ce750e9 · typeable/oauth2-proxy

git.iko.soy/typeable/oauth2-proxy

PKCE Support (#1541)

* Add the allowed_email_domains and the allowed_groups on the auth_request endpoint + support standard wildcard char for validation with sub-domain and email-domain. Signed-off-by: Valentin Pichard <github@w3st.fr> * Fix provider data initialisation * PKCE Support Adds Code Challenge PKCE support (RFC-7636) and partial Authorization Server Metadata (RFC-8414) for detecting PKCE support. - Introduces new option `--force-code-challenge-method` to force a specific code challenge method (either `S256` or `plain`) for instances when the server has not implemented RFC-8414 in order to detect PKCE support on the discovery document. - In all other cases, if the PKCE support can be determined during discovery then the `code_challenge_methods_supported` is used and S256 is always preferred. - The force command line argument is helpful with some providers like Azure who supports PKCE but does not list it in their discovery document yet. - Initial thought was given to just always attempt PKCE since according to spec additional URL parameters should be dropped by servers which implemented OAuth 2, however other projects found cases in the wild where this causes 500 errors by buggy implementations. See: https://github.com/spring-projects/spring-security/pull/7804#issuecomment-578323810 - Due to the fact that the `code_verifier` must be saved between the redirect and callback, sessions are now created when the redirect takes place with `Authenticated: false`. The session will be recreated and marked as `Authenticated` on callback. - Individual provider implementations can choose to include or ignore code_challenge and code_verifier function parameters passed to them Note: Technically speaking `plain` is not required to be implemented since oauth2-proxy will always be able to handle S256 and servers MUST implement S256 support. > If the client is capable of using "S256", it MUST use "S256", as "S256" > is Mandatory To Implement (MTI) on the server. Clients are permitted > to use "plain" only if they cannot support "S256" for some technical > reason and know via out-of-band configuration that the server supports > "plain". Ref: RFC-7636 Sec 4.2 oauth2-proxy will always use S256 unless the user explicitly forces `plain`. Fixes #1361 * Address PR comments by moving pkce generation * Make PKCE opt-in, move to using the Nonce generater for code verifier * Make PKCE opt-in, move to using the Nonce generater for code verifier * Encrypt CodeVerifier in CSRF Token instead of Session - Update Dex for PKCE support - Expose HTTPBin for further use cases * Correct the tests * Move code challenges into extra params * Correct typo in code challenge method Co-authored-by: Joel Speed <Joel.speed@hotmail.co.uk> * Correct the extra space in docs Co-authored-by: Joel Speed <Joel.speed@hotmail.co.uk> * Address changelog and new line nits * Add generated docs Co-authored-by: Valentin Pichard <github@w3st.fr> Co-authored-by: Joel Speed <joel.speed@hotmail.co.uk>

commitce750e9b301ac66b3b8cbdbd0bc8ac7415bfed79 authorBraunson <5280764+braunsonm@users.noreply.github.com> dateSun 13 Mar 2022, 06:08 (5 years ago) parent45ef848

27 files changed+250−59

compare with master

unifiedsplit

.gitignore+1

@@ -18,6 +18,7 @@ c.out

1818_obj

1919_test

2020.idea/

21+.vscode/

2122

2223# Architecture specific extensions/prefixes

2324*.[568vq]

CHANGELOG.md+6

@@ -2,6 +2,11 @@

33## Release Highlights

5+- [#1361](https://github.com/oauth2-proxy/oauth2-proxy/pull/1541) PKCE Code Challenge Support - RFC-7636 (@braunsonm)

6+ - At this time the `--code-challenge-method` flag can be used to enable it with the method of your choice.

7+- Parital support for OAuth2 Authorization Server Metadata for detecting code challenge methods (@braunsonm)

8+ - A warning will be displayed when your provider advertises support for PKCE but you have not enabled it.

510## Important Notes

611

712## Breaking Changes

@@ -23,6 +28,7 @@

2328- [#1474](https://github.com/oauth2-proxy/oauth2-proxy/pull/1474) Support configuration of minimal acceptable TLS version (@polarctos)

2429- [#1545](https://github.com/oauth2-proxy/oauth2-proxy/pull/1545) Fix issue with query string allowed group panic on skip methods (@andytson)

2530- [#1286](https://github.com/oauth2-proxy/oauth2-proxy/pull/1286) Add the `allowed_email_domains` and the `allowed_groups` on the `auth_request` + support standard wildcard char for validation with sub-domain and email-domain. (@w3st3ry @armandpicard)

31+- [#1361](https://github.com/oauth2-proxy/oauth2-proxy/pull/1541) PKCE Code Challenge Support - RFC-7636 (@braunsonm)

2632

2733# V7.2.1

2834

contrib/local-environment/docker-compose.yaml+4−2

@@ -29,8 +29,8 @@ services:

2929 - httpbin

3030 dex:

3131 container_name: dex

32− image: quay.io/dexidp/dex:v2.23.0

33− command: serve /dex.yaml

32+ image: ghcr.io/dexidp/dex:v2.30.3

33+ command: dex serve /dex.yaml

3434 ports:

3535 - 4190:4190/tcp

3636 hostname: dex

@@ -47,6 +47,8 @@ services:

4747 httpbin:

4848 container_name: httpbin

4949 image: kennethreitz/httpbin

50+ ports:

51+ - 8080:80/tcp

5052 networks:

5153 httpbin: {}

5254 etcd:

docs/docs/configuration/alpha_config.md+1

@@ -419,6 +419,7 @@ Provider holds all configuration for a single provider

419419| `validateURL` | _string_ | ValidateURL is the access token validation endpoint |

420420| `scope` | _string_ | Scope is the OAuth scope specification |

421421| `allowedGroups` | _[]string_ | AllowedGroups is a list of restrict logins to members of this group |

422+| `force_code_challenge_method` | _string_ | The forced code challenge method |

422423

423424### ProviderType

424425#### (`string` alias)

docs/docs/configuration/overview.md+1

@@ -84,6 +84,7 @@ An example [oauth2-proxy.cfg](https://github.com/oauth2-proxy/oauth2-proxy/blob/

8889| `--cookie-domain` | string \| list | Optional cookie domains to force cookies to (e.g. `.yourcompany.com`). The longest domain matching the request's host will be used (or the shortest cookie domain if there is no match). | |

oauthproxy.go+35−11

@@ -2,6 +2,7 @@ package main

33import (

44 "context"

5+ "encoding/base64"

56 "encoding/json"

67 "errors"

78 "fmt"

@@ -25,6 +26,7 @@ import (

2526 "github.com/oauth2-proxy/oauth2-proxy/v7/pkg/app/redirect"

2627 "github.com/oauth2-proxy/oauth2-proxy/v7/pkg/authentication/basic"

2728 "github.com/oauth2-proxy/oauth2-proxy/v7/pkg/cookies"

29+ "github.com/oauth2-proxy/oauth2-proxy/v7/pkg/encryption"

2830 proxyhttp "github.com/oauth2-proxy/oauth2-proxy/v7/pkg/http"

2931 "github.com/oauth2-proxy/oauth2-proxy/v7/pkg/util"

3032

@@ -680,7 +682,29 @@ func (p *OAuthProxy) doOAuthStart(rw http.ResponseWriter, req *http.Request, ove

680682 extraParams := p.provider.Data().LoginURLParams(overrides)

681683 prepareNoCache(rw)

682684

683− csrf, err := cookies.NewCSRF(p.CookieOptions)

685+ var codeChallenge, codeVerifier, codeChallengeMethod string

686+ if p.provider.Data().CodeChallengeMethod != "" {

687+ codeChallengeMethod = p.provider.Data().CodeChallengeMethod

688+ preEncodedCodeVerifier, err := encryption.Nonce(96)

689+ if err != nil {

690+ logger.Errorf("Unable to build random string: %v", err)

691+ p.ErrorPage(rw, req, http.StatusInternalServerError, err.Error())

692+ return

693+ }

694+ codeVerifier = base64.RawURLEncoding.EncodeToString(preEncodedCodeVerifier)

695+

696+ codeChallenge, err = encryption.GenerateCodeChallenge(p.provider.Data().CodeChallengeMethod, codeVerifier)

697+ if err != nil {

698+ logger.Errorf("Error creating code challenge: %v", err)

699+ p.ErrorPage(rw, req, http.StatusInternalServerError, err.Error())

700+ return

701+ }

702+

703+ extraParams.Add("code_challenge", codeChallenge)

704+ extraParams.Add("code_challenge_method", codeChallengeMethod)

705+ }

706+

707+ csrf, err := cookies.NewCSRF(p.CookieOptions, codeVerifier)

684708 if err != nil {

685709 logger.Errorf("Error creating CSRF nonce: %v", err)

686710 p.ErrorPage(rw, req, http.StatusInternalServerError, err.Error())

@@ -732,24 +756,24 @@ func (p *OAuthProxy) OAuthCallback(rw http.ResponseWriter, req *http.Request) {

732756 return

733757 }

734758

735− session, err := p.redeemCode(req)

759+ csrf, err := cookies.LoadCSRFCookie(req, p.CookieOptions)

736760 if err != nil {

737− logger.Errorf("Error redeeming code during OAuth2 callback: %v", err)

738− p.ErrorPage(rw, req, http.StatusInternalServerError, err.Error())

761+ logger.Println(req, logger.AuthFailure, "Invalid authentication via OAuth2: unable to obtain CSRF cookie")

762+ p.ErrorPage(rw, req, http.StatusForbidden, err.Error(), "Login Failed: Unable to find a valid CSRF token. Please try again.")

739763 return

740764 }

741765

742− err = p.enrichSessionState(req.Context(), session)

766+ session, err := p.redeemCode(req, csrf.GetCodeVerifier())

743767 if err != nil {

744− logger.Errorf("Error creating session during OAuth2 callback: %v", err)

768+ logger.Errorf("Error redeeming code during OAuth2 callback: %v", err)

745769 p.ErrorPage(rw, req, http.StatusInternalServerError, err.Error())

746770 return

747771 }

748772

749− csrf, err := cookies.LoadCSRFCookie(req, p.CookieOptions)

773+ err = p.enrichSessionState(req.Context(), session)

750774 if err != nil {

751− logger.PrintAuthf(session.Email, req, logger.AuthFailure, "Invalid authentication via OAuth2: unable to obtain CSRF cookie")

752− p.ErrorPage(rw, req, http.StatusForbidden, err.Error(), "Login Failed: Unable to find a valid CSRF token. Please try again.")

775+ logger.Errorf("Error creating session during OAuth2 callback: %v", err)

776+ p.ErrorPage(rw, req, http.StatusInternalServerError, err.Error())

753777 return

754778 }

755779

@@ -799,14 +823,14 @@ func (p *OAuthProxy) OAuthCallback(rw http.ResponseWriter, req *http.Request) {

799823 }

800824}

801825

802−func (p *OAuthProxy) redeemCode(req *http.Request) (*sessionsapi.SessionState, error) {

826+func (p *OAuthProxy) redeemCode(req *http.Request, codeVerifier string) (*sessionsapi.SessionState, error) {

803827 code := req.Form.Get("code")

804828 if code == "" {

805829 return nil, providers.ErrMissingCode

806830 }

807831

808832 redirectURI := p.getOAuthRedirectURI(req)

809− s, err := p.provider.Redeem(req.Context(), redirectURI, code)

833+ s, err := p.provider.Redeem(req.Context(), redirectURI, code, codeVerifier)

810834 if err != nil {

811835 return nil, err

812836 }

oauthproxy_test.go+2−2

@@ -115,7 +115,7 @@ func Test_redeemCode(t *testing.T) {

115115 }

116116

117117 req := httptest.NewRequest(http.MethodGet, "/", nil)

118− _, err = proxy.redeemCode(req)

118+ _, err = proxy.redeemCode(req, "")

119119 assert.Equal(t, providers.ErrMissingCode, err)

120120}

121121

@@ -405,7 +405,7 @@ func (patTest *PassAccessTokenTest) Close() {

405405func (patTest *PassAccessTokenTest) getCallbackEndpoint() (httpCode int, cookie string) {

406406 rw := httptest.NewRecorder()

407407

408− csrf, err := cookies.NewCSRF(patTest.proxy.CookieOptions)

408+ csrf, err := cookies.NewCSRF(patTest.proxy.CookieOptions, "")

409409 if err != nil {

410410 panic(err)

411411 }

pkg/apis/options/legacy_options.go+16−12

@@ -519,6 +519,8 @@ type LegacyProvider struct {

519519 JWTKey string `flag:"jwt-key" cfg:"jwt_key"`

520520 JWTKeyFile string `flag:"jwt-key-file" cfg:"jwt_key_file"`

521521 PubJWKURL string `flag:"pubjwk-url" cfg:"pubjwk_url"`

522+ // PKCE Code Challenge method to use (either S256 or plain)

523+ CodeChallengeMethod string `flag:"code-challenge-method" cfg:"force_code_challenge_method"`

522524}

523525

524526func legacyProviderFlagSet() *pflag.FlagSet {

@@ -563,6 +565,7 @@ func legacyProviderFlagSet() *pflag.FlagSet {

563565 flagSet.String("scope", "", "OAuth scope specification")

564566 flagSet.String("prompt", "", "OIDC prompt")

565567 flagSet.String("approval-prompt", "force", "OAuth approval_prompt")

568+ flagSet.String("code-challenge-method", "", "use PKCE code challenges with the specified method. Either 'plain' or 'S256'")

566569

567570 flagSet.String("acr-values", "", "acr values string: optional")

568571 flagSet.String("jwt-key", "", "private key in PEM format used to sign JWT, so that you can say something like -jwt-key=\"${OAUTH2_PROXY_JWT_KEY}\": required by login.gov")

@@ -621,18 +624,19 @@ func (l *LegacyProvider) convert() (Providers, error) {

621624 providers := Providers{}

622625

623626 provider := Provider{

624− ClientID: l.ClientID,

625− ClientSecret: l.ClientSecret,

626− ClientSecretFile: l.ClientSecretFile,

627− Type: ProviderType(l.ProviderType),

628− CAFiles: l.ProviderCAFiles,

629− LoginURL: l.LoginURL,

630− RedeemURL: l.RedeemURL,

631− ProfileURL: l.ProfileURL,

632− ProtectedResource: l.ProtectedResource,

633− ValidateURL: l.ValidateURL,

634− Scope: l.Scope,

635− AllowedGroups: l.AllowedGroups,

627+ ClientID: l.ClientID,

628+ ClientSecret: l.ClientSecret,

629+ ClientSecretFile: l.ClientSecretFile,

630+ Type: ProviderType(l.ProviderType),

631+ CAFiles: l.ProviderCAFiles,

632+ LoginURL: l.LoginURL,

633+ RedeemURL: l.RedeemURL,

634+ ProfileURL: l.ProfileURL,

635+ ProtectedResource: l.ProtectedResource,

636+ ValidateURL: l.ValidateURL,

637+ Scope: l.Scope,

638+ AllowedGroups: l.AllowedGroups,

639+ CodeChallengeMethod: l.CodeChallengeMethod,

636640 }

637641

638642 // This part is out of the switch section for all providers that support OIDC

pkg/apis/options/providers.go+2

@@ -76,6 +76,8 @@ type Provider struct {

7676 Scope string `json:"scope,omitempty"`

7777 // AllowedGroups is a list of restrict logins to members of this group

7878 AllowedGroups []string `json:"allowedGroups,omitempty"`

79+ // The forced code challenge method

80+ CodeChallengeMethod string `json:"force_code_challenge_method,omitempty"`

7981}

8082

8183// ProviderType is used to enumerate the different provider type options

pkg/cookies/csrf.go+16−5

@@ -20,6 +20,7 @@ type CSRF interface {

2020 HashOIDCNonce() string

2121 CheckOAuthState(string) bool

2222 CheckOIDCNonce(string) bool

23+ GetCodeVerifier() string

2324

2425 SetSessionNonce(s *sessions.SessionState)

2526

@@ -38,24 +39,30 @@ type csrf struct {

3839 // is used to mitigate replay attacks.

3940 OIDCNonce []byte `msgpack:"n,omitempty"`

4041

42+ // CodeVerifier holds the unobfuscated PKCE code verification string

43+ // which is used to compare the code challenge when exchanging the

44+ // authentication code.

45+ CodeVerifier string `msgpack:"cv,omitempty"`

46+

4147 cookieOpts *options.Cookie

4248 time clock.Clock

4349}

4450

4551// NewCSRF creates a CSRF with random nonces

46−func NewCSRF(opts *options.Cookie) (CSRF, error) {

47− state, err := encryption.Nonce()

52+func NewCSRF(opts *options.Cookie, codeVerifier string) (CSRF, error) {

53+ state, err := encryption.Nonce(32)

4854 if err != nil {

4955 return nil, err

5056 }

51− nonce, err := encryption.Nonce()

57+ nonce, err := encryption.Nonce(32)

5258 if err != nil {

5359 return nil, err

5460 }

5561

5662 return &csrf{

57− OAuthState: state,

58− OIDCNonce: nonce,

63+ OAuthState: state,

64+ OIDCNonce: nonce,

65+ CodeVerifier: codeVerifier,

5966

6067 cookieOpts: opts,

6168 }, nil

@@ -71,6 +78,10 @@ func LoadCSRFCookie(req *http.Request, opts *options.Cookie) (CSRF, error) {

7178 return decodeCSRFCookie(cookie, opts)

7279}

7380

81+func (c *csrf) GetCodeVerifier() string {

82+ return c.CodeVerifier

83+}

84+

7485// HashOAuthState returns the hash of the OAuth state nonce

7586func (c *csrf) HashOAuthState() string {

7687 return encryption.HashNonce(c.OAuthState)

pkg/cookies/csrf_test.go+5−2

@@ -33,7 +33,7 @@ var _ = Describe("CSRF Cookie Tests", func() {

3333 }

3434

3535 var err error

36− publicCSRF, err = NewCSRF(cookieOpts)

36+ publicCSRF, err = NewCSRF(cookieOpts, "verifier")

3737 Expect(err).ToNot(HaveOccurred())

3838

3939 privateCSRF = publicCSRF.(*csrf)

@@ -44,14 +44,16 @@ var _ = Describe("CSRF Cookie Tests", func() {

4444 Expect(privateCSRF.OAuthState).ToNot(BeEmpty())

4545 Expect(privateCSRF.OIDCNonce).ToNot(BeEmpty())

4646 Expect(privateCSRF.OAuthState).ToNot(Equal(privateCSRF.OIDCNonce))

47+ Expect(privateCSRF.CodeVerifier).To(Equal("verifier"))

4748 })

4849

4950 It("makes unique nonces between multiple CSRFs", func() {

50− other, err := NewCSRF(cookieOpts)

51+ other, err := NewCSRF(cookieOpts, "verifier")

5152 Expect(err).ToNot(HaveOccurred())

5253

5354 Expect(privateCSRF.OAuthState).ToNot(Equal(other.(*csrf).OAuthState))

5455 Expect(privateCSRF.OIDCNonce).ToNot(Equal(other.(*csrf).OIDCNonce))

56+ Expect(privateCSRF.CodeVerifier).To(Equal("verifier"))

5557 })

5658 })

5759

@@ -72,6 +74,7 @@ var _ = Describe("CSRF Cookie Tests", func() {

7274 Expect(publicCSRF.CheckOIDCNonce(csrfNonce + csrfState)).To(BeFalse())

7375 Expect(publicCSRF.CheckOAuthState("")).To(BeFalse())

7476 Expect(publicCSRF.CheckOIDCNonce("")).To(BeFalse())

77+ Expect(publicCSRF.GetCodeVerifier()).To(Equal("verifier"))

7578 })

7679 })

7780

pkg/encryption/nonce.go+3−3

@@ -8,9 +8,9 @@ import (

88 "golang.org/x/crypto/blake2b"

99)

1010

11−// Nonce generates a random 32-byte slice to be used as a nonce

12−func Nonce() ([]byte, error) {

13− b := make([]byte, 32)

11+// Nonce generates a random n-byte slice

12+func Nonce(length int) ([]byte, error) {

13+ b := make([]byte, length)

1414 _, err := rand.Read(b)

1515 if err != nil {

1616 return nil, err

pkg/encryption/utils.go+17

@@ -12,6 +12,11 @@ import (

1212 "time"

1313)

1414

15+const (

16+ CodeChallengeMethodPlain = "plain"

17+ CodeChallengeMethodS256 = "S256"

18+)

19+

1520// SecretBytes attempts to base64 decode the secret, if that fails it treats the secret as binary

1621func SecretBytes(secret string) []byte {

1722 b, err := base64.RawURLEncoding.DecodeString(strings.TrimRight(secret, "="))

@@ -75,6 +80,18 @@ func SignedValue(seed string, key string, value []byte, now time.Time) (string,

7580 return cookieVal, nil

7681}

7782

83+func GenerateCodeChallenge(method, codeVerifier string) (string, error) {

84+ switch method {

85+ case CodeChallengeMethodPlain:

86+ return codeVerifier, nil

87+ case CodeChallengeMethodS256:

88+ shaSum := sha256.Sum256([]byte(codeVerifier))

89+ return base64.RawURLEncoding.EncodeToString(shaSum[:]), nil

90+ default:

91+ return "", fmt.Errorf("unknown challenge method: %v", method)

92+ }

93+}

94+

7895func cookieSignature(signer func() hash.Hash, args ...string) (string, error) {

7996 h := hmac.New(signer, []byte(args[0]))

8097 for _, arg := range args[1:] {

pkg/validation/sessions.go+1−1

@@ -51,7 +51,7 @@ func validateRedisSessionStore(o *options.Options) []string {

5151 return []string{fmt.Sprintf("unable to initialize a redis client: %v", err)}

5252 }

5353

54− n, err := encryption.Nonce()

54+ n, err := encryption.Nonce(32)

5555 if err != nil {

5656 return []string{fmt.Sprintf("unable to generate a redis initialization test key: %v", err)}

5757 }

providers/azure.go+6−3

@@ -112,8 +112,8 @@ func (p *AzureProvider) GetLoginURL(redirectURI, state, _ string, extraParams ur

112112}

113113

114114// Redeem exchanges the OAuth2 authentication token for an ID token

115−func (p *AzureProvider) Redeem(ctx context.Context, redirectURL, code string) (*sessions.SessionState, error) {

116− params, err := p.prepareRedeem(redirectURL, code)

115+func (p *AzureProvider) Redeem(ctx context.Context, redirectURL, code, codeVerifier string) (*sessions.SessionState, error) {

116+ params, err := p.prepareRedeem(redirectURL, code, codeVerifier)

117117 if err != nil {

118118 return nil, err

119119 }

@@ -187,7 +187,7 @@ func (p *AzureProvider) EnrichSession(ctx context.Context, s *sessions.SessionSt

187187 return nil

188188}

189189

190−func (p *AzureProvider) prepareRedeem(redirectURL, code string) (url.Values, error) {

190+func (p *AzureProvider) prepareRedeem(redirectURL, code, codeVerifier string) (url.Values, error) {

191191 params := url.Values{}

192192 if code == "" {

193193 return params, ErrMissingCode

@@ -202,6 +202,9 @@ func (p *AzureProvider) prepareRedeem(redirectURL, code string) (url.Values, err

202202 params.Add("client_secret", clientSecret)

203203 params.Add("code", code)

204204 params.Add("grant_type", "authorization_code")

205+ if codeVerifier != "" {

206+ params.Add("code_verifier", codeVerifier)

207+ }

205208 if p.ProtectedResource != nil && p.ProtectedResource.String() != "" {

206209 params.Add("resource", p.ProtectedResource.String())

207210 }

providers/azure_test.go+1−1

@@ -326,7 +326,7 @@ func TestAzureProviderRedeem(t *testing.T) {

326326 bURL, _ := url.Parse(b.URL)

327327 p := testAzureProvider(bURL.Host, options.AzureOptions{})

328328 p.Data().RedeemURL.Path = "/common/oauth2/token"

329− s, err := p.Redeem(context.Background(), "https://localhost", "1234")

329+ s, err := p.Redeem(context.Background(), "https://localhost", "1234", "123")

330330 if testCase.InjectRedeemURLError {

331331 assert.NotNil(t, err)

332332 } else {

providers/google.go+4−1

@@ -141,7 +141,7 @@ func claimsFromIDToken(idToken string) (*claims, error) {

141141}

142142

143143// Redeem exchanges the OAuth2 authentication token for an ID token

144−func (p *GoogleProvider) Redeem(ctx context.Context, redirectURL, code string) (*sessions.SessionState, error) {

144+func (p *GoogleProvider) Redeem(ctx context.Context, redirectURL, code, codeVerifier string) (*sessions.SessionState, error) {

145145 if code == "" {

146146 return nil, ErrMissingCode

147147 }

@@ -156,6 +156,9 @@ func (p *GoogleProvider) Redeem(ctx context.Context, redirectURL, code string) (

156156 params.Add("client_secret", clientSecret)

157157 params.Add("code", code)

158158 params.Add("grant_type", "authorization_code")

159+ if codeVerifier != "" {

160+ params.Add("code_verifier", codeVerifier)

161+ }

159162

160163 var jsonResponse struct {

161164 AccessToken string `json:"access_token"`

providers/google_test.go+5−5

@@ -112,7 +112,7 @@ func TestGoogleProviderGetEmailAddress(t *testing.T) {

112112 p.RedeemURL, server = newRedeemServer(body)

113113 defer server.Close()

114114

115− session, err := p.Redeem(context.Background(), "http://redirect/", "code1234")

115+ session, err := p.Redeem(context.Background(), "http://redirect/", "code1234", "123")

116116 assert.Equal(t, nil, err)

117117 assert.NotEqual(t, session, nil)

118118 assert.Equal(t, "michael.bland@gsa.gov", session.Email)

@@ -178,7 +178,7 @@ func TestGoogleProviderGetEmailAddressInvalidEncoding(t *testing.T) {

178178 p.RedeemURL, server = newRedeemServer(body)

179179 defer server.Close()

180180

181− session, err := p.Redeem(context.Background(), "http://redirect/", "code1234")

181+ session, err := p.Redeem(context.Background(), "http://redirect/", "code1234", "123")

182182 assert.NotEqual(t, nil, err)

183183 if session != nil {

184184 t.Errorf("expect nill session %#v", session)

@@ -189,7 +189,7 @@ func TestGoogleProviderRedeemFailsNoCLientSecret(t *testing.T) {

189189 p := newGoogleProvider(t)

190190 p.ProviderData.ClientSecretFile = "srvnoerre"

191191

192− session, err := p.Redeem(context.Background(), "http://redirect/", "code1234")

192+ session, err := p.Redeem(context.Background(), "http://redirect/", "code1234", "123")

193193 assert.NotEqual(t, nil, err)

194194 if session != nil {

195195 t.Errorf("expect nill session %#v", session)

@@ -209,7 +209,7 @@ func TestGoogleProviderGetEmailAddressInvalidJson(t *testing.T) {

209209 p.RedeemURL, server = newRedeemServer(body)

210210 defer server.Close()

211211

212− session, err := p.Redeem(context.Background(), "http://redirect/", "code1234")

212+ session, err := p.Redeem(context.Background(), "http://redirect/", "code1234", "123")

213213 assert.NotEqual(t, nil, err)

214214 if session != nil {

215215 t.Errorf("expect nill session %#v", session)

@@ -228,7 +228,7 @@ func TestGoogleProviderGetEmailAddressEmailMissing(t *testing.T) {

228228 p.RedeemURL, server = newRedeemServer(body)

229229 defer server.Close()

230230

231− session, err := p.Redeem(context.Background(), "http://redirect/", "code1234")

231+ session, err := p.Redeem(context.Background(), "http://redirect/", "code1234", "123")

232232 assert.NotEqual(t, nil, err)

233233 if session != nil {

234234 t.Errorf("expect nill session %#v", session)

providers/logingov.go+4−1

@@ -202,7 +202,7 @@ func emailFromUserInfo(ctx context.Context, accessToken string, userInfoEndpoint

202202}

203203

204204// Redeem exchanges the OAuth2 authentication token for an ID token

205−func (p *LoginGovProvider) Redeem(ctx context.Context, _, code string) (*sessions.SessionState, error) {

205+func (p *LoginGovProvider) Redeem(ctx context.Context, _, code, codeVerifier string) (*sessions.SessionState, error) {

206206 if code == "" {

207207 return nil, ErrMissingCode

208208 }

@@ -225,6 +225,9 @@ func (p *LoginGovProvider) Redeem(ctx context.Context, _, code string) (*session

225225 params.Add("client_assertion_type", "urn:ietf:params:oauth:client-assertion-type:jwt-bearer")

226226 params.Add("code", code)

227227 params.Add("grant_type", "authorization_code")

228+ if codeVerifier != "" {

229+ params.Add("code_verifier", codeVerifier)

230+ }

228231

229232 // Get the token from the body that we got from the token endpoint.

230233 var jsonResponse struct {

providers/logingov_test.go+2−2

@@ -235,7 +235,7 @@ func TestLoginGovProviderSessionData(t *testing.T) {

235235 p.PubJWKURL, pubjwkserver = newLoginGovServer(pubjwkbody)

236236 defer pubjwkserver.Close()

237237

238− session, err := p.Redeem(context.Background(), "http://redirect/", "code1234")

238+ session, err := p.Redeem(context.Background(), "http://redirect/", "code1234", "123")

239239 assert.NoError(t, err)

240240 assert.NotEqual(t, session, nil)

241241 assert.Equal(t, "timothy.spencer@gsa.gov", session.Email)

@@ -329,7 +329,7 @@ func TestLoginGovProviderBadNonce(t *testing.T) {

329329 p.PubJWKURL, pubjwkserver = newLoginGovServer(pubjwkbody)

330330 defer pubjwkserver.Close()

331331

332− _, err = p.Redeem(context.Background(), "http://redirect/", "code1234")

332+ _, err = p.Redeem(context.Background(), "http://redirect/", "code1234", "123")

333333

334334 // The "badfakenonce" in the idtoken above should cause this to error out

335335 assert.Error(t, err)

providers/oidc.go+7−2

@@ -43,12 +43,17 @@ func (p *OIDCProvider) GetLoginURL(redirectURI, state, nonce string, extraParams

4343}

4444

4545// Redeem exchanges the OAuth2 authentication token for an ID token

46−func (p *OIDCProvider) Redeem(ctx context.Context, redirectURL, code string) (*sessions.SessionState, error) {

46+func (p *OIDCProvider) Redeem(ctx context.Context, redirectURL, code, codeVerifier string) (*sessions.SessionState, error) {

4747 clientSecret, err := p.GetClientSecret()

4848 if err != nil {

4949 return nil, err

5050 }

5151

52+ var opts []oauth2.AuthCodeOption

53+ if codeVerifier != "" {

54+ opts = append(opts, oauth2.SetAuthURLParam("code_verifier", codeVerifier))

55+ }

56+

5257 c := oauth2.Config{

5358 ClientID: p.ClientID,

5459 ClientSecret: clientSecret,

@@ -57,7 +62,7 @@ func (p *OIDCProvider) Redeem(ctx context.Context, redirectURL, code string) (*s

5762 },

5863 RedirectURL: redirectURL,

5964 }

60− token, err := c.Exchange(ctx, code)

65+ token, err := c.Exchange(ctx, code, opts...)

6166 if err != nil {

6267 return nil, fmt.Errorf("token exchange failed: %v", err)

6368 }

providers/oidc_test.go+5−3

@@ -91,7 +91,7 @@ func TestOIDCProviderGetLoginURL(t *testing.T) {

9191 }

9292 provider := newOIDCProvider(serverURL, true)

9393

94− n, err := encryption.Nonce()

94+ n, err := encryption.Nonce(32)

9595 assert.NoError(t, err)

9696 nonce := base64.RawURLEncoding.EncodeToString(n)

9797

@@ -102,6 +102,8 @@ func TestOIDCProviderGetLoginURL(t *testing.T) {

102102 provider.SkipNonce = false

103103 withNonce := provider.GetLoginURL("http://redirect/", "", nonce, url.Values{})

104104 assert.Contains(t, withNonce, fmt.Sprintf("nonce=%s", nonce))

105+ assert.NotContains(t, withNonce, "code_challenge")

106+ assert.NotContains(t, withNonce, "code_challenge_method")

105107}

106108

107109func TestOIDCProviderRedeem(t *testing.T) {

@@ -117,7 +119,7 @@ func TestOIDCProviderRedeem(t *testing.T) {

117119 server, provider := newTestOIDCSetup(body)

118120 defer server.Close()

119121

120− session, err := provider.Redeem(context.Background(), provider.RedeemURL.String(), "code1234")

122+ session, err := provider.Redeem(context.Background(), provider.RedeemURL.String(), "code1234", "")

121123 assert.Equal(t, nil, err)

122124 assert.Equal(t, defaultIDToken.Email, session.Email)

123125 assert.Equal(t, accessToken, session.AccessToken)

@@ -140,7 +142,7 @@ func TestOIDCProviderRedeem_custom_userid(t *testing.T) {

140142 provider.EmailClaim = "phone_number"

141143 defer server.Close()

142144

143− session, err := provider.Redeem(context.Background(), provider.RedeemURL.String(), "code1234")

145+ session, err := provider.Redeem(context.Background(), provider.RedeemURL.String(), "code1234", "")

144146 assert.Equal(t, nil, err)

145147 assert.Equal(t, defaultIDToken.Phone, session.Email)

146148}

providers/provider_data.go+4

@@ -37,6 +37,10 @@ type ProviderData struct {

3737 ClientSecret string

3838 ClientSecretFile string

3939 Scope string

40+ // The picked CodeChallenge Method or empty if none.

41+ CodeChallengeMethod string

42+ // Code challenge methods supported by the Provider

43+ SupportedCodeChallengeMethods []string `json:"code_challenge_methods_supported,omitempty"`

4044

4145 // Common OIDC options for any OIDC-based providers to consume

4246 AllowUnverifiedEmail bool

providers/provider_default.go+7−1

@@ -33,13 +33,16 @@ var (

3333)

3434

3535// GetLoginURL with typical oauth parameters

36+// codeChallenge and codeChallengeMethod are the PKCE challenge and method to append to the URL params.

37+// they will be empty strings if no code challenge should be presented

3638func (p *ProviderData) GetLoginURL(redirectURI, state, _ string, extraParams url.Values) string {

3739 loginURL := makeLoginURL(p, redirectURI, state, extraParams)

3840 return loginURL.String()

3941}

4042

4143// Redeem provides a default implementation of the OAuth2 token redemption process

42−func (p *ProviderData) Redeem(ctx context.Context, redirectURL, code string) (*sessions.SessionState, error) {

44+// The codeVerifier is set if a code_verifier parameter should be sent for PKCE

45+func (p *ProviderData) Redeem(ctx context.Context, redirectURL, code, codeVerifier string) (*sessions.SessionState, error) {

4346 if code == "" {

4447 return nil, ErrMissingCode

4548 }

@@ -54,6 +57,9 @@ func (p *ProviderData) Redeem(ctx context.Context, redirectURL, code string) (*s

5457 params.Add("client_secret", clientSecret)

5558 params.Add("code", code)

5659 params.Add("grant_type", "authorization_code")

60+ if codeVerifier != "" {

61+ params.Add("code_verifier", codeVerifier)

62+ }

5763 if p.ProtectedResource != nil && p.ProtectedResource.String() != "" {

5864 params.Add("resource", p.ProtectedResource.String())

5965 }

providers/provider_default_test.go+32

@@ -2,6 +2,7 @@ package providers

33import (

44 "context"

5+ "net/url"

56 "testing"

67 "time"

@@ -29,6 +30,37 @@ func TestRefresh(t *testing.T) {

2930 assert.Equal(t, ErrNotImplemented, err)

3031}

3132

33+func TestCodeChallengeConfigured(t *testing.T) {

34+ p := &ProviderData{

35+ LoginURL: &url.URL{

36+ Scheme: "http",

37+ Host: "my.test.idp",

38+ Path: "/oauth/authorize",

39+ },

40+ }

41+

42+ extraValues := url.Values{}

43+ extraValues["code_challenge"] = []string{"challenge"}

44+ extraValues["code_challenge_method"] = []string{"method"}

45+ result := p.GetLoginURL("https://my.test.app/oauth", "", "", extraValues)

46+ assert.Contains(t, result, "code_challenge=challenge")

47+ assert.Contains(t, result, "code_challenge_method=method")

48+}

49+

50+func TestCodeChallengeNotConfigured(t *testing.T) {

51+ p := &ProviderData{

52+ LoginURL: &url.URL{

53+ Scheme: "http",

54+ Host: "my.test.idp",

55+ Path: "/oauth/authorize",

56+ },

57+ }

58+

59+ result := p.GetLoginURL("https://my.test.app/oauth", "", "", url.Values{})

60+ assert.NotContains(t, result, "code_challenge")

61+ assert.NotContains(t, result, "code_challenge_method")

62+}

63+

3264func TestProviderDataEnrichSession(t *testing.T) {

3365 g := NewWithT(t)

3466 p := &ProviderData{}

providers/providers.go+28−2

@@ -7,15 +7,21 @@ import (

88 "github.com/oauth2-proxy/oauth2-proxy/v7/pkg/apis/options"

99 "github.com/oauth2-proxy/oauth2-proxy/v7/pkg/apis/sessions"

10+ "github.com/oauth2-proxy/oauth2-proxy/v7/pkg/logger"

1011 internaloidc "github.com/oauth2-proxy/oauth2-proxy/v7/pkg/providers/oidc"

1112 k8serrors "k8s.io/apimachinery/pkg/util/errors"

1213)

1314

15+const (

16+ CodeChallengeMethodPlain = "plain"

17+ CodeChallengeMethodS256 = "S256"

18+)

19+

1420// Provider represents an upstream identity provider implementation

1521type Provider interface {

1622 Data() *ProviderData

17− GetLoginURL(redirectURI, finalRedirect string, nonce string, extraParams url.Values) string

18− Redeem(ctx context.Context, redirectURI, code string) (*sessions.SessionState, error)

23+ GetLoginURL(redirectURI, finalRedirect, nonce string, extraParams url.Values) string

24+ Redeem(ctx context.Context, redirectURI, code, codeVerifier string) (*sessions.SessionState, error)

1925 // Deprecated: Migrate to EnrichSession

2026 GetEmailAddress(ctx context.Context, s *sessions.SessionState) (string, error)

2127 EnrichSession(ctx context.Context, s *sessions.SessionState) error

@@ -95,10 +101,12 @@ func newProviderDataFromConfig(providerConfig options.Provider) (*ProviderData,

95101 if pv.DiscoveryEnabled() {

96102 // Use the discovered values rather than any specified values

97103 endpoints := pv.Provider().Endpoints()

104+ pkce := pv.Provider().PKCE()

98105 providerConfig.LoginURL = endpoints.AuthURL

99106 providerConfig.RedeemURL = endpoints.TokenURL

100107 providerConfig.ProfileURL = endpoints.UserInfoURL

101108 providerConfig.OIDCConfig.JwksURL = endpoints.JWKsURL

109+ p.SupportedCodeChallengeMethods = pkce.CodeChallengeAlgs

102110 }

103111 }

104112

@@ -131,6 +139,12 @@ func newProviderDataFromConfig(providerConfig options.Provider) (*ProviderData,

131139 p.EmailClaim = providerConfig.OIDCConfig.EmailClaim

132140 p.GroupsClaim = providerConfig.OIDCConfig.GroupsClaim

133141

142+ // Set PKCE enabled or disabled based on discovery and force options

143+ p.CodeChallengeMethod = parseCodeChallengeMethod(providerConfig)

144+ if len(p.SupportedCodeChallengeMethods) != 0 && p.CodeChallengeMethod == "" {

145+ logger.Printf("Warning: Your provider supports PKCE methods %+q, but you have not enabled one with --code-challenge-method", p.SupportedCodeChallengeMethods)

146+ }

147+

134148 // TODO (@NickMeves) - Remove This

135149 // Backwards Compatibility for Deprecated UserIDClaim option

136150 if providerConfig.OIDCConfig.EmailClaim == options.OIDCEmailClaim &&

@@ -154,6 +168,18 @@ func newProviderDataFromConfig(providerConfig options.Provider) (*ProviderData,

154168 return p, nil

155169}

156170

171+// Pick the most appropriate code challenge method for PKCE

172+// At this time we do not consider what the server supports to be safe and

173+// only enable PKCE if the user opts-in

174+func parseCodeChallengeMethod(providerConfig options.Provider) string {

175+ switch {

176+ case providerConfig.CodeChallengeMethod != "":

177+ return providerConfig.CodeChallengeMethod

178+ default:

179+ return ""

180+ }

181+}

182+

157183func providerRequiresOIDCProviderVerifier(providerType options.ProviderType) (bool, error) {

158184 switch providerType {

159185 case options.BitbucketProvider, options.DigitalOceanProvider, options.FacebookProvider, options.GitHubProvider,

providers/providers_test.go+35

@@ -171,3 +171,38 @@ func TestScope(t *testing.T) {

171171 g.Expect(pd.Scope).To(Equal(tc.expectedScope))

172172 }

173173}

174+

175+func TestForcedMethodS256(t *testing.T) {

176+ g := NewWithT(t)

177+ options := options.NewOptions()

178+ options.Providers[0].CodeChallengeMethod = CodeChallengeMethodS256

179+ method := parseCodeChallengeMethod(options.Providers[0])

180+

181+ g.Expect(method).To(Equal(CodeChallengeMethodS256))

182+}

183+

184+func TestForcedMethodPlain(t *testing.T) {

185+ g := NewWithT(t)

186+ options := options.NewOptions()

187+ options.Providers[0].CodeChallengeMethod = CodeChallengeMethodPlain

188+ method := parseCodeChallengeMethod(options.Providers[0])

189+

190+ g.Expect(method).To(Equal(CodeChallengeMethodPlain))

191+}

192+

193+func TestPrefersS256(t *testing.T) {

194+ g := NewWithT(t)

195+ options := options.NewOptions()

196+ method := parseCodeChallengeMethod(options.Providers[0])

197+

198+ g.Expect(method).To(Equal(""))

199+}

200+

201+func TestCanOverwriteS256(t *testing.T) {

202+ g := NewWithT(t)

203+ options := options.NewOptions()

204+ options.Providers[0].CodeChallengeMethod = "plain"

205+ method := parseCodeChallengeMethod(options.Providers[0])

206+

207+ g.Expect(method).To(Equal(CodeChallengeMethodPlain))

208+}