git.iko.soy/typeable/oauth2-proxy
on master help sign in
Merge pull request #700 from grnhse/oidc-no-email-tokens

Allow OIDC Bearer Tokens without emails

commitaceb9e2762f41fe80f4364570572b2770440b4ca authorJoel Speed <Joel.speed@hotmail.co.uk> dateSun 16 Aug 2020, 13:03 (6 years ago) parent8515da30645e19
3 files changed+82−13
compare with master
unifiedsplit
CHANGELOG.md+1 providers/oidc.go+18−13 providers/oidc_test.go+63
CHANGELOG.md+1
@@ -16,6 +16,7 @@
1616- [#718](https://github.com/oauth2-proxy/oauth2-proxy/pull/718) Allow Logging to stdout with separate Error Log Channel
1717- [#690](https://github.com/oauth2-proxy/oauth2-proxy/pull/690) Address GoSec security findings & remediate (@NickMeves)
1818- [#689](https://github.com/oauth2-proxy/oauth2-proxy/pull/689) Fix finicky logging_handler_test from time drift (@NickMeves)
19+- [#700](https://github.com/oauth2-proxy/oauth2-proxy/pull/700) Allow OIDC Bearer auth IDTokens to have empty email claim & profile URL (@NickMeves)
1920- [#699](https://github.com/oauth2-proxy/oauth2-proxy/pull/699) Align persistence ginkgo tests with conventions (@NickMeves)
2021- [#696](https://github.com/oauth2-proxy/oauth2-proxy/pull/696) Preserve query when building redirect
2122- [#561](https://github.com/oauth2-proxy/oauth2-proxy/pull/561) Refactor provider URLs to package level vars (@JoelSpeed)
providers/oidc.go+18−13
@@ -157,7 +157,7 @@ func (p *OIDCProvider) createSessionState(ctx context.Context, token *oauth2.Tok
157157 newSession = &sessions.SessionState{}
158158 } else {
159159 var err error
160− newSession, err = p.createSessionStateInternal(ctx, token.Extra("id_token").(string), idToken, token)
160+ newSession, err = p.createSessionStateInternal(ctx, idToken, token)
161161 if err != nil {
162162 return nil, err
163163 }
@@ -172,7 +172,7 @@ func (p *OIDCProvider) createSessionState(ctx context.Context, token *oauth2.Tok
172172}
173173
174174func (p *OIDCProvider) CreateSessionStateFromBearerToken(ctx context.Context, rawIDToken string, idToken *oidc.IDToken) (*sessions.SessionState, error) {
175− newSession, err := p.createSessionStateInternal(ctx, rawIDToken, idToken, nil)
175+ newSession, err := p.createSessionStateInternal(ctx, idToken, nil)
176176 if err != nil {
177177 return nil, err
178178 }
@@ -185,24 +185,22 @@ func (p *OIDCProvider) CreateSessionStateFromBearerToken(ctx context.Context, ra
185185 return newSession, nil
186186}
187187
188−func (p *OIDCProvider) createSessionStateInternal(ctx context.Context, rawIDToken string, idToken *oidc.IDToken, token *oauth2.Token) (*sessions.SessionState, error) {
188+func (p *OIDCProvider) createSessionStateInternal(ctx context.Context, idToken *oidc.IDToken, token *oauth2.Token) (*sessions.SessionState, error) {
189189
190190 newSession := &sessions.SessionState{}
191191
192192 if idToken == nil {
193193 return newSession, nil
194194 }
195− accessToken := ""
196− if token != nil {
197− accessToken = token.AccessToken
198− }
199195
200− claims, err := p.findClaimsFromIDToken(ctx, idToken, accessToken, p.ProfileURL.String())
196+ claims, err := p.findClaimsFromIDToken(ctx, idToken, token)
201197 if err != nil {
202198 return nil, fmt.Errorf("couldn't extract claims from id_token (%v)", err)
203199 }
204200
205− newSession.IDToken = rawIDToken
201+ if token != nil {
202+ newSession.IDToken = token.Extra("id_token").(string)
203+ }
206204
207205 newSession.Email = claims.UserID // TODO Rename SessionState.Email to .UserID in the near future
208206
@@ -230,8 +228,7 @@ func getOIDCHeader(accessToken string) http.Header {
230228 return header
231229}
232230
233−func (p *OIDCProvider) findClaimsFromIDToken(ctx context.Context, idToken *oidc.IDToken, accessToken string, profileURL string) (*OIDCClaims, error) {
234−
231+func (p *OIDCProvider) findClaimsFromIDToken(ctx context.Context, idToken *oidc.IDToken, token *oauth2.Token) (*OIDCClaims, error) {
235232 claims := &OIDCClaims{}
236233 // Extract default claims.
237234 if err := idToken.Claims(&claims); err != nil {
@@ -249,7 +246,15 @@ func (p *OIDCProvider) findClaimsFromIDToken(ctx context.Context, idToken *oidc.
249246
250247 // userID claim was not present or was empty in the ID Token
251248 if claims.UserID == "" {
252− if profileURL == "" {
249+ // BearerToken case, allow empty UserID
250+ // ProfileURL checks below won't work since we don't have an access token
251+ if token == nil {
252+ claims.UserID = claims.Subject
253+ return claims, nil
254+ }
255+
256+ profileURL := p.ProfileURL.String()
257+ if profileURL == "" || token.AccessToken == "" {
253258 return nil, fmt.Errorf("id_token did not contain user ID claim (%q)", p.UserIDClaim)
254259 }
255260
@@ -258,7 +263,7 @@ func (p *OIDCProvider) findClaimsFromIDToken(ctx context.Context, idToken *oidc.
258263 // Make a query to the userinfo endpoint, and attempt to locate the email from there.
259264 respJSON, err := requests.New(profileURL).
260265 WithContext(ctx).
261− WithHeaders(getOIDCHeader(accessToken)).
266+ WithHeaders(getOIDCHeader(token.AccessToken)).
262267 Do().
263268 UnmarshalJSON()
264269 if err != nil {
providers/oidc_test.go+63
@@ -60,6 +60,22 @@ var defaultIDToken idTokenClaims = idTokenClaims{
6060 },
6161}
6262
63+var minimalIDToken idTokenClaims = idTokenClaims{
64+ "",
65+ "",
66+ "",
67+ "",
68+ jwt.StandardClaims{
69+ Audience: "https://test.myapp.com",
70+ ExpiresAt: time.Now().Add(time.Duration(5) * time.Minute).Unix(),
71+ Id: "id-some-id",
72+ IssuedAt: time.Now().Unix(),
73+ Issuer: "https://issuer.example.com",
74+ NotBefore: 0,
75+ Subject: "minimal",
76+ },
77+}
78+
6379type fakeKeySetStub struct{}
6480
6581func (fakeKeySetStub) VerifySignature(_ context.Context, jwt string) (payload []byte, err error) {
@@ -253,6 +269,53 @@ func TestOIDCProviderRefreshSessionIfNeededWithIdToken(t *testing.T) {
253269 assert.Equal(t, refreshToken, existingSession.RefreshToken)
254270}
255271
272+func TestCreateSessionStateFromBearerToken(t *testing.T) {
273+ const profileURLEmail = "janed@me.com"
274+
275+ testCases := map[string]struct {
276+ IDToken idTokenClaims
277+ ExpectedUser string
278+ ExpectedEmail string
279+ }{
280+ "Default IDToken": {
281+ IDToken: defaultIDToken,
282+ ExpectedUser: defaultIDToken.Subject,
283+ ExpectedEmail: defaultIDToken.Email,
284+ },
285+ "Minimal IDToken with no email claim": {
286+ IDToken: minimalIDToken,
287+ ExpectedUser: minimalIDToken.Subject,
288+ ExpectedEmail: minimalIDToken.Subject,
289+ },
290+ }
291+ for testName, tc := range testCases {
292+ t.Run(testName, func(t *testing.T) {
293+ jsonResp := []byte(fmt.Sprintf(`{"email":"%s"}`, profileURLEmail))
294+ server, provider := newTestSetup(jsonResp)
295+ defer server.Close()
296+
297+ rawIDToken, err := newSignedTestIDToken(tc.IDToken)
298+ assert.NoError(t, err)
299+
300+ keyset := fakeKeySetStub{}
301+ verifier := oidc.NewVerifier("https://issuer.example.com", keyset,
302+ &oidc.Config{ClientID: "https://test.myapp.com", SkipExpiryCheck: true})
303+
304+ idToken, err := verifier.Verify(context.Background(), rawIDToken)
305+ assert.NoError(t, err)
306+
307+ ss, err := provider.CreateSessionStateFromBearerToken(context.Background(), rawIDToken, idToken)
308+ assert.NoError(t, err)
309+
310+ assert.Equal(t, tc.ExpectedUser, ss.User)
311+ assert.Equal(t, tc.ExpectedEmail, ss.Email)
312+ assert.Equal(t, rawIDToken, ss.IDToken)
313+ assert.Equal(t, rawIDToken, ss.AccessToken)
314+ assert.Equal(t, "", ss.RefreshToken)
315+ })
316+ }
317+}
318+
256319func TestOIDCProvider_findVerifiedIdToken(t *testing.T) {
257320
258321 server, provider := newTestSetup([]byte(""))