git.iko.soy/typeable/oauth2-proxy
on master help sign in
Merge pull request #1086 from oauth2-proxy/early-refresh

Convert RefreshSessionIfNeeded into RefreshSession

commit16a9893a19dfe832a1acc72af70b27d0150caa7f authorNick Meves <nicholas.meves@gmail.com> dateTue 22 Jun 2021, 17:13 (5 years ago) parent5f4ac25ff914d7
21 files changed+301−247
compare with master
unifiedsplit
CHANGELOG.md+6 oauthproxy.go+13−4 pkg/apis/sessions/session_state.go+27−43 pkg/apis/sessions/session_state_test.go+24 pkg/clock/clock.go−7 pkg/middleware/stored_session.go+46−35 pkg/middleware/stored_session_test.go+79−37 pkg/sessions/cookie/session_store.go+1−2 pkg/sessions/persistence/manager.go+1−2 providers/azure.go+17−21 providers/azure_test.go+4−13 providers/facebook.go+1−1 providers/gitlab.go+15−18 providers/google.go+22−27 providers/linkedin.go+1−1 providers/logingov.go+8−9 providers/oidc.go+8−9 providers/oidc_test.go+2−2 providers/provider_default.go+9−7 providers/provider_default_test.go+14−6 providers/providers.go+3−3
CHANGELOG.md+6
@@ -4,10 +4,16 @@
44
55## Important Notes
66
7+- [#1086](https://github.com/oauth2-proxy/oauth2-proxy/pull/1086) The extra validation to protect invalid session
8+ deserialization from v6.0.0 (only) has been removed to improve performance. If you are on v6.0.0, either upgrade
9+ to a version before this first and allow legacy sessions to expire gracefully or change your `cookie-secret`
10+ value and force all sessions to reauthenticate.
11+
712## Breaking Changes
813
914## Changes since v7.1.3
1015
16+- [#1086](https://github.com/oauth2-proxy/oauth2-proxy/pull/1086) Refresh sessions before token expiration if configured (@NickMeves)
1117- [#1226](https://github.com/oauth2-proxy/oauth2-proxy/pull/1226) Move app redirection logic to its own package (@JoelSpeed)
1218- [#1128](https://github.com/oauth2-proxy/oauth2-proxy/pull/1128) Use gorilla mux for OAuth Proxy routing (@JoelSpeed)
1319- [#1238](https://github.com/oauth2-proxy/oauth2-proxy/pull/1238) Added ADFS provider (@samirachoadi)
oauthproxy.go+13−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
@@ -786,6 +786,15 @@ func (p *OAuthProxy) redeemCode(req *http.Request) (*sessionsapi.SessionState, e
786786 if err != nil {
787787 return nil, err
788788 }
789+
790+ // Force setting these in case the Provider didn't
791+ if s.CreatedAt == nil {
792+ s.CreatedAtNow()
793+ }
794+ if s.ExpiresOn == nil {
795+ s.ExpiresIn(p.CookieOptions.Expire)
796+ }
797+
789798 return s, nil
790799}
791800
pkg/apis/sessions/session_state.go+27−43
@@ -3,14 +3,12 @@ package sessions
33import (
44 "bytes"
55 "context"
6− "errors"
76 "fmt"
87 "io"
98 "io/ioutil"
10− "reflect"
119 "time"
12− "unicode/utf8"
1310
11+ "github.com/oauth2-proxy/oauth2-proxy/v7/pkg/clock"
1412 "github.com/oauth2-proxy/oauth2-proxy/v7/pkg/encryption"
1513 "github.com/pierrec/lz4"
1614 "github.com/vmihailenco/msgpack/v4"
@@ -32,7 +30,9 @@ type SessionState struct {
3230 Groups []string `msgpack:"g,omitempty"`
3331 PreferredUsername string `msgpack:"pu,omitempty"`
3432
35− Lock Lock `msgpack:"-"`
33+ // Internal helpers, not serialized
34+ Clock clock.Clock `msgpack:"-"`
35+ Lock Lock `msgpack:"-"`
3636}
3737
3838func (s *SessionState) ObtainLock(ctx context.Context, expiration time.Duration) error {
@@ -63,9 +63,30 @@ func (s *SessionState) PeekLock(ctx context.Context) (bool, error) {
6363 return s.Lock.Peek(ctx)
6464}
6565
66+// CreatedAtNow sets a SessionState's CreatedAt to now
67+func (s *SessionState) CreatedAtNow() {
68+ now := s.Clock.Now()
69+ s.CreatedAt = &now
70+}
71+
72+// SetExpiresOn sets an expiration
73+func (s *SessionState) SetExpiresOn(exp time.Time) {
74+ s.ExpiresOn = &exp
75+}
76+
77+// ExpiresIn sets an expiration a certain duration from CreatedAt.
78+// CreatedAt will be set to time.Now if it is unset.
79+func (s *SessionState) ExpiresIn(d time.Duration) {
80+ if s.CreatedAt == nil {
81+ s.CreatedAtNow()
82+ }
83+ exp := s.CreatedAt.Add(d)
84+ s.ExpiresOn = &exp
85+}
86+
6687// IsExpired checks whether the session has expired
6788func (s *SessionState) IsExpired() bool {
68− if s.ExpiresOn != nil && !s.ExpiresOn.IsZero() && s.ExpiresOn.Before(time.Now()) {
89+ if s.ExpiresOn != nil && !s.ExpiresOn.IsZero() && s.ExpiresOn.Before(s.Clock.Now()) {
6990 return true
7091 }
7192 return false
@@ -74,7 +95,7 @@ func (s *SessionState) IsExpired() bool {
7495// Age returns the age of a session
7596func (s *SessionState) Age() time.Duration {
7697 if s.CreatedAt != nil && !s.CreatedAt.IsZero() {
77− return time.Now().Truncate(time.Second).Sub(*s.CreatedAt)
98+ return s.Clock.Now().Truncate(time.Second).Sub(*s.CreatedAt)
7899 }
79100 return 0
80101}
@@ -177,11 +198,6 @@ func DecodeSessionState(data []byte, c encryption.Cipher, compressed bool) (*Ses
177198 return nil, fmt.Errorf("error unmarshalling data to session state: %w", err)
178199 }
179200
180− err = ss.validate()
181− if err != nil {
182− return nil, err
183− }
184−
185201 return &ss, nil
186202}
187203
@@ -235,35 +251,3 @@ func lz4Decompress(compressed []byte) ([]byte, error) {
235251
236252 return payload, nil
237253}
238−
239−// validate ensures the decoded session is non-empty and contains valid data
240−//
241−// Non-empty check is needed due to ensure the non-authenticated AES-CFB
242−// decryption doesn't result in garbage data that collides with a valid
243−// MessagePack header bytes (which MessagePack will unmarshal to an empty
244−// default SessionState). <1% chance, but observed with random test data.
245−//
246−// UTF-8 check ensures the strings are valid and not raw bytes overloaded
247−// into Latin-1 encoding. The occurs when legacy unencrypted fields are
248−// decrypted with AES-CFB which results in random bytes.
249−func (s *SessionState) validate() error {
250− for _, field := range []string{
251− s.User,
252− s.Email,
253− s.PreferredUsername,
254− s.AccessToken,
255− s.IDToken,
256− s.RefreshToken,
257− } {
258− if !utf8.ValidString(field) {
259− return errors.New("invalid non-UTF8 field in session")
260− }
261− }
262−
263− empty := new(SessionState)
264− if reflect.DeepEqual(*s, *empty) {
265− return errors.New("invalid empty session unmarshalled")
266− }
267−
268− return nil
269−}
pkg/apis/sessions/session_state_test.go+24
@@ -16,6 +16,30 @@ func timePtr(t time.Time) *time.Time {
1616 return &t
1717}
1818
19+func TestCreatedAtNow(t *testing.T) {
20+ g := NewWithT(t)
21+ ss := &SessionState{}
22+
23+ now := time.Unix(1234567890, 0)
24+ ss.Clock.Set(now)
25+
26+ ss.CreatedAtNow()
27+ g.Expect(*ss.CreatedAt).To(Equal(now))
28+}
29+
30+func TestExpiresIn(t *testing.T) {
31+ g := NewWithT(t)
32+ ss := &SessionState{}
33+
34+ now := time.Unix(1234567890, 0)
35+ ss.Clock.Set(now)
36+
37+ ttl := time.Duration(743) * time.Second
38+ ss.ExpiresIn(ttl)
39+
40+ g.Expect(*ss.ExpiresOn).To(Equal(ss.CreatedAt.Add(ttl)))
41+}
42+
1943func TestString(t *testing.T) {
2044 g := NewWithT(t)
2145 created, err := time.Parse(time.RFC3339, "2000-01-01T00:00:00Z")
pkg/clock/clock.go−7
@@ -63,13 +63,10 @@ func Reset() *clockapi.Mock {
6363// package.
6464type Clock struct {
6565 mock *clockapi.Mock
66− sync.Mutex
6766}
6867
6968// Set sets the Clock to a clock.Mock at the given time.Time
7069func (c *Clock) Set(t time.Time) {
71− c.Lock()
72− defer c.Unlock()
7370 if c.mock == nil {
7471 c.mock = clockapi.NewMock()
7572 }
@@ -79,8 +76,6 @@ func (c *Clock) Set(t time.Time) {
7976// Add moves clock forward time.Duration if it is mocked. It will error
8077// if the clock is not mocked.
8178func (c *Clock) Add(d time.Duration) error {
82− c.Lock()
83− defer c.Unlock()
8479 if c.mock == nil {
8580 return errors.New("clock not mocked")
8681 }
@@ -91,8 +86,6 @@ func (c *Clock) Add(d time.Duration) error {
9186// Reset removes local clock.Mock. Returns any existing Mock if set in case
9287// lingering time operations are attached to it.
9388func (c *Clock) Reset() *clockapi.Mock {
94− c.Lock()
95− defer c.Unlock()
9689 existing := c.mock
9790 c.mock = nil
9891 return existing
pkg/middleware/stored_session.go+46−35
@@ -11,25 +11,26 @@ import (
1111 middlewareapi "github.com/oauth2-proxy/oauth2-proxy/v7/pkg/apis/middleware"
1212 sessionsapi "github.com/oauth2-proxy/oauth2-proxy/v7/pkg/apis/sessions"
1313 "github.com/oauth2-proxy/oauth2-proxy/v7/pkg/logger"
14+ "github.com/oauth2-proxy/oauth2-proxy/v7/providers"
1415)
1516
16−// StoredSessionLoaderOptions cotnains all of the requirements to construct
17+// StoredSessionLoaderOptions contains all of the requirements to construct
1718// a stored session loader.
1819// All options must be provided.
1920type StoredSessionLoaderOptions struct {
20− // Session storage basckend
21+ // Session storage backend
2122 SessionStore sessionsapi.SessionStore
2223
2324 // How often should sessions be refreshed
2425 RefreshPeriod time.Duration
2526
26− // Provider based sesssion refreshing
27− RefreshSessionIfNeeded func(context.Context, *sessionsapi.SessionState) (bool, error)
27+ // Provider based session refreshing
28+ RefreshSession func(context.Context, *sessionsapi.SessionState) (bool, error)
2829
2930 // Provider based session validation.
3031 // If the sesssion is older than `RefreshPeriod` but the provider doesn't
3132 // refresh it, we must re-validate using this validation.
32− ValidateSessionState func(context.Context, *sessionsapi.SessionState) bool
33+ ValidateSession func(context.Context, *sessionsapi.SessionState) bool
3334}
3435
3536// NewStoredSessionLoader creates a new storedSessionLoader which loads
@@ -38,10 +39,10 @@ type StoredSessionLoaderOptions struct {
3839// If a session was loader by a previous handler, it will not be replaced.
3940func NewStoredSessionLoader(opts *StoredSessionLoaderOptions) alice.Constructor {
4041 ss := &storedSessionLoader{
41− store: opts.SessionStore,
42− refreshPeriod: opts.RefreshPeriod,
43− refreshSessionWithProviderIfNeeded: opts.RefreshSessionIfNeeded,
44− validateSessionState: opts.ValidateSessionState,
42+ store: opts.SessionStore,
43+ refreshPeriod: opts.RefreshPeriod,
44+ sessionRefresher: opts.RefreshSession,
45+ sessionValidator: opts.ValidateSession,
4546 }
4647 return ss.loadSession
4748}
@@ -49,10 +50,10 @@ func NewStoredSessionLoader(opts *StoredSessionLoaderOptions) alice.Constructor
4950// storedSessionLoader is responsible for loading sessions from cookie
5051// identified sessions in the session store.
5152type 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
53+ store sessionsapi.SessionStore
54+ refreshPeriod time.Duration
55+ sessionRefresher func(context.Context, *sessionsapi.SessionState) (bool, error)
56+ sessionValidator func(context.Context, *sessionsapi.SessionState) bool
5657}
5758
5859// loadSession attempts to load a session as identified by the request cookies.
@@ -108,49 +109,59 @@ func (s *storedSessionLoader) getValidatedSession(rw http.ResponseWriter, req *h
108109
109110// refreshSessionIfNeeded will attempt to refresh a session if the session
110111// is older than the refresh period.
111−// It is assumed that if the provider refreshes the session, the session is now
112−// valid.
113−// If the session requires refreshing but the provider does not refresh it,
114−// we must validate the session to ensure that the returned session is still
115−// valid.
112+// Success or fail, we will then validate the session.
116113func (s *storedSessionLoader) refreshSessionIfNeeded(rw http.ResponseWriter, req *http.Request, session *sessionsapi.SessionState) error {
117114 if s.refreshPeriod <= time.Duration(0) || session.Age() < s.refreshPeriod {
118115 // Refresh is disabled or the session is not old enough, do nothing
119116 return nil
120117 }
121118
122− 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)
119+ logger.Printf("Refreshing session - User: %s; SessionAge: %s", session.User, session.Age())
120+ err := s.refreshSession(rw, req, session)
124121 if err != nil {
125− return err
122+ // If a preemptive refresh fails, we still keep the session
123+ // if validateSession succeeds.
124+ logger.Errorf("Unable to refresh session: %v", err)
126125 }
127126
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
127+ // Validate all sessions after any Redeem/Refresh operation (fail or success)
128+ return s.validateSession(req.Context(), session)
133129}
134130
135−// refreshSessionWithProvider attempts to refresh the sessinon with the provider
131+// refreshSession attempts to refresh the session with the provider
136132// 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)
139− if err != nil {
140− return false, fmt.Errorf("error refreshing access token: %v", err)
133+func (s *storedSessionLoader) refreshSession(rw http.ResponseWriter, req *http.Request, session *sessionsapi.SessionState) error {
134+ refreshed, err := s.sessionRefresher(req.Context(), session)
135+ if err != nil && !errors.Is(err, providers.ErrNotImplemented) {
136+ return fmt.Errorf("error refreshing tokens: %v", err)
141137 }
142138
139+ // HACK:
140+ // Providers that don't implement `RefreshSession` use the default
141+ // implementation which returns `ErrNotImplemented`.
142+ // Pretend it refreshed to reset the refresh timer so that `ValidateSession`
143+ // isn't triggered every subsequent request and is only called once during
144+ // this request.
145+ if errors.Is(err, providers.ErrNotImplemented) {
146+ refreshed = true
147+ }
148+
149+ // Session not refreshed, nothing to persist.
143150 if !refreshed {
144− return false, nil
151+ return nil
145152 }
146153
154+ // If we refreshed, update the `CreatedAt` time to reset the refresh timer
155+ // (In case underlying provider implementations forget)
156+ session.CreatedAtNow()
157+
147158 // Because the session was refreshed, make sure to save it
148159 err = s.store.Save(rw, req, session)
149160 if err != nil {
150161 logger.PrintAuthf(session.Email, req, logger.AuthError, "error saving session: %v", err)
151− return false, fmt.Errorf("error saving session: %v", err)
162+ return fmt.Errorf("error saving session: %v", err)
152163 }
153− return true, nil
164+ return nil
154165}
155166
156167// validateSession checks whether the session has expired and performs
@@ -161,7 +172,7 @@ func (s *storedSessionLoader) validateSession(ctx context.Context, session *sess
161172 return errors.New("session is expired")
162173 }
163174
164− if !s.validateSessionState(ctx, session) {
175+ if !s.sessionValidator(ctx, session) {
165176 return errors.New("session is invalid")
166177 }
167178
pkg/middleware/stored_session_test.go+79−37
@@ -10,6 +10,8 @@ import (
1010
1111 middlewareapi "github.com/oauth2-proxy/oauth2-proxy/v7/pkg/apis/middleware"
1212 sessionsapi "github.com/oauth2-proxy/oauth2-proxy/v7/pkg/apis/sessions"
13+ "github.com/oauth2-proxy/oauth2-proxy/v7/pkg/clock"
14+ "github.com/oauth2-proxy/oauth2-proxy/v7/providers"
1315 . "github.com/onsi/ginkgo"
1416 . "github.com/onsi/ginkgo/extensions/table"
1517 . "github.com/onsi/gomega"
@@ -17,15 +19,17 @@ import (
1719
1820var _ = Describe("Stored Session Suite", func() {
1921 const (
20− refresh = "Refresh"
21− noRefresh = "NoRefresh"
22+ refresh = "Refresh"
23+ noRefresh = "NoRefresh"
24+ notImplemented = "NotImplemented"
2225 )
2326
2427 var ctx = context.Background()
2528
2629 Context("StoredSessionLoader", func() {
27− createdPast := time.Now().Add(-5 * time.Minute)
28− createdFuture := time.Now().Add(5 * time.Minute)
30+ now := time.Now()
31+ createdPast := now.Add(-5 * time.Minute)
32+ createdFuture := now.Add(5 * time.Minute)
2933
3034 var defaultRefreshFunc = func(_ context.Context, ss *sessionsapi.SessionState) (bool, error) {
3135 switch ss.RefreshToken {
@@ -85,6 +89,14 @@ var _ = Describe("Stored Session Suite", func() {
8589 },
8690 }
8791
92+ BeforeEach(func() {
93+ clock.Set(now)
94+ })
95+
96+ AfterEach(func() {
97+ clock.Reset()
98+ })
99+
88100 type storedSessionLoaderTableInput struct {
89101 requestHeaders http.Header
90102 existingSession *sessionsapi.SessionState
@@ -109,10 +121,10 @@ var _ = Describe("Stored Session Suite", func() {
109121 rw := httptest.NewRecorder()
110122
111123 opts := &StoredSessionLoaderOptions{
112− SessionStore: in.store,
113− RefreshPeriod: in.refreshPeriod,
114− RefreshSessionIfNeeded: in.refreshSession,
115− ValidateSessionState: in.validateSession,
124+ SessionStore: in.store,
125+ RefreshPeriod: in.refreshPeriod,
126+ RefreshSession: in.refreshSession,
127+ ValidateSession: in.validateSession,
116128 }
117129
118130 // Create the handler with a next handler that will capture the session
@@ -208,6 +220,21 @@ var _ = Describe("Stored Session Suite", func() {
208220 existingSession: nil,
209221 expectedSession: &sessionsapi.SessionState{
210222 RefreshToken: "Refreshed",
223+ CreatedAt: &now,
224+ ExpiresOn: &createdFuture,
225+ },
226+ store: defaultSessionStore,
227+ refreshPeriod: 1 * time.Minute,
228+ refreshSession: defaultRefreshFunc,
229+ validateSession: defaultValidateFunc,
230+ }),
231+ Entry("when the provider refresh fails but validation succeeds", storedSessionLoaderTableInput{
232+ requestHeaders: http.Header{
233+ "Cookie": []string{"_oauth2_proxy=RefreshError"},
234+ },
235+ existingSession: nil,
236+ expectedSession: &sessionsapi.SessionState{
237+ RefreshToken: "RefreshError",
211238 CreatedAt: &createdPast,
212239 ExpiresOn: &createdFuture,
213240 },
@@ -216,7 +243,7 @@ var _ = Describe("Stored Session Suite", func() {
216243 refreshSession: defaultRefreshFunc,
217244 validateSession: defaultValidateFunc,
218245 }),
219− Entry("when the provider refresh fails", storedSessionLoaderTableInput{
246+ Entry("when the provider refresh fails and validation fails", storedSessionLoaderTableInput{
220247 requestHeaders: http.Header{
221248 "Cookie": []string{"_oauth2_proxy=RefreshError"},
222249 },
@@ -225,7 +252,7 @@ var _ = Describe("Stored Session Suite", func() {
225252 store: defaultSessionStore,
226253 refreshPeriod: 1 * time.Minute,
227254 refreshSession: defaultRefreshFunc,
228− validateSession: defaultValidateFunc,
255+ validateSession: func(context.Context, *sessionsapi.SessionState) bool { return false },
229256 }),
230257 Entry("when the session is not refreshed and is no longer valid", storedSessionLoaderTableInput{
231258 requestHeaders: http.Header{
@@ -261,18 +288,20 @@ var _ = Describe("Stored Session Suite", func() {
261288 s := &storedSessionLoader{
262289 refreshPeriod: in.refreshPeriod,
263290 store: &fakeSessionStore{},
264− refreshSessionWithProviderIfNeeded: func(_ context.Context, ss *sessionsapi.SessionState) (bool, error) {
291+ sessionRefresher: func(_ context.Context, ss *sessionsapi.SessionState) (bool, error) {
265292 refreshed = true
266293 switch ss.RefreshToken {
267294 case refresh:
268295 return true, nil
269296 case noRefresh:
270297 return false, nil
298+ case notImplemented:
299+ return false, providers.ErrNotImplemented
271300 default:
272301 return false, errors.New("error refreshing session")
273302 }
274303 },
275− validateSessionState: func(_ context.Context, ss *sessionsapi.SessionState) bool {
304+ sessionValidator: func(_ context.Context, ss *sessionsapi.SessionState) bool {
276305 validated = true
277306 return ss.AccessToken != "Invalid"
278307 },
@@ -326,7 +355,7 @@ var _ = Describe("Stored Session Suite", func() {
326355 },
327356 expectedErr: nil,
328357 expectRefreshed: true,
329− expectValidated: false,
358+ expectValidated: true,
330359 }),
331360 Entry("when the session is not refreshed by the provider", refreshSessionIfNeededTableInput{
332361 refreshPeriod: 1 * time.Minute,
@@ -339,15 +368,25 @@ var _ = Describe("Stored Session Suite", func() {
339368 expectRefreshed: true,
340369 expectValidated: true,
341370 }),
342− Entry("when the provider refresh fails", refreshSessionIfNeededTableInput{
371+ Entry("when the provider doesn't implement refresh but validation succeeds", refreshSessionIfNeededTableInput{
372+ refreshPeriod: 1 * time.Minute,
373+ session: &sessionsapi.SessionState{
374+ RefreshToken: notImplemented,
375+ CreatedAt: &createdPast,
376+ },
377+ expectedErr: nil,
378+ expectRefreshed: true,
379+ expectValidated: true,
380+ }),
381+ Entry("when the provider refresh fails but validation succeeds", refreshSessionIfNeededTableInput{
343382 refreshPeriod: 1 * time.Minute,
344383 session: &sessionsapi.SessionState{
345384 RefreshToken: "RefreshError",
346385 CreatedAt: &createdPast,
347386 },
348− expectedErr: errors.New("error refreshing access token: error refreshing session"),
387+ expectedErr: nil,
349388 expectRefreshed: true,
350− expectValidated: false,
389+ expectValidated: true,
351390 }),
352391 Entry("when the session is not refreshed by the provider and validation fails", refreshSessionIfNeededTableInput{
353392 refreshPeriod: 1 * time.Minute,
@@ -364,12 +403,11 @@ var _ = Describe("Stored Session Suite", func() {
364403 )
365404 })
366405
367− Context("refreshSessionWithProvider", func() {
406+ Context("refreshSession", func() {
368407 type refreshSessionWithProviderTableInput struct {
369− session *sessionsapi.SessionState
370− expectedErr error
371− expectRefreshed bool
372− expectSaved bool
408+ session *sessionsapi.SessionState
409+ expectedErr error
410+ expectSaved bool
373411 }
374412
375413 now := time.Now()
@@ -388,12 +426,14 @@ var _ = Describe("Stored Session Suite", func() {
388426 return nil
389427 },
390428 },
391− refreshSessionWithProviderIfNeeded: func(_ context.Context, ss *sessionsapi.SessionState) (bool, error) {
429+ sessionRefresher: func(_ context.Context, ss *sessionsapi.SessionState) (bool, error) {
392430 switch ss.RefreshToken {
393431 case refresh:
394432 return true, nil
395433 case noRefresh:
396434 return false, nil
435+ case notImplemented:
436+ return false, providers.ErrNotImplemented
397437 default:
398438 return false, errors.New("error refreshing session")
399439 }
@@ -402,30 +442,34 @@ var _ = Describe("Stored Session Suite", func() {
402442
403443 req := httptest.NewRequest("", "/", nil)
404444 req = middlewareapi.AddRequestScope(req, &middlewareapi.RequestScope{})
405− refreshed, err := s.refreshSessionWithProvider(nil, req, in.session)
445+ err := s.refreshSession(nil, req, in.session)
406446 if in.expectedErr != nil {
407447 Expect(err).To(MatchError(in.expectedErr))
408448 } else {
409449 Expect(err).ToNot(HaveOccurred())
410450 }
411− Expect(refreshed).To(Equal(in.expectRefreshed))
412451 Expect(saved).To(Equal(in.expectSaved))
413452 },
414453 Entry("when the provider does not refresh the session", refreshSessionWithProviderTableInput{
415454 session: &sessionsapi.SessionState{
416455 RefreshToken: noRefresh,
417456 },
418− expectedErr: nil,
419− expectRefreshed: false,
420− expectSaved: false,
457+ expectedErr: nil,
458+ expectSaved: false,
421459 }),
422460 Entry("when the provider refreshes the session", refreshSessionWithProviderTableInput{
423461 session: &sessionsapi.SessionState{
424462 RefreshToken: refresh,
425463 },
426− expectedErr: nil,
427− expectRefreshed: true,
428− expectSaved: true,
464+ expectedErr: nil,
465+ expectSaved: true,
466+ }),
467+ Entry("when the provider doesn't implement refresh", refreshSessionWithProviderTableInput{
468+ session: &sessionsapi.SessionState{
469+ RefreshToken: notImplemented,
470+ },
471+ expectedErr: nil,
472+ expectSaved: true,
429473 }),
430474 Entry("when the provider returns an error", refreshSessionWithProviderTableInput{
431475 session: &sessionsapi.SessionState{
@@ -433,18 +477,16 @@ var _ = Describe("Stored Session Suite", func() {
433477 CreatedAt: &now,
434478 ExpiresOn: &now,
435479 },
436− expectedErr: errors.New("error refreshing access token: error refreshing session"),
437− expectRefreshed: false,
438− expectSaved: false,
480+ expectedErr: errors.New("error refreshing tokens: error refreshing session"),
481+ expectSaved: false,
439482 }),
440483 Entry("when the saving the session returns an error", refreshSessionWithProviderTableInput{
441484 session: &sessionsapi.SessionState{
442485 RefreshToken: refresh,
443486 AccessToken: "NoSave",
444487 },
445− expectedErr: errors.New("error saving session: unable to save session"),
446− expectRefreshed: false,
447− expectSaved: true,
488+ expectedErr: errors.New("error saving session: unable to save session"),
489+ expectSaved: true,
448490 }),
449491 )
450492 })
@@ -454,7 +496,7 @@ var _ = Describe("Stored Session Suite", func() {
454496
455497 BeforeEach(func() {
456498 s = &storedSessionLoader{
457− validateSessionState: func(_ context.Context, ss *sessionsapi.SessionState) bool {
499+ sessionValidator: func(_ context.Context, ss *sessionsapi.SessionState) bool {
458500 return ss.AccessToken == "Valid"
459501 },
460502 }
pkg/sessions/cookie/session_store.go+1−2
@@ -36,8 +36,7 @@ type SessionStore struct {
3636// within Cookies set on the HTTP response writer
3737func (s *SessionStore) Save(rw http.ResponseWriter, req *http.Request, ss *sessions.SessionState) error {
3838 if ss.CreatedAt == nil || ss.CreatedAt.IsZero() {
39− now := time.Now()
40− ss.CreatedAt = &now
39+ ss.CreatedAtNow()
4140 }
4241 value, err := s.cookieForSession(ss)
4342 if err != nil {
pkg/sessions/persistence/manager.go+1−2
@@ -30,8 +30,7 @@ func NewManager(store Store, cookieOpts *options.Cookie) *Manager {
3030// from the persistent data store.
3131func (m *Manager) Save(rw http.ResponseWriter, req *http.Request, s *sessions.SessionState) error {
3232 if s.CreatedAt == nil || s.CreatedAt.IsZero() {
33− now := time.Now()
34− s.CreatedAt = &now
33+ s.CreatedAtNow()
3534 }
3635
3736 tckt, err := decodeTicketFromRequest(req, m.Options)
providers/azure.go+17−21
@@ -142,16 +142,13 @@ func (p *AzureProvider) Redeem(ctx context.Context, redirectURL, code string) (*
142142 return nil, err
143143 }
144144
145− created := time.Now()
146− expires := time.Unix(jsonResponse.ExpiresOn, 0)
147−
148145 session := &sessions.SessionState{
149146 AccessToken: jsonResponse.AccessToken,
150147 IDToken: jsonResponse.IDToken,
151− CreatedAt: &created,
152− ExpiresOn: &expires,
153148 RefreshToken: jsonResponse.RefreshToken,
154149 }
150+ session.CreatedAtNow()
151+ session.SetExpiresOn(time.Unix(jsonResponse.ExpiresOn, 0))
155152
156153 email, err := p.verifyTokenAndExtractEmail(ctx, session.IDToken)
157154
@@ -239,28 +236,29 @@ func (p *AzureProvider) verifyTokenAndExtractEmail(ctx context.Context, token st
239236 return email, nil
240237}
241238
242−// RefreshSessionIfNeeded checks if the session has expired and uses the
243−// RefreshToken to fetch a new ID token if required
244−func (p *AzureProvider) RefreshSessionIfNeeded(ctx context.Context, s *sessions.SessionState) (bool, error) {
245− if s == nil || s.ExpiresOn.After(time.Now()) || s.RefreshToken == "" {
239+// RefreshSession uses the RefreshToken to fetch new Access and ID Tokens
240+func (p *AzureProvider) RefreshSession(ctx context.Context, s *sessions.SessionState) (bool, error) {
241+ if s == nil || s.RefreshToken == "" {
246242 return false, nil
247243 }
248244
249− origExpiration := s.ExpiresOn
250−
251245 err := p.redeemRefreshToken(ctx, s)
252246 if err != nil {
253247 return false, fmt.Errorf("unable to redeem refresh token: %v", err)
254248 }
255249
256− logger.Printf("refreshed id token %s (expired on %s)\n", s, origExpiration)
257250 return true, nil
258251}
259252
260−func (p *AzureProvider) redeemRefreshToken(ctx context.Context, s *sessions.SessionState) (err error) {
253+func (p *AzureProvider) redeemRefreshToken(ctx context.Context, s *sessions.SessionState) error {
254+ clientSecret, err := p.GetClientSecret()
255+ if err != nil {
256+ return err
257+ }
258+
261259 params := url.Values{}
262260 params.Add("client_id", p.ClientID)
263− params.Add("client_secret", p.ClientSecret)
261+ params.Add("client_secret", clientSecret)
264262 params.Add("refresh_token", s.RefreshToken)
265263 params.Add("grant_type", "refresh_token")
266264
@@ -278,18 +276,16 @@ func (p *AzureProvider) redeemRefreshToken(ctx context.Context, s *sessions.Sess
278276 SetHeader("Content-Type", "application/x-www-form-urlencoded").
279277 Do().
280278 UnmarshalInto(&jsonResponse)
281−
282279 if err != nil {
283− return
280+ return err
284281 }
285282
286− now := time.Now()
287− expires := time.Unix(jsonResponse.ExpiresOn, 0)
288283 s.AccessToken = jsonResponse.AccessToken
289284 s.IDToken = jsonResponse.IDToken
290285 s.RefreshToken = jsonResponse.RefreshToken
291− s.CreatedAt = &now
292− s.ExpiresOn = &expires
286+
287+ s.CreatedAtNow()
288+ s.SetExpiresOn(time.Unix(jsonResponse.ExpiresOn, 0))
293289
294290 email, err := p.verifyTokenAndExtractEmail(ctx, s.IDToken)
295291
@@ -312,7 +308,7 @@ func (p *AzureProvider) redeemRefreshToken(ctx context.Context, s *sessions.Sess
312308 }
313309 }
314310
315− return
311+ return nil
316312}
317313
318314func makeAzureHeader(accessToken string) http.Header {
providers/azure_test.go+4−13
@@ -340,17 +340,7 @@ func TestAzureProviderProtectedResourceConfigured(t *testing.T) {
340340 assert.Contains(t, result, "resource="+url.QueryEscape("http://my.resource.test"))
341341}
342342
343−func TestAzureProviderNotRefreshWhenNotExpired(t *testing.T) {
344− p := testAzureProvider("")
345−
346− expires := time.Now().Add(time.Duration(1) * time.Hour)
347− 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)
349− assert.Equal(t, nil, err)
350− assert.False(t, refreshNeeded)
351−}
352−
353−func TestAzureProviderRefreshWhenExpired(t *testing.T) {
343+func TestAzureProviderRefresh(t *testing.T) {
354344 email := "foo@example.com"
355345 idToken := idTokenClaims{Email: email}
356346 idTokenString, err := newSignedTestIDToken(idToken)
@@ -373,9 +363,10 @@ func TestAzureProviderRefreshWhenExpired(t *testing.T) {
373363
374364 expires := time.Now().Add(time.Duration(-1) * time.Hour)
375365 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)
366+
367+ refreshed, err := p.RefreshSession(context.Background(), session)
377368 assert.Equal(t, nil, err)
378− assert.True(t, refreshNeeded)
369+ assert.True(t, refreshed)
379370 assert.NotEqual(t, session, nil)
380371 assert.Equal(t, "new_some_access_token", session.AccessToken)
381372 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+15−18
@@ -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 {
@@ -264,14 +259,16 @@ func (p *GitLabProvider) createSession(ctx context.Context, token *oauth2.Token)
264259 }
265260 }
266261
267− created := time.Now()
268− return &sessions.SessionState{
262+ ss := &sessions.SessionState{
269263 AccessToken: token.AccessToken,
270264 IDToken: getIDToken(token),
271265 RefreshToken: token.RefreshToken,
272− CreatedAt: &created,
273− ExpiresOn: &idToken.Expiry,
274− }, nil
266+ }
267+
268+ ss.CreatedAtNow()
269+ ss.SetExpiresOn(idToken.Expiry)
270+
271+ return ss, nil
275272}
276273
277274// ValidateSession checks that the session's IDToken is still valid
providers/google.go+22−27
@@ -163,23 +163,22 @@ func (p *GoogleProvider) Redeem(ctx context.Context, redirectURL, code string) (
163163 return nil, err
164164 }
165165
166− created := time.Now()
167− expires := time.Now().Add(time.Duration(jsonResponse.ExpiresIn) * time.Second).Truncate(time.Second)
168−
169− return &sessions.SessionState{
166+ ss := &sessions.SessionState{
170167 AccessToken: jsonResponse.AccessToken,
171168 IDToken: jsonResponse.IDToken,
172− CreatedAt: &created,
173− ExpiresOn: &expires,
174169 RefreshToken: jsonResponse.RefreshToken,
175170 Email: c.Email,
176171 User: c.Subject,
177− }, nil
172+ }
173+ ss.CreatedAtNow()
174+ ss.ExpiresIn(time.Duration(jsonResponse.ExpiresIn) * time.Second)
175+
176+ return ss, nil
178177}
179178
180179// EnrichSession checks the listed Google Groups configured and adds any
181180// that the user is a member of to session.Groups.
182−func (p *GoogleProvider) EnrichSession(ctx context.Context, s *sessions.SessionState) error {
181+func (p *GoogleProvider) EnrichSession(_ context.Context, s *sessions.SessionState) error {
183182 // TODO (@NickMeves) - Move to pure EnrichSession logic and stop
184183 // reusing legacy `groupValidator`.
185184 //
@@ -266,14 +265,13 @@ func userInGroup(service *admin.Service, group string, email string) bool {
266265 return false
267266}
268267
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 == "" {
268+// RefreshSession uses the RefreshToken to fetch new Access and ID Tokens
269+func (p *GoogleProvider) RefreshSession(ctx context.Context, s *sessions.SessionState) (bool, error) {
270+ if s == nil || s.RefreshToken == "" {
273271 return false, nil
274272 }
275273
276− newToken, newIDToken, duration, err := p.redeemRefreshToken(ctx, s.RefreshToken)
274+ err := p.redeemRefreshToken(ctx, s)
277275 if err != nil {
278276 return false, err
279277 }
@@ -286,26 +284,20 @@ func (p *GoogleProvider) RefreshSessionIfNeeded(ctx context.Context, s *sessions
286284 return false, fmt.Errorf("%s is no longer in the group(s)", s.Email)
287285 }
288286
289− origExpiration := s.ExpiresOn
290− expires := time.Now().Add(duration).Truncate(time.Second)
291− s.AccessToken = newToken
292− s.IDToken = newIDToken
293− s.ExpiresOn = &expires
294− logger.Printf("refreshed access token %s (expired on %s)", s, origExpiration)
295287 return true, nil
296288}
297289
298−func (p *GoogleProvider) redeemRefreshToken(ctx context.Context, refreshToken string) (token string, idToken string, expires time.Duration, err error) {
290+func (p *GoogleProvider) redeemRefreshToken(ctx context.Context, s *sessions.SessionState) error {
299291 // https://developers.google.com/identity/protocols/OAuth2WebServer#refresh
300292 clientSecret, err := p.GetClientSecret()
301293 if err != nil {
302− return
294+ return err
303295 }
304296
305297 params := url.Values{}
306298 params.Add("client_id", p.ClientID)
307299 params.Add("client_secret", clientSecret)
308− params.Add("refresh_token", refreshToken)
300+ params.Add("refresh_token", s.RefreshToken)
309301 params.Add("grant_type", "refresh_token")
310302
311303 var data struct {
@@ -322,11 +314,14 @@ func (p *GoogleProvider) redeemRefreshToken(ctx context.Context, refreshToken st
322314 Do().
323315 UnmarshalInto(&data)
324316 if err != nil {
325− return "", "", 0, err
317+ return err
326318 }
327319
328− token = data.AccessToken
329− idToken = data.IDToken
330− expires = time.Duration(data.ExpiresIn) * time.Second
331− return
320+ s.AccessToken = data.AccessToken
321+ s.IDToken = data.IDToken
322+
323+ s.CreatedAtNow()
324+ s.ExpiresIn(time.Duration(data.ExpiresIn) * time.Second)
325+
326+ return nil
332327}
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/logingov.go+8−9
@@ -159,7 +159,7 @@ func emailFromUserInfo(ctx context.Context, accessToken string, userInfoEndpoint
159159}
160160
161161// Redeem exchanges the OAuth2 authentication token for an ID token
162−func (p *LoginGovProvider) Redeem(ctx context.Context, redirectURL, code string) (*sessions.SessionState, error) {
162+func (p *LoginGovProvider) Redeem(ctx context.Context, _, code string) (*sessions.SessionState, error) {
163163 if code == "" {
164164 return nil, ErrMissingCode
165165 }
@@ -214,17 +214,16 @@ func (p *LoginGovProvider) Redeem(ctx context.Context, redirectURL, code string)
214214 return nil, err
215215 }
216216
217− created := time.Now()
218− expires := time.Now().Add(time.Duration(jsonResponse.ExpiresIn) * time.Second).Truncate(time.Second)
219−
220− // Store the data that we found in the session state
221− return &sessions.SessionState{
217+ session := &sessions.SessionState{
222218 AccessToken: jsonResponse.AccessToken,
223219 IDToken: jsonResponse.IDToken,
224− CreatedAt: &created,
225− ExpiresOn: &expires,
226220 Email: email,
227− }, nil
221+ }
222+
223+ session.CreatedAtNow()
224+ session.ExpiresIn(time.Duration(jsonResponse.ExpiresIn) * time.Second)
225+
226+ return session, nil
228227}
229228
230229// GetLoginURL overrides GetLoginURL to add login.gov parameters
providers/oidc.go+8−9
@@ -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
@@ -155,7 +154,6 @@ func (p *OIDCProvider) RefreshSessionIfNeeded(ctx context.Context, s *sessions.S
155154 return false, fmt.Errorf("unable to redeem refresh token: %v", err)
156155 }
157156
158− logger.Printf("refreshed session: %s", s)
159157 return true, nil
160158}
161159
@@ -227,7 +225,9 @@ func (p *OIDCProvider) CreateSessionFromToken(ctx context.Context, token string)
227225 ss.AccessToken = token
228226 ss.IDToken = token
229227 ss.RefreshToken = ""
230− ss.ExpiresOn = &idToken.Expiry
228+
229+ ss.CreatedAtNow()
230+ ss.SetExpiresOn(idToken.Expiry)
231231
232232 return ss, nil
233233}
@@ -257,9 +257,8 @@ func (p *OIDCProvider) createSession(ctx context.Context, token *oauth2.Token, r
257257 ss.RefreshToken = token.RefreshToken
258258 ss.IDToken = getIDToken(token)
259259
260− created := time.Now()
261− ss.CreatedAt = &created
262− ss.ExpiresOn = &token.Expiry
260+ ss.CreatedAtNow()
261+ ss.SetExpiresOn(token.Expiry)
263262
264263 return ss, nil
265264}
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−7
@@ -6,7 +6,6 @@ import (
66 "errors"
77 "fmt"
88 "net/url"
9− "time"
109
1110 "github.com/oauth2-proxy/oauth2-proxy/v7/pkg/apis/middleware"
1211 "github.com/oauth2-proxy/oauth2-proxy/v7/pkg/apis/sessions"
@@ -85,9 +84,13 @@ func (p *ProviderData) Redeem(ctx context.Context, redirectURL, code string) (*s
8584 if err != nil {
8685 return nil, err
8786 }
87+ // TODO (@NickMeves): Uses OAuth `expires_in` to set an expiration
8888 if token := values.Get("access_token"); token != "" {
89− created := time.Now()
90− return &sessions.SessionState{AccessToken: token, CreatedAt: &created}, nil
89+ ss := &sessions.SessionState{
90+ AccessToken: token,
91+ }
92+ ss.CreatedAtNow()
93+ return ss, nil
9194 }
9295
9396 return nil, fmt.Errorf("no access token found %s", result.Body())
@@ -126,10 +129,9 @@ func (p *ProviderData) ValidateSession(ctx context.Context, s *sessions.SessionS
126129 return validateToken(ctx, p, s.AccessToken, nil)
127130}
128131
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
132+// RefreshSession refreshes the user's session
133+func (p *ProviderData) RefreshSession(_ context.Context, _ *sessions.SessionState) (bool, error) {
134+ return false, ErrNotImplemented
133135}
134136
135137// CreateSessionFromToken converts Bearer IDTokens into sessions
providers/provider_default_test.go+14−6
@@ -14,12 +14,20 @@ import (
1414func TestRefresh(t *testing.T) {
1515 p := &ProviderData{}
1616
17− expires := time.Now().Add(time.Duration(-11) * time.Minute)
18− refreshed, err := p.RefreshSessionIfNeeded(context.Background(), &sessions.SessionState{
19− ExpiresOn: &expires,
20− })
21− assert.Equal(t, false, refreshed)
22− assert.Equal(t, nil, err)
17+ now := time.Unix(1234567890, 10)
18+ expires := time.Unix(1234567890, 0)
19+
20+ ss := &sessions.SessionState{}
21+ ss.Clock.Set(now)
22+ ss.SetExpiresOn(expires)
23+
24+ refreshed, err := p.RefreshSession(context.Background(), ss)
25+ assert.False(t, refreshed)
26+ assert.Equal(t, ErrNotImplemented, err)
27+
28+ refreshed, err = p.RefreshSession(context.Background(), nil)
29+ assert.False(t, refreshed)
30+ assert.Equal(t, ErrNotImplemented, err)
2331}
2432
2533func TestAcrValuesNotConfigured(t *testing.T) {
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