From 2879801c4e28887c3072cb12f3add590dc75c2ac Mon Sep 17 00:00:00 2001 From: Tadeusz Magura-Witkowski Date: Thu, 13 Feb 2020 15:43:30 +0100 Subject: [PATCH 01/24] Basic implementation of PKCE Signed-off-by: Tadeusz Magura-Witkowski --- server/handlers.go | 37 +++++++++++++++++++ server/oauth2.go | 16 +++++++++ server/server_test.go | 57 +++++++++++++++++++++++------- storage/conformance/conformance.go | 10 ++++++ storage/etcd/types.go | 32 ++++++++++++----- storage/kubernetes/types.go | 34 +++++++++++++----- storage/sql/crud.go | 31 ++++++++++------ storage/sql/migrate.go | 15 ++++++++ storage/storage.go | 10 ++++++ 9 files changed, 202 insertions(+), 40 deletions(-) diff --git a/server/handlers.go b/server/handlers.go index 24d3999973..054b39ec59 100644 --- a/server/handlers.go +++ b/server/handlers.go @@ -2,6 +2,8 @@ package server import ( "context" + "crypto/sha256" + "encoding/base64" "encoding/json" "errors" "fmt" @@ -23,6 +25,11 @@ import ( "github.com/dexidp/dex/storage" ) +const ( + CodeChallengeMethodPlain = "plain" + CodeChallengeMethodS256 = "S256" +) + // newHealthChecker returns the healthz handler. The handler runs until the // provided context is canceled. func (s *Server) newHealthChecker(ctx context.Context) http.Handler { @@ -633,6 +640,7 @@ func (s *Server) sendCodeResponse(w http.ResponseWriter, r *http.Request, authRe Expiry: s.now().Add(time.Minute * 30), RedirectURI: authReq.RedirectURI, ConnectorData: authReq.ConnectorData, + CodeChallenge: authReq.CodeChallenge, } if err := s.storage.CreateAuthCode(code); err != nil { s.logger.Errorf("Failed to create auth code: %v", err) @@ -761,6 +769,19 @@ func (s *Server) handleToken(w http.ResponseWriter, r *http.Request) { } } +func (s *Server) calculateCodeChallenge(codeVerifier, codeChallengeMethod string) (string, error) { + switch codeChallengeMethod { + case CodeChallengeMethodPlain: + return codeVerifier, nil + case CodeChallengeMethodS256: + shaSum := sha256.Sum256([]byte(codeVerifier)) + return base64.RawURLEncoding.EncodeToString(shaSum[:]), nil + default: + s.logger.Errorf("unknown challenge method (%v)", codeChallengeMethod) + return "", fmt.Errorf("unknown challenge method (%v)", codeChallengeMethod) + } +} + // handle an access token request https://tools.ietf.org/html/rfc6749#section-4.1.3 func (s *Server) handleAuthCode(w http.ResponseWriter, r *http.Request, client storage.Client) { code := r.PostFormValue("code") @@ -777,6 +798,22 @@ func (s *Server) handleAuthCode(w http.ResponseWriter, r *http.Request, client s return } + // RFC 7636 (PKCE) + codeChallengeFromStorage := authCode.CodeChallenge.CodeChallenge + if codeChallengeFromStorage != "" { + providedCodeVerifier := r.PostFormValue("code_verifier") + calculatedCodeChallenge, err := s.calculateCodeChallenge(providedCodeVerifier, authCode.CodeChallenge.CodeChallengeMethod) + if err != nil { + s.tokenErrHelper(w, errServerError, "", http.StatusInternalServerError) + return + } + + if codeChallengeFromStorage != calculatedCodeChallenge { + s.tokenErrHelper(w, errInvalidRequest, "invalid code_verifier.", http.StatusBadRequest) + return + } + } + if authCode.RedirectURI != redirectURI { s.tokenErrHelper(w, errInvalidRequest, "redirect_uri did not match URI from initial request.", http.StatusBadRequest) return diff --git a/server/oauth2.go b/server/oauth2.go index 0cd26814ee..0cdfc79468 100644 --- a/server/oauth2.go +++ b/server/oauth2.go @@ -400,6 +400,18 @@ func (s *Server) parseAuthorizationRequest(r *http.Request) (*storage.AuthReques scopes := strings.Fields(q.Get("scope")) responseTypes := strings.Fields(q.Get("response_type")) + codeChallenge := q.Get("code_challenge") + codeChallengeMethod := q.Get("code_challenge_method") + + if codeChallengeMethod == "" { + codeChallengeMethod = CodeChallengeMethodPlain + } + + if codeChallengeMethod != CodeChallengeMethodS256 && codeChallengeMethod != CodeChallengeMethodPlain { + description := fmt.Sprintf("Unsupported PKCE challenge method (%q).", codeChallengeMethod) + return nil, &authErr{"", "", errInvalidRequest, description} + } + client, err := s.storage.GetClient(clientID) if err != nil { if err == storage.ErrNotFound { @@ -525,6 +537,10 @@ func (s *Server) parseAuthorizationRequest(r *http.Request) (*storage.AuthReques RedirectURI: redirectURI, ResponseTypes: responseTypes, ConnectorID: connectorID, + CodeChallenge: storage.CodeChallenge{ + CodeChallenge: codeChallenge, + CodeChallengeMethod: codeChallengeMethod, + }, }, nil } diff --git a/server/server_test.go b/server/server_test.go index 8fe84c9a5d..69696e6d30 100644 --- a/server/server_test.go +++ b/server/server_test.go @@ -228,25 +228,33 @@ func TestOAuth2CodeFlow(t *testing.T) { oidcConfig := &oidc.Config{SkipClientIDCheck: true} + basicIDTokenVerify := func(ctx context.Context, p *oidc.Provider, config *oauth2.Config, token *oauth2.Token) error { + idToken, ok := token.Extra("id_token").(string) + if !ok { + return fmt.Errorf("no id token found") + } + if _, err := p.Verifier(oidcConfig).Verify(ctx, idToken); err != nil { + return fmt.Errorf("failed to verify id token: %v", err) + } + return nil + } + tests := []struct { name string // If specified these set of scopes will be used during the test case. scopes []string // handleToken provides the OAuth2 token response for the integration test. handleToken func(context.Context, *oidc.Provider, *oauth2.Config, *oauth2.Token) error + + // extra parameters to pass when requesting auth_code + authCodeOptions []oauth2.AuthCodeOption + + // extra parameters to pass when retrieving id token + retreiveTokenOptions []oauth2.AuthCodeOption }{ { - name: "verify ID Token", - handleToken: func(ctx context.Context, p *oidc.Provider, config *oauth2.Config, token *oauth2.Token) error { - idToken, ok := token.Extra("id_token").(string) - if !ok { - return fmt.Errorf("no id token found") - } - if _, err := p.Verifier(oidcConfig).Verify(ctx, idToken); err != nil { - return fmt.Errorf("failed to verify id token: %v", err) - } - return nil - }, + name: "verify ID Token", + handleToken: basicIDTokenVerify, }, { name: "fetch userinfo", @@ -472,6 +480,29 @@ func TestOAuth2CodeFlow(t *testing.T) { return nil }, }, + { + // This test ensures that PKCE work in "plain" mode (no code_challenge_method specified) + name: "PKCE with plain", + authCodeOptions: []oauth2.AuthCodeOption{ + oauth2.SetAuthURLParam("code_challenge", "challenge123"), + }, + retreiveTokenOptions: []oauth2.AuthCodeOption{ + oauth2.SetAuthURLParam("code_verifier", "challenge123"), + }, + handleToken: basicIDTokenVerify, + }, + { + // This test ensures that PKCE work in "S256" mode + name: "PKCE with S256", + authCodeOptions: []oauth2.AuthCodeOption{ + oauth2.SetAuthURLParam("code_challenge", "lyyl-X4a69qrqgEfUL8wodWic3Be9ZZ5eovBgIKKi-w"), + oauth2.SetAuthURLParam("code_challenge_method", "S256"), + }, + retreiveTokenOptions: []oauth2.AuthCodeOption{ + oauth2.SetAuthURLParam("code_verifier", "challenge123"), + }, + handleToken: basicIDTokenVerify, + }, } for _, tc := range tests { @@ -514,7 +545,7 @@ func TestOAuth2CodeFlow(t *testing.T) { oauth2Client := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { if r.URL.Path != "/callback" { // User is visiting app first time. Redirect to dex. - http.Redirect(w, r, oauth2Config.AuthCodeURL(state), http.StatusSeeOther) + http.Redirect(w, r, oauth2Config.AuthCodeURL(state, tc.authCodeOptions...), http.StatusSeeOther) return } @@ -535,7 +566,7 @@ func TestOAuth2CodeFlow(t *testing.T) { // Grab code, exchange for token. if code := q.Get("code"); code != "" { gotCode = true - token, err := oauth2Config.Exchange(ctx, code) + token, err := oauth2Config.Exchange(ctx, code, tc.retreiveTokenOptions...) if err != nil { t.Errorf("failed to exchange code for token: %v", err) return diff --git a/storage/conformance/conformance.go b/storage/conformance/conformance.go index 8a55a75ba3..e853adfcc4 100644 --- a/storage/conformance/conformance.go +++ b/storage/conformance/conformance.go @@ -79,6 +79,11 @@ func mustBeErrAlreadyExists(t *testing.T, kind string, err error) { } func testAuthRequestCRUD(t *testing.T, s storage.Storage) { + codeChallenge := storage.CodeChallenge{ + CodeChallenge: "code_challenge_test", + CodeChallengeMethod: "plain", + } + a1 := storage.AuthRequest{ ID: storage.NewID(), ClientID: "client1", @@ -99,6 +104,7 @@ func testAuthRequestCRUD(t *testing.T, s storage.Storage) { EmailVerified: true, Groups: []string{"a", "b"}, }, + CodeChallenge: codeChallenge, } identity := storage.Claims{Email: "foobar"} @@ -153,6 +159,10 @@ func testAuthRequestCRUD(t *testing.T, s storage.Storage) { t.Fatalf("update failed, wanted identity=%#v got %#v", identity, got.Claims) } + if !reflect.DeepEqual(got.CodeChallenge, codeChallenge) { + t.Fatalf("storage does not support PKCE, wanted challenge=%#v got %#v", codeChallenge, got.CodeChallenge) + } + if err := s.DeleteAuthRequest(a1.ID); err != nil { t.Fatalf("failed to delete auth request: %v", err) } diff --git a/storage/etcd/types.go b/storage/etcd/types.go index a16eae8e94..ae36bf4295 100644 --- a/storage/etcd/types.go +++ b/storage/etcd/types.go @@ -21,19 +21,24 @@ type AuthCode struct { Claims Claims `json:"claims,omitempty"` Expiry time.Time `json:"expiry"` + + CodeChallenge string `json:"code_challenge,omitempty"` + CodeChallengeMethod string `json:"code_challenge_method,omitempty"` } func fromStorageAuthCode(a storage.AuthCode) AuthCode { return AuthCode{ - ID: a.ID, - ClientID: a.ClientID, - RedirectURI: a.RedirectURI, - ConnectorID: a.ConnectorID, - ConnectorData: a.ConnectorData, - Nonce: a.Nonce, - Scopes: a.Scopes, - Claims: fromStorageClaims(a.Claims), - Expiry: a.Expiry, + ID: a.ID, + ClientID: a.ClientID, + RedirectURI: a.RedirectURI, + ConnectorID: a.ConnectorID, + ConnectorData: a.ConnectorData, + Nonce: a.Nonce, + Scopes: a.Scopes, + Claims: fromStorageClaims(a.Claims), + Expiry: a.Expiry, + CodeChallenge: a.CodeChallenge.CodeChallenge, + CodeChallengeMethod: a.CodeChallenge.CodeChallengeMethod, } } @@ -58,6 +63,9 @@ type AuthRequest struct { ConnectorID string `json:"connector_id"` ConnectorData []byte `json:"connector_data"` + + CodeChallenge string `json:"code_challenge,omitempty"` + CodeChallengeMethod string `json:"code_challenge_method,omitempty"` } func fromStorageAuthRequest(a storage.AuthRequest) AuthRequest { @@ -75,6 +83,8 @@ func fromStorageAuthRequest(a storage.AuthRequest) AuthRequest { Claims: fromStorageClaims(a.Claims), ConnectorID: a.ConnectorID, ConnectorData: a.ConnectorData, + CodeChallenge: a.CodeChallenge.CodeChallenge, + CodeChallengeMethod: a.CodeChallenge.CodeChallengeMethod, } } @@ -93,6 +103,10 @@ func toStorageAuthRequest(a AuthRequest) storage.AuthRequest { ConnectorData: a.ConnectorData, Expiry: a.Expiry, Claims: toStorageClaims(a.Claims), + CodeChallenge: storage.CodeChallenge{ + CodeChallenge: a.CodeChallenge, + CodeChallengeMethod: a.CodeChallengeMethod, + }, } } diff --git a/storage/kubernetes/types.go b/storage/kubernetes/types.go index 5eda178111..03b463ffd6 100644 --- a/storage/kubernetes/types.go +++ b/storage/kubernetes/types.go @@ -269,6 +269,9 @@ type AuthRequest struct { ConnectorData []byte `json:"connectorData,omitempty"` Expiry time.Time `json:"expiry"` + + CodeChallenge string `json:"code_challenge,omitempty"` + CodeChallengeMethod string `json:"code_challenge_method,omitempty"` } // AuthRequestList is a list of AuthRequests. @@ -293,6 +296,10 @@ func toStorageAuthRequest(req AuthRequest) storage.AuthRequest { ConnectorData: req.ConnectorData, Expiry: req.Expiry, Claims: toStorageClaims(req.Claims), + CodeChallenge: storage.CodeChallenge{ + CodeChallenge: req.CodeChallenge, + CodeChallengeMethod: req.CodeChallengeMethod, + }, } return a } @@ -319,6 +326,8 @@ func (cli *client) fromStorageAuthRequest(a storage.AuthRequest) AuthRequest { ConnectorData: a.ConnectorData, Expiry: a.Expiry, Claims: fromStorageClaims(a.Claims), + CodeChallenge: a.CodeChallenge.CodeChallenge, + CodeChallengeMethod: a.CodeChallenge.CodeChallengeMethod, } return req } @@ -392,6 +401,9 @@ type AuthCode struct { ConnectorData []byte `json:"connectorData,omitempty"` Expiry time.Time `json:"expiry"` + + CodeChallenge string `json:"code_challenge,omitempty"` + CodeChallengeMethod string `json:"code_challenge_method,omitempty"` } // AuthCodeList is a list of AuthCodes. @@ -411,14 +423,16 @@ func (cli *client) fromStorageAuthCode(a storage.AuthCode) AuthCode { Name: a.ID, Namespace: cli.namespace, }, - ClientID: a.ClientID, - RedirectURI: a.RedirectURI, - ConnectorID: a.ConnectorID, - ConnectorData: a.ConnectorData, - Nonce: a.Nonce, - Scopes: a.Scopes, - Claims: fromStorageClaims(a.Claims), - Expiry: a.Expiry, + ClientID: a.ClientID, + RedirectURI: a.RedirectURI, + ConnectorID: a.ConnectorID, + ConnectorData: a.ConnectorData, + Nonce: a.Nonce, + Scopes: a.Scopes, + Claims: fromStorageClaims(a.Claims), + Expiry: a.Expiry, + CodeChallenge: a.CodeChallenge.CodeChallenge, + CodeChallengeMethod: a.CodeChallenge.CodeChallengeMethod, } } @@ -433,6 +447,10 @@ func toStorageAuthCode(a AuthCode) storage.AuthCode { Scopes: a.Scopes, Claims: toStorageClaims(a.Claims), Expiry: a.Expiry, + CodeChallenge: storage.CodeChallenge{ + CodeChallenge: a.CodeChallenge, + CodeChallengeMethod: a.CodeChallengeMethod, + }, } } diff --git a/storage/sql/crud.go b/storage/sql/crud.go index e87dc56aa5..8abe75f56d 100644 --- a/storage/sql/crud.go +++ b/storage/sql/crud.go @@ -111,10 +111,11 @@ func (c *conn) CreateAuthRequest(a storage.AuthRequest) error { claims_user_id, claims_username, claims_preferred_username, claims_email, claims_email_verified, claims_groups, connector_id, connector_data, - expiry + expiry, + code_challenge, code_challenge_method ) values ( - $1, $2, $3, $4, $5, $6, $7, $8, $9, $10, $11, $12, $13, $14, $15, $16, $17, $18 + $1, $2, $3, $4, $5, $6, $7, $8, $9, $10, $11, $12, $13, $14, $15, $16, $17, $18, $19, $20 ); `, a.ID, a.ClientID, encoder(a.ResponseTypes), encoder(a.Scopes), a.RedirectURI, a.Nonce, a.State, @@ -123,6 +124,7 @@ func (c *conn) CreateAuthRequest(a storage.AuthRequest) error { a.Claims.Email, a.Claims.EmailVerified, encoder(a.Claims.Groups), a.ConnectorID, a.ConnectorData, a.Expiry, + a.CodeChallenge.CodeChallenge, a.CodeChallenge.CodeChallengeMethod, ) if err != nil { if c.alreadyExistsCheck(err) { @@ -153,8 +155,9 @@ func (c *conn) UpdateAuthRequest(id string, updater func(a storage.AuthRequest) claims_email = $12, claims_email_verified = $13, claims_groups = $14, connector_id = $15, connector_data = $16, - expiry = $17 - where id = $18; + expiry = $17, + code_challenge = $18, code_challenge_method = $19 + where id = $20; `, a.ClientID, encoder(a.ResponseTypes), encoder(a.Scopes), a.RedirectURI, a.Nonce, a.State, a.ForceApprovalPrompt, a.LoggedIn, @@ -162,7 +165,9 @@ func (c *conn) UpdateAuthRequest(id string, updater func(a storage.AuthRequest) a.Claims.Email, a.Claims.EmailVerified, encoder(a.Claims.Groups), a.ConnectorID, a.ConnectorData, - a.Expiry, r.ID, + a.Expiry, + a.CodeChallenge.CodeChallenge, a.CodeChallenge.CodeChallengeMethod, + r.ID, ) if err != nil { return fmt.Errorf("update auth request: %v", err) @@ -182,7 +187,8 @@ func getAuthRequest(q querier, id string) (a storage.AuthRequest, err error) { force_approval_prompt, logged_in, claims_user_id, claims_username, claims_preferred_username, claims_email, claims_email_verified, claims_groups, - connector_id, connector_data, expiry + connector_id, connector_data, expiry, + code_challenge, code_challenge_method from auth_request where id = $1; `, id).Scan( &a.ID, &a.ClientID, decoder(&a.ResponseTypes), decoder(&a.Scopes), &a.RedirectURI, &a.Nonce, &a.State, @@ -191,6 +197,7 @@ func getAuthRequest(q querier, id string) (a storage.AuthRequest, err error) { &a.Claims.Email, &a.Claims.EmailVerified, decoder(&a.Claims.Groups), &a.ConnectorID, &a.ConnectorData, &a.Expiry, + &a.CodeChallenge.CodeChallenge, &a.CodeChallenge.CodeChallengeMethod, ) if err != nil { if err == sql.ErrNoRows { @@ -208,13 +215,15 @@ func (c *conn) CreateAuthCode(a storage.AuthCode) error { claims_user_id, claims_username, claims_preferred_username, claims_email, claims_email_verified, claims_groups, connector_id, connector_data, - expiry + expiry, + code_challenge, code_challenge_method ) - values ($1, $2, $3, $4, $5, $6, $7, $8, $9, $10, $11, $12, $13, $14); + values ($1, $2, $3, $4, $5, $6, $7, $8, $9, $10, $11, $12, $13, $14, $15, $16); `, a.ID, a.ClientID, encoder(a.Scopes), a.Nonce, a.RedirectURI, a.Claims.UserID, a.Claims.Username, a.Claims.PreferredUsername, a.Claims.Email, a.Claims.EmailVerified, encoder(a.Claims.Groups), a.ConnectorID, a.ConnectorData, a.Expiry, + a.CodeChallenge.CodeChallenge, a.CodeChallenge.CodeChallengeMethod, ) if err != nil { @@ -233,12 +242,14 @@ func (c *conn) GetAuthCode(id string) (a storage.AuthCode, err error) { claims_user_id, claims_username, claims_preferred_username, claims_email, claims_email_verified, claims_groups, connector_id, connector_data, - expiry + expiry, + code_challenge, code_challenge_method from auth_code where id = $1; `, id).Scan( &a.ID, &a.ClientID, decoder(&a.Scopes), &a.Nonce, &a.RedirectURI, &a.Claims.UserID, &a.Claims.Username, &a.Claims.PreferredUsername, &a.Claims.Email, &a.Claims.EmailVerified, decoder(&a.Claims.Groups), &a.ConnectorID, &a.ConnectorData, &a.Expiry, + &a.CodeChallenge.CodeChallenge, &a.CodeChallenge.CodeChallengeMethod, ) if err != nil { if err == sql.ErrNoRows { @@ -298,7 +309,7 @@ func (c *conn) UpdateRefreshToken(id string, updater func(old storage.RefreshTok claims_email_verified = $8, claims_groups = $9, connector_id = $10, - connector_data = $11, + connector_data = $11, token = $12, created_at = $13, last_used = $14 diff --git a/storage/sql/migrate.go b/storage/sql/migrate.go index 5b86bc78d8..f2f16f878f 100644 --- a/storage/sql/migrate.go +++ b/storage/sql/migrate.go @@ -209,4 +209,19 @@ var migrations = []migration{ `, }, }, + { + stmts: []string{` + alter table auth_request + add column code_challenge text not null default '';`, + ` + alter table auth_request + add column code_challenge_method text not null default '';`, + ` + alter table auth_code + add column code_challenge text not null default '';`, + ` + alter table auth_code + add column code_challenge_method text not null default '';`, + }, + }, } diff --git a/storage/storage.go b/storage/storage.go index 42ecd8ed41..f8b0265735 100644 --- a/storage/storage.go +++ b/storage/storage.go @@ -146,6 +146,12 @@ type Claims struct { Groups []string } +// Data needed for PKCE (RFC 7636) +type CodeChallenge struct { + CodeChallenge string + CodeChallengeMethod string +} + // AuthRequest represents a OAuth2 client authorization request. It holds the state // of a single auth flow up to the point that the user authorizes the client. type AuthRequest struct { @@ -183,6 +189,8 @@ type AuthRequest struct { // Set when the user authenticates. ConnectorID string ConnectorData []byte + + CodeChallenge } // AuthCode represents a code which can be exchanged for an OAuth2 token response. @@ -218,6 +226,8 @@ type AuthCode struct { Claims Claims Expiry time.Time + + CodeChallenge } // RefreshToken is an OAuth2 refresh token which allows a client to request new From 551a29262dc1127341ded34cc7e28aa44b3285c8 Mon Sep 17 00:00:00 2001 From: Bernd Eckstein Date: Wed, 22 Jul 2020 12:10:52 +0200 Subject: [PATCH 02/24] @mfmarche on 24 Feb: when code_verifier is set, don't check client_secret In PKCE flow, no client_secret is used, so the check for a valid client_secret would always fail. Signed-off-by: Bernd Eckstein --- server/handlers.go | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/server/handlers.go b/server/handlers.go index 054b39ec59..b0a3304cc6 100644 --- a/server/handlers.go +++ b/server/handlers.go @@ -753,7 +753,9 @@ func (s *Server) handleToken(w http.ResponseWriter, r *http.Request) { } return } - if client.Secret != clientSecret { + if r.PostFormValue("code_verifier") != "" { + // RFC 7636 (PKCE) if code_verifier is received use PKCE where we do not use the client_secret + } else if client.Secret != clientSecret { s.tokenErrHelper(w, errInvalidClient, "Invalid client credentials.", http.StatusUnauthorized) return } From 2688931ffa2d712d2e48447024fb86c6740b20cc Mon Sep 17 00:00:00 2001 From: Bernd Eckstein Date: Wed, 22 Jul 2020 12:33:00 +0200 Subject: [PATCH 03/24] @deric on 16 Jun: return invalid_grant when wrong code_verifier Signed-off-by: Bernd Eckstein --- server/handlers.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/server/handlers.go b/server/handlers.go index b0a3304cc6..1df6451383 100644 --- a/server/handlers.go +++ b/server/handlers.go @@ -811,7 +811,7 @@ func (s *Server) handleAuthCode(w http.ResponseWriter, r *http.Request, client s } if codeChallengeFromStorage != calculatedCodeChallenge { - s.tokenErrHelper(w, errInvalidRequest, "invalid code_verifier.", http.StatusBadRequest) + s.tokenErrHelper(w, errInvalidGrant, "invalid code_verifier.", http.StatusBadRequest) return } } From 3c47734d9e0e2fef3744bb8d622d14b6920d9d4c Mon Sep 17 00:00:00 2001 From: Bernd Eckstein Date: Wed, 22 Jul 2020 13:29:11 +0200 Subject: [PATCH 04/24] Enforce PKCE flow on /token when PKCE flow was started on /auth Also dissallow PKCE on /token, when PKCE flow was not started on /auth Signed-off-by: Bernd Eckstein --- server/handlers.go | 17 ++++++++++++++--- 1 file changed, 14 insertions(+), 3 deletions(-) diff --git a/server/handlers.go b/server/handlers.go index 1df6451383..295d3da024 100644 --- a/server/handlers.go +++ b/server/handlers.go @@ -754,7 +754,7 @@ func (s *Server) handleToken(w http.ResponseWriter, r *http.Request) { return } if r.PostFormValue("code_verifier") != "" { - // RFC 7636 (PKCE) if code_verifier is received use PKCE where we do not use the client_secret + // RFC 7636 (PKCE) if code_verifier is received, use PKCE and not the client_secret } else if client.Secret != clientSecret { s.tokenErrHelper(w, errInvalidClient, "Invalid client credentials.", http.StatusUnauthorized) return @@ -802,7 +802,10 @@ func (s *Server) handleAuthCode(w http.ResponseWriter, r *http.Request, client s // RFC 7636 (PKCE) codeChallengeFromStorage := authCode.CodeChallenge.CodeChallenge - if codeChallengeFromStorage != "" { + providedCodeVerifier := r.PostFormValue("code_verifier") + + if providedCodeVerifier != "" && codeChallengeFromStorage != "" { + providedCodeVerifier := r.PostFormValue("code_verifier") calculatedCodeChallenge, err := s.calculateCodeChallenge(providedCodeVerifier, authCode.CodeChallenge.CodeChallengeMethod) if err != nil { @@ -811,9 +814,17 @@ func (s *Server) handleAuthCode(w http.ResponseWriter, r *http.Request, client s } if codeChallengeFromStorage != calculatedCodeChallenge { - s.tokenErrHelper(w, errInvalidGrant, "invalid code_verifier.", http.StatusBadRequest) + s.tokenErrHelper(w, errInvalidGrant, "Invalid code_verifier.", http.StatusBadRequest) return } + } else if providedCodeVerifier != "" { + // Received PKCE request on /auth, but no code_verifier on /token + s.tokenErrHelper(w, errInvalidGrant, "Expecting code_verifier in PKCE flow.", http.StatusBadRequest) + return + } else if codeChallengeFromStorage != "" { + // Received no code_challenge on /auth, but a code_verifier on /token + s.tokenErrHelper(w, errInvalidRequest, "No PKCE flow started. Cannot check code_verifier.", http.StatusBadRequest) + return } if authCode.RedirectURI != redirectURI { From d0fafb0f875fa4f4968158780c5a7372ce4085c2 Mon Sep 17 00:00:00 2001 From: Bernd Eckstein Date: Fri, 24 Jul 2020 10:43:01 +0200 Subject: [PATCH 05/24] fixed error messages when mixed PKCE/no PKCE flow. Signed-off-by: Bernd Eckstein --- server/handlers.go | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/server/handlers.go b/server/handlers.go index 295d3da024..b5a67cb9da 100644 --- a/server/handlers.go +++ b/server/handlers.go @@ -805,7 +805,6 @@ func (s *Server) handleAuthCode(w http.ResponseWriter, r *http.Request, client s providedCodeVerifier := r.PostFormValue("code_verifier") if providedCodeVerifier != "" && codeChallengeFromStorage != "" { - providedCodeVerifier := r.PostFormValue("code_verifier") calculatedCodeChallenge, err := s.calculateCodeChallenge(providedCodeVerifier, authCode.CodeChallenge.CodeChallengeMethod) if err != nil { @@ -818,13 +817,13 @@ func (s *Server) handleAuthCode(w http.ResponseWriter, r *http.Request, client s return } } else if providedCodeVerifier != "" { - // Received PKCE request on /auth, but no code_verifier on /token - s.tokenErrHelper(w, errInvalidGrant, "Expecting code_verifier in PKCE flow.", http.StatusBadRequest) - return - } else if codeChallengeFromStorage != "" { // Received no code_challenge on /auth, but a code_verifier on /token s.tokenErrHelper(w, errInvalidRequest, "No PKCE flow started. Cannot check code_verifier.", http.StatusBadRequest) return + } else if codeChallengeFromStorage != "" { + // Received PKCE request on /auth, but no code_verifier on /token + s.tokenErrHelper(w, errInvalidGrant, "Expecting parameter code_verifier in PKCE flow.", http.StatusBadRequest) + return } if authCode.RedirectURI != redirectURI { From 492fecf5677efbbfdbc789f6777b6d0807191a1f Mon Sep 17 00:00:00 2001 From: Bernd Eckstein Date: Fri, 24 Jul 2020 10:47:22 +0200 Subject: [PATCH 06/24] server_test.go: Added PKCE error cases on /token endpoint * Added test for invalid_grant, when wrong code_verifier is sent * Added test for mixed PKCE / no PKCE auth flows. Signed-off-by: Bernd Eckstein --- server/server_test.go | 105 +++++++++++++++++++++++++++++++++++++++++- 1 file changed, 104 insertions(+), 1 deletion(-) diff --git a/server/server_test.go b/server/server_test.go index 69696e6d30..5d6a708ecd 100644 --- a/server/server_test.go +++ b/server/server_test.go @@ -203,6 +203,20 @@ func TestDiscovery(t *testing.T) { } } +// Defines an expected error by HTTP Status Code and +// the OAuth2 error int the response json +type ErrorResponse struct { + Error string + StatusCode int +} + +// https://tools.ietf.org/html/rfc6749#section-5.2 +type OAuth2ErrorResponse struct { + Error string `json:"error"` + ErrorDescription string `json:"error_description"` + ErrorURI string `json:"error_uri"` +} + // TestOAuth2CodeFlow runs integration tests against a test server. The tests stand up a server // which requires no interaction to login, logs in through a test client, then passes the client // and returned token to the test. @@ -251,6 +265,9 @@ func TestOAuth2CodeFlow(t *testing.T) { // extra parameters to pass when retrieving id token retreiveTokenOptions []oauth2.AuthCodeOption + + // define an error response, when the test expects a error on the token endpoint + tokenError ErrorResponse }{ { name: "verify ID Token", @@ -412,7 +429,7 @@ func TestOAuth2CodeFlow(t *testing.T) { v.Add("client_secret", clientSecret) v.Add("grant_type", "refresh_token") v.Add("refresh_token", token.RefreshToken) - // Request a scope that wasn't requestd initially. + // Request a scope that wasn't requested initially. v.Add("scope", "oidc email profile") resp, err := http.PostForm(p.Endpoint().TokenURL, v) if err != nil { @@ -503,6 +520,68 @@ func TestOAuth2CodeFlow(t *testing.T) { }, handleToken: basicIDTokenVerify, }, + { + // This test ensures that PKCE does fail with wrong code_verifier in "plain" mode + name: "PKCE with plain and wrong code_verifier", + authCodeOptions: []oauth2.AuthCodeOption{ + oauth2.SetAuthURLParam("code_challenge", "challenge123"), + }, + retreiveTokenOptions: []oauth2.AuthCodeOption{ + oauth2.SetAuthURLParam("code_verifier", "challenge124"), + }, + handleToken: basicIDTokenVerify, + tokenError: ErrorResponse{ + Error: errInvalidGrant, + StatusCode: http.StatusBadRequest, + }, + }, + { + // This test ensures that PKCE fail with wrong code_verifier in "S256" mode + name: "PKCE with S256 and wrong code_verifier", + authCodeOptions: []oauth2.AuthCodeOption{ + oauth2.SetAuthURLParam("code_challenge", "lyyl-X4a69qrqgEfUL8wodWic3Be9ZZ5eovBgIKKi-w"), + oauth2.SetAuthURLParam("code_challenge_method", "S256"), + }, + retreiveTokenOptions: []oauth2.AuthCodeOption{ + oauth2.SetAuthURLParam("code_verifier", "challenge124"), + }, + handleToken: basicIDTokenVerify, + tokenError: ErrorResponse{ + Error: errInvalidGrant, + StatusCode: http.StatusBadRequest, + }, + }, + { + // Ensure that when no PKCE flow was started on /auth + // we cannot switch to PKCE on /token + name: "No PKCE flow started", + authCodeOptions: []oauth2.AuthCodeOption{ + oauth2.SetAuthURLParam("code_challenge", "lyyl-X4a69qrqgEfUL8wodWic3Be9ZZ5eovBgIKKi-w"), + oauth2.SetAuthURLParam("code_challenge_method", "S256"), + }, + retreiveTokenOptions: []oauth2.AuthCodeOption{}, + handleToken: basicIDTokenVerify, + tokenError: ErrorResponse{ + Error: errInvalidGrant, + StatusCode: http.StatusBadRequest, + }, + }, + { + // Ensure that, when PKCE flow started on /auth + // we stay in PKCE flow on /token + name: "No PKCE flow started", + authCodeOptions: []oauth2.AuthCodeOption{ + // No PKCE call on /auth + }, + retreiveTokenOptions: []oauth2.AuthCodeOption{ + oauth2.SetAuthURLParam("code_verifier", "challenge123"), + }, + handleToken: basicIDTokenVerify, + tokenError: ErrorResponse{ + Error: errInvalidRequest, + StatusCode: http.StatusBadRequest, + }, + }, } for _, tc := range tests { @@ -567,6 +646,30 @@ func TestOAuth2CodeFlow(t *testing.T) { if code := q.Get("code"); code != "" { gotCode = true token, err := oauth2Config.Exchange(ctx, code, tc.retreiveTokenOptions...) + if tc.tokenError.StatusCode != 0 { + if err == nil { + t.Errorf("%s: DANGEROUS! got a token when we should not get one!", tc.name) + return + } + if rErr, ok := err.(*oauth2.RetrieveError); ok { + if rErr.Response.StatusCode != tc.tokenError.StatusCode { + t.Errorf("%s: got wrong StatusCode from server %d. expected %d", + tc.name, rErr.Response.StatusCode, tc.tokenError.StatusCode) + } + details := new(OAuth2ErrorResponse) + if err := json.Unmarshal(rErr.Body, details); err != nil { + t.Errorf("%s: could not parse return json: %s", tc.name, err) + return + } + if details.Error != tc.tokenError.Error { + t.Errorf("%s: got wrong Error in response: %s (%s). expected %s", + tc.name, details.Error, details.ErrorDescription, tc.tokenError.Error) + } + } else { + t.Errorf("%s: unexpedted error type: %s. expected *oauth2.RetrieveError", tc.name, reflect.TypeOf(err)) + } + return + } if err != nil { t.Errorf("failed to exchange code for token: %v", err) return From 0c8032807aef6f1530ed7fe6736b14f0719198b5 Mon Sep 17 00:00:00 2001 From: Bernd Eckstein Date: Fri, 24 Jul 2020 11:37:15 +0200 Subject: [PATCH 07/24] cleanup: extracted method checkErrorResponse and type TestDefinition * fixed connector being overwritten Signed-off-by: Bernd Eckstein --- server/server_test.go | 102 ++++++++++++++++++++++-------------------- 1 file changed, 54 insertions(+), 48 deletions(-) diff --git a/server/server_test.go b/server/server_test.go index 5d6a708ecd..0c9d57ec88 100644 --- a/server/server_test.go +++ b/server/server_test.go @@ -99,13 +99,13 @@ func newTestServer(ctx context.Context, t *testing.T, updateConfig func(c *Confi } s.URL = config.Issuer - connector := storage.Connector{ + connector1 := storage.Connector{ ID: "mock", Type: "mockCallback", Name: "Mock", ResourceVersion: "1", } - if err := config.Storage.CreateConnector(connector); err != nil { + if err := config.Storage.CreateConnector(connector1); err != nil { t.Fatalf("create connector: %v", err) } @@ -137,7 +137,7 @@ func newTestServerMultipleConnectors(ctx context.Context, t *testing.T, updateCo } s.URL = config.Issuer - connector := storage.Connector{ + connector1 := storage.Connector{ ID: "mock", Type: "mockCallback", Name: "Mock", @@ -149,7 +149,7 @@ func newTestServerMultipleConnectors(ctx context.Context, t *testing.T, updateCo Name: "Mock", ResourceVersion: "1", } - if err := config.Storage.CreateConnector(connector); err != nil { + if err := config.Storage.CreateConnector(connector1); err != nil { t.Fatalf("create connector: %v", err) } if err := config.Storage.CreateConnector(connector2); err != nil { @@ -217,6 +217,23 @@ type OAuth2ErrorResponse struct { ErrorURI string `json:"error_uri"` } +type TestDefinition struct { + name string + // If specified these set of scopes will be used during the test case. + scopes []string + // handleToken provides the OAuth2 token response for the integration test. + handleToken func(context.Context, *oidc.Provider, *oauth2.Config, *oauth2.Token) error + + // extra parameters to pass when requesting auth_code + authCodeOptions []oauth2.AuthCodeOption + + // extra parameters to pass when retrieving id token + retrieveTokenOptions []oauth2.AuthCodeOption + + // define an error response, when the test expects an error on the token endpoint + tokenError ErrorResponse +} + // TestOAuth2CodeFlow runs integration tests against a test server. The tests stand up a server // which requires no interaction to login, logs in through a test client, then passes the client // and returned token to the test. @@ -253,22 +270,7 @@ func TestOAuth2CodeFlow(t *testing.T) { return nil } - tests := []struct { - name string - // If specified these set of scopes will be used during the test case. - scopes []string - // handleToken provides the OAuth2 token response for the integration test. - handleToken func(context.Context, *oidc.Provider, *oauth2.Config, *oauth2.Token) error - - // extra parameters to pass when requesting auth_code - authCodeOptions []oauth2.AuthCodeOption - - // extra parameters to pass when retrieving id token - retreiveTokenOptions []oauth2.AuthCodeOption - - // define an error response, when the test expects a error on the token endpoint - tokenError ErrorResponse - }{ + tests := []TestDefinition{ { name: "verify ID Token", handleToken: basicIDTokenVerify, @@ -503,7 +505,7 @@ func TestOAuth2CodeFlow(t *testing.T) { authCodeOptions: []oauth2.AuthCodeOption{ oauth2.SetAuthURLParam("code_challenge", "challenge123"), }, - retreiveTokenOptions: []oauth2.AuthCodeOption{ + retrieveTokenOptions: []oauth2.AuthCodeOption{ oauth2.SetAuthURLParam("code_verifier", "challenge123"), }, handleToken: basicIDTokenVerify, @@ -515,7 +517,7 @@ func TestOAuth2CodeFlow(t *testing.T) { oauth2.SetAuthURLParam("code_challenge", "lyyl-X4a69qrqgEfUL8wodWic3Be9ZZ5eovBgIKKi-w"), oauth2.SetAuthURLParam("code_challenge_method", "S256"), }, - retreiveTokenOptions: []oauth2.AuthCodeOption{ + retrieveTokenOptions: []oauth2.AuthCodeOption{ oauth2.SetAuthURLParam("code_verifier", "challenge123"), }, handleToken: basicIDTokenVerify, @@ -526,7 +528,7 @@ func TestOAuth2CodeFlow(t *testing.T) { authCodeOptions: []oauth2.AuthCodeOption{ oauth2.SetAuthURLParam("code_challenge", "challenge123"), }, - retreiveTokenOptions: []oauth2.AuthCodeOption{ + retrieveTokenOptions: []oauth2.AuthCodeOption{ oauth2.SetAuthURLParam("code_verifier", "challenge124"), }, handleToken: basicIDTokenVerify, @@ -542,7 +544,7 @@ func TestOAuth2CodeFlow(t *testing.T) { oauth2.SetAuthURLParam("code_challenge", "lyyl-X4a69qrqgEfUL8wodWic3Be9ZZ5eovBgIKKi-w"), oauth2.SetAuthURLParam("code_challenge_method", "S256"), }, - retreiveTokenOptions: []oauth2.AuthCodeOption{ + retrieveTokenOptions: []oauth2.AuthCodeOption{ oauth2.SetAuthURLParam("code_verifier", "challenge124"), }, handleToken: basicIDTokenVerify, @@ -559,7 +561,7 @@ func TestOAuth2CodeFlow(t *testing.T) { oauth2.SetAuthURLParam("code_challenge", "lyyl-X4a69qrqgEfUL8wodWic3Be9ZZ5eovBgIKKi-w"), oauth2.SetAuthURLParam("code_challenge_method", "S256"), }, - retreiveTokenOptions: []oauth2.AuthCodeOption{}, + retrieveTokenOptions: []oauth2.AuthCodeOption{}, handleToken: basicIDTokenVerify, tokenError: ErrorResponse{ Error: errInvalidGrant, @@ -573,7 +575,7 @@ func TestOAuth2CodeFlow(t *testing.T) { authCodeOptions: []oauth2.AuthCodeOption{ // No PKCE call on /auth }, - retreiveTokenOptions: []oauth2.AuthCodeOption{ + retrieveTokenOptions: []oauth2.AuthCodeOption{ oauth2.SetAuthURLParam("code_verifier", "challenge123"), }, handleToken: basicIDTokenVerify, @@ -645,29 +647,9 @@ func TestOAuth2CodeFlow(t *testing.T) { // Grab code, exchange for token. if code := q.Get("code"); code != "" { gotCode = true - token, err := oauth2Config.Exchange(ctx, code, tc.retreiveTokenOptions...) + token, err := oauth2Config.Exchange(ctx, code, tc.retrieveTokenOptions...) if tc.tokenError.StatusCode != 0 { - if err == nil { - t.Errorf("%s: DANGEROUS! got a token when we should not get one!", tc.name) - return - } - if rErr, ok := err.(*oauth2.RetrieveError); ok { - if rErr.Response.StatusCode != tc.tokenError.StatusCode { - t.Errorf("%s: got wrong StatusCode from server %d. expected %d", - tc.name, rErr.Response.StatusCode, tc.tokenError.StatusCode) - } - details := new(OAuth2ErrorResponse) - if err := json.Unmarshal(rErr.Body, details); err != nil { - t.Errorf("%s: could not parse return json: %s", tc.name, err) - return - } - if details.Error != tc.tokenError.Error { - t.Errorf("%s: got wrong Error in response: %s (%s). expected %s", - tc.name, details.Error, details.ErrorDescription, tc.tokenError.Error) - } - } else { - t.Errorf("%s: unexpedted error type: %s. expected *oauth2.RetrieveError", tc.name, reflect.TypeOf(err)) - } + checkErrorResponse(err, t, tc) return } if err != nil { @@ -736,6 +718,30 @@ func TestOAuth2CodeFlow(t *testing.T) { } } +func checkErrorResponse(err error, t *testing.T, tc TestDefinition) { + if err == nil { + t.Errorf("%s: DANGEROUS! got a token when we should not get one!", tc.name) + return + } + if rErr, ok := err.(*oauth2.RetrieveError); ok { + if rErr.Response.StatusCode != tc.tokenError.StatusCode { + t.Errorf("%s: got wrong StatusCode from server %d. expected %d", + tc.name, rErr.Response.StatusCode, tc.tokenError.StatusCode) + } + details := new(OAuth2ErrorResponse) + if err := json.Unmarshal(rErr.Body, details); err != nil { + t.Errorf("%s: could not parse return json: %s", tc.name, err) + return + } + if tc.tokenError.Error != "" && details.Error != tc.tokenError.Error { + t.Errorf("%s: got wrong Error in response: %s (%s). expected %s", + tc.name, details.Error, details.ErrorDescription, tc.tokenError.Error) + } + } else { + t.Errorf("%s: unexpedted error type: %s. expected *oauth2.RetrieveError", tc.name, reflect.TypeOf(err)) + } +} + func TestOAuth2ImplicitFlow(t *testing.T) { ctx, cancel := context.WithCancel(context.Background()) defer cancel() From 118ab1035a5ac3e312f5323e8444ca3d3e301f0e Mon Sep 17 00:00:00 2001 From: Bernd Eckstein Date: Tue, 28 Jul 2020 11:19:09 +0200 Subject: [PATCH 08/24] /token endpoint: skip client_secret verification only for grand type authorization_code with PKCE extension Signed-off-by: Bernd Eckstein --- server/handlers.go | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/server/handlers.go b/server/handlers.go index b5a67cb9da..8c23849873 100644 --- a/server/handlers.go +++ b/server/handlers.go @@ -753,14 +753,17 @@ func (s *Server) handleToken(w http.ResponseWriter, r *http.Request) { } return } - if r.PostFormValue("code_verifier") != "" { + + grantType := r.PostFormValue("grant_type") + codeVerifier := r.PostFormValue("code_verifier") + + if grantType == grantTypeAuthorizationCode && codeVerifier != "" { // RFC 7636 (PKCE) if code_verifier is received, use PKCE and not the client_secret } else if client.Secret != clientSecret { s.tokenErrHelper(w, errInvalidClient, "Invalid client credentials.", http.StatusUnauthorized) return } - grantType := r.PostFormValue("grant_type") switch grantType { case grantTypeAuthorizationCode: s.handleAuthCode(w, r, client) From c58264de62f275ba9afc82b0c7cf8efa86ca7fed Mon Sep 17 00:00:00 2001 From: Bernd Eckstein Date: Tue, 25 Aug 2020 16:59:18 +0200 Subject: [PATCH 09/24] Allow "Authorization" header in CORS handlers * Adds "Authorization" to the default CORS headers{"Accept", "Accept-Language", "Content-Language", "Origin"} Signed-off-by: Bernd Eckstein --- server/server.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/server/server.go b/server/server.go index a0a075fbfc..9a82b9e094 100644 --- a/server/server.go +++ b/server/server.go @@ -284,7 +284,8 @@ func newServer(ctx context.Context, c Config, rotationStrategy rotationStrategy) var handler http.Handler = h if len(c.AllowedOrigins) > 0 { corsOption := handlers.AllowedOrigins(c.AllowedOrigins) - handler = handlers.CORS(corsOption)(handler) + corsHeaders := handlers.AllowedHeaders([]string{"Authorization"}) + handler = handlers.CORS(corsOption, corsHeaders)(handler) } r.Handle(path.Join(issuerURL.Path, p), instrumentHandlerCounter(p, handler)) } From 739beefbdcfd386c65bd4df833fee9a332b90f4e Mon Sep 17 00:00:00 2001 From: Bernd Eckstein Date: Mon, 31 Aug 2020 10:50:28 +0200 Subject: [PATCH 10/24] Add "code_challenge_methods_supported" to discovery endpoint discovery endpoint /dex/.well-known/openid-configuration now has the following entry: "code_challenge_methods_supported": [ "S256", "plain" ] Signed-off-by: Bernd Eckstein --- server/handlers.go | 42 ++++++++++++++++++++++-------------------- 1 file changed, 22 insertions(+), 20 deletions(-) diff --git a/server/handlers.go b/server/handlers.go index 1b609a8f47..ee4ff76138 100644 --- a/server/handlers.go +++ b/server/handlers.go @@ -155,30 +155,32 @@ func (s *Server) handlePublicKeys(w http.ResponseWriter, r *http.Request) { } type discovery struct { - Issuer string `json:"issuer"` - Auth string `json:"authorization_endpoint"` - Token string `json:"token_endpoint"` - Keys string `json:"jwks_uri"` - UserInfo string `json:"userinfo_endpoint"` - ResponseTypes []string `json:"response_types_supported"` - Subjects []string `json:"subject_types_supported"` - IDTokenAlgs []string `json:"id_token_signing_alg_values_supported"` - Scopes []string `json:"scopes_supported"` - AuthMethods []string `json:"token_endpoint_auth_methods_supported"` - Claims []string `json:"claims_supported"` + Issuer string `json:"issuer"` + Auth string `json:"authorization_endpoint"` + Token string `json:"token_endpoint"` + Keys string `json:"jwks_uri"` + UserInfo string `json:"userinfo_endpoint"` + ResponseTypes []string `json:"response_types_supported"` + Subjects []string `json:"subject_types_supported"` + IDTokenAlgs []string `json:"id_token_signing_alg_values_supported"` + CodeChallengeAlgs []string `json:"code_challenge_methods_supported"` + Scopes []string `json:"scopes_supported"` + AuthMethods []string `json:"token_endpoint_auth_methods_supported"` + Claims []string `json:"claims_supported"` } func (s *Server) discoveryHandler() (http.HandlerFunc, error) { d := discovery{ - Issuer: s.issuerURL.String(), - Auth: s.absURL("/auth"), - Token: s.absURL("/token"), - Keys: s.absURL("/keys"), - UserInfo: s.absURL("/userinfo"), - Subjects: []string{"public"}, - IDTokenAlgs: []string{string(jose.RS256)}, - Scopes: []string{"openid", "email", "groups", "profile", "offline_access"}, - AuthMethods: []string{"client_secret_basic"}, + Issuer: s.issuerURL.String(), + Auth: s.absURL("/auth"), + Token: s.absURL("/token"), + Keys: s.absURL("/keys"), + UserInfo: s.absURL("/userinfo"), + Subjects: []string{"public"}, + IDTokenAlgs: []string{string(jose.RS256)}, + CodeChallengeAlgs: []string{CodeChallengeMethodS256, CodeChallengeMethodPlain}, + Scopes: []string{"openid", "email", "groups", "profile", "offline_access"}, + AuthMethods: []string{"client_secret_basic"}, Claims: []string{ "aud", "email", "email_verified", "exp", "iat", "iss", "locale", "name", "sub", From 1683a178a1303b4976a790f6673ed0b9d9c22472 Mon Sep 17 00:00:00 2001 From: Bernd Eckstein Date: Thu, 10 Sep 2020 16:09:30 +0200 Subject: [PATCH 11/24] Updated tests (mixed-up comments), added a PKCE test * @asoorm added test that checks if downgrade to "plain" on /token endpoint Signed-off-by: Bernd Eckstein --- server/server_test.go | 38 ++++++++++++++++++++++++++++---------- 1 file changed, 28 insertions(+), 10 deletions(-) diff --git a/server/server_test.go b/server/server_test.go index 007a5df13a..a2379a2680 100644 --- a/server/server_test.go +++ b/server/server_test.go @@ -241,7 +241,6 @@ type OAuth2ErrorResponse struct { ErrorURI string `json:"error_uri"` } - func makeOAuth2Tests(clientID string, clientSecret string, now func() time.Time) oauth2Tests { requestedScopes := []string{oidc.ScopeOpenID, "email", "profile", "groups", "offline_access"} @@ -559,24 +558,26 @@ func makeOAuth2Tests(clientID string, clientSecret string, now func() time.Time) }, }, { - // Ensure that when no PKCE flow was started on /auth - // we cannot switch to PKCE on /token - name: "No PKCE flow started on /auth", + // Ensure that, when PKCE flow started on /auth + // we stay in PKCE flow on /token + name: "PKCE flow expected on /token", authCodeOptions: []oauth2.AuthCodeOption{ oauth2.SetAuthURLParam("code_challenge", "lyyl-X4a69qrqgEfUL8wodWic3Be9ZZ5eovBgIKKi-w"), oauth2.SetAuthURLParam("code_challenge_method", "S256"), }, - retrieveTokenOptions: []oauth2.AuthCodeOption{}, - handleToken: basicIDTokenVerify, + retrieveTokenOptions: []oauth2.AuthCodeOption{ + // No PKCE call on /token + }, + handleToken: basicIDTokenVerify, tokenError: ErrorResponse{ Error: errInvalidGrant, StatusCode: http.StatusBadRequest, }, }, { - // Ensure that, when PKCE flow started on /auth - // we stay in PKCE flow on /token - name: "PKCE flow expected on /token", + // Ensure that when no PKCE flow was started on /auth + // we cannot switch to PKCE on /token + name: "No PKCE flow started on /auth", authCodeOptions: []oauth2.AuthCodeOption{ // No PKCE call on /auth }, @@ -589,6 +590,23 @@ func makeOAuth2Tests(clientID string, clientSecret string, now func() time.Time) StatusCode: http.StatusBadRequest, }, }, + { + // Make sure that, when we start with "S256" on /auth, we cannot downgrade to "plain" on /token + name: "PKCE with S256 and try to downgrade to plain", + authCodeOptions: []oauth2.AuthCodeOption{ + oauth2.SetAuthURLParam("code_challenge", "lyyl-X4a69qrqgEfUL8wodWic3Be9ZZ5eovBgIKKi-w"), + oauth2.SetAuthURLParam("code_challenge_method", "S256"), + }, + retrieveTokenOptions: []oauth2.AuthCodeOption{ + oauth2.SetAuthURLParam("code_verifier", "lyyl-X4a69qrqgEfUL8wodWic3Be9ZZ5eovBgIKKi-w"), + oauth2.SetAuthURLParam("code_challenge_method", "plain"), + }, + handleToken: basicIDTokenVerify, + tokenError: ErrorResponse{ + Error: errInvalidGrant, + StatusCode: http.StatusBadRequest, + }, + }, }, } } @@ -1577,4 +1595,4 @@ func TestOAuth2DeviceFlow(t *testing.T) { } }() } -} \ No newline at end of file +} From a1aab00d2dc5bc3b53646885549911b2568975cb Mon Sep 17 00:00:00 2001 From: Bernd Eckstein Date: Wed, 16 Sep 2020 10:22:49 +0200 Subject: [PATCH 12/24] remove redefinition of providedCodeVerifier, fixed spelling (#6) Signed-off-by: Bernd Eckstein Signed-off-by: Bernd Eckstein --- server/handlers.go | 2 -- server/server_test.go | 2 +- 2 files changed, 1 insertion(+), 3 deletions(-) diff --git a/server/handlers.go b/server/handlers.go index d2a4652035..b2d0bd86b5 100644 --- a/server/handlers.go +++ b/server/handlers.go @@ -816,13 +816,11 @@ func (s *Server) handleAuthCode(w http.ResponseWriter, r *http.Request, client s providedCodeVerifier := r.PostFormValue("code_verifier") if providedCodeVerifier != "" && codeChallengeFromStorage != "" { - providedCodeVerifier := r.PostFormValue("code_verifier") calculatedCodeChallenge, err := s.calculateCodeChallenge(providedCodeVerifier, authCode.CodeChallenge.CodeChallengeMethod) if err != nil { s.tokenErrHelper(w, errServerError, "", http.StatusInternalServerError) return } - if codeChallengeFromStorage != calculatedCodeChallenge { s.tokenErrHelper(w, errInvalidGrant, "Invalid code_verifier.", http.StatusBadRequest) return diff --git a/server/server_test.go b/server/server_test.go index a2379a2680..7783d891e3 100644 --- a/server/server_test.go +++ b/server/server_test.go @@ -1332,7 +1332,7 @@ func checkErrorResponse(err error, t *testing.T, tc test) { tc.name, details.Error, details.ErrorDescription, tc.tokenError.Error) } } else { - t.Errorf("%s: unexpedted error type: %s. expected *oauth2.RetrieveError", tc.name, reflect.TypeOf(err)) + t.Errorf("%s: unexpected error type: %s. expected *oauth2.RetrieveError", tc.name, reflect.TypeOf(err)) } } From 7251cd6923d0874b6a4f9bc2a46d3d5aac01f55f Mon Sep 17 00:00:00 2001 From: Bernd Eckstein Date: Thu, 17 Sep 2020 11:17:16 +0200 Subject: [PATCH 13/24] Rename struct CodeChallenge to PKCE Signed-off-by: Bernd Eckstein --- go.mod | 10 +++++++--- go.sum | 31 ++++++++++++++++++++++++++++++ server/handlers.go | 6 +++--- server/oauth2.go | 2 +- storage/conformance/conformance.go | 8 ++++---- storage/etcd/types.go | 10 +++++----- storage/kubernetes/types.go | 12 ++++++------ storage/sql/crud.go | 10 +++++----- storage/storage.go | 8 +++++--- 9 files changed, 67 insertions(+), 30 deletions(-) diff --git a/go.mod b/go.mod index 22480a0170..985baaeb77 100644 --- a/go.mod +++ b/go.mod @@ -5,11 +5,14 @@ go 1.14 require ( github.com/Microsoft/hcsshim v0.8.7 // indirect github.com/beevik/etree v1.1.0 + github.com/coreos/go-etcd v2.0.0+incompatible // indirect github.com/coreos/go-oidc v2.2.1+incompatible github.com/coreos/go-semver v0.3.0 // indirect github.com/coreos/go-systemd v0.0.0-20190719114852-fd7a80b32e1f // indirect github.com/coreos/pkg v0.0.0-20180928190104-399ea9e2e55f // indirect + github.com/cpuguy83/go-md2man v1.0.10 // indirect github.com/dexidp/dex/api/v2 v2.0.0 + github.com/dexidp/dex/examples v0.0.0-20200916154904-458059cc89d8 // indirect github.com/felixge/httpsnoop v1.0.1 github.com/ghodss/yaml v1.0.0 github.com/go-sql-driver/mysql v1.5.0 @@ -28,22 +31,23 @@ require ( github.com/prometheus/client_golang v1.4.0 github.com/russellhaering/goxmldsig v0.0.0-20180430223755-7acd5e4a6ef7 github.com/sirupsen/logrus v1.4.2 - github.com/spf13/cobra v0.0.5 + github.com/spf13/cobra v1.0.0 github.com/stretchr/testify v1.4.0 github.com/testcontainers/testcontainers-go v0.0.9 github.com/tmc/grpc-websocket-proxy v0.0.0-20190109142713-0ad062ec5ee5 // indirect + github.com/ugorji/go/codec v0.0.0-20181204163529-d75b2dcb6bc8 // indirect go.etcd.io/etcd v0.0.0-20191023171146-3cf2f69b5738 go.uber.org/atomic v1.4.0 // indirect golang.org/x/crypto v0.0.0-20190701094942-4def268fd1a4 golang.org/x/net v0.0.0-20190813141303-74dc4d7220e7 - golang.org/x/oauth2 v0.0.0-20190604053449-0f29369cfe45 + golang.org/x/oauth2 v0.0.0-20200107190931-bf48bf16ab8d golang.org/x/time v0.0.0-20190308202827-9d24e82272b4 // indirect google.golang.org/api v0.15.0 google.golang.org/appengine v1.6.1 // indirect google.golang.org/grpc v1.26.0 gopkg.in/asn1-ber.v1 v1.0.0-20181015200546-f715ec2f112d // indirect gopkg.in/ldap.v2 v2.5.1 - gopkg.in/square/go-jose.v2 v2.4.1 + gopkg.in/square/go-jose.v2 v2.5.1 sigs.k8s.io/testing_frameworks v0.1.2 ) diff --git a/go.sum b/go.sum index 54abd1b102..09f0c48e74 100644 --- a/go.sum +++ b/go.sum @@ -13,6 +13,7 @@ github.com/Microsoft/go-winio v0.4.15-0.20190919025122-fc70bd9a86b5 h1:ygIc8M6tr github.com/Microsoft/go-winio v0.4.15-0.20190919025122-fc70bd9a86b5/go.mod h1:tTuCMEN+UleMWgg9dVx4Hu52b1bJo+59jBh3ajtinzw= github.com/Microsoft/hcsshim v0.8.7 h1:ptnOoufxGSzauVTsdE+wMYnCWA301PdoN4xg5oRdZpg= github.com/Microsoft/hcsshim v0.8.7/go.mod h1:OHd7sQqRFrYd3RmSgbgji+ctCwkbq2wbEYNSzOYtcBQ= +github.com/OneOfOne/xxhash v1.2.2/go.mod h1:HSdplMjZKSmBqAxg5vPj2TmRDmfkzw+cTzAElWljhcU= github.com/alecthomas/template v0.0.0-20160405071501-a0175ee3bccc/go.mod h1:LOuyumcjzFXgccqObfd/Ljyb9UuFJ6TxHnclSeseNhc= github.com/alecthomas/template v0.0.0-20190718012654-fb15b899a751/go.mod h1:LOuyumcjzFXgccqObfd/Ljyb9UuFJ6TxHnclSeseNhc= github.com/alecthomas/units v0.0.0-20151022065526-2efee857e7cf/go.mod h1:ybxpYRFXyAe+OPACYpWeL0wqObRcbAqCMya13uyzqw0= @@ -30,6 +31,8 @@ github.com/blang/semver v3.1.0+incompatible/go.mod h1:kRBLl5iJ+tD4TcOOxsy/0fnweb github.com/cenkalti/backoff v2.2.1+incompatible h1:tNowT99t7UNflLxfYYSlKYsBpXdEet03Pg2g16Swow4= github.com/cenkalti/backoff v2.2.1+incompatible/go.mod h1:90ReRw6GdpyfrHakVjL/QHaoyV4aDUVVkXQJJJ3NXXM= github.com/census-instrumentation/opencensus-proto v0.2.1/go.mod h1:f6KPmirojxKA12rnyqOA5BBL4O983OfeGPqjHWSTneU= +github.com/cespare/xxhash v1.1.0 h1:a6HrQnmkObjyL+Gs60czilIUGqrzKutQD6XZog3p+ko= +github.com/cespare/xxhash v1.1.0/go.mod h1:XrSqR1VqqWfGrhpAt58auRo0WTKS1nRRg3ghfAqPWnc= github.com/cespare/xxhash/v2 v2.1.1 h1:6MnRN8NT7+YBpUIWxHtefFZOKTAPgGjpQSxqLNn0+qY= github.com/cespare/xxhash/v2 v2.1.1/go.mod h1:VGX0DQ3Q6kWi7AoAeZDth3/j3BFtOZR5XLFGgcrjCOs= github.com/client9/misspell v0.3.4/go.mod h1:qj6jICC3Q7zFZvVWo7KLAzC3yx5G7kyvSDkc90ppPyw= @@ -44,6 +47,7 @@ github.com/containerd/fifo v0.0.0-20190226154929-a9fb20d87448/go.mod h1:ODA38xgv github.com/containerd/go-runc v0.0.0-20180907222934-5a6d9f37cfa3/go.mod h1:IV7qH3hrUgRmyYrtgEeGWJfWbgcHL9CSRruz2Vqcph0= github.com/containerd/ttrpc v0.0.0-20190828154514-0e0f228740de/go.mod h1:PvCDdDGpgqzQIzDW1TphrGLssLDZp2GuS+X5DkEJB8o= github.com/containerd/typeurl v0.0.0-20180627222232-a93fcdb778cd/go.mod h1:Cm3kwCdlkCfMSHURc+r6fwoGH6/F1hH3S4sg0rLFWPc= +github.com/coreos/bbolt v1.3.2/go.mod h1:iRUV2dpdMOn7Bo10OQBFzIJO9kkE559Wcmn+qkEiiKk= github.com/coreos/etcd v3.3.10+incompatible/go.mod h1:uF7uidLiAD3TWHmW31ZFd/JWoc32PjwdhPthX9715RE= github.com/coreos/go-etcd v2.0.0+incompatible/go.mod h1:Jez6KQU2B/sWsbdaef3ED8NzMklzPG4d5KIOhIy30Tk= github.com/coreos/go-oidc v2.2.1+incompatible h1:mh48q/BqXqgjVHpy2ZY7WnWAbenxRjsz9N1i1YxjHAk= @@ -60,12 +64,16 @@ github.com/coreos/pkg v0.0.0-20160727233714-3ac0863d7acf/go.mod h1:E3G3o1h8I7cfc github.com/coreos/pkg v0.0.0-20180928190104-399ea9e2e55f h1:lBNOc5arjvs8E5mO2tbpBpLoyyu8B6e44T7hJy6potg= github.com/coreos/pkg v0.0.0-20180928190104-399ea9e2e55f/go.mod h1:E3G3o1h8I7cfcXa63jLwjI0eiQQMgzzUDFVpN/nH/eA= github.com/cpuguy83/go-md2man v1.0.10/go.mod h1:SmD6nW6nTyfqj6ABTjUi3V3JVMnlJmwcJI5acqYI6dE= +github.com/cpuguy83/go-md2man/v2 v2.0.0/go.mod h1:maD7wRr/U5Z6m/iR4s+kqSMx2CaBsrgA7czyZG/E6dU= github.com/creack/pty v1.1.7/go.mod h1:lj5s0c3V2DBrqTV7llrYr5NG6My20zk30Fl46Y7DoTY= github.com/davecgh/go-spew v1.1.0/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38= github.com/davecgh/go-spew v1.1.1 h1:vj9j/u1bqnvCEfJOwUhtlOARqs3+rkHYY13jYWTU97c= github.com/davecgh/go-spew v1.1.1/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38= +github.com/dexidp/dex/examples v0.0.0-20200916154904-458059cc89d8 h1:igNt1Em0/r1PeqK7KkXXd/eP/nlKwSvtvKDe+/ZfKfY= +github.com/dexidp/dex/examples v0.0.0-20200916154904-458059cc89d8/go.mod h1:7vocppXTfbsaYcPDFuo9Oj8N0FDnsKlt/3lSL+opISY= github.com/dgrijalva/jwt-go v3.2.0+incompatible h1:7qlOGliEKZXTDg6OTjfoBKDXWrumCAMpl/TFQ4/5kLM= github.com/dgrijalva/jwt-go v3.2.0+incompatible/go.mod h1:E3ru+11k8xSBh+hMPgOLZmtrrCbhqsmaPHjLKYnJCaQ= +github.com/dgryski/go-sip13 v0.0.0-20181026042036-e10d5fee7954/go.mod h1:vAd38F8PWV+bWy6jNmig1y/TA+kYO4g3RSRF0IAv0no= github.com/docker/distribution v2.7.1-0.20190205005809-0d3efadf0154+incompatible h1:dvc1KSkIYTVjZgHf/CTC2diTYC8PzhaA5sFISRfNVrE= github.com/docker/distribution v2.7.1-0.20190205005809-0d3efadf0154+incompatible/go.mod h1:J2gT2udsDAN96Uj4KfcMRqY0/ypR+oyYUYmja8H+y+w= github.com/docker/docker v0.7.3-0.20190506211059-b20a14b54661 h1:ZuxGvIvF01nfc/G9RJ5Q7Va1zQE2WJyG18Zv3DqCEf4= @@ -107,6 +115,7 @@ github.com/gogo/protobuf v1.2.1/go.mod h1:hp+jE20tsWTFYpLwKvXlhS1hjn+gTNwPg2I6zV github.com/golang/glog v0.0.0-20160126235308-23def4e6c14b h1:VKtxabqXZkF25pY9ekfRL6a582T4P37/31XEstQ5p58= github.com/golang/glog v0.0.0-20160126235308-23def4e6c14b/go.mod h1:SBH7ygxi8pfUlaOkMMuAQtPIUF8ecWP5IEl/CR7VP2Q= github.com/golang/groupcache v0.0.0-20160516000752-02826c3e7903/go.mod h1:cIg4eruTrX1D+g88fzRXU5OdNfaM+9IcxsU14FzY7Hc= +github.com/golang/groupcache v0.0.0-20190129154638-5b532d6fd5ef/go.mod h1:cIg4eruTrX1D+g88fzRXU5OdNfaM+9IcxsU14FzY7Hc= github.com/golang/groupcache v0.0.0-20190702054246-869f871628b6 h1:ZgQEtGgCBiWRM39fZuwSd1LwSqqSW0hOdXCYYDX0R3I= github.com/golang/groupcache v0.0.0-20190702054246-869f871628b6/go.mod h1:cIg4eruTrX1D+g88fzRXU5OdNfaM+9IcxsU14FzY7Hc= github.com/golang/mock v1.1.1/go.mod h1:oTYuIxOrZwtPieC+H1uAHpcLFnEyAGVDL/k47Jfbm0A= @@ -143,10 +152,12 @@ github.com/gorilla/mux v1.7.3/go.mod h1:1lud6UwP+6orDFRuTfBEV8e9/aOM/c4fVVCaMa2z github.com/gorilla/websocket v0.0.0-20170926233335-4201258b820c/go.mod h1:E7qHFY5m1UJ88s3WnNqhKjPHQ0heANvMoAMk2YaljkQ= github.com/gorilla/websocket v1.4.0 h1:WDFjx/TMzVgy9VdMMQi2K2Emtwi2QcUQsztZ/zLaH/Q= github.com/gorilla/websocket v1.4.0/go.mod h1:E7qHFY5m1UJ88s3WnNqhKjPHQ0heANvMoAMk2YaljkQ= +github.com/grpc-ecosystem/go-grpc-middleware v1.0.0/go.mod h1:FiyG127CGDf3tlThmgyCl78X/SZQqEOJBCDaAfeWzPs= github.com/grpc-ecosystem/go-grpc-middleware v1.0.1-0.20190118093823-f849b5445de4 h1:z53tR0945TRRQO/fLEVPI6SMv7ZflF0TEaTAoU7tOzg= github.com/grpc-ecosystem/go-grpc-middleware v1.0.1-0.20190118093823-f849b5445de4/go.mod h1:FiyG127CGDf3tlThmgyCl78X/SZQqEOJBCDaAfeWzPs= github.com/grpc-ecosystem/go-grpc-prometheus v1.2.0 h1:Ovs26xHkKqVztRpIrF/92BcuyuQ/YW4NSIpoGtfXNho= github.com/grpc-ecosystem/go-grpc-prometheus v1.2.0/go.mod h1:8NvIoxWQoOIhqOTXgfV/d3M/q6VIi02HzZEHgUlZvzk= +github.com/grpc-ecosystem/grpc-gateway v1.9.0/go.mod h1:vNeuVxBJEsws4ogUvrchl83t/GYV9WGTSLVdBhOQFDY= github.com/grpc-ecosystem/grpc-gateway v1.9.5 h1:UImYN5qQ8tuGpGE16ZmjvcTtTw24zw1QAp/SlnNrZhI= github.com/grpc-ecosystem/grpc-gateway v1.9.5/go.mod h1:vNeuVxBJEsws4ogUvrchl83t/GYV9WGTSLVdBhOQFDY= github.com/hashicorp/errwrap v0.0.0-20141028054710-7554cd9344ce/go.mod h1:YH+1FKiLXxHSkmPseP+kNlulaMuP3n2brvKWEqk/Jc4= @@ -202,6 +213,7 @@ github.com/modern-go/reflect2 v1.0.1/go.mod h1:bx2lNnkwVCuqBIxFjflWJWanXIb3Rllmb github.com/morikuni/aec v0.0.0-20170113033406-39771216ff4c h1:nXxl5PrvVm2L/wCy8dQu6DMTwH4oIuGN8GJDAlqDdVE= github.com/morikuni/aec v0.0.0-20170113033406-39771216ff4c/go.mod h1:BbKIizmSmc5MMPqRYbxO4ZU0S0+P200+tUnFx7PXmsc= github.com/mwitkow/go-conntrack v0.0.0-20161129095857-cc309e4a2223/go.mod h1:qRWi+5nqEBWmkhHvq77mSJWrCKwh8bxhgT7d/eI7P4U= +github.com/oklog/ulid v1.3.1/go.mod h1:CirwcVhetQ6Lv90oh/F+FBtV6XMibvdAFo93nm5qn4U= github.com/olekukonko/tablewriter v0.0.0-20170122224234-a0225b3f23b5/go.mod h1:vsDQFd/mU46D+Z4whnwzcISnGGzXWMclvtLoiIKAKIo= github.com/onsi/ginkgo v1.4.0/go.mod h1:lLunBs/Ym6LB5Z9jYTR76FiuTmxDTDusOGeTQH+WWjE= github.com/onsi/ginkgo v1.6.0/go.mod h1:lLunBs/Ym6LB5Z9jYTR76FiuTmxDTDusOGeTQH+WWjE= @@ -229,6 +241,7 @@ github.com/pmezard/go-difflib v1.0.0/go.mod h1:iKH77koFhYxTK1pcRnkKkqfTogsbg7gZN github.com/pquerna/cachecontrol v0.0.0-20180517163645-1555304b9b35 h1:J9b7z+QKAmPf4YLrFg6oQUotqHQeUNWwkvo7jZp1GLU= github.com/pquerna/cachecontrol v0.0.0-20180517163645-1555304b9b35/go.mod h1:prYjPmNq4d1NPVmpShWobRqXY3q7Vp+80DqgxxUrUIA= github.com/prometheus/client_golang v0.9.1/go.mod h1:7SWBe2y4D6OKWSNQJUaRYU/AaXPKyh/dDVn+NZz0KFw= +github.com/prometheus/client_golang v0.9.3/go.mod h1:/TN21ttK/J9q6uSwhBd54HahCDft0ttaMvbicHlPoso= github.com/prometheus/client_golang v1.0.0 h1:vrDKnkGzuGvhNAL56c7DBz29ZL+KxnoR0x7enabFceM= github.com/prometheus/client_golang v1.0.0/go.mod h1:db9x61etRT2tGnBNRi70OPL5FsnadC4Ky3P0J6CfImo= github.com/prometheus/client_golang v1.4.0 h1:YVIb/fVcOTMSqtqZWSKnHpSLBxu8DKgxq8z6RuBZwqI= @@ -239,37 +252,47 @@ github.com/prometheus/client_model v0.0.0-20190129233127-fd36f4220a90/go.mod h1: github.com/prometheus/client_model v0.0.0-20190812154241-14fe0d1b01d4/go.mod h1:xMI15A0UPsDsEKsMN9yxemIoYk6Tm2C1GtYGdfGttqA= github.com/prometheus/client_model v0.2.0 h1:uq5h0d+GuxiXLJLNABMgp2qUWDPiLvgCzz2dUR+/W/M= github.com/prometheus/client_model v0.2.0/go.mod h1:xMI15A0UPsDsEKsMN9yxemIoYk6Tm2C1GtYGdfGttqA= +github.com/prometheus/common v0.0.0-20181113130724-41aa239b4cce/go.mod h1:daVV7qP5qjZbuso7PdcryaAu0sAZbrN9i7WWcTMWvro= +github.com/prometheus/common v0.4.0/go.mod h1:TNfzLD0ON7rHzMJeJkieUDPYmFC7Snx/y86RQel1bk4= github.com/prometheus/common v0.4.1/go.mod h1:TNfzLD0ON7rHzMJeJkieUDPYmFC7Snx/y86RQel1bk4= github.com/prometheus/common v0.9.1 h1:KOMtN28tlbam3/7ZKEYKHhKoJZYYj3gMH4uc62x7X7U= github.com/prometheus/common v0.9.1/go.mod h1:yhUN8i9wzaXS3w1O07YhxHEBxD+W35wd8bs7vj7HSQ4= github.com/prometheus/procfs v0.0.0-20181005140218-185b4288413d/go.mod h1:c3At6R/oaqEKCNdg8wHV1ftS6bRYblBhIjjI8uT2IGk= +github.com/prometheus/procfs v0.0.0-20190507164030-5867b95ac084/go.mod h1:TjEm7ze935MbeOT/UhFTIMYKhuLP4wbCsTZCD3I8kEA= github.com/prometheus/procfs v0.0.2/go.mod h1:TjEm7ze935MbeOT/UhFTIMYKhuLP4wbCsTZCD3I8kEA= github.com/prometheus/procfs v0.0.5 h1:3+auTFlqw+ZaQYJARz6ArODtkaIwtvBTx3N2NehQlL8= github.com/prometheus/procfs v0.0.5/go.mod h1:4A/X28fw3Fc593LaREMrKMqOKvUAntwMDaekg4FpcdQ= github.com/prometheus/procfs v0.0.8 h1:+fpWZdT24pJBiqJdAwYBjPSk+5YmQzYNPYzQsdzLkt8= github.com/prometheus/procfs v0.0.8/go.mod h1:7Qr8sr6344vo1JqZ6HhLceV9o3AJ1Ff+GxbHq6oeK9A= +github.com/prometheus/tsdb v0.7.1/go.mod h1:qhTCs0VvXwvX/y3TZrWD7rabWM+ijKTux40TwIPHuXU= github.com/rogpeppe/fastuuid v0.0.0-20150106093220-6724a57986af/go.mod h1:XWv6SoW27p1b0cqNHllgS5HIMJraePCO15w5zCzIWYg= github.com/russellhaering/goxmldsig v0.0.0-20180430223755-7acd5e4a6ef7 h1:J4AOUcOh/t1XbQcJfkEqhzgvMJ2tDxdCVvmHxW5QXao= github.com/russellhaering/goxmldsig v0.0.0-20180430223755-7acd5e4a6ef7/go.mod h1:Oz4y6ImuOQZxynhbSXk7btjEfNBtGlj2dcaOvXl2FSM= github.com/russross/blackfriday v1.5.2/go.mod h1:JO/DiYxRf+HjHt06OyowR9PTA263kcR/rfWxYHBV53g= +github.com/russross/blackfriday/v2 v2.0.1/go.mod h1:+Rmxgy9KzJVeS9/2gXHxylqXiyQDYRxCVz55jmeOWTM= github.com/satori/go.uuid v1.2.0 h1:0uYX9dsZ2yD7q2RtLRtPSdGDWzjeM3TbMJP9utgA0ww= github.com/satori/go.uuid v1.2.0/go.mod h1:dA0hQrYB0VpLJoorglMZABFdXlWrHn1NEOzdhQKdks0= +github.com/shurcooL/sanitized_anchor_name v1.0.0/go.mod h1:1NzhyTcUVG4SuEtjjoZeVRXNmyL/1OwPU0+IJeTBvfc= github.com/sirupsen/logrus v1.2.0/go.mod h1:LxeOpSwHxABJmUn/MG1IvRgCAasNZTLOkJPxbbu5VWo= github.com/sirupsen/logrus v1.4.1/go.mod h1:ni0Sbl8bgC9z8RoU9G6nDWqqs/fq4eDPysMBDgk/93Q= github.com/sirupsen/logrus v1.4.2 h1:SPIRibHv4MatM3XXNO2BJeFLZwZ2LvZgfQ5+UNI2im4= github.com/sirupsen/logrus v1.4.2/go.mod h1:tLMulIdttU9McNUspp0xgXVQah82FyeX6MwdIuYE2rE= github.com/soheilhy/cmux v0.1.4 h1:0HKaf1o97UwFjHH9o5XsHUOF+tqmdA7KEzXLpiyaw0E= github.com/soheilhy/cmux v0.1.4/go.mod h1:IM3LyeVVIOuxMH7sFAkER9+bJ4dT7Ms6E4xg4kGIyLM= +github.com/spaolacci/murmur3 v0.0.0-20180118202830-f09979ecbc72/go.mod h1:JwIasOWyU6f++ZhiEuf87xNszmSA2myDM2Kzu9HwQUA= github.com/spf13/afero v1.1.2/go.mod h1:j4pytiNVoe2o6bmDsKpLACNPDBIoEAkihy7loJ1B0CQ= github.com/spf13/cast v1.3.0/go.mod h1:Qx5cxh0v+4UWYiBimWS+eyWzqEqokIECu5etghLkUJE= github.com/spf13/cobra v0.0.3/go.mod h1:1l0Ry5zgKvJasoi3XT1TypsSe7PqH0Sj9dhYf7v3XqQ= github.com/spf13/cobra v0.0.5 h1:f0B+LkLX6DtmRH1isoNA9VTtNUK9K8xYd28JNNfOv/s= github.com/spf13/cobra v0.0.5/go.mod h1:3K3wKZymM7VvHMDS9+Akkh4K60UwM26emMESw8tLCHU= +github.com/spf13/cobra v1.0.0 h1:6m/oheQuQ13N9ks4hubMG6BnvwOeaJrqSPLahSnczz8= +github.com/spf13/cobra v1.0.0/go.mod h1:/6GTrnGXV9HjY+aR4k0oJ5tcvakLuG6EuKReYlHNrgE= github.com/spf13/jwalterweatherman v1.0.0/go.mod h1:cQK4TGJAtQXfYWX+Ddv3mKDzgVb68N+wFjFa4jdeBTo= github.com/spf13/pflag v1.0.1/go.mod h1:DYY7MBk1bdzusC3SYhjObp+wFpr4gzcvqqNjLnInEg4= github.com/spf13/pflag v1.0.3 h1:zPAT6CGy6wXeQ7NtTnaTerfKOsV6V6F8agHXFiazDkg= github.com/spf13/pflag v1.0.3/go.mod h1:DYY7MBk1bdzusC3SYhjObp+wFpr4gzcvqqNjLnInEg4= github.com/spf13/viper v1.3.2/go.mod h1:ZiWeW+zYFKm7srdB9IoDzzZXaJaI5eL9QjNiN/DMA2s= +github.com/spf13/viper v1.4.0/go.mod h1:PTJ7Z/lr49W6bUbkmS1V3by4uWynFiR9p7+dSq/yZzE= github.com/stretchr/objx v0.1.0/go.mod h1:HFkY916IF+rwdDfMAkV7OtwuqBVzrE8GR6GFx+wExME= github.com/stretchr/objx v0.1.1/go.mod h1:HFkY916IF+rwdDfMAkV7OtwuqBVzrE8GR6GFx+wExME= github.com/stretchr/testify v1.2.2 h1:bSDNvY7ZPG5RlJ8otE/7V6gMiyenm9RtJ7IUVIAoJ1w= @@ -284,6 +307,7 @@ github.com/testcontainers/testcontainers-go v0.0.9/go.mod h1:0Qe9qqjNZgxHzzdHPWw github.com/tmc/grpc-websocket-proxy v0.0.0-20170815181823-89b8d40f7ca8/go.mod h1:ncp9v5uamzpCO7NfCPTXjqaC+bZgJeR0sMTm6dMHP7U= github.com/tmc/grpc-websocket-proxy v0.0.0-20190109142713-0ad062ec5ee5 h1:LnC5Kc/wtumK+WB441p7ynQJzVuNRJiqddSIE3IlSEQ= github.com/tmc/grpc-websocket-proxy v0.0.0-20190109142713-0ad062ec5ee5/go.mod h1:ncp9v5uamzpCO7NfCPTXjqaC+bZgJeR0sMTm6dMHP7U= +github.com/ugorji/go v1.1.4/go.mod h1:uQMGLiO92mf5W77hV/PUCpI3pbzQx3CRekS0kk+RGrc= github.com/ugorji/go/codec v0.0.0-20181204163529-d75b2dcb6bc8/go.mod h1:VFNgLljTbGfSG7qAOspJ7OScBnGdDN/yBr0sguwnwf0= github.com/urfave/cli v0.0.0-20171014202726-7bc6a0acffa5/go.mod h1:70zkFmudgCuE/ngEzBv17Jvp/497gISqfk5gWijbERA= github.com/urfave/cli v1.20.0/go.mod h1:70zkFmudgCuE/ngEzBv17Jvp/497gISqfk5gWijbERA= @@ -293,6 +317,7 @@ github.com/xeipuuv/gojsonschema v0.0.0-20180618132009-1d523034197f/go.mod h1:5yf github.com/xiang90/probing v0.0.0-20190116061207-43a291ad63a2 h1:eY9dn8+vbi4tKz5Qo6v2eYzo7kUS51QINcR5jNpbZS8= github.com/xiang90/probing v0.0.0-20190116061207-43a291ad63a2/go.mod h1:UETIi67q53MR2AWcXfiuqkDkRtnGDLqkBTpCHuJHxtU= github.com/xordataexchange/crypt v0.0.3-0.20170626215501-b2862e3d0a77/go.mod h1:aYKd//L2LvnjZzWKhF00oedf4jCCReLcmhLdhm1A27Q= +go.etcd.io/bbolt v1.3.2/go.mod h1:IbVyRI1SCnLcuJnV2u8VeU0CEYM7e686BmAb1XKL+uU= go.etcd.io/bbolt v1.3.3 h1:MUGmc65QhB3pIlaQ5bB4LwqSj6GIonVJXpZiaKNyaKk= go.etcd.io/bbolt v1.3.3/go.mod h1:IbVyRI1SCnLcuJnV2u8VeU0CEYM7e686BmAb1XKL+uU= go.etcd.io/etcd v0.0.0-20191023171146-3cf2f69b5738 h1:VcrIfasaLFkyjk6KNlXQSzO+B0fZcnECiDrKJsfxka0= @@ -334,6 +359,7 @@ golang.org/x/net v0.0.0-20190311183353-d8887717615a/go.mod h1:t9HGtf8HONx5eT2rtn golang.org/x/net v0.0.0-20190404232315-eb5bcb51f2a3/go.mod h1:t9HGtf8HONx5eT2rtn7q6eTqICYqUVnKs3thJo3Qplg= golang.org/x/net v0.0.0-20190501004415-9ce7a6920f09/go.mod h1:t9HGtf8HONx5eT2rtn7q6eTqICYqUVnKs3thJo3Qplg= golang.org/x/net v0.0.0-20190503192946-f4e77d36d62c/go.mod h1:t9HGtf8HONx5eT2rtn7q6eTqICYqUVnKs3thJo3Qplg= +golang.org/x/net v0.0.0-20190522155817-f3200d17e092/go.mod h1:HSz+uSET+XFnRR8LxR5pz3Of3rY3CfYBVs4xY44aLks= golang.org/x/net v0.0.0-20190603091049-60506f45cf65/go.mod h1:HSz+uSET+XFnRR8LxR5pz3Of3rY3CfYBVs4xY44aLks= golang.org/x/net v0.0.0-20190613194153-d28f0bde5980/go.mod h1:z5CRVTTTmAJ677TzLLGU+0bjPO0LkuOLi4/5GtJWs/s= golang.org/x/net v0.0.0-20190813141303-74dc4d7220e7 h1:fHDIZ2oxGnUZRN6WgWFCbYBjH9uqVPRCUVUDhs0wnbA= @@ -342,6 +368,8 @@ golang.org/x/oauth2 v0.0.0-20180821212333-d2e6202438be/go.mod h1:N/0e6XlmueqKjAG golang.org/x/oauth2 v0.0.0-20190226205417-e64efc72b421/go.mod h1:gOpvHmFTYa4IltrdGE7lF6nIHvwfUNPOp7c8zoXwtLw= golang.org/x/oauth2 v0.0.0-20190604053449-0f29369cfe45 h1:SVwTIAaPC2U/AvvLNZ2a7OVsmBpC8L5BlwK1whH3hm0= golang.org/x/oauth2 v0.0.0-20190604053449-0f29369cfe45/go.mod h1:gOpvHmFTYa4IltrdGE7lF6nIHvwfUNPOp7c8zoXwtLw= +golang.org/x/oauth2 v0.0.0-20200107190931-bf48bf16ab8d h1:TzXSXBo42m9gQenoE3b9BGiEpg5IG2JkU5FkPIawgtw= +golang.org/x/oauth2 v0.0.0-20200107190931-bf48bf16ab8d/go.mod h1:gOpvHmFTYa4IltrdGE7lF6nIHvwfUNPOp7c8zoXwtLw= golang.org/x/sync v0.0.0-20180314180146-1d60e4601c6f/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM= golang.org/x/sync v0.0.0-20181108010431-42b317875d0f h1:Bl/8QSvNqXvPGPGXa2z5xUTmV7VDcZyvRZ+QQXkXTZQ= golang.org/x/sync v0.0.0-20181108010431-42b317875d0f/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM= @@ -411,6 +439,7 @@ google.golang.org/genproto v0.0.0-20190819201941-24fa4b261c55/go.mod h1:DMBHOl98 google.golang.org/grpc v1.17.0/go.mod h1:6QZJwpn2B+Zp71q/5VxRsJ6NXXVCE5NRUHRo+f3cWCs= google.golang.org/grpc v1.19.0/go.mod h1:mqu4LbDTu4XGKhr4mRzUsmM4RtVoemTSY81AxZiDr8c= google.golang.org/grpc v1.20.1/go.mod h1:10oTOabMzJvdu6/UiuZezV6QK5dSlG84ov/aaiqXj38= +google.golang.org/grpc v1.21.0/go.mod h1:oYelfM1adQP15Ek0mdvEgi9Df8B9CZIaU1084ijfRaM= google.golang.org/grpc v1.23.0 h1:AzbTB6ux+okLTzP8Ru1Xs41C303zdcfEht7MQnYJt5A= google.golang.org/grpc v1.23.0/go.mod h1:Y5yQAOtifL1yxbo5wqy6BxZv8vAUGQwXBOALyacEbxg= google.golang.org/grpc v1.23.1 h1:q4XQuHFC6I28BKZpo6IYyb3mNO+l7lSOxRuYTCiDfXk= @@ -433,6 +462,8 @@ gopkg.in/ldap.v2 v2.5.1/go.mod h1:oI0cpe/D7HRtBQl8aTg+ZmzFUAvu4lsv3eLXMLGFxWk= gopkg.in/resty.v1 v1.12.0/go.mod h1:mDo4pnntr5jdWRML875a/NmxYqAlA73dVijT2AXvQQo= gopkg.in/square/go-jose.v2 v2.4.1 h1:H0TmLt7/KmzlrDOpa1F+zr0Tk90PbJYBfsVUmRLrf9Y= gopkg.in/square/go-jose.v2 v2.4.1/go.mod h1:M9dMgbHiYLoDGQrXy7OpJDJWiKiU//h+vD76mk0e1AI= +gopkg.in/square/go-jose.v2 v2.5.1 h1:7odma5RETjNHWJnR32wx8t+Io4djHE1PqxCFx3iiZ2w= +gopkg.in/square/go-jose.v2 v2.5.1/go.mod h1:M9dMgbHiYLoDGQrXy7OpJDJWiKiU//h+vD76mk0e1AI= gopkg.in/tomb.v1 v1.0.0-20141024135613-dd632973f1e7 h1:uRGJdciOHaEIrze2W8Q3AKkepLTh2hOroT7a+7czfdQ= gopkg.in/tomb.v1 v1.0.0-20141024135613-dd632973f1e7/go.mod h1:dt/ZhP58zS4L8KSrWDmTeBkI65Dw0HsyUHuEVlX15mw= gopkg.in/yaml.v2 v2.0.0-20170812160011-eb3733d160e7/go.mod h1:JAlM8MvJe8wmxCU4Bli9HhUf9+ttbYbLASfIpnQbh74= diff --git a/server/handlers.go b/server/handlers.go index b2d0bd86b5..d5a56b10f5 100644 --- a/server/handlers.go +++ b/server/handlers.go @@ -646,7 +646,7 @@ func (s *Server) sendCodeResponse(w http.ResponseWriter, r *http.Request, authRe Expiry: s.now().Add(time.Minute * 30), RedirectURI: authReq.RedirectURI, ConnectorData: authReq.ConnectorData, - CodeChallenge: authReq.CodeChallenge, + PKCE: authReq.PKCE, } if err := s.storage.CreateAuthCode(code); err != nil { s.logger.Errorf("Failed to create auth code: %v", err) @@ -812,11 +812,11 @@ func (s *Server) handleAuthCode(w http.ResponseWriter, r *http.Request, client s } // RFC 7636 (PKCE) - codeChallengeFromStorage := authCode.CodeChallenge.CodeChallenge + codeChallengeFromStorage := authCode.PKCE.CodeChallenge providedCodeVerifier := r.PostFormValue("code_verifier") if providedCodeVerifier != "" && codeChallengeFromStorage != "" { - calculatedCodeChallenge, err := s.calculateCodeChallenge(providedCodeVerifier, authCode.CodeChallenge.CodeChallengeMethod) + calculatedCodeChallenge, err := s.calculateCodeChallenge(providedCodeVerifier, authCode.PKCE.CodeChallengeMethod) if err != nil { s.tokenErrHelper(w, errServerError, "", http.StatusInternalServerError) return diff --git a/server/oauth2.go b/server/oauth2.go index 02d1c6afa4..d12cf40177 100644 --- a/server/oauth2.go +++ b/server/oauth2.go @@ -553,7 +553,7 @@ func (s *Server) parseAuthorizationRequest(r *http.Request) (*storage.AuthReques RedirectURI: redirectURI, ResponseTypes: responseTypes, ConnectorID: connectorID, - CodeChallenge: storage.CodeChallenge{ + PKCE: storage.PKCE{ CodeChallenge: codeChallenge, CodeChallengeMethod: codeChallengeMethod, }, diff --git a/storage/conformance/conformance.go b/storage/conformance/conformance.go index 57db921b6d..c38be0b055 100644 --- a/storage/conformance/conformance.go +++ b/storage/conformance/conformance.go @@ -81,7 +81,7 @@ func mustBeErrAlreadyExists(t *testing.T, kind string, err error) { } func testAuthRequestCRUD(t *testing.T, s storage.Storage) { - codeChallenge := storage.CodeChallenge{ + codeChallenge := storage.PKCE{ CodeChallenge: "code_challenge_test", CodeChallengeMethod: "plain", } @@ -106,7 +106,7 @@ func testAuthRequestCRUD(t *testing.T, s storage.Storage) { EmailVerified: true, Groups: []string{"a", "b"}, }, - CodeChallenge: codeChallenge, + PKCE: codeChallenge, } identity := storage.Claims{Email: "foobar"} @@ -161,8 +161,8 @@ func testAuthRequestCRUD(t *testing.T, s storage.Storage) { t.Fatalf("update failed, wanted identity=%#v got %#v", identity, got.Claims) } - if !reflect.DeepEqual(got.CodeChallenge, codeChallenge) { - t.Fatalf("storage does not support PKCE, wanted challenge=%#v got %#v", codeChallenge, got.CodeChallenge) + if !reflect.DeepEqual(got.PKCE, codeChallenge) { + t.Fatalf("storage does not support PKCE, wanted challenge=%#v got %#v", codeChallenge, got.PKCE) } if err := s.DeleteAuthRequest(a1.ID); err != nil { diff --git a/storage/etcd/types.go b/storage/etcd/types.go index 7fd577a228..22e083afe5 100644 --- a/storage/etcd/types.go +++ b/storage/etcd/types.go @@ -37,8 +37,8 @@ func fromStorageAuthCode(a storage.AuthCode) AuthCode { Scopes: a.Scopes, Claims: fromStorageClaims(a.Claims), Expiry: a.Expiry, - CodeChallenge: a.CodeChallenge.CodeChallenge, - CodeChallengeMethod: a.CodeChallenge.CodeChallengeMethod, + CodeChallenge: a.PKCE.CodeChallenge, + CodeChallengeMethod: a.PKCE.CodeChallengeMethod, } } @@ -83,8 +83,8 @@ func fromStorageAuthRequest(a storage.AuthRequest) AuthRequest { Claims: fromStorageClaims(a.Claims), ConnectorID: a.ConnectorID, ConnectorData: a.ConnectorData, - CodeChallenge: a.CodeChallenge.CodeChallenge, - CodeChallengeMethod: a.CodeChallenge.CodeChallengeMethod, + CodeChallenge: a.PKCE.CodeChallenge, + CodeChallengeMethod: a.PKCE.CodeChallengeMethod, } } @@ -103,7 +103,7 @@ func toStorageAuthRequest(a AuthRequest) storage.AuthRequest { ConnectorData: a.ConnectorData, Expiry: a.Expiry, Claims: toStorageClaims(a.Claims), - CodeChallenge: storage.CodeChallenge{ + PKCE: storage.PKCE{ CodeChallenge: a.CodeChallenge, CodeChallengeMethod: a.CodeChallengeMethod, }, diff --git a/storage/kubernetes/types.go b/storage/kubernetes/types.go index f2c35277ce..e851ab6d1f 100644 --- a/storage/kubernetes/types.go +++ b/storage/kubernetes/types.go @@ -326,7 +326,7 @@ func toStorageAuthRequest(req AuthRequest) storage.AuthRequest { ConnectorData: req.ConnectorData, Expiry: req.Expiry, Claims: toStorageClaims(req.Claims), - CodeChallenge: storage.CodeChallenge{ + PKCE: storage.PKCE{ CodeChallenge: req.CodeChallenge, CodeChallengeMethod: req.CodeChallengeMethod, }, @@ -356,8 +356,8 @@ func (cli *client) fromStorageAuthRequest(a storage.AuthRequest) AuthRequest { ConnectorData: a.ConnectorData, Expiry: a.Expiry, Claims: fromStorageClaims(a.Claims), - CodeChallenge: a.CodeChallenge.CodeChallenge, - CodeChallengeMethod: a.CodeChallenge.CodeChallengeMethod, + CodeChallenge: a.PKCE.CodeChallenge, + CodeChallengeMethod: a.PKCE.CodeChallengeMethod, } return req } @@ -461,8 +461,8 @@ func (cli *client) fromStorageAuthCode(a storage.AuthCode) AuthCode { Scopes: a.Scopes, Claims: fromStorageClaims(a.Claims), Expiry: a.Expiry, - CodeChallenge: a.CodeChallenge.CodeChallenge, - CodeChallengeMethod: a.CodeChallenge.CodeChallengeMethod, + CodeChallenge: a.PKCE.CodeChallenge, + CodeChallengeMethod: a.PKCE.CodeChallengeMethod, } } @@ -477,7 +477,7 @@ func toStorageAuthCode(a AuthCode) storage.AuthCode { Scopes: a.Scopes, Claims: toStorageClaims(a.Claims), Expiry: a.Expiry, - CodeChallenge: storage.CodeChallenge{ + PKCE: storage.PKCE{ CodeChallenge: a.CodeChallenge, CodeChallengeMethod: a.CodeChallengeMethod, }, diff --git a/storage/sql/crud.go b/storage/sql/crud.go index 19b11bf207..6c76eac561 100644 --- a/storage/sql/crud.go +++ b/storage/sql/crud.go @@ -141,7 +141,7 @@ func (c *conn) CreateAuthRequest(a storage.AuthRequest) error { a.Claims.Email, a.Claims.EmailVerified, encoder(a.Claims.Groups), a.ConnectorID, a.ConnectorData, a.Expiry, - a.CodeChallenge.CodeChallenge, a.CodeChallenge.CodeChallengeMethod, + a.PKCE.CodeChallenge, a.PKCE.CodeChallengeMethod, ) if err != nil { if c.alreadyExistsCheck(err) { @@ -183,7 +183,7 @@ func (c *conn) UpdateAuthRequest(id string, updater func(a storage.AuthRequest) encoder(a.Claims.Groups), a.ConnectorID, a.ConnectorData, a.Expiry, - a.CodeChallenge.CodeChallenge, a.CodeChallenge.CodeChallengeMethod, + a.PKCE.CodeChallenge, a.PKCE.CodeChallengeMethod, r.ID, ) if err != nil { @@ -214,7 +214,7 @@ func getAuthRequest(q querier, id string) (a storage.AuthRequest, err error) { &a.Claims.Email, &a.Claims.EmailVerified, decoder(&a.Claims.Groups), &a.ConnectorID, &a.ConnectorData, &a.Expiry, - &a.CodeChallenge.CodeChallenge, &a.CodeChallenge.CodeChallengeMethod, + &a.PKCE.CodeChallenge, &a.PKCE.CodeChallengeMethod, ) if err != nil { if err == sql.ErrNoRows { @@ -240,7 +240,7 @@ func (c *conn) CreateAuthCode(a storage.AuthCode) error { a.ID, a.ClientID, encoder(a.Scopes), a.Nonce, a.RedirectURI, a.Claims.UserID, a.Claims.Username, a.Claims.PreferredUsername, a.Claims.Email, a.Claims.EmailVerified, encoder(a.Claims.Groups), a.ConnectorID, a.ConnectorData, a.Expiry, - a.CodeChallenge.CodeChallenge, a.CodeChallenge.CodeChallengeMethod, + a.PKCE.CodeChallenge, a.PKCE.CodeChallengeMethod, ) if err != nil { @@ -266,7 +266,7 @@ func (c *conn) GetAuthCode(id string) (a storage.AuthCode, err error) { &a.ID, &a.ClientID, decoder(&a.Scopes), &a.Nonce, &a.RedirectURI, &a.Claims.UserID, &a.Claims.Username, &a.Claims.PreferredUsername, &a.Claims.Email, &a.Claims.EmailVerified, decoder(&a.Claims.Groups), &a.ConnectorID, &a.ConnectorData, &a.Expiry, - &a.CodeChallenge.CodeChallenge, &a.CodeChallenge.CodeChallengeMethod, + &a.PKCE.CodeChallenge, &a.PKCE.CodeChallengeMethod, ) if err != nil { if err == sql.ErrNoRows { diff --git a/storage/storage.go b/storage/storage.go index 373619894a..4781d0987b 100644 --- a/storage/storage.go +++ b/storage/storage.go @@ -170,7 +170,7 @@ type Claims struct { } // Data needed for PKCE (RFC 7636) -type CodeChallenge struct { +type PKCE struct { CodeChallenge string CodeChallengeMethod string } @@ -213,7 +213,8 @@ type AuthRequest struct { ConnectorID string ConnectorData []byte - CodeChallenge + // PKCE CodeChallenge and CodeChallengeMethod + PKCE PKCE } // AuthCode represents a code which can be exchanged for an OAuth2 token response. @@ -250,7 +251,8 @@ type AuthCode struct { Expiry time.Time - CodeChallenge + // PKCE CodeChallenge and CodeChallengeMethod + PKCE PKCE } // RefreshToken is an OAuth2 refresh token which allows a client to request new From b24e4d5110a08356cf7cd042df77e5c53cadc8ca Mon Sep 17 00:00:00 2001 From: Bernd Eckstein Date: Thu, 17 Sep 2020 11:31:18 +0200 Subject: [PATCH 14/24] PKCE: Check clientSecret when available In authorization_code flow with PKCE, allow empty client_secret on /auth and /token endpoints. But check the client_secret when it is given. Signed-off-by: Bernd Eckstein --- server/handlers.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/server/handlers.go b/server/handlers.go index d5a56b10f5..3ad44b2b6f 100644 --- a/server/handlers.go +++ b/server/handlers.go @@ -763,8 +763,8 @@ func (s *Server) handleToken(w http.ResponseWriter, r *http.Request) { grantType := r.PostFormValue("grant_type") codeVerifier := r.PostFormValue("code_verifier") - if grantType == grantTypeAuthorizationCode && codeVerifier != "" { - // RFC 7636 (PKCE) if code_verifier is received, use PKCE and not the client_secret + if grantType == grantTypeAuthorizationCode && codeVerifier != "" && clientSecret == "" { + // RFC 7636 (PKCE) if code_verifier is received, use PKCE and allow empty clientSecret } else if client.Secret != clientSecret { s.tokenErrHelper(w, errInvalidClient, "Invalid client credentials.", http.StatusUnauthorized) return From 9faf988c8342bea950d126cf6022b262bd122bb9 Mon Sep 17 00:00:00 2001 From: Bernd Eckstein Date: Thu, 17 Sep 2020 13:47:36 +0200 Subject: [PATCH 15/24] Enable PKCE with public: true dex configuration public on staticClients now enables the following behavior in PKCE: - Public: false, PKCE will always check client_secret. This means PKCE in it's natural form is disabled. - Public: true, PKCE is enabled. It will only check client_secret if the client has sent one. But it allows the code flow if the client didn't sent one. Signed-off-by: Bernd Eckstein --- server/handlers.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/server/handlers.go b/server/handlers.go index 3ad44b2b6f..49b8b6241e 100644 --- a/server/handlers.go +++ b/server/handlers.go @@ -763,8 +763,8 @@ func (s *Server) handleToken(w http.ResponseWriter, r *http.Request) { grantType := r.PostFormValue("grant_type") codeVerifier := r.PostFormValue("code_verifier") - if grantType == grantTypeAuthorizationCode && codeVerifier != "" && clientSecret == "" { - // RFC 7636 (PKCE) if code_verifier is received, use PKCE and allow empty clientSecret + if client.Public && grantType == grantTypeAuthorizationCode && codeVerifier != "" && clientSecret == "" { + // RFC 7636 (PKCE) if code_verifier is received, use PKCE and allow empty clientSecret, when client is public } else if client.Secret != clientSecret { s.tokenErrHelper(w, errInvalidClient, "Invalid client credentials.", http.StatusUnauthorized) return From a17bfc11400853e9915f6c7f502bb7a933f1f3c5 Mon Sep 17 00:00:00 2001 From: Bernd Eckstein Date: Thu, 17 Sep 2020 15:20:24 +0200 Subject: [PATCH 16/24] Redirect error on unsupported code_challenge_method - Check for unsupported code_challenge_method after redirect uri is validated, and use newErr() to return the error. - Add PKCE tests to oauth2_test.go Signed-off-by: Bernd Eckstein --- server/oauth2.go | 10 +++--- server/oauth2_test.go | 72 +++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 77 insertions(+), 5 deletions(-) diff --git a/server/oauth2.go b/server/oauth2.go index d12cf40177..79e54c3297 100644 --- a/server/oauth2.go +++ b/server/oauth2.go @@ -420,11 +420,6 @@ func (s *Server) parseAuthorizationRequest(r *http.Request) (*storage.AuthReques codeChallengeMethod = CodeChallengeMethodPlain } - if codeChallengeMethod != CodeChallengeMethodS256 && codeChallengeMethod != CodeChallengeMethodPlain { - description := fmt.Sprintf("Unsupported PKCE challenge method (%q).", codeChallengeMethod) - return nil, &authErr{"", "", errInvalidRequest, description} - } - client, err := s.storage.GetClient(clientID) if err != nil { if err == storage.ErrNotFound { @@ -458,6 +453,11 @@ func (s *Server) parseAuthorizationRequest(r *http.Request) (*storage.AuthReques return &authErr{state, redirectURI, typ, fmt.Sprintf(format, a...)} } + if codeChallengeMethod != CodeChallengeMethodS256 && codeChallengeMethod != CodeChallengeMethodPlain { + description := fmt.Sprintf("Unsupported PKCE challenge method (%q).", codeChallengeMethod) + return nil, newErr(errInvalidRequest, description) + } + var ( unrecognized []string invalidScopes []string diff --git a/server/oauth2_test.go b/server/oauth2_test.go index ad122055c2..8db9ea5929 100644 --- a/server/oauth2_test.go +++ b/server/oauth2_test.go @@ -197,6 +197,78 @@ func TestParseAuthorizationRequest(t *testing.T) { }, wantErr: true, }, + { + name: "PKCE code_challenge_method plain", + clients: []storage.Client{ + { + ID: "bar", + RedirectURIs: []string{"https://example.com/bar"}, + }, + }, + supportedResponseTypes: []string{"code"}, + queryParams: map[string]string{ + "client_id": "bar", + "redirect_uri": "https://example.com/bar", + "response_type": "code", + "code_challenge": "123", + "code_challenge_method": "plain", + "scope": "openid email profile", + }, + }, + { + name: "PKCE code_challenge_method default plain", + clients: []storage.Client{ + { + ID: "bar", + RedirectURIs: []string{"https://example.com/bar"}, + }, + }, + supportedResponseTypes: []string{"code"}, + queryParams: map[string]string{ + "client_id": "bar", + "redirect_uri": "https://example.com/bar", + "response_type": "code", + "code_challenge": "123", + "scope": "openid email profile", + }, + }, + { + name: "PKCE code_challenge_method S256", + clients: []storage.Client{ + { + ID: "bar", + RedirectURIs: []string{"https://example.com/bar"}, + }, + }, + supportedResponseTypes: []string{"code"}, + queryParams: map[string]string{ + "client_id": "bar", + "redirect_uri": "https://example.com/bar", + "response_type": "code", + "code_challenge": "123", + "code_challenge_method": "S256", + "scope": "openid email profile", + }, + }, + { + name: "PKCE invalid code_challenge_method", + clients: []storage.Client{ + { + ID: "bar", + RedirectURIs: []string{"https://example.com/bar"}, + }, + }, + supportedResponseTypes: []string{"code"}, + queryParams: map[string]string{ + "client_id": "bar", + "redirect_uri": "https://example.com/bar", + "response_type": "code", + "code_challenge": "123", + "code_challenge_method": "invalid_method", + "scope": "openid email profile", + }, + wantErr: true, + }, } for _, tc := range tests { From f78dc9dfcf23f60e09818bb5f565d4381be7fd20 Mon Sep 17 00:00:00 2001 From: Bernd Eckstein Date: Thu, 17 Sep 2020 15:28:05 +0200 Subject: [PATCH 17/24] Reverted go.mod and go.sum to the state of master Signed-off-by: Bernd Eckstein --- go.mod | 10 +++------- go.sum | 31 ------------------------------- 2 files changed, 3 insertions(+), 38 deletions(-) diff --git a/go.mod b/go.mod index 985baaeb77..22480a0170 100644 --- a/go.mod +++ b/go.mod @@ -5,14 +5,11 @@ go 1.14 require ( github.com/Microsoft/hcsshim v0.8.7 // indirect github.com/beevik/etree v1.1.0 - github.com/coreos/go-etcd v2.0.0+incompatible // indirect github.com/coreos/go-oidc v2.2.1+incompatible github.com/coreos/go-semver v0.3.0 // indirect github.com/coreos/go-systemd v0.0.0-20190719114852-fd7a80b32e1f // indirect github.com/coreos/pkg v0.0.0-20180928190104-399ea9e2e55f // indirect - github.com/cpuguy83/go-md2man v1.0.10 // indirect github.com/dexidp/dex/api/v2 v2.0.0 - github.com/dexidp/dex/examples v0.0.0-20200916154904-458059cc89d8 // indirect github.com/felixge/httpsnoop v1.0.1 github.com/ghodss/yaml v1.0.0 github.com/go-sql-driver/mysql v1.5.0 @@ -31,23 +28,22 @@ require ( github.com/prometheus/client_golang v1.4.0 github.com/russellhaering/goxmldsig v0.0.0-20180430223755-7acd5e4a6ef7 github.com/sirupsen/logrus v1.4.2 - github.com/spf13/cobra v1.0.0 + github.com/spf13/cobra v0.0.5 github.com/stretchr/testify v1.4.0 github.com/testcontainers/testcontainers-go v0.0.9 github.com/tmc/grpc-websocket-proxy v0.0.0-20190109142713-0ad062ec5ee5 // indirect - github.com/ugorji/go/codec v0.0.0-20181204163529-d75b2dcb6bc8 // indirect go.etcd.io/etcd v0.0.0-20191023171146-3cf2f69b5738 go.uber.org/atomic v1.4.0 // indirect golang.org/x/crypto v0.0.0-20190701094942-4def268fd1a4 golang.org/x/net v0.0.0-20190813141303-74dc4d7220e7 - golang.org/x/oauth2 v0.0.0-20200107190931-bf48bf16ab8d + golang.org/x/oauth2 v0.0.0-20190604053449-0f29369cfe45 golang.org/x/time v0.0.0-20190308202827-9d24e82272b4 // indirect google.golang.org/api v0.15.0 google.golang.org/appengine v1.6.1 // indirect google.golang.org/grpc v1.26.0 gopkg.in/asn1-ber.v1 v1.0.0-20181015200546-f715ec2f112d // indirect gopkg.in/ldap.v2 v2.5.1 - gopkg.in/square/go-jose.v2 v2.5.1 + gopkg.in/square/go-jose.v2 v2.4.1 sigs.k8s.io/testing_frameworks v0.1.2 ) diff --git a/go.sum b/go.sum index 09f0c48e74..54abd1b102 100644 --- a/go.sum +++ b/go.sum @@ -13,7 +13,6 @@ github.com/Microsoft/go-winio v0.4.15-0.20190919025122-fc70bd9a86b5 h1:ygIc8M6tr github.com/Microsoft/go-winio v0.4.15-0.20190919025122-fc70bd9a86b5/go.mod h1:tTuCMEN+UleMWgg9dVx4Hu52b1bJo+59jBh3ajtinzw= github.com/Microsoft/hcsshim v0.8.7 h1:ptnOoufxGSzauVTsdE+wMYnCWA301PdoN4xg5oRdZpg= github.com/Microsoft/hcsshim v0.8.7/go.mod h1:OHd7sQqRFrYd3RmSgbgji+ctCwkbq2wbEYNSzOYtcBQ= -github.com/OneOfOne/xxhash v1.2.2/go.mod h1:HSdplMjZKSmBqAxg5vPj2TmRDmfkzw+cTzAElWljhcU= github.com/alecthomas/template v0.0.0-20160405071501-a0175ee3bccc/go.mod h1:LOuyumcjzFXgccqObfd/Ljyb9UuFJ6TxHnclSeseNhc= github.com/alecthomas/template v0.0.0-20190718012654-fb15b899a751/go.mod h1:LOuyumcjzFXgccqObfd/Ljyb9UuFJ6TxHnclSeseNhc= github.com/alecthomas/units v0.0.0-20151022065526-2efee857e7cf/go.mod h1:ybxpYRFXyAe+OPACYpWeL0wqObRcbAqCMya13uyzqw0= @@ -31,8 +30,6 @@ github.com/blang/semver v3.1.0+incompatible/go.mod h1:kRBLl5iJ+tD4TcOOxsy/0fnweb github.com/cenkalti/backoff v2.2.1+incompatible h1:tNowT99t7UNflLxfYYSlKYsBpXdEet03Pg2g16Swow4= github.com/cenkalti/backoff v2.2.1+incompatible/go.mod h1:90ReRw6GdpyfrHakVjL/QHaoyV4aDUVVkXQJJJ3NXXM= github.com/census-instrumentation/opencensus-proto v0.2.1/go.mod h1:f6KPmirojxKA12rnyqOA5BBL4O983OfeGPqjHWSTneU= -github.com/cespare/xxhash v1.1.0 h1:a6HrQnmkObjyL+Gs60czilIUGqrzKutQD6XZog3p+ko= -github.com/cespare/xxhash v1.1.0/go.mod h1:XrSqR1VqqWfGrhpAt58auRo0WTKS1nRRg3ghfAqPWnc= github.com/cespare/xxhash/v2 v2.1.1 h1:6MnRN8NT7+YBpUIWxHtefFZOKTAPgGjpQSxqLNn0+qY= github.com/cespare/xxhash/v2 v2.1.1/go.mod h1:VGX0DQ3Q6kWi7AoAeZDth3/j3BFtOZR5XLFGgcrjCOs= github.com/client9/misspell v0.3.4/go.mod h1:qj6jICC3Q7zFZvVWo7KLAzC3yx5G7kyvSDkc90ppPyw= @@ -47,7 +44,6 @@ github.com/containerd/fifo v0.0.0-20190226154929-a9fb20d87448/go.mod h1:ODA38xgv github.com/containerd/go-runc v0.0.0-20180907222934-5a6d9f37cfa3/go.mod h1:IV7qH3hrUgRmyYrtgEeGWJfWbgcHL9CSRruz2Vqcph0= github.com/containerd/ttrpc v0.0.0-20190828154514-0e0f228740de/go.mod h1:PvCDdDGpgqzQIzDW1TphrGLssLDZp2GuS+X5DkEJB8o= github.com/containerd/typeurl v0.0.0-20180627222232-a93fcdb778cd/go.mod h1:Cm3kwCdlkCfMSHURc+r6fwoGH6/F1hH3S4sg0rLFWPc= -github.com/coreos/bbolt v1.3.2/go.mod h1:iRUV2dpdMOn7Bo10OQBFzIJO9kkE559Wcmn+qkEiiKk= github.com/coreos/etcd v3.3.10+incompatible/go.mod h1:uF7uidLiAD3TWHmW31ZFd/JWoc32PjwdhPthX9715RE= github.com/coreos/go-etcd v2.0.0+incompatible/go.mod h1:Jez6KQU2B/sWsbdaef3ED8NzMklzPG4d5KIOhIy30Tk= github.com/coreos/go-oidc v2.2.1+incompatible h1:mh48q/BqXqgjVHpy2ZY7WnWAbenxRjsz9N1i1YxjHAk= @@ -64,16 +60,12 @@ github.com/coreos/pkg v0.0.0-20160727233714-3ac0863d7acf/go.mod h1:E3G3o1h8I7cfc github.com/coreos/pkg v0.0.0-20180928190104-399ea9e2e55f h1:lBNOc5arjvs8E5mO2tbpBpLoyyu8B6e44T7hJy6potg= github.com/coreos/pkg v0.0.0-20180928190104-399ea9e2e55f/go.mod h1:E3G3o1h8I7cfcXa63jLwjI0eiQQMgzzUDFVpN/nH/eA= github.com/cpuguy83/go-md2man v1.0.10/go.mod h1:SmD6nW6nTyfqj6ABTjUi3V3JVMnlJmwcJI5acqYI6dE= -github.com/cpuguy83/go-md2man/v2 v2.0.0/go.mod h1:maD7wRr/U5Z6m/iR4s+kqSMx2CaBsrgA7czyZG/E6dU= github.com/creack/pty v1.1.7/go.mod h1:lj5s0c3V2DBrqTV7llrYr5NG6My20zk30Fl46Y7DoTY= github.com/davecgh/go-spew v1.1.0/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38= github.com/davecgh/go-spew v1.1.1 h1:vj9j/u1bqnvCEfJOwUhtlOARqs3+rkHYY13jYWTU97c= github.com/davecgh/go-spew v1.1.1/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38= -github.com/dexidp/dex/examples v0.0.0-20200916154904-458059cc89d8 h1:igNt1Em0/r1PeqK7KkXXd/eP/nlKwSvtvKDe+/ZfKfY= -github.com/dexidp/dex/examples v0.0.0-20200916154904-458059cc89d8/go.mod h1:7vocppXTfbsaYcPDFuo9Oj8N0FDnsKlt/3lSL+opISY= github.com/dgrijalva/jwt-go v3.2.0+incompatible h1:7qlOGliEKZXTDg6OTjfoBKDXWrumCAMpl/TFQ4/5kLM= github.com/dgrijalva/jwt-go v3.2.0+incompatible/go.mod h1:E3ru+11k8xSBh+hMPgOLZmtrrCbhqsmaPHjLKYnJCaQ= -github.com/dgryski/go-sip13 v0.0.0-20181026042036-e10d5fee7954/go.mod h1:vAd38F8PWV+bWy6jNmig1y/TA+kYO4g3RSRF0IAv0no= github.com/docker/distribution v2.7.1-0.20190205005809-0d3efadf0154+incompatible h1:dvc1KSkIYTVjZgHf/CTC2diTYC8PzhaA5sFISRfNVrE= github.com/docker/distribution v2.7.1-0.20190205005809-0d3efadf0154+incompatible/go.mod h1:J2gT2udsDAN96Uj4KfcMRqY0/ypR+oyYUYmja8H+y+w= github.com/docker/docker v0.7.3-0.20190506211059-b20a14b54661 h1:ZuxGvIvF01nfc/G9RJ5Q7Va1zQE2WJyG18Zv3DqCEf4= @@ -115,7 +107,6 @@ github.com/gogo/protobuf v1.2.1/go.mod h1:hp+jE20tsWTFYpLwKvXlhS1hjn+gTNwPg2I6zV github.com/golang/glog v0.0.0-20160126235308-23def4e6c14b h1:VKtxabqXZkF25pY9ekfRL6a582T4P37/31XEstQ5p58= github.com/golang/glog v0.0.0-20160126235308-23def4e6c14b/go.mod h1:SBH7ygxi8pfUlaOkMMuAQtPIUF8ecWP5IEl/CR7VP2Q= github.com/golang/groupcache v0.0.0-20160516000752-02826c3e7903/go.mod h1:cIg4eruTrX1D+g88fzRXU5OdNfaM+9IcxsU14FzY7Hc= -github.com/golang/groupcache v0.0.0-20190129154638-5b532d6fd5ef/go.mod h1:cIg4eruTrX1D+g88fzRXU5OdNfaM+9IcxsU14FzY7Hc= github.com/golang/groupcache v0.0.0-20190702054246-869f871628b6 h1:ZgQEtGgCBiWRM39fZuwSd1LwSqqSW0hOdXCYYDX0R3I= github.com/golang/groupcache v0.0.0-20190702054246-869f871628b6/go.mod h1:cIg4eruTrX1D+g88fzRXU5OdNfaM+9IcxsU14FzY7Hc= github.com/golang/mock v1.1.1/go.mod h1:oTYuIxOrZwtPieC+H1uAHpcLFnEyAGVDL/k47Jfbm0A= @@ -152,12 +143,10 @@ github.com/gorilla/mux v1.7.3/go.mod h1:1lud6UwP+6orDFRuTfBEV8e9/aOM/c4fVVCaMa2z github.com/gorilla/websocket v0.0.0-20170926233335-4201258b820c/go.mod h1:E7qHFY5m1UJ88s3WnNqhKjPHQ0heANvMoAMk2YaljkQ= github.com/gorilla/websocket v1.4.0 h1:WDFjx/TMzVgy9VdMMQi2K2Emtwi2QcUQsztZ/zLaH/Q= github.com/gorilla/websocket v1.4.0/go.mod h1:E7qHFY5m1UJ88s3WnNqhKjPHQ0heANvMoAMk2YaljkQ= -github.com/grpc-ecosystem/go-grpc-middleware v1.0.0/go.mod h1:FiyG127CGDf3tlThmgyCl78X/SZQqEOJBCDaAfeWzPs= github.com/grpc-ecosystem/go-grpc-middleware v1.0.1-0.20190118093823-f849b5445de4 h1:z53tR0945TRRQO/fLEVPI6SMv7ZflF0TEaTAoU7tOzg= github.com/grpc-ecosystem/go-grpc-middleware v1.0.1-0.20190118093823-f849b5445de4/go.mod h1:FiyG127CGDf3tlThmgyCl78X/SZQqEOJBCDaAfeWzPs= github.com/grpc-ecosystem/go-grpc-prometheus v1.2.0 h1:Ovs26xHkKqVztRpIrF/92BcuyuQ/YW4NSIpoGtfXNho= github.com/grpc-ecosystem/go-grpc-prometheus v1.2.0/go.mod h1:8NvIoxWQoOIhqOTXgfV/d3M/q6VIi02HzZEHgUlZvzk= -github.com/grpc-ecosystem/grpc-gateway v1.9.0/go.mod h1:vNeuVxBJEsws4ogUvrchl83t/GYV9WGTSLVdBhOQFDY= github.com/grpc-ecosystem/grpc-gateway v1.9.5 h1:UImYN5qQ8tuGpGE16ZmjvcTtTw24zw1QAp/SlnNrZhI= github.com/grpc-ecosystem/grpc-gateway v1.9.5/go.mod h1:vNeuVxBJEsws4ogUvrchl83t/GYV9WGTSLVdBhOQFDY= github.com/hashicorp/errwrap v0.0.0-20141028054710-7554cd9344ce/go.mod h1:YH+1FKiLXxHSkmPseP+kNlulaMuP3n2brvKWEqk/Jc4= @@ -213,7 +202,6 @@ github.com/modern-go/reflect2 v1.0.1/go.mod h1:bx2lNnkwVCuqBIxFjflWJWanXIb3Rllmb github.com/morikuni/aec v0.0.0-20170113033406-39771216ff4c h1:nXxl5PrvVm2L/wCy8dQu6DMTwH4oIuGN8GJDAlqDdVE= github.com/morikuni/aec v0.0.0-20170113033406-39771216ff4c/go.mod h1:BbKIizmSmc5MMPqRYbxO4ZU0S0+P200+tUnFx7PXmsc= github.com/mwitkow/go-conntrack v0.0.0-20161129095857-cc309e4a2223/go.mod h1:qRWi+5nqEBWmkhHvq77mSJWrCKwh8bxhgT7d/eI7P4U= -github.com/oklog/ulid v1.3.1/go.mod h1:CirwcVhetQ6Lv90oh/F+FBtV6XMibvdAFo93nm5qn4U= github.com/olekukonko/tablewriter v0.0.0-20170122224234-a0225b3f23b5/go.mod h1:vsDQFd/mU46D+Z4whnwzcISnGGzXWMclvtLoiIKAKIo= github.com/onsi/ginkgo v1.4.0/go.mod h1:lLunBs/Ym6LB5Z9jYTR76FiuTmxDTDusOGeTQH+WWjE= github.com/onsi/ginkgo v1.6.0/go.mod h1:lLunBs/Ym6LB5Z9jYTR76FiuTmxDTDusOGeTQH+WWjE= @@ -241,7 +229,6 @@ github.com/pmezard/go-difflib v1.0.0/go.mod h1:iKH77koFhYxTK1pcRnkKkqfTogsbg7gZN github.com/pquerna/cachecontrol v0.0.0-20180517163645-1555304b9b35 h1:J9b7z+QKAmPf4YLrFg6oQUotqHQeUNWwkvo7jZp1GLU= github.com/pquerna/cachecontrol v0.0.0-20180517163645-1555304b9b35/go.mod h1:prYjPmNq4d1NPVmpShWobRqXY3q7Vp+80DqgxxUrUIA= github.com/prometheus/client_golang v0.9.1/go.mod h1:7SWBe2y4D6OKWSNQJUaRYU/AaXPKyh/dDVn+NZz0KFw= -github.com/prometheus/client_golang v0.9.3/go.mod h1:/TN21ttK/J9q6uSwhBd54HahCDft0ttaMvbicHlPoso= github.com/prometheus/client_golang v1.0.0 h1:vrDKnkGzuGvhNAL56c7DBz29ZL+KxnoR0x7enabFceM= github.com/prometheus/client_golang v1.0.0/go.mod h1:db9x61etRT2tGnBNRi70OPL5FsnadC4Ky3P0J6CfImo= github.com/prometheus/client_golang v1.4.0 h1:YVIb/fVcOTMSqtqZWSKnHpSLBxu8DKgxq8z6RuBZwqI= @@ -252,47 +239,37 @@ github.com/prometheus/client_model v0.0.0-20190129233127-fd36f4220a90/go.mod h1: github.com/prometheus/client_model v0.0.0-20190812154241-14fe0d1b01d4/go.mod h1:xMI15A0UPsDsEKsMN9yxemIoYk6Tm2C1GtYGdfGttqA= github.com/prometheus/client_model v0.2.0 h1:uq5h0d+GuxiXLJLNABMgp2qUWDPiLvgCzz2dUR+/W/M= github.com/prometheus/client_model v0.2.0/go.mod h1:xMI15A0UPsDsEKsMN9yxemIoYk6Tm2C1GtYGdfGttqA= -github.com/prometheus/common v0.0.0-20181113130724-41aa239b4cce/go.mod h1:daVV7qP5qjZbuso7PdcryaAu0sAZbrN9i7WWcTMWvro= -github.com/prometheus/common v0.4.0/go.mod h1:TNfzLD0ON7rHzMJeJkieUDPYmFC7Snx/y86RQel1bk4= github.com/prometheus/common v0.4.1/go.mod h1:TNfzLD0ON7rHzMJeJkieUDPYmFC7Snx/y86RQel1bk4= github.com/prometheus/common v0.9.1 h1:KOMtN28tlbam3/7ZKEYKHhKoJZYYj3gMH4uc62x7X7U= github.com/prometheus/common v0.9.1/go.mod h1:yhUN8i9wzaXS3w1O07YhxHEBxD+W35wd8bs7vj7HSQ4= github.com/prometheus/procfs v0.0.0-20181005140218-185b4288413d/go.mod h1:c3At6R/oaqEKCNdg8wHV1ftS6bRYblBhIjjI8uT2IGk= -github.com/prometheus/procfs v0.0.0-20190507164030-5867b95ac084/go.mod h1:TjEm7ze935MbeOT/UhFTIMYKhuLP4wbCsTZCD3I8kEA= github.com/prometheus/procfs v0.0.2/go.mod h1:TjEm7ze935MbeOT/UhFTIMYKhuLP4wbCsTZCD3I8kEA= github.com/prometheus/procfs v0.0.5 h1:3+auTFlqw+ZaQYJARz6ArODtkaIwtvBTx3N2NehQlL8= github.com/prometheus/procfs v0.0.5/go.mod h1:4A/X28fw3Fc593LaREMrKMqOKvUAntwMDaekg4FpcdQ= github.com/prometheus/procfs v0.0.8 h1:+fpWZdT24pJBiqJdAwYBjPSk+5YmQzYNPYzQsdzLkt8= github.com/prometheus/procfs v0.0.8/go.mod h1:7Qr8sr6344vo1JqZ6HhLceV9o3AJ1Ff+GxbHq6oeK9A= -github.com/prometheus/tsdb v0.7.1/go.mod h1:qhTCs0VvXwvX/y3TZrWD7rabWM+ijKTux40TwIPHuXU= github.com/rogpeppe/fastuuid v0.0.0-20150106093220-6724a57986af/go.mod h1:XWv6SoW27p1b0cqNHllgS5HIMJraePCO15w5zCzIWYg= github.com/russellhaering/goxmldsig v0.0.0-20180430223755-7acd5e4a6ef7 h1:J4AOUcOh/t1XbQcJfkEqhzgvMJ2tDxdCVvmHxW5QXao= github.com/russellhaering/goxmldsig v0.0.0-20180430223755-7acd5e4a6ef7/go.mod h1:Oz4y6ImuOQZxynhbSXk7btjEfNBtGlj2dcaOvXl2FSM= github.com/russross/blackfriday v1.5.2/go.mod h1:JO/DiYxRf+HjHt06OyowR9PTA263kcR/rfWxYHBV53g= -github.com/russross/blackfriday/v2 v2.0.1/go.mod h1:+Rmxgy9KzJVeS9/2gXHxylqXiyQDYRxCVz55jmeOWTM= github.com/satori/go.uuid v1.2.0 h1:0uYX9dsZ2yD7q2RtLRtPSdGDWzjeM3TbMJP9utgA0ww= github.com/satori/go.uuid v1.2.0/go.mod h1:dA0hQrYB0VpLJoorglMZABFdXlWrHn1NEOzdhQKdks0= -github.com/shurcooL/sanitized_anchor_name v1.0.0/go.mod h1:1NzhyTcUVG4SuEtjjoZeVRXNmyL/1OwPU0+IJeTBvfc= github.com/sirupsen/logrus v1.2.0/go.mod h1:LxeOpSwHxABJmUn/MG1IvRgCAasNZTLOkJPxbbu5VWo= github.com/sirupsen/logrus v1.4.1/go.mod h1:ni0Sbl8bgC9z8RoU9G6nDWqqs/fq4eDPysMBDgk/93Q= github.com/sirupsen/logrus v1.4.2 h1:SPIRibHv4MatM3XXNO2BJeFLZwZ2LvZgfQ5+UNI2im4= github.com/sirupsen/logrus v1.4.2/go.mod h1:tLMulIdttU9McNUspp0xgXVQah82FyeX6MwdIuYE2rE= github.com/soheilhy/cmux v0.1.4 h1:0HKaf1o97UwFjHH9o5XsHUOF+tqmdA7KEzXLpiyaw0E= github.com/soheilhy/cmux v0.1.4/go.mod h1:IM3LyeVVIOuxMH7sFAkER9+bJ4dT7Ms6E4xg4kGIyLM= -github.com/spaolacci/murmur3 v0.0.0-20180118202830-f09979ecbc72/go.mod h1:JwIasOWyU6f++ZhiEuf87xNszmSA2myDM2Kzu9HwQUA= github.com/spf13/afero v1.1.2/go.mod h1:j4pytiNVoe2o6bmDsKpLACNPDBIoEAkihy7loJ1B0CQ= github.com/spf13/cast v1.3.0/go.mod h1:Qx5cxh0v+4UWYiBimWS+eyWzqEqokIECu5etghLkUJE= github.com/spf13/cobra v0.0.3/go.mod h1:1l0Ry5zgKvJasoi3XT1TypsSe7PqH0Sj9dhYf7v3XqQ= github.com/spf13/cobra v0.0.5 h1:f0B+LkLX6DtmRH1isoNA9VTtNUK9K8xYd28JNNfOv/s= github.com/spf13/cobra v0.0.5/go.mod h1:3K3wKZymM7VvHMDS9+Akkh4K60UwM26emMESw8tLCHU= -github.com/spf13/cobra v1.0.0 h1:6m/oheQuQ13N9ks4hubMG6BnvwOeaJrqSPLahSnczz8= -github.com/spf13/cobra v1.0.0/go.mod h1:/6GTrnGXV9HjY+aR4k0oJ5tcvakLuG6EuKReYlHNrgE= github.com/spf13/jwalterweatherman v1.0.0/go.mod h1:cQK4TGJAtQXfYWX+Ddv3mKDzgVb68N+wFjFa4jdeBTo= github.com/spf13/pflag v1.0.1/go.mod h1:DYY7MBk1bdzusC3SYhjObp+wFpr4gzcvqqNjLnInEg4= github.com/spf13/pflag v1.0.3 h1:zPAT6CGy6wXeQ7NtTnaTerfKOsV6V6F8agHXFiazDkg= github.com/spf13/pflag v1.0.3/go.mod h1:DYY7MBk1bdzusC3SYhjObp+wFpr4gzcvqqNjLnInEg4= github.com/spf13/viper v1.3.2/go.mod h1:ZiWeW+zYFKm7srdB9IoDzzZXaJaI5eL9QjNiN/DMA2s= -github.com/spf13/viper v1.4.0/go.mod h1:PTJ7Z/lr49W6bUbkmS1V3by4uWynFiR9p7+dSq/yZzE= github.com/stretchr/objx v0.1.0/go.mod h1:HFkY916IF+rwdDfMAkV7OtwuqBVzrE8GR6GFx+wExME= github.com/stretchr/objx v0.1.1/go.mod h1:HFkY916IF+rwdDfMAkV7OtwuqBVzrE8GR6GFx+wExME= github.com/stretchr/testify v1.2.2 h1:bSDNvY7ZPG5RlJ8otE/7V6gMiyenm9RtJ7IUVIAoJ1w= @@ -307,7 +284,6 @@ github.com/testcontainers/testcontainers-go v0.0.9/go.mod h1:0Qe9qqjNZgxHzzdHPWw github.com/tmc/grpc-websocket-proxy v0.0.0-20170815181823-89b8d40f7ca8/go.mod h1:ncp9v5uamzpCO7NfCPTXjqaC+bZgJeR0sMTm6dMHP7U= github.com/tmc/grpc-websocket-proxy v0.0.0-20190109142713-0ad062ec5ee5 h1:LnC5Kc/wtumK+WB441p7ynQJzVuNRJiqddSIE3IlSEQ= github.com/tmc/grpc-websocket-proxy v0.0.0-20190109142713-0ad062ec5ee5/go.mod h1:ncp9v5uamzpCO7NfCPTXjqaC+bZgJeR0sMTm6dMHP7U= -github.com/ugorji/go v1.1.4/go.mod h1:uQMGLiO92mf5W77hV/PUCpI3pbzQx3CRekS0kk+RGrc= github.com/ugorji/go/codec v0.0.0-20181204163529-d75b2dcb6bc8/go.mod h1:VFNgLljTbGfSG7qAOspJ7OScBnGdDN/yBr0sguwnwf0= github.com/urfave/cli v0.0.0-20171014202726-7bc6a0acffa5/go.mod h1:70zkFmudgCuE/ngEzBv17Jvp/497gISqfk5gWijbERA= github.com/urfave/cli v1.20.0/go.mod h1:70zkFmudgCuE/ngEzBv17Jvp/497gISqfk5gWijbERA= @@ -317,7 +293,6 @@ github.com/xeipuuv/gojsonschema v0.0.0-20180618132009-1d523034197f/go.mod h1:5yf github.com/xiang90/probing v0.0.0-20190116061207-43a291ad63a2 h1:eY9dn8+vbi4tKz5Qo6v2eYzo7kUS51QINcR5jNpbZS8= github.com/xiang90/probing v0.0.0-20190116061207-43a291ad63a2/go.mod h1:UETIi67q53MR2AWcXfiuqkDkRtnGDLqkBTpCHuJHxtU= github.com/xordataexchange/crypt v0.0.3-0.20170626215501-b2862e3d0a77/go.mod h1:aYKd//L2LvnjZzWKhF00oedf4jCCReLcmhLdhm1A27Q= -go.etcd.io/bbolt v1.3.2/go.mod h1:IbVyRI1SCnLcuJnV2u8VeU0CEYM7e686BmAb1XKL+uU= go.etcd.io/bbolt v1.3.3 h1:MUGmc65QhB3pIlaQ5bB4LwqSj6GIonVJXpZiaKNyaKk= go.etcd.io/bbolt v1.3.3/go.mod h1:IbVyRI1SCnLcuJnV2u8VeU0CEYM7e686BmAb1XKL+uU= go.etcd.io/etcd v0.0.0-20191023171146-3cf2f69b5738 h1:VcrIfasaLFkyjk6KNlXQSzO+B0fZcnECiDrKJsfxka0= @@ -359,7 +334,6 @@ golang.org/x/net v0.0.0-20190311183353-d8887717615a/go.mod h1:t9HGtf8HONx5eT2rtn golang.org/x/net v0.0.0-20190404232315-eb5bcb51f2a3/go.mod h1:t9HGtf8HONx5eT2rtn7q6eTqICYqUVnKs3thJo3Qplg= golang.org/x/net v0.0.0-20190501004415-9ce7a6920f09/go.mod h1:t9HGtf8HONx5eT2rtn7q6eTqICYqUVnKs3thJo3Qplg= golang.org/x/net v0.0.0-20190503192946-f4e77d36d62c/go.mod h1:t9HGtf8HONx5eT2rtn7q6eTqICYqUVnKs3thJo3Qplg= -golang.org/x/net v0.0.0-20190522155817-f3200d17e092/go.mod h1:HSz+uSET+XFnRR8LxR5pz3Of3rY3CfYBVs4xY44aLks= golang.org/x/net v0.0.0-20190603091049-60506f45cf65/go.mod h1:HSz+uSET+XFnRR8LxR5pz3Of3rY3CfYBVs4xY44aLks= golang.org/x/net v0.0.0-20190613194153-d28f0bde5980/go.mod h1:z5CRVTTTmAJ677TzLLGU+0bjPO0LkuOLi4/5GtJWs/s= golang.org/x/net v0.0.0-20190813141303-74dc4d7220e7 h1:fHDIZ2oxGnUZRN6WgWFCbYBjH9uqVPRCUVUDhs0wnbA= @@ -368,8 +342,6 @@ golang.org/x/oauth2 v0.0.0-20180821212333-d2e6202438be/go.mod h1:N/0e6XlmueqKjAG golang.org/x/oauth2 v0.0.0-20190226205417-e64efc72b421/go.mod h1:gOpvHmFTYa4IltrdGE7lF6nIHvwfUNPOp7c8zoXwtLw= golang.org/x/oauth2 v0.0.0-20190604053449-0f29369cfe45 h1:SVwTIAaPC2U/AvvLNZ2a7OVsmBpC8L5BlwK1whH3hm0= golang.org/x/oauth2 v0.0.0-20190604053449-0f29369cfe45/go.mod h1:gOpvHmFTYa4IltrdGE7lF6nIHvwfUNPOp7c8zoXwtLw= -golang.org/x/oauth2 v0.0.0-20200107190931-bf48bf16ab8d h1:TzXSXBo42m9gQenoE3b9BGiEpg5IG2JkU5FkPIawgtw= -golang.org/x/oauth2 v0.0.0-20200107190931-bf48bf16ab8d/go.mod h1:gOpvHmFTYa4IltrdGE7lF6nIHvwfUNPOp7c8zoXwtLw= golang.org/x/sync v0.0.0-20180314180146-1d60e4601c6f/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM= golang.org/x/sync v0.0.0-20181108010431-42b317875d0f h1:Bl/8QSvNqXvPGPGXa2z5xUTmV7VDcZyvRZ+QQXkXTZQ= golang.org/x/sync v0.0.0-20181108010431-42b317875d0f/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM= @@ -439,7 +411,6 @@ google.golang.org/genproto v0.0.0-20190819201941-24fa4b261c55/go.mod h1:DMBHOl98 google.golang.org/grpc v1.17.0/go.mod h1:6QZJwpn2B+Zp71q/5VxRsJ6NXXVCE5NRUHRo+f3cWCs= google.golang.org/grpc v1.19.0/go.mod h1:mqu4LbDTu4XGKhr4mRzUsmM4RtVoemTSY81AxZiDr8c= google.golang.org/grpc v1.20.1/go.mod h1:10oTOabMzJvdu6/UiuZezV6QK5dSlG84ov/aaiqXj38= -google.golang.org/grpc v1.21.0/go.mod h1:oYelfM1adQP15Ek0mdvEgi9Df8B9CZIaU1084ijfRaM= google.golang.org/grpc v1.23.0 h1:AzbTB6ux+okLTzP8Ru1Xs41C303zdcfEht7MQnYJt5A= google.golang.org/grpc v1.23.0/go.mod h1:Y5yQAOtifL1yxbo5wqy6BxZv8vAUGQwXBOALyacEbxg= google.golang.org/grpc v1.23.1 h1:q4XQuHFC6I28BKZpo6IYyb3mNO+l7lSOxRuYTCiDfXk= @@ -462,8 +433,6 @@ gopkg.in/ldap.v2 v2.5.1/go.mod h1:oI0cpe/D7HRtBQl8aTg+ZmzFUAvu4lsv3eLXMLGFxWk= gopkg.in/resty.v1 v1.12.0/go.mod h1:mDo4pnntr5jdWRML875a/NmxYqAlA73dVijT2AXvQQo= gopkg.in/square/go-jose.v2 v2.4.1 h1:H0TmLt7/KmzlrDOpa1F+zr0Tk90PbJYBfsVUmRLrf9Y= gopkg.in/square/go-jose.v2 v2.4.1/go.mod h1:M9dMgbHiYLoDGQrXy7OpJDJWiKiU//h+vD76mk0e1AI= -gopkg.in/square/go-jose.v2 v2.5.1 h1:7odma5RETjNHWJnR32wx8t+Io4djHE1PqxCFx3iiZ2w= -gopkg.in/square/go-jose.v2 v2.5.1/go.mod h1:M9dMgbHiYLoDGQrXy7OpJDJWiKiU//h+vD76mk0e1AI= gopkg.in/tomb.v1 v1.0.0-20141024135613-dd632973f1e7 h1:uRGJdciOHaEIrze2W8Q3AKkepLTh2hOroT7a+7czfdQ= gopkg.in/tomb.v1 v1.0.0-20141024135613-dd632973f1e7/go.mod h1:dt/ZhP58zS4L8KSrWDmTeBkI65Dw0HsyUHuEVlX15mw= gopkg.in/yaml.v2 v2.0.0-20170812160011-eb3733d160e7/go.mod h1:JAlM8MvJe8wmxCU4Bli9HhUf9+ttbYbLASfIpnQbh74= From 1059ba7392b1cc49a8379ead595b50eda4e9da5a Mon Sep 17 00:00:00 2001 From: Bernd Eckstein Date: Mon, 21 Sep 2020 14:07:15 +0200 Subject: [PATCH 18/24] Don't omit client secret check for PKCE Signed-off-by: Bernd Eckstein --- server/handlers.go | 9 ++------- 1 file changed, 2 insertions(+), 7 deletions(-) diff --git a/server/handlers.go b/server/handlers.go index 49b8b6241e..cb1eae1f3f 100644 --- a/server/handlers.go +++ b/server/handlers.go @@ -759,17 +759,12 @@ func (s *Server) handleToken(w http.ResponseWriter, r *http.Request) { } return } - - grantType := r.PostFormValue("grant_type") - codeVerifier := r.PostFormValue("code_verifier") - - if client.Public && grantType == grantTypeAuthorizationCode && codeVerifier != "" && clientSecret == "" { - // RFC 7636 (PKCE) if code_verifier is received, use PKCE and allow empty clientSecret, when client is public - } else if client.Secret != clientSecret { + if client.Secret != clientSecret { s.tokenErrHelper(w, errInvalidClient, "Invalid client credentials.", http.StatusUnauthorized) return } + grantType := r.PostFormValue("grant_type") switch grantType { case grantTypeAuthorizationCode: s.handleAuthCode(w, r, client) From b6e297b78537dc44cd3e1374f0b4d34bf89404ac Mon Sep 17 00:00:00 2001 From: Martin Heide Date: Mon, 5 Oct 2020 09:14:43 +0000 Subject: [PATCH 19/24] Allow public clients (e.g. with PKCE) to have redirect URIs configured Signed-off-by: Martin Heide --- server/oauth2.go | 13 ++++++++----- server/oauth2_test.go | 25 +++++++++++++++++++++++++ 2 files changed, 33 insertions(+), 5 deletions(-) diff --git a/server/oauth2.go b/server/oauth2.go index 79e54c3297..9ae825ab4e 100644 --- a/server/oauth2.go +++ b/server/oauth2.go @@ -588,12 +588,15 @@ func (s *Server) validateCrossClientTrust(clientID, peerID string) (trusted bool } func validateRedirectURI(client storage.Client, redirectURI string) bool { - if !client.Public { - for _, uri := range client.RedirectURIs { - if redirectURI == uri { - return true - } + // Allow named RedirectURIs for both public and non-public clients. + // This is required make PKCE-enabled web apps work, when configured as public clients. + for _, uri := range client.RedirectURIs { + if redirectURI == uri { + return true } + } + // For non-public clients, only named RedirectURIs are allowed. + if !client.Public { return false } diff --git a/server/oauth2_test.go b/server/oauth2_test.go index 8db9ea5929..f75015f053 100644 --- a/server/oauth2_test.go +++ b/server/oauth2_test.go @@ -340,6 +340,7 @@ func TestValidRedirectURI(t *testing.T) { RedirectURIs: []string{"http://foo.com/bar"}, }, redirectURI: "http://foo.com/bar/baz", + wantValid: false, }, { client: storage.Client{ @@ -369,6 +370,30 @@ func TestValidRedirectURI(t *testing.T) { redirectURI: "http://localhost", wantValid: true, }, + // Both Public + RedirectURIs configured: Could e.g. be a PKCE-enabled web app. + { + client: storage.Client{ + Public: true, + RedirectURIs: []string{"http://foo.com/bar"}, + }, + redirectURI: "http://foo.com/bar", + wantValid: true, + }, + { + client: storage.Client{ + Public: true, + RedirectURIs: []string{"http://foo.com/bar"}, + }, + redirectURI: "http://foo.com/bar/baz", + wantValid: false, + }, + { + client: storage.Client{ + Public: true, + }, + redirectURI: "http://foo.com/bar", + wantValid: false, + }, { client: storage.Client{ Public: true, From 46c6d9d9341fd3a2044e7a8b4f34fdd98f0f1eb6 Mon Sep 17 00:00:00 2001 From: Bernd Eckstein Date: Mon, 5 Oct 2020 11:24:06 +0200 Subject: [PATCH 20/24] Remove "Authorization" as Accepted Headers on CORS, small fixes Signed-off-by: Bernd Eckstein --- server/handlers.go | 2 +- server/server.go | 3 +-- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/server/handlers.go b/server/handlers.go index cb1eae1f3f..4b5dddf9f3 100644 --- a/server/handlers.go +++ b/server/handlers.go @@ -785,7 +785,6 @@ func (s *Server) calculateCodeChallenge(codeVerifier, codeChallengeMethod string shaSum := sha256.Sum256([]byte(codeVerifier)) return base64.RawURLEncoding.EncodeToString(shaSum[:]), nil default: - s.logger.Errorf("unknown challenge method (%v)", codeChallengeMethod) return "", fmt.Errorf("unknown challenge method (%v)", codeChallengeMethod) } } @@ -813,6 +812,7 @@ func (s *Server) handleAuthCode(w http.ResponseWriter, r *http.Request, client s if providedCodeVerifier != "" && codeChallengeFromStorage != "" { calculatedCodeChallenge, err := s.calculateCodeChallenge(providedCodeVerifier, authCode.PKCE.CodeChallengeMethod) if err != nil { + s.logger.Error(err) s.tokenErrHelper(w, errServerError, "", http.StatusInternalServerError) return } diff --git a/server/server.go b/server/server.go index 904d0fb0c0..f4d139d151 100644 --- a/server/server.go +++ b/server/server.go @@ -287,8 +287,7 @@ func newServer(ctx context.Context, c Config, rotationStrategy rotationStrategy) var handler http.Handler = h if len(c.AllowedOrigins) > 0 { corsOption := handlers.AllowedOrigins(c.AllowedOrigins) - corsHeaders := handlers.AllowedHeaders([]string{"Authorization"}) - handler = handlers.CORS(corsOption, corsHeaders)(handler) + handler = handlers.CORS(corsOption)(handler) } r.Handle(path.Join(issuerURL.Path, p), instrumentHandlerCounter(p, handler)) } From 5435741ef9a8933811024a7011460879846e4f6d Mon Sep 17 00:00:00 2001 From: Martin Heide Date: Mon, 5 Oct 2020 18:26:43 +0000 Subject: [PATCH 21/24] Revert "Allow public clients (e.g. with PKCE) to have redirect URIs configured" This reverts commit b6e297b78537dc44cd3e1374f0b4d34bf89404ac. Signed-off-by: Martin Heide --- server/oauth2.go | 13 +++++-------- server/oauth2_test.go | 25 ------------------------- 2 files changed, 5 insertions(+), 33 deletions(-) diff --git a/server/oauth2.go b/server/oauth2.go index 9ae825ab4e..79e54c3297 100644 --- a/server/oauth2.go +++ b/server/oauth2.go @@ -588,15 +588,12 @@ func (s *Server) validateCrossClientTrust(clientID, peerID string) (trusted bool } func validateRedirectURI(client storage.Client, redirectURI string) bool { - // Allow named RedirectURIs for both public and non-public clients. - // This is required make PKCE-enabled web apps work, when configured as public clients. - for _, uri := range client.RedirectURIs { - if redirectURI == uri { - return true - } - } - // For non-public clients, only named RedirectURIs are allowed. if !client.Public { + for _, uri := range client.RedirectURIs { + if redirectURI == uri { + return true + } + } return false } diff --git a/server/oauth2_test.go b/server/oauth2_test.go index f75015f053..8db9ea5929 100644 --- a/server/oauth2_test.go +++ b/server/oauth2_test.go @@ -340,7 +340,6 @@ func TestValidRedirectURI(t *testing.T) { RedirectURIs: []string{"http://foo.com/bar"}, }, redirectURI: "http://foo.com/bar/baz", - wantValid: false, }, { client: storage.Client{ @@ -370,30 +369,6 @@ func TestValidRedirectURI(t *testing.T) { redirectURI: "http://localhost", wantValid: true, }, - // Both Public + RedirectURIs configured: Could e.g. be a PKCE-enabled web app. - { - client: storage.Client{ - Public: true, - RedirectURIs: []string{"http://foo.com/bar"}, - }, - redirectURI: "http://foo.com/bar", - wantValid: true, - }, - { - client: storage.Client{ - Public: true, - RedirectURIs: []string{"http://foo.com/bar"}, - }, - redirectURI: "http://foo.com/bar/baz", - wantValid: false, - }, - { - client: storage.Client{ - Public: true, - }, - redirectURI: "http://foo.com/bar", - wantValid: false, - }, { client: storage.Client{ Public: true, From dd6de362c5678363955795a66eca0b1070386161 Mon Sep 17 00:00:00 2001 From: Bernd Eckstein Date: Wed, 14 Oct 2020 11:22:20 +0200 Subject: [PATCH 22/24] PKCE on client_secret client error message * When connecting to the token endpoint with PKCE without client_secret, but the client is configured with a client_secret, generate a special error message. Signed-off-by: Bernd Eckstein --- server/handlers.go | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/server/handlers.go b/server/handlers.go index 4b5dddf9f3..f368777454 100644 --- a/server/handlers.go +++ b/server/handlers.go @@ -759,6 +759,10 @@ func (s *Server) handleToken(w http.ResponseWriter, r *http.Request) { } return } + if clientSecret == "" && client.Secret != "" && r.PostFormValue("code_verifier") != "" { + s.tokenErrHelper(w, errInvalidClient, "Missing client credentials. If you want to use PKCE without client_secret, create a public dex client.", http.StatusUnauthorized) + return + } if client.Secret != clientSecret { s.tokenErrHelper(w, errInvalidClient, "Invalid client credentials.", http.StatusUnauthorized) return From a3ed22930e22263eb4132f7b50278ed1d87b674f Mon Sep 17 00:00:00 2001 From: Bernd Eckstein Date: Thu, 15 Oct 2020 09:07:05 +0200 Subject: [PATCH 23/24] Output info message when PKCE without client_secret used on confidential client * removes the special error message Signed-off-by: Bernd Eckstein --- server/handlers.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/server/handlers.go b/server/handlers.go index f368777454..dc80f6c828 100644 --- a/server/handlers.go +++ b/server/handlers.go @@ -760,8 +760,8 @@ func (s *Server) handleToken(w http.ResponseWriter, r *http.Request) { return } if clientSecret == "" && client.Secret != "" && r.PostFormValue("code_verifier") != "" { - s.tokenErrHelper(w, errInvalidClient, "Missing client credentials. If you want to use PKCE without client_secret, create a public dex client.", http.StatusUnauthorized) - return + s.logger.Infof("detected PKCE token request without client_secret on client %s. "+ + "Set the client to be pubic without client_secret, if you want to allow this.", client.ID) } if client.Secret != clientSecret { s.tokenErrHelper(w, errInvalidClient, "Invalid client credentials.", http.StatusUnauthorized) From 7b369a63df62f020ec2cb6041a850bff6b99d123 Mon Sep 17 00:00:00 2001 From: Bernd Eckstein Date: Fri, 16 Oct 2020 11:55:20 +0200 Subject: [PATCH 24/24] General missing/invalid client_secret message on token endpoint Signed-off-by: Bernd Eckstein --- server/handlers.go | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/server/handlers.go b/server/handlers.go index dc80f6c828..add2031ccc 100644 --- a/server/handlers.go +++ b/server/handlers.go @@ -759,11 +759,12 @@ func (s *Server) handleToken(w http.ResponseWriter, r *http.Request) { } return } - if clientSecret == "" && client.Secret != "" && r.PostFormValue("code_verifier") != "" { - s.logger.Infof("detected PKCE token request without client_secret on client %s. "+ - "Set the client to be pubic without client_secret, if you want to allow this.", client.ID) - } if client.Secret != clientSecret { + if clientSecret == "" { + s.logger.Infof("missing client_secret on token request for client: %s", client.ID) + } else { + s.logger.Infof("invalid client_secret on token request for client: %s", client.ID) + } s.tokenErrHelper(w, errInvalidClient, "Invalid client credentials.", http.StatusUnauthorized) return }