git.iko.soy/typeable/oauth2-proxy
on master help sign in
Support optional id_tokens in refresh responses (#335)

* OIDC Token Refresh works without id_tokens Addresses https://github.com/pusher/oauth2_proxy/issues/318 Refactoring the OIDC provider so that the refresh process works when there are no id_tokens present in the response. Added unit tests to the oidc_test.go to prove the redeem and refresh still work. The expiry time of the session is now taken from the outh token expiry and not the id_token (preventing stale access_tokens in sessions). * Refactoring the to use a KeySetStub in the oidc_test.go. This allows the elimination of the slightly contrived function passing elements used previously. (This change is being applied to address the bug #318) * Changes as per the PR comments and preparing for 5.x release * Fixup changelog Co-authored-by: Joel Speed <Joel.speed@hotmail.co.uk>

commit10adb5c516b5a15756a7baa50aa2d8551a6655b8 authorBen Letton <benletton@gmail.com> dateThu 6 Feb 2020, 18:09 (7 years ago) parent18d2036
3 files changed+369−55
CHANGELOG.md+2−1 providers/oidc.go+103−54 providers/oidc_test.go+264
CHANGELOG.md+2−1
@@ -3,11 +3,13 @@
33## Release Hightlights
44
55## Important Notes
6+- [#335] The session expiry for the OIDC provider is now taken from the Token Response (expires_in) rather than from the id_token (exp)
67
78## Breaking Changes
89
910## Changes since v5.0.0
1011
12+- [#335](https://github.com/pusher/oauth2_proxy/pull/335) OIDC Provider support for empty id_tokens in the access token refresh response (@howzat)
1113- [#363](https://github.com/pusher/oauth2_proxy/pull/363) Extension of Redis Session Store to Support Redis Cluster (@yan-dblinf)
1214- [#353](https://github.com/pusher/oauth2_proxy/pull/353) Fix login page fragment handling after soft reload on Firefox (@ffdybuster)
1315
@@ -37,7 +39,6 @@
3739- [#179](https://github.com/pusher/oauth2_proxy/pull/179) Add Nextcloud provider (@Ramblurr)
3840- [#280](https://github.com/pusher/oauth2_proxy/pull/280) whitelisted redirect domains: add support for whitelisting specific ports or allowing wildcard ports (@kamaln7)
3941- [#351](https://github.com/pusher/oauth2_proxy/pull/351) Add DigitalOcean Auth provider (@kamaln7)
40−
4142# v4.1.0
4243
4344## Release Highlights
providers/oidc.go+103−54
@@ -4,6 +4,7 @@ import (
44 "context"
55 "fmt"
66 "net/http"
7+ "strings"
78 "time"
89
910 oidc "github.com/coreos/go-oidc"
@@ -42,28 +43,36 @@ func (p *OIDCProvider) Redeem(redirectURL, code string) (s *sessions.SessionStat
4243 if err != nil {
4344 return nil, fmt.Errorf("token exchange: %v", err)
4445 }
45− s, err = p.createSessionState(ctx, token)
46+
47+ // in the initial exchange the id token is mandatory
48+ idToken, err := p.findVerifiedIDToken(ctx, token)
49+ if err != nil {
50+ return nil, fmt.Errorf("could not verify id_token: %v", err)
51+ } else if idToken == nil {
52+ return nil, fmt.Errorf("token response did not contain an id_token")
53+ }
54+
55+ s, err = p.createSessionState(token, idToken)
4656 if err != nil {
4757 return nil, fmt.Errorf("unable to update session: %v", err)
4858 }
59+
4960 return
5061}
5162
5263// RefreshSessionIfNeeded checks if the session has expired and uses the
53−// RefreshToken to fetch a new ID token if required
64+// RefreshToken to fetch a new Access Token (and optional ID token) if required
5465func (p *OIDCProvider) RefreshSessionIfNeeded(s *sessions.SessionState) (bool, error) {
5566 if s == nil || s.ExpiresOn.After(time.Now()) || s.RefreshToken == "" {
5667 return false, nil
5768 }
5869
59− origExpiration := s.ExpiresOn
60−
6170 err := p.redeemRefreshToken(s)
6271 if err != nil {
6372 return false, fmt.Errorf("unable to redeem refresh token: %v", err)
6473 }
6574
66− fmt.Printf("refreshed id token %s (expired on %s)\n", s, origExpiration)
75+ fmt.Printf("refreshed access token %s (expired on %s)\n", s, s.ExpiresOn)
6776 return true, nil
6877}
6978
@@ -84,43 +93,106 @@ func (p *OIDCProvider) redeemRefreshToken(s *sessions.SessionState) (err error)
8493 if err != nil {
8594 return fmt.Errorf("failed to get token: %v", err)
8695 }
87− newSession, err := p.createSessionState(ctx, token)
96+
97+ // in the token refresh response the id_token is optional
98+ idToken, err := p.findVerifiedIDToken(ctx, token)
8899 if err != nil {
89− return fmt.Errorf("unable to update session: %v", err)
100+ return fmt.Errorf("unable to extract id_token from response: %v", err)
90101 }
102+
103+ newSession, err := p.createSessionState(token, idToken)
104+ if err != nil {
105+ return fmt.Errorf("unable create new session state from response: %v", err)
106+ }
107+
108+ // It's possible that if the refresh token isn't in the token response the session will not contain an id token
109+ // if it doesn't it's probably better to retain the old one
110+ if newSession.IDToken != "" {
111+ s.IDToken = newSession.IDToken
112+ s.Email = newSession.Email
113+ s.User = newSession.User
114+ }
115+
91116 s.AccessToken = newSession.AccessToken
92− s.IDToken = newSession.IDToken
93117 s.RefreshToken = newSession.RefreshToken
94118 s.CreatedAt = newSession.CreatedAt
95119 s.ExpiresOn = newSession.ExpiresOn
96− s.Email = newSession.Email
120+
97121 return
98122}
99123
100−func (p *OIDCProvider) createSessionState(ctx context.Context, token *oauth2.Token) (*sessions.SessionState, error) {
101− rawIDToken, ok := token.Extra("id_token").(string)
102− if !ok {
103− return nil, fmt.Errorf("token response did not contain an id_token")
124+func (p *OIDCProvider) findVerifiedIDToken(ctx context.Context, token *oauth2.Token) (*oidc.IDToken, error) {
125+
126+ getIDToken := func() (string, bool) {
127+ rawIDToken, _ := token.Extra("id_token").(string)
128+ return rawIDToken, len(strings.TrimSpace(rawIDToken)) > 0
129+ }
130+
131+ if rawIDToken, present := getIDToken(); present {
132+ verifiedIdToken, err := p.Verifier.Verify(ctx, rawIDToken)
133+ return verifiedIdToken, err
134+ } else {
135+ return nil, nil
104136 }
137+}
138+
139+func (p *OIDCProvider) createSessionState(token *oauth2.Token, idToken *oidc.IDToken) (*sessions.SessionState, error) {
140+
141+ newSession := &sessions.SessionState{}
142+
143+ if idToken != nil {
144+ claims, err := findClaimsFromIDToken(idToken, token.AccessToken, p.ProfileURL.String())
145+ if err != nil {
146+ return nil, fmt.Errorf("couldn't extract claims from id_token (%e)", err)
147+ }
105148
106− // Parse and verify ID Token payload.
107− idToken, err := p.Verifier.Verify(ctx, rawIDToken)
149+ if claims != nil {
150+
151+ if !p.AllowUnverifiedEmail && claims.Verified != nil && !*claims.Verified {
152+ return nil, fmt.Errorf("email in id_token (%s) isn't verified", claims.Email)
153+ }
154+
155+ newSession.IDToken = token.Extra("id_token").(string)
156+ newSession.Email = claims.Email
157+ newSession.User = claims.Subject
158+ }
159+ }
160+
161+ newSession.AccessToken = token.AccessToken
162+ newSession.RefreshToken = token.RefreshToken
163+ newSession.CreatedAt = time.Now()
164+ newSession.ExpiresOn = token.Expiry
165+ return newSession, nil
166+}
167+
168+// ValidateSessionState checks that the session's IDToken is still valid
169+func (p *OIDCProvider) ValidateSessionState(s *sessions.SessionState) bool {
170+ ctx := context.Background()
171+ _, err := p.Verifier.Verify(ctx, s.IDToken)
108172 if err != nil {
109− return nil, fmt.Errorf("could not verify id_token: %v", err)
173+ return false
110174 }
111175
176+ return true
177+}
178+
179+func getOIDCHeader(accessToken string) http.Header {
180+ header := make(http.Header)
181+ header.Set("Accept", "application/json")
182+ header.Set("Authorization", fmt.Sprintf("Bearer %s", accessToken))
183+ return header
184+}
185+
186+func findClaimsFromIDToken(idToken *oidc.IDToken, accessToken string, profileURL string) (*OIDCClaims, error) {
187+
112188 // Extract custom claims.
113− var claims struct {
114− Subject string `json:"sub"`
115− Email string `json:"email"`
116− Verified *bool `json:"email_verified"`
117− }
118− if err := idToken.Claims(&claims); err != nil {
189+ claims := &OIDCClaims{}
190+ if err := idToken.Claims(claims); err != nil {
119191 return nil, fmt.Errorf("failed to parse id_token claims: %v", err)
120192 }
121193
122194 if claims.Email == "" {
123− if p.ProfileURL.String() == "" {
195+ if profileURL == "" {
124196 return nil, fmt.Errorf("id_token did not contain an email")
125197 }
126198
@@ -128,11 +200,11 @@ func (p *OIDCProvider) createSessionState(ctx context.Context, token *oauth2.Tok
128200 // contents at the profileURL contains the email.
129201 // Make a query to the userinfo endpoint, and attempt to locate the email from there.
130202
131− req, err := http.NewRequest("GET", p.ProfileURL.String(), nil)
203+ req, err := http.NewRequest("GET", profileURL, nil)
132204 if err != nil {
133205 return nil, err
134206 }
135− req.Header = getOIDCHeader(token.AccessToken)
207+ req.Header = getOIDCHeader(accessToken)
136208
137209 respJSON, err := requests.Request(req)
138210 if err != nil {
@@ -141,40 +213,17 @@ func (p *OIDCProvider) createSessionState(ctx context.Context, token *oauth2.Tok
141213
142214 email, err := respJSON.Get("email").String()
143215 if err != nil {
144− return nil, fmt.Errorf("Neither id_token nor userinfo endpoint contained an email")
216+ return nil, fmt.Errorf("neither id_token nor userinfo endpoint contained an email")
145217 }
146218
147219 claims.Email = email
148220 }
149− if !p.AllowUnverifiedEmail && claims.Verified != nil && !*claims.Verified {
150− return nil, fmt.Errorf("email in id_token (%s) isn't verified", claims.Email)
151− }
152221
153− return &sessions.SessionState{
154− AccessToken: token.AccessToken,
155− IDToken: rawIDToken,
156− RefreshToken: token.RefreshToken,
157− CreatedAt: time.Now(),
158− ExpiresOn: idToken.Expiry,
159− Email: claims.Email,
160− User: claims.Subject,
161− }, nil
222+ return claims, nil
162223}
163224
164−// ValidateSessionState checks that the session's IDToken is still valid
165−func (p *OIDCProvider) ValidateSessionState(s *sessions.SessionState) bool {
166− ctx := context.Background()
167− _, err := p.Verifier.Verify(ctx, s.IDToken)
168− if err != nil {
169− return false
170− }
171−
172− return true
173−}
174−
175−func getOIDCHeader(accessToken string) http.Header {
176− header := make(http.Header)
177− header.Set("Accept", "application/json")
178− header.Set("Authorization", fmt.Sprintf("Bearer %s", accessToken))
179− return header
225+type OIDCClaims struct {
226+ Subject string `json:"sub"`
227+ Email string `json:"email"`
228+ Verified *bool `json:"email_verified"`
180229}
providers/oidc_test.gonew file+264
@@ -0,0 +1,264 @@
1+package providers
2+
3+import (
4+ "context"
5+ "crypto/rand"
6+ "crypto/rsa"
7+ "encoding/base64"
8+ "encoding/json"
9+ "errors"
10+ "fmt"
11+ "golang.org/x/oauth2"
12+
13+ "github.com/bmizerany/assert"
14+ "github.com/coreos/go-oidc"
15+ "github.com/dgrijalva/jwt-go"
16+ "github.com/pusher/oauth2_proxy/pkg/apis/sessions"
17+
18+ "net/http"
19+ "net/http/httptest"
20+ "net/url"
21+ "strings"
22+ "testing"
23+ "time"
24+)
25+
26+const accessToken = "access_token"
27+const refreshToken = "refresh_token"
28+const clientID = "https://test.myapp.com"
29+const secret = "secret"
30+
31+type idTokenClaims struct {
32+ Name string `json:"name,omitempty"`
33+ Email string `json:"email,omitempty"`
34+ Picture string `json:"picture,omitempty"`
35+ jwt.StandardClaims
36+}
37+
38+type redeemTokenResponse struct {
39+ AccessToken string `json:"access_token"`
40+ RefreshToken string `json:"refresh_token"`
41+ ExpiresIn int64 `json:"expires_in"`
42+ TokenType string `json:"token_type"`
43+ IDToken string `json:"id_token,omitempty"`
44+}
45+
46+var defaultIDToken idTokenClaims = idTokenClaims{
47+ "Jane Dobbs",
48+ "janed@me.com",
49+ "http://mugbook.com/janed/me.jpg",
50+ jwt.StandardClaims{
51+ Audience: "https://test.myapp.com",
52+ ExpiresAt: time.Now().Add(time.Duration(5) * time.Minute).Unix(),
53+ Id: "id-some-id",
54+ IssuedAt: time.Now().Unix(),
55+ Issuer: "https://issuer.example.com",
56+ NotBefore: 0,
57+ Subject: "123456789",
58+ },
59+}
60+
61+type fakeKeySetStub struct {}
62+
63+func (fakeKeySetStub) VerifySignature(_ context.Context, jwt string) (payload []byte, err error) {
64+ decodeString, err := base64.RawURLEncoding.DecodeString(strings.Split(jwt, ".")[1])
65+ tokenClaims := &idTokenClaims{}
66+ err = json.Unmarshal(decodeString, tokenClaims)
67+
68+ if err != nil || tokenClaims.Id == "this-id-fails-validation" {
69+ return nil, fmt.Errorf("the validation failed for subject [%v]", tokenClaims.Subject)
70+ }
71+
72+ return decodeString, err
73+}
74+
75+func newOIDCProvider(serverURL *url.URL) *OIDCProvider {
76+
77+ providerData := &ProviderData{
78+ ProviderName: "oidc",
79+ ClientID: clientID,
80+ ClientSecret: secret,
81+ LoginURL: &url.URL{
82+ Scheme: serverURL.Scheme,
83+ Host: serverURL.Host,
84+ Path: "/login/oauth/authorize"},
85+ RedeemURL: &url.URL{
86+ Scheme: serverURL.Scheme,
87+ Host: serverURL.Host,
88+ Path: "/login/oauth/access_token"},
89+ ProfileURL: &url.URL{
90+ Scheme: serverURL.Scheme,
91+ Host: serverURL.Host,
92+ Path: "/profile"},
93+ ValidateURL: &url.URL{
94+ Scheme: serverURL.Scheme,
95+ Host: serverURL.Host,
96+ Path: "/api"},
97+ Scope: "openid profile offline_access"}
98+
99+ p := &OIDCProvider{
100+ ProviderData: providerData,
101+ Verifier: oidc.NewVerifier(
102+ "https://issuer.example.com",
103+ fakeKeySetStub{},
104+ &oidc.Config{ClientID: clientID},
105+ ),
106+ }
107+
108+ return p
109+}
110+
111+func newOIDCServer(body []byte) (*url.URL, *httptest.Server) {
112+ s := httptest.NewServer(http.HandlerFunc(func(rw http.ResponseWriter, r *http.Request) {
113+ rw.Header().Add("content-type", "application/json")
114+ _, _ = rw.Write(body)
115+ }))
116+ u, _ := url.Parse(s.URL)
117+ return u, s
118+}
119+
120+func newSignedTestIDToken(tokenClaims idTokenClaims) (string, error) {
121+
122+ key, _ := rsa.GenerateKey(rand.Reader, 2048)
123+ standardClaims := jwt.NewWithClaims(jwt.SigningMethodRS256, tokenClaims)
124+ return standardClaims.SignedString(key)
125+}
126+
127+func newOauth2Token() *oauth2.Token {
128+ return &oauth2.Token{
129+ AccessToken: accessToken,
130+ TokenType: "Bearer",
131+ RefreshToken: refreshToken,
132+ Expiry: time.Time{}.Add(time.Duration(5) * time.Second),
133+ }
134+}
135+
136+func newTestSetup(body []byte) (*httptest.Server, *OIDCProvider) {
137+ redeemURL, server := newOIDCServer(body)
138+ provider := newOIDCProvider(redeemURL)
139+ return server, provider
140+}
141+
142+func TestOIDCProviderRedeem(t *testing.T) {
143+
144+ idToken, _ := newSignedTestIDToken(defaultIDToken)
145+ body, _ := json.Marshal(redeemTokenResponse{
146+ AccessToken: accessToken,
147+ ExpiresIn: 10,
148+ TokenType: "Bearer",
149+ RefreshToken: refreshToken,
150+ IDToken: idToken,
151+ })
152+
153+ server, provider := newTestSetup(body)
154+ defer server.Close()
155+
156+ session, err := provider.Redeem(provider.RedeemURL.String(), "code1234")
157+ assert.Equal(t, nil, err)
158+ assert.Equal(t, defaultIDToken.Email, session.Email)
159+ assert.Equal(t, accessToken, session.AccessToken)
160+ assert.Equal(t, idToken, session.IDToken)
161+ assert.Equal(t, refreshToken, session.RefreshToken)
162+ assert.Equal(t, "123456789", session.User)
163+}
164+
165+func TestOIDCProviderRefreshSessionIfNeededWithoutIdToken(t *testing.T) {
166+
167+ idToken, _ := newSignedTestIDToken(defaultIDToken)
168+ body, _ := json.Marshal(redeemTokenResponse{
169+ AccessToken: accessToken,
170+ ExpiresIn: 10,
171+ TokenType: "Bearer",
172+ RefreshToken: refreshToken,
173+ })
174+
175+ server, provider := newTestSetup(body)
176+ defer server.Close()
177+
178+ existingSession := &sessions.SessionState{
179+ AccessToken: "changeit",
180+ IDToken: idToken,
181+ CreatedAt: time.Time{},
182+ ExpiresOn: time.Time{},
183+ RefreshToken: refreshToken,
184+ Email: "janedoe@example.com",
185+ User: "11223344",
186+ }
187+
188+ refreshed, err := provider.RefreshSessionIfNeeded(existingSession)
189+ assert.Equal(t, nil, err)
190+ assert.Equal(t, refreshed, true)
191+ assert.Equal(t, "janedoe@example.com", existingSession.Email)
192+ assert.Equal(t, accessToken, existingSession.AccessToken)
193+ assert.Equal(t, idToken, existingSession.IDToken)
194+ assert.Equal(t, refreshToken, existingSession.RefreshToken)
195+ assert.Equal(t, "11223344", existingSession.User)
196+}
197+
198+func TestOIDCProviderRefreshSessionIfNeededWithIdToken(t *testing.T) {
199+
200+ idToken, _ := newSignedTestIDToken(defaultIDToken)
201+ body, _ := json.Marshal(redeemTokenResponse{
202+ AccessToken: accessToken,
203+ ExpiresIn: 10,
204+ TokenType: "Bearer",
205+ RefreshToken: refreshToken,
206+ IDToken: idToken,
207+ })
208+
209+ server, provider := newTestSetup(body)
210+ defer server.Close()
211+
212+ existingSession := &sessions.SessionState{
213+ AccessToken: "changeit",
214+ IDToken: "changeit",
215+ CreatedAt: time.Time{},
216+ ExpiresOn: time.Time{},
217+ RefreshToken: refreshToken,
218+ Email: "changeit",
219+ User: "changeit",
220+ }
221+ refreshed, err := provider.RefreshSessionIfNeeded(existingSession)
222+ assert.Equal(t, nil, err)
223+ assert.Equal(t, refreshed, true)
224+ assert.Equal(t, defaultIDToken.Email, existingSession.Email)
225+ assert.Equal(t, defaultIDToken.Subject, existingSession.User)
226+ assert.Equal(t, accessToken, existingSession.AccessToken)
227+ assert.Equal(t, idToken, existingSession.IDToken)
228+ assert.Equal(t, refreshToken, existingSession.RefreshToken)
229+}
230+
231+func TestOIDCProvider_findVerifiedIdToken(t *testing.T) {
232+
233+ server, provider := newTestSetup([]byte(""))
234+
235+ defer server.Close()
236+
237+ token := newOauth2Token()
238+ signedIdToken, _ := newSignedTestIDToken(defaultIDToken)
239+ tokenWithIdToken := token.WithExtra(map[string]interface{}{
240+ "id_token": signedIdToken,
241+ })
242+
243+ verifiedIdToken, err := provider.findVerifiedIDToken(context.Background(), tokenWithIdToken)
244+ assert.Equal(t, true, err == nil)
245+ assert.Equal(t, true, verifiedIdToken != nil)
246+ assert.Equal(t, defaultIDToken.Issuer, verifiedIdToken.Issuer)
247+ assert.Equal(t, defaultIDToken.Subject, verifiedIdToken.Subject)
248+
249+ // When the validation fails the response should be nil
250+ defaultIDToken.Id = "this-id-fails-validation"
251+ signedIdToken, _ = newSignedTestIDToken(defaultIDToken)
252+ tokenWithIdToken = token.WithExtra(map[string]interface{}{
253+ "id_token": signedIdToken,
254+ })
255+
256+ verifiedIdToken, err = provider.findVerifiedIDToken(context.Background(), tokenWithIdToken)
257+ assert.Equal(t, errors.New("failed to verify signature: the validation failed for subject [123456789]"), err)
258+ assert.Equal(t, true, verifiedIdToken == nil)
259+
260+ // When there is no id token in the oauth token
261+ verifiedIdToken, err = provider.findVerifiedIDToken(context.Background(), newOauth2Token())
262+ assert.Equal(t, nil, err)
263+ assert.Equal(t, true, verifiedIdToken == nil)
264+}