From b304361ce9085665af0bcbf4ac2a6b1f0e75cab9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tiziano=20M=C3=BCller?= Date: Fri, 24 May 2024 16:34:38 +0200 Subject: [PATCH 1/6] server: fix error reporting and logic for /keys handler restores proper error reporting to include the host dialed, and fixes the tautological comparison `jwks == nil` in the recovery path to properly refetch the server config and try again as intended --- internal/server/server.go | 49 +++++++++++++++++++++++---------------- 1 file changed, 29 insertions(+), 20 deletions(-) diff --git a/internal/server/server.go b/internal/server/server.go index 16b7b51..3fdae97 100644 --- a/internal/server/server.go +++ b/internal/server/server.go @@ -141,38 +141,47 @@ func (s *Server) StartLogin(clients []oauth.Client, params ServerParams) error { p = params.AuthProvider jwks []byte ) - // try and get the JWKS from param first - if p.Endpoints.JwksUri != "" { - err := p.FetchJwks() + + fetchAndMarshal := func() (err error) { + err = p.FetchJwks() if err != nil { - fmt.Printf("failed to fetch keys using JWKS url...trying to fetch config and try again...\n") + fmt.Printf("failed to fetch keys: %v\n", err) + return } jwks, err = json.Marshal(p.KeySet) if err != nil { fmt.Printf("failed to marshal JWKS: %v\n", err) } - } else if p.Endpoints.Config != "" && jwks == nil { - // otherwise, try and fetch the whole config and try again - err := p.FetchServerConfig() - if err != nil { - fmt.Printf("failed to fetch server config: %v\n", err) - http.Redirect(w, r, "/error", http.StatusInternalServerError) - return - } - err = p.FetchJwks() - if err != nil { - fmt.Printf("failed to fetch JWKS after fetching server config: %v\n", err) - http.Redirect(w, r, "/error", http.StatusInternalServerError) + return + } + + // try and get the JWKS from param first + if p.Endpoints.JwksUri != "" { + if err := fetchAndMarshal(); err != nil { + w.Write(jwks) return } } - // forward the JWKS from the authorization server - if jwks == nil { - fmt.Printf("no JWKS was fetched from authorization server\n") - http.Redirect(w, r, "/error", http.StatusInternalServerError) + // otherwise or if fetching the JWKS failed, try and fetch the whole config first and try again + if p.Endpoints.Config != "" { + if err := p.FetchServerConfig(); err != nil { + fmt.Printf("failed to fetch server config: %v\n", err) + http.Error(w, http.StatusText(http.StatusInternalServerError), http.StatusInternalServerError) + return + } + } else { + fmt.Printf("getting JWKS from param failed and endpoints config unavailable\n") + http.Error(w, http.StatusText(http.StatusInternalServerError), http.StatusInternalServerError) return } + + if err := fetchAndMarshal(); err != nil { + fmt.Printf("failed to fetch and marshal JWKS after config update: %v\n", err) + http.Error(w, http.StatusText(http.StatusInternalServerError), http.StatusInternalServerError) + return + } + w.Write(jwks) }) r.HandleFunc("/token", func(w http.ResponseWriter, r *http.Request) { From 2612978a98d1391175b2c7bbdbb06b977f6e4583 Mon Sep 17 00:00:00 2001 From: David Allen Date: Wed, 12 Jun 2024 12:50:53 -0600 Subject: [PATCH 2/6] Added verbose print to show ID and access tokens from IDP --- internal/flows/jwt_bearer.go | 3 +++ 1 file changed, 3 insertions(+) diff --git a/internal/flows/jwt_bearer.go b/internal/flows/jwt_bearer.go index 2e93265..a0287d9 100644 --- a/internal/flows/jwt_bearer.go +++ b/internal/flows/jwt_bearer.go @@ -51,6 +51,9 @@ func NewJwtBearerFlow(eps JwtBearerFlowEndpoints, params JwtBearerFlowParams) (s if client == nil { return "", fmt.Errorf("invalid client (client is nil)") } + if verbose { + fmt.Printf("ID token (IDP): %s\n access token (IDP): %s", accessToken, idToken) + } if accessToken != "" { _, err := jws.Verify([]byte(accessToken), jws.WithKeySet(client.Provider.KeySet), jws.WithValidateKey(true)) if err != nil { From 8570064235bb8a7616070c9def275efa5b799a8c Mon Sep 17 00:00:00 2001 From: David Allen Date: Wed, 12 Jun 2024 14:01:24 -0600 Subject: [PATCH 3/6] Added response body print to debug ID token --- internal/oauth/authenticate.go | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/internal/oauth/authenticate.go b/internal/oauth/authenticate.go index b579e8e..175aad1 100644 --- a/internal/oauth/authenticate.go +++ b/internal/oauth/authenticate.go @@ -109,12 +109,14 @@ func (client *Client) FetchTokenFromAuthenticationServer(code string, state stri } res, err := http.PostForm(client.Provider.Endpoints.Token, body) if err != nil { - return nil, fmt.Errorf("failed to get ID token: %s", err) + return nil, fmt.Errorf("failed to get ID token: %v", err) } + b, err := io.ReadAll(res.Body) + if err != nil { + return nil, fmt.Errorf("failed to read response body: %v", err) + } + fmt.Printf("%s\n", string(b)) defer res.Body.Close() - // domain, _ := url.Parse("http://127.0.0.1") - // client.Jar.SetCookies(domain, res.Cookies()) - return io.ReadAll(res.Body) } From 8c01ba897ff72c81cc49b8ced6f1f310c1b930a1 Mon Sep 17 00:00:00 2001 From: David Allen Date: Wed, 12 Jun 2024 12:50:53 -0600 Subject: [PATCH 4/6] Added verbose print to show ID and access tokens from IDP --- internal/flows/jwt_bearer.go | 3 +++ 1 file changed, 3 insertions(+) diff --git a/internal/flows/jwt_bearer.go b/internal/flows/jwt_bearer.go index 2e93265..a0287d9 100644 --- a/internal/flows/jwt_bearer.go +++ b/internal/flows/jwt_bearer.go @@ -51,6 +51,9 @@ func NewJwtBearerFlow(eps JwtBearerFlowEndpoints, params JwtBearerFlowParams) (s if client == nil { return "", fmt.Errorf("invalid client (client is nil)") } + if verbose { + fmt.Printf("ID token (IDP): %s\n access token (IDP): %s", accessToken, idToken) + } if accessToken != "" { _, err := jws.Verify([]byte(accessToken), jws.WithKeySet(client.Provider.KeySet), jws.WithValidateKey(true)) if err != nil { From a7e0e73e4560a979151a0c57037034c64b61810b Mon Sep 17 00:00:00 2001 From: David Allen Date: Wed, 12 Jun 2024 14:01:24 -0600 Subject: [PATCH 5/6] Added response body print to debug ID token --- internal/oauth/authenticate.go | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/internal/oauth/authenticate.go b/internal/oauth/authenticate.go index b579e8e..175aad1 100644 --- a/internal/oauth/authenticate.go +++ b/internal/oauth/authenticate.go @@ -109,12 +109,14 @@ func (client *Client) FetchTokenFromAuthenticationServer(code string, state stri } res, err := http.PostForm(client.Provider.Endpoints.Token, body) if err != nil { - return nil, fmt.Errorf("failed to get ID token: %s", err) + return nil, fmt.Errorf("failed to get ID token: %v", err) } + b, err := io.ReadAll(res.Body) + if err != nil { + return nil, fmt.Errorf("failed to read response body: %v", err) + } + fmt.Printf("%s\n", string(b)) defer res.Body.Close() - // domain, _ := url.Parse("http://127.0.0.1") - // client.Jar.SetCookies(domain, res.Cookies()) - return io.ReadAll(res.Body) } From e0a8d434211c29be3b4808e3dfcedaa2a713bf8a Mon Sep 17 00:00:00 2001 From: David Allen Date: Wed, 12 Jun 2024 14:42:47 -0600 Subject: [PATCH 6/6] Fixed token fetch from IDP --- internal/oauth/authenticate.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/oauth/authenticate.go b/internal/oauth/authenticate.go index 175aad1..4af65cb 100644 --- a/internal/oauth/authenticate.go +++ b/internal/oauth/authenticate.go @@ -118,5 +118,5 @@ func (client *Client) FetchTokenFromAuthenticationServer(code string, state stri fmt.Printf("%s\n", string(b)) defer res.Body.Close() - return io.ReadAll(res.Body) + return b, nil }