git.iko.soy/typeable/oauth2-proxy
on master help sign in
RefreshSessions immediately when called
commit7e80e5596b64ed9d25c9e2e119c7025da104f941 authorNick Meves <nick.meves@greenhouse.io> dateSat 6 Mar 2021, 15:33 (6 years ago) parent5f4ac25
13 files changed+74−79
compare with master
unifiedsplit
oauthproxy.go+4−4 pkg/middleware/stored_session.go+25−24 pkg/middleware/stored_session_test.go+10−15 providers/azure_test.go+4−3 providers/facebook.go+1−1 providers/gitlab.go+8−13 providers/google.go+3−4 providers/linkedin.go+1−1 providers/oidc.go+3−4 providers/oidc_test.go+2−2 providers/provider_default.go+9−4 providers/provider_default_test.go+1−1 providers/providers.go+3−3
oauthproxy.go+4−4
@@ -361,10 +361,10 @@ func buildSessionChain(opts *options.Options, sessionStore sessionsapi.SessionSt
361361 }
362362
363363 chain = chain.Append(middleware.NewStoredSessionLoader(&middleware.StoredSessionLoaderOptions{
364− SessionStore: sessionStore,
365− RefreshPeriod: opts.Cookie.Refresh,
366− RefreshSessionIfNeeded: opts.GetProvider().RefreshSessionIfNeeded,
367− ValidateSessionState: opts.GetProvider().ValidateSession,
364+ SessionStore: sessionStore,
365+ RefreshPeriod: opts.Cookie.Refresh,
366+ RefreshSession: opts.GetProvider().RefreshSession,
367+ ValidateSession: opts.GetProvider().ValidateSession,
368368 }))
369369
370370 return chain
pkg/middleware/stored_session.go+25−24
@@ -24,12 +24,12 @@ type StoredSessionLoaderOptions struct {
2424 RefreshPeriod time.Duration
2525
2626 // Provider based sesssion refreshing
27− RefreshSessionIfNeeded func(context.Context, *sessionsapi.SessionState) (bool, error)
27+ RefreshSession func(context.Context, *sessionsapi.SessionState) (bool, error)
2828
2929 // Provider based session validation.
3030 // If the sesssion is older than `RefreshPeriod` but the provider doesn't
3131 // refresh it, we must re-validate using this validation.
32− ValidateSessionState func(context.Context, *sessionsapi.SessionState) bool
32+ ValidateSession func(context.Context, *sessionsapi.SessionState) bool
3333}
3434
3535// NewStoredSessionLoader creates a new storedSessionLoader which loads
@@ -38,10 +38,10 @@ type StoredSessionLoaderOptions struct {
3838// If a session was loader by a previous handler, it will not be replaced.
3939func NewStoredSessionLoader(opts *StoredSessionLoaderOptions) alice.Constructor {
4040 ss := &storedSessionLoader{
41− store: opts.SessionStore,
42− refreshPeriod: opts.RefreshPeriod,
43− refreshSessionWithProviderIfNeeded: opts.RefreshSessionIfNeeded,
44− validateSessionState: opts.ValidateSessionState,
41+ store: opts.SessionStore,
42+ refreshPeriod: opts.RefreshPeriod,
43+ sessionRefresher: opts.RefreshSession,
44+ sessionValidator: opts.ValidateSession,
4545 }
4646 return ss.loadSession
4747}
@@ -49,10 +49,10 @@ func NewStoredSessionLoader(opts *StoredSessionLoaderOptions) alice.Constructor
4949// storedSessionLoader is responsible for loading sessions from cookie
5050// identified sessions in the session store.
5151type storedSessionLoader struct {
52− store sessionsapi.SessionStore
53− refreshPeriod time.Duration
54− refreshSessionWithProviderIfNeeded func(context.Context, *sessionsapi.SessionState) (bool, error)
55− validateSessionState func(context.Context, *sessionsapi.SessionState) bool
52+ store sessionsapi.SessionStore
53+ refreshPeriod time.Duration
54+ sessionRefresher func(context.Context, *sessionsapi.SessionState) (bool, error)
55+ sessionValidator func(context.Context, *sessionsapi.SessionState) bool
5656}
5757
5858// loadSession attempts to load a session as identified by the request cookies.
@@ -120,37 +120,38 @@ func (s *storedSessionLoader) refreshSessionIfNeeded(rw http.ResponseWriter, req
120120 }
121121
122122 logger.Printf("Refreshing %s old session cookie for %s (refresh after %s)", session.Age(), session, s.refreshPeriod)
123− refreshed, err := s.refreshSessionWithProvider(rw, req, session)
123+ err := s.refreshSession(rw, req, session)
124124 if err != nil {
125125 return err
126126 }
127127
128− if !refreshed {
129− // Session wasn't refreshed, so make sure it's still valid
130− return s.validateSession(req.Context(), session)
131− }
132− return nil
128+ // Validate all sessions after any Redeem/Refresh operation
129+ return s.validateSession(req.Context(), session)
133130}
134131
135−// refreshSessionWithProvider attempts to refresh the sessinon with the provider
132+// refreshSession attempts to refresh the session with the provider
136133// and will save the session if it was updated.
137−func (s *storedSessionLoader) refreshSessionWithProvider(rw http.ResponseWriter, req *http.Request, session *sessionsapi.SessionState) (bool, error) {
138− refreshed, err := s.refreshSessionWithProviderIfNeeded(req.Context(), session)
134+func (s *storedSessionLoader) refreshSession(rw http.ResponseWriter, req *http.Request, session *sessionsapi.SessionState) error {
135+ refreshed, err := s.sessionRefresher(req.Context(), session)
139136 if err != nil {
140− return false, fmt.Errorf("error refreshing access token: %v", err)
137+ return fmt.Errorf("error refreshing access token: %v", err)
141138 }
142139
143140 if !refreshed {
144− return false, nil
141+ return nil
145142 }
146143
144+ // If we refreshed, update the `CreatedAt` time to reset the refresh timer
145+ // TODO: Implement
146+ // session.CreatedAtNow()
147+
147148 // Because the session was refreshed, make sure to save it
148149 err = s.store.Save(rw, req, session)
149150 if err != nil {
150151 logger.PrintAuthf(session.Email, req, logger.AuthError, "error saving session: %v", err)
151− return false, fmt.Errorf("error saving session: %v", err)
152+ return fmt.Errorf("error saving session: %v", err)
152153 }
153− return true, nil
154+ return nil
154155}
155156
156157// validateSession checks whether the session has expired and performs
@@ -161,7 +162,7 @@ func (s *storedSessionLoader) validateSession(ctx context.Context, session *sess
161162 return errors.New("session is expired")
162163 }
163164
164− if !s.validateSessionState(ctx, session) {
165+ if !s.sessionValidator(ctx, session) {
165166 return errors.New("session is invalid")
166167 }
167168
pkg/middleware/stored_session_test.go+10−15
@@ -109,10 +109,10 @@ var _ = Describe("Stored Session Suite", func() {
109109 rw := httptest.NewRecorder()
110110
111111 opts := &StoredSessionLoaderOptions{
112− SessionStore: in.store,
113− RefreshPeriod: in.refreshPeriod,
114− RefreshSessionIfNeeded: in.refreshSession,
115− ValidateSessionState: in.validateSession,
112+ SessionStore: in.store,
113+ RefreshPeriod: in.refreshPeriod,
114+ RefreshSession: in.refreshSession,
115+ ValidateSession: in.validateSession,
116116 }
117117
118118 // Create the handler with a next handler that will capture the session
@@ -261,7 +261,7 @@ var _ = Describe("Stored Session Suite", func() {
261261 s := &storedSessionLoader{
262262 refreshPeriod: in.refreshPeriod,
263263 store: &fakeSessionStore{},
264− refreshSessionWithProviderIfNeeded: func(_ context.Context, ss *sessionsapi.SessionState) (bool, error) {
264+ sessionRefresher: func(_ context.Context, ss *sessionsapi.SessionState) (bool, error) {
265265 refreshed = true
266266 switch ss.RefreshToken {
267267 case refresh:
@@ -272,7 +272,7 @@ var _ = Describe("Stored Session Suite", func() {
272272 return false, errors.New("error refreshing session")
273273 }
274274 },
275− validateSessionState: func(_ context.Context, ss *sessionsapi.SessionState) bool {
275+ sessionValidator: func(_ context.Context, ss *sessionsapi.SessionState) bool {
276276 validated = true
277277 return ss.AccessToken != "Invalid"
278278 },
@@ -364,7 +364,7 @@ var _ = Describe("Stored Session Suite", func() {
364364 )
365365 })
366366
367− Context("refreshSessionWithProvider", func() {
367+ Context("refreshSession", func() {
368368 type refreshSessionWithProviderTableInput struct {
369369 session *sessionsapi.SessionState
370370 expectedErr error
@@ -388,7 +388,7 @@ var _ = Describe("Stored Session Suite", func() {
388388 return nil
389389 },
390390 },
391− refreshSessionWithProviderIfNeeded: func(_ context.Context, ss *sessionsapi.SessionState) (bool, error) {
391+ sessionRefresher: func(_ context.Context, ss *sessionsapi.SessionState) (bool, error) {
392392 switch ss.RefreshToken {
393393 case refresh:
394394 return true, nil
@@ -402,13 +402,12 @@ var _ = Describe("Stored Session Suite", func() {
402402
403403 req := httptest.NewRequest("", "/", nil)
404404 req = middlewareapi.AddRequestScope(req, &middlewareapi.RequestScope{})
405− refreshed, err := s.refreshSessionWithProvider(nil, req, in.session)
405+ err := s.refreshSession(nil, req, in.session)
406406 if in.expectedErr != nil {
407407 Expect(err).To(MatchError(in.expectedErr))
408408 } else {
409409 Expect(err).ToNot(HaveOccurred())
410410 }
411− Expect(refreshed).To(Equal(in.expectRefreshed))
412411 Expect(saved).To(Equal(in.expectSaved))
413412 },
414413 Entry("when the provider does not refresh the session", refreshSessionWithProviderTableInput{
@@ -416,7 +415,6 @@ var _ = Describe("Stored Session Suite", func() {
416415 RefreshToken: noRefresh,
417416 },
418417 expectedErr: nil,
419− expectRefreshed: false,
420418 expectSaved: false,
421419 }),
422420 Entry("when the provider refreshes the session", refreshSessionWithProviderTableInput{
@@ -424,7 +422,6 @@ var _ = Describe("Stored Session Suite", func() {
424422 RefreshToken: refresh,
425423 },
426424 expectedErr: nil,
427− expectRefreshed: true,
428425 expectSaved: true,
429426 }),
430427 Entry("when the provider returns an error", refreshSessionWithProviderTableInput{
@@ -434,7 +431,6 @@ var _ = Describe("Stored Session Suite", func() {
434431 ExpiresOn: &now,
435432 },
436433 expectedErr: errors.New("error refreshing access token: error refreshing session"),
437− expectRefreshed: false,
438434 expectSaved: false,
439435 }),
440436 Entry("when the saving the session returns an error", refreshSessionWithProviderTableInput{
@@ -443,7 +439,6 @@ var _ = Describe("Stored Session Suite", func() {
443439 AccessToken: "NoSave",
444440 },
445441 expectedErr: errors.New("error saving session: unable to save session"),
446− expectRefreshed: false,
447442 expectSaved: true,
448443 }),
449444 )
@@ -454,7 +449,7 @@ var _ = Describe("Stored Session Suite", func() {
454449
455450 BeforeEach(func() {
456451 s = &storedSessionLoader{
457− validateSessionState: func(_ context.Context, ss *sessionsapi.SessionState) bool {
452+ sessionValidator: func(_ context.Context, ss *sessionsapi.SessionState) bool {
458453 return ss.AccessToken == "Valid"
459454 },
460455 }
providers/azure_test.go+4−3
@@ -345,7 +345,7 @@ func TestAzureProviderNotRefreshWhenNotExpired(t *testing.T) {
345345
346346 expires := time.Now().Add(time.Duration(1) * time.Hour)
347347 session := &sessions.SessionState{AccessToken: "some_access_token", RefreshToken: "some_refresh_token", IDToken: "some_id_token", ExpiresOn: &expires}
348− refreshNeeded, err := p.RefreshSessionIfNeeded(context.Background(), session)
348+ refreshNeeded, err := p.RefreshSession(context.Background(), session)
349349 assert.Equal(t, nil, err)
350350 assert.False(t, refreshNeeded)
351351}
@@ -373,9 +373,10 @@ func TestAzureProviderRefreshWhenExpired(t *testing.T) {
373373
374374 expires := time.Now().Add(time.Duration(-1) * time.Hour)
375375 session := &sessions.SessionState{AccessToken: "some_access_token", RefreshToken: "some_refresh_token", IDToken: "some_id_token", ExpiresOn: &expires}
376− refreshNeeded, err := p.RefreshSessionIfNeeded(context.Background(), session)
376+
377+ refreshed, err := p.RefreshSession(context.Background(), session)
377378 assert.Equal(t, nil, err)
378− assert.True(t, refreshNeeded)
379+ assert.True(t, refreshed)
379380 assert.NotEqual(t, session, nil)
380381 assert.Equal(t, "new_some_access_token", session.AccessToken)
381382 assert.Equal(t, "new_some_refresh_token", session.RefreshToken)
providers/facebook.go+1−1
@@ -88,7 +88,7 @@ func (p *FacebookProvider) GetEmailAddress(ctx context.Context, s *sessions.Sess
8888 return r.Email, nil
8989}
9090
91−// ValidateSessionState validates the AccessToken
91+// ValidateSession validates the AccessToken
9292func (p *FacebookProvider) ValidateSession(ctx context.Context, s *sessions.SessionState) bool {
9393 return validateToken(ctx, p, s.AccessToken, makeOIDCHeader(s.AccessToken))
9494}
providers/gitlab.go+8−13
@@ -121,10 +121,9 @@ func (p *GitLabProvider) SetProjectScope() {
121121 }
122122}
123123
124−// RefreshSessionIfNeeded checks if the session has expired and uses the
125−// RefreshToken to fetch a new ID token if required
126−func (p *GitLabProvider) RefreshSessionIfNeeded(ctx context.Context, s *sessions.SessionState) (bool, error) {
127− if s == nil || (s.ExpiresOn != nil && s.ExpiresOn.After(time.Now())) || s.RefreshToken == "" {
124+// RefreshSession uses the RefreshToken to fetch new Access and ID Tokens
125+func (p *GitLabProvider) RefreshSession(ctx context.Context, s *sessions.SessionState) (bool, error) {
126+ if s == nil || s.RefreshToken == "" {
128127 return false, nil
129128 }
130129
@@ -139,10 +138,10 @@ func (p *GitLabProvider) RefreshSessionIfNeeded(ctx context.Context, s *sessions
139138 return true, nil
140139}
141140
142−func (p *GitLabProvider) redeemRefreshToken(ctx context.Context, s *sessions.SessionState) (err error) {
141+func (p *GitLabProvider) redeemRefreshToken(ctx context.Context, s *sessions.SessionState) error {
143142 clientSecret, err := p.GetClientSecret()
144143 if err != nil {
145− return
144+ return err
146145 }
147146
148147 c := oauth2.Config{
@@ -164,13 +163,9 @@ func (p *GitLabProvider) redeemRefreshToken(ctx context.Context, s *sessions.Ses
164163 if err != nil {
165164 return fmt.Errorf("unable to update session: %v", err)
166165 }
167− s.AccessToken = newSession.AccessToken
168− s.IDToken = newSession.IDToken
169− s.RefreshToken = newSession.RefreshToken
170− s.CreatedAt = newSession.CreatedAt
171− s.ExpiresOn = newSession.ExpiresOn
172− s.Email = newSession.Email
173− return
166+ *s = *newSession
167+
168+ return nil
174169}
175170
176171type gitlabUserInfo struct {
providers/google.go+3−4
@@ -266,10 +266,9 @@ func userInGroup(service *admin.Service, group string, email string) bool {
266266 return false
267267}
268268
269−// RefreshSessionIfNeeded checks if the session has expired and uses the
270−// RefreshToken to fetch a new ID token if required
271−func (p *GoogleProvider) RefreshSessionIfNeeded(ctx context.Context, s *sessions.SessionState) (bool, error) {
272− if s == nil || (s.ExpiresOn != nil && s.ExpiresOn.After(time.Now())) || s.RefreshToken == "" {
269+// RefreshSession uses the RefreshToken to fetch new Access and ID Tokens
270+func (p *GoogleProvider) RefreshSession(ctx context.Context, s *sessions.SessionState) (bool, error) {
271+ if s == nil || s.RefreshToken == "" {
273272 return false, nil
274273 }
275274
providers/linkedin.go+1−1
@@ -93,7 +93,7 @@ func (p *LinkedInProvider) GetEmailAddress(ctx context.Context, s *sessions.Sess
9393 return email, nil
9494}
9595
96−// ValidateSessionState validates the AccessToken
96+// ValidateSession validates the AccessToken
9797func (p *LinkedInProvider) ValidateSession(ctx context.Context, s *sessions.SessionState) bool {
9898 return validateToken(ctx, p, s.AccessToken, makeLinkedInHeader(s.AccessToken))
9999}
providers/oidc.go+3−4
@@ -143,10 +143,9 @@ func (p *OIDCProvider) ValidateSession(ctx context.Context, s *sessions.SessionS
143143 return true
144144}
145145
146−// RefreshSessionIfNeeded checks if the session has expired and uses the
147−// RefreshToken to fetch a new Access Token (and optional ID token) if required
148−func (p *OIDCProvider) RefreshSessionIfNeeded(ctx context.Context, s *sessions.SessionState) (bool, error) {
149− if s == nil || (s.ExpiresOn != nil && s.ExpiresOn.After(time.Now())) || s.RefreshToken == "" {
146+// RefreshSession uses the RefreshToken to fetch new Access and ID Tokens
147+func (p *OIDCProvider) RefreshSession(ctx context.Context, s *sessions.SessionState) (bool, error) {
148+ if s == nil || s.RefreshToken == "" {
150149 return false, nil
151150 }
152151
providers/oidc_test.go+2−2
@@ -487,7 +487,7 @@ func TestOIDCProviderRefreshSessionIfNeededWithoutIdToken(t *testing.T) {
487487 User: "11223344",
488488 }
489489
490− refreshed, err := provider.RefreshSessionIfNeeded(context.Background(), existingSession)
490+ refreshed, err := provider.RefreshSession(context.Background(), existingSession)
491491 assert.Equal(t, nil, err)
492492 assert.Equal(t, refreshed, true)
493493 assert.Equal(t, "janedoe@example.com", existingSession.Email)
@@ -520,7 +520,7 @@ func TestOIDCProviderRefreshSessionIfNeededWithIdToken(t *testing.T) {
520520 Email: "changeit",
521521 User: "changeit",
522522 }
523− refreshed, err := provider.RefreshSessionIfNeeded(context.Background(), existingSession)
523+ refreshed, err := provider.RefreshSession(context.Background(), existingSession)
524524 assert.Equal(t, nil, err)
525525 assert.Equal(t, refreshed, true)
526526 assert.Equal(t, defaultIDToken.Email, existingSession.Email)
providers/provider_default.go+9−4
@@ -126,10 +126,15 @@ func (p *ProviderData) ValidateSession(ctx context.Context, s *sessions.SessionS
126126 return validateToken(ctx, p, s.AccessToken, nil)
127127}
128128
129−// RefreshSessionIfNeeded should refresh the user's session if required and
130−// do nothing if a refresh is not required
131−func (p *ProviderData) RefreshSessionIfNeeded(_ context.Context, _ *sessions.SessionState) (bool, error) {
132− return false, nil
129+// RefreshSession refreshes the user's session
130+func (p *ProviderData) RefreshSession(_ context.Context, s *sessions.SessionState) (bool, error) {
131+ if s == nil {
132+ return false, nil
133+ }
134+
135+ // Pretend `RefreshSession` occured so `ValidateSession` isn't called
136+ // on every request after any potential set refresh period elapses.
137+ return true, nil
133138}
134139
135140// CreateSessionFromToken converts Bearer IDTokens into sessions
providers/provider_default_test.go+1−1
@@ -15,7 +15,7 @@ func TestRefresh(t *testing.T) {
1515 p := &ProviderData{}
1616
1717 expires := time.Now().Add(time.Duration(-11) * time.Minute)
18− refreshed, err := p.RefreshSessionIfNeeded(context.Background(), &sessions.SessionState{
18+ refreshed, err := p.RefreshSession(context.Background(), &sessions.SessionState{
1919 ExpiresOn: &expires,
2020 })
2121 assert.Equal(t, false, refreshed)
providers/providers.go+3−3
@@ -9,14 +9,14 @@ import (
99// Provider represents an upstream identity provider implementation
1010type Provider interface {
1111 Data() *ProviderData
12+ GetLoginURL(redirectURI, finalRedirect string, nonce string) string
13+ Redeem(ctx context.Context, redirectURI, code string) (*sessions.SessionState, error)
1214 // Deprecated: Migrate to EnrichSession
1315 GetEmailAddress(ctx context.Context, s *sessions.SessionState) (string, error)
14− GetLoginURL(redirectURI, state, nonce string) string
15− Redeem(ctx context.Context, redirectURI, code string) (*sessions.SessionState, error)
1616 EnrichSession(ctx context.Context, s *sessions.SessionState) error
1717 Authorize(ctx context.Context, s *sessions.SessionState) (bool, error)
1818 ValidateSession(ctx context.Context, s *sessions.SessionState) bool
19− RefreshSessionIfNeeded(ctx context.Context, s *sessions.SessionState) (bool, error)
19+ RefreshSession(ctx context.Context, s *sessions.SessionState) (bool, error)
2020 CreateSessionFromToken(ctx context.Context, token string) (*sessions.SessionState, error)
2121}
2222