git.iko.soy/typeable/oauth2-proxy
on master help sign in
Merge pull request #1468 from oauth2-proxy/session-refresh-using-lock
44dc3caJoel Speed
CHANGELOG.md+1
compare with master
unifiedsplit
@@ -8,6 +8,7 @@
8
8
9## Changes since v7.2.1
9## Changes since v7.2.1
10
10
11+- [#1468](https://github.com/oauth2-proxy/oauth2-proxy/pull/1468) Implement session locking with session state lock (@JoelSpeed, @Bibob7)
11- [#1489](https://github.com/oauth2-proxy/oauth2-proxy/pull/1489) Fix Docker Buildx push to include build version (@JoelSpeed)
12- [#1489](https://github.com/oauth2-proxy/oauth2-proxy/pull/1489) Fix Docker Buildx push to include build version (@JoelSpeed)
12- [#1477](https://github.com/oauth2-proxy/oauth2-proxy/pull/1477) Remove provider documentation for `Microsoft Azure AD` (@omBratteng)
13- [#1477](https://github.com/oauth2-proxy/oauth2-proxy/pull/1477) Remove provider documentation for `Microsoft Azure AD` (@omBratteng)
13- [#1204](https://github.com/oauth2-proxy/oauth2-proxy/pull/1204) Added configuration for audience claim (`--oidc-extra-audience`) and ability to specify extra audiences (`--oidc-extra-audience`) allowed passing audience verification. This enables support for AWS Cognito and other issuers that have custom audience claims. Also, this adds the ability to allow multiple audiences. (@kschu91)
14- [#1204](https://github.com/oauth2-proxy/oauth2-proxy/pull/1204) Added configuration for audience claim (`--oidc-extra-audience`) and ability to specify extra audiences (`--oidc-extra-audience`) allowed passing audience verification. This enables support for AWS Cognito and other issuers that have custom audience claims. Also, this adds the ability to allow multiple audiences. (@kschu91)
pkg/middleware/stored_session.go+82−8
@@ -14,6 +14,23 @@ import (
14 "github.com/oauth2-proxy/oauth2-proxy/v7/providers"
14 "github.com/oauth2-proxy/oauth2-proxy/v7/providers"
15)
15)
16
16
17+const (
18+ // When attempting to obtain the lock, if it's not done before this timeout
19+ // then exit and fail the refresh attempt.
20+ // TODO: This should probably be configurable by the end user.
21+ sessionRefreshObtainTimeout = 5 * time.Second
22+
23+ // Maximum time allowed for a session refresh attempt.
24+ // If the refresh request isn't finished within this time, the lock will be
25+ // released.
26+ // TODO: This should probably be configurable by the end user.
27+ sessionRefreshLockDuration = 2 * time.Second
28+
29+ // How long to wait after failing to obtain the lock before trying again.
30+ // TODO: This should probably be configurable by the end user.
31+ sessionRefreshRetryPeriod = 10 * time.Millisecond
32+)
33+
17// StoredSessionLoaderOptions contains all of the requirements to construct
34// StoredSessionLoaderOptions contains all of the requirements to construct
18// a stored session loader.
35// a stored session loader.
19// All options must be provided.
36// All options must be provided.
@@ -91,13 +108,10 @@ func (s *storedSessionLoader) loadSession(next http.Handler) http.Handler {
91// that is is valid.
108// that is is valid.
92func (s *storedSessionLoader) getValidatedSession(rw http.ResponseWriter, req *http.Request) (*sessionsapi.SessionState, error) {
109func (s *storedSessionLoader) getValidatedSession(rw http.ResponseWriter, req *http.Request) (*sessionsapi.SessionState, error) {
93 session, err := s.store.Load(req)
110 session, err := s.store.Load(req)
94− if err != nil {
111+ if err != nil || session == nil {
112+ // No session was found in the storage or error occurred, nothing more to do
95 return nil, err
113 return nil, err
96 }
114 }
97− if session == nil {
98− // No session was found in the storage, nothing more to do
99− return nil, nil
100− }
101
115
102 err = s.refreshSessionIfNeeded(rw, req, session)
116 err = s.refreshSessionIfNeeded(rw, req, session)
103 if err != nil {
117 if err != nil {
@@ -111,14 +125,69 @@ func (s *storedSessionLoader) getValidatedSession(rw http.ResponseWriter, req *h
111// is older than the refresh period.
125// is older than the refresh period.
112// Success or fail, we will then validate the session.
126// Success or fail, we will then validate the session.
113func (s *storedSessionLoader) refreshSessionIfNeeded(rw http.ResponseWriter, req *http.Request, session *sessionsapi.SessionState) error {
127func (s *storedSessionLoader) refreshSessionIfNeeded(rw http.ResponseWriter, req *http.Request, session *sessionsapi.SessionState) error {
114− if s.refreshPeriod <= time.Duration(0) || session.Age() < s.refreshPeriod {
128+ if !needsRefresh(s.refreshPeriod, session) {
115 // Refresh is disabled or the session is not old enough, do nothing
129 // Refresh is disabled or the session is not old enough, do nothing
116 return nil
130 return nil
117 }
131 }
118
132
119− logger.Printf("Refreshing session - User: %s; SessionAge: %s", session.User, session.Age())
133+ var lockObtained bool
120− err := s.refreshSession(rw, req, session)
134+ ctx, cancel := context.WithTimeout(context.Background(), sessionRefreshObtainTimeout)
135+ defer cancel()
136+
137+ for !lockObtained {
138+ select {
139+ case <-ctx.Done():
140+ return errors.New("timeout obtaining session lock")
141+ default:
142+ err := session.ObtainLock(req.Context(), sessionRefreshLockDuration)
143+ if err != nil && !errors.Is(err, sessionsapi.ErrLockNotObtained) {
144+ return fmt.Errorf("error occurred while trying to obtain lock: %v", err)
145+ } else if errors.Is(err, sessionsapi.ErrLockNotObtained) {
146+ time.Sleep(sessionRefreshRetryPeriod)
147+ continue
148+ }
149+ // No error means we obtained the lock
150+ lockObtained = true
151+ }
152+ }
153+
154+ // The rest of this function is carried out under lock, but we must release it
155+ // wherever we exit from this function.
156+ defer func() {
157+ if session == nil {
158+ return
159+ }
160+ if err := session.ReleaseLock(req.Context()); err != nil {
161+ logger.Errorf("unable to release lock: %v", err)
162+ }
163+ }()
164+
165+ // Reload the session in case it was changed underneath us.
166+ freshSession, err := s.store.Load(req)
121 if err != nil {
167 if err != nil {
168+ return fmt.Errorf("could not load session: %v", err)
169+ }
170+ if freshSession == nil {
171+ return errors.New("session no longer exists, it may have been removed by another request")
172+ }
173+ // Restore the state of the fresh session into the original pointer.
174+ // This is important so that changes are passed up the to the parent scope.
175+ lock := session.Lock
176+ *session = *freshSession
177+
178+ // Ensure we maintain the session lock after we have refreshed the session.
179+ // Loading from the session store creates a new lock in the session.
180+ session.Lock = lock
181+
182+ if !needsRefresh(s.refreshPeriod, session) {
183+ // The session must have already been refreshed while we were waiting to
184+ // obtain the lock.
185+ return nil
186+ }
187+
188+ // We are holding the lock and the session needs a refresh
189+ logger.Printf("Refreshing session - User: %s; SessionAge: %s", session.User, session.Age())
190+ if err := s.refreshSession(rw, req, session); err != nil {
122 // If a preemptive refresh fails, we still keep the session
191 // If a preemptive refresh fails, we still keep the session
123 // if validateSession succeeds.
192 // if validateSession succeeds.
124 logger.Errorf("Unable to refresh session: %v", err)
193 logger.Errorf("Unable to refresh session: %v", err)
@@ -128,6 +197,11 @@ func (s *storedSessionLoader) refreshSessionIfNeeded(rw http.ResponseWriter, req
128 return s.validateSession(req.Context(), session)
197 return s.validateSession(req.Context(), session)
129}
198}
130
199
200+// needsRefresh determines whether we should attempt to refresh a session or not.
201+func needsRefresh(refreshPeriod time.Duration, session *sessionsapi.SessionState) bool {
202+ return refreshPeriod > time.Duration(0) && session.Age() > refreshPeriod
203+}
204+
131// refreshSession attempts to refresh the session with the provider
205// refreshSession attempts to refresh the session with the provider
132// and will save the session if it was updated.
206// and will save the session if it was updated.
133func (s *storedSessionLoader) refreshSession(rw http.ResponseWriter, req *http.Request, session *sessionsapi.SessionState) error {
207func (s *storedSessionLoader) refreshSession(rw http.ResponseWriter, req *http.Request, session *sessionsapi.SessionState) error {
pkg/middleware/stored_session_test.go+267−38
@@ -6,6 +6,7 @@ import (
6 "fmt"
6 "fmt"
7 "net/http"
7 "net/http"
8 "net/http/httptest"
8 "net/http/httptest"
9+ "sync"
9 "time"
10 "time"
10
11
11 middlewareapi "github.com/oauth2-proxy/oauth2-proxy/v7/pkg/apis/middleware"
12 middlewareapi "github.com/oauth2-proxy/oauth2-proxy/v7/pkg/apis/middleware"
@@ -17,9 +18,74 @@ import (
17 . "github.com/onsi/gomega"
18 . "github.com/onsi/gomega"
18)
19)
19
20
21+type testLock struct {
22+ locked bool
23+ obtainOnAttempt int
24+ obtainAttempts int
25+ obtainError error
26+}
27+
28+func (l *testLock) Obtain(_ context.Context, _ time.Duration) error {
29+ l.obtainAttempts++
30+ if l.obtainAttempts < l.obtainOnAttempt {
31+ return sessionsapi.ErrLockNotObtained
32+ }
33+ if l.obtainError != nil {
34+ return l.obtainError
35+ }
36+ l.locked = true
37+ return nil
38+}
39+
40+func (l *testLock) Peek(_ context.Context) (bool, error) {
41+ return l.locked, nil
42+}
43+
44+func (l *testLock) Refresh(_ context.Context, _ time.Duration) error {
45+ return nil
46+}
47+
48+func (l *testLock) Release(_ context.Context) error {
49+ l.locked = false
50+ return nil
51+}
52+
53+type testLockConcurrent struct {
54+ mu sync.RWMutex
55+ locked bool
56+}
57+
58+func (l *testLockConcurrent) Obtain(_ context.Context, _ time.Duration) error {
59+ l.mu.Lock()
60+ defer l.mu.Unlock()
61+ if l.locked {
62+ return sessionsapi.ErrLockNotObtained
63+ }
64+ l.locked = true
65+ return nil
66+}
67+
68+func (l *testLockConcurrent) Peek(_ context.Context) (bool, error) {
69+ l.mu.RLock()
70+ defer l.mu.RUnlock()
71+ return l.locked, nil
72+}
73+
74+func (l *testLockConcurrent) Refresh(_ context.Context, _ time.Duration) error {
75+ return nil
76+}
77+
78+func (l *testLockConcurrent) Release(_ context.Context) error {
79+ l.mu.Lock()
80+ defer l.mu.Unlock()
81+ l.locked = false
82+ return nil
83+}
84+
20var _ = Describe("Stored Session Suite", func() {
85var _ = Describe("Stored Session Suite", func() {
21 const (
86 const (
22 refresh = "Refresh"
87 refresh = "Refresh"
88+ refreshed = "Refreshed"
23 noRefresh = "NoRefresh"
89 noRefresh = "NoRefresh"
24 notImplemented = "NotImplemented"
90 notImplemented = "NotImplemented"
25 )
91 )
@@ -34,7 +100,7 @@ var _ = Describe("Stored Session Suite", func() {
34 var defaultRefreshFunc = func(_ context.Context, ss *sessionsapi.SessionState) (bool, error) {
100 var defaultRefreshFunc = func(_ context.Context, ss *sessionsapi.SessionState) (bool, error) {
35 switch ss.RefreshToken {
101 switch ss.RefreshToken {
36 case refresh:
102 case refresh:
37− ss.RefreshToken = "Refreshed"
103+ ss.RefreshToken = refreshed
38 return true, nil
104 return true, nil
39 case noRefresh:
105 case noRefresh:
40 return false, nil
106 return false, nil
@@ -181,6 +247,7 @@ var _ = Describe("Stored Session Suite", func() {
181 RefreshToken: noRefresh,
247 RefreshToken: noRefresh,
182 CreatedAt: &createdPast,
248 CreatedAt: &createdPast,
183 ExpiresOn: &createdFuture,
249 ExpiresOn: &createdFuture,
250+ Lock: &sessionsapi.NoOpLock{},
184 },
251 },
185 store: defaultSessionStore,
252 store: defaultSessionStore,
186 refreshPeriod: 1 * time.Minute,
253 refreshPeriod: 1 * time.Minute,
@@ -222,6 +289,7 @@ var _ = Describe("Stored Session Suite", func() {
222 RefreshToken: "Refreshed",
289 RefreshToken: "Refreshed",
223 CreatedAt: &now,
290 CreatedAt: &now,
224 ExpiresOn: &createdFuture,
291 ExpiresOn: &createdFuture,
292+ Lock: &sessionsapi.NoOpLock{},
225 },
293 },
226 store: defaultSessionStore,
294 store: defaultSessionStore,
227 refreshPeriod: 1 * time.Minute,
295 refreshPeriod: 1 * time.Minute,
@@ -237,6 +305,7 @@ var _ = Describe("Stored Session Suite", func() {
237 RefreshToken: "RefreshError",
305 RefreshToken: "RefreshError",
238 CreatedAt: &createdPast,
306 CreatedAt: &createdPast,
239 ExpiresOn: &createdFuture,
307 ExpiresOn: &createdFuture,
308+ Lock: &sessionsapi.NoOpLock{},
240 },
309 },
241 store: defaultSessionStore,
310 store: defaultSessionStore,
242 refreshPeriod: 1 * time.Minute,
311 refreshPeriod: 1 * time.Minute,
@@ -266,15 +335,116 @@ var _ = Describe("Stored Session Suite", func() {
266 validateSession: defaultValidateFunc,
335 validateSession: defaultValidateFunc,
267 }),
336 }),
268 )
337 )
338+
339+ type storedSessionLoaderConcurrentTableInput struct {
340+ existingSession *sessionsapi.SessionState
341+ refreshPeriod time.Duration
342+ numConcReqs int
343+ }
344+
345+ DescribeTable("when serving concurrent requests",
346+ func(in storedSessionLoaderConcurrentTableInput) {
347+ lockConc := &testLockConcurrent{}
348+
349+ lock := &sync.RWMutex{}
350+ existingSession := *in.existingSession // deep copy existingSession state
351+ existingSession.Lock = lockConc
352+ store := &fakeSessionStore{
353+ LoadFunc: func(req *http.Request) (*sessionsapi.SessionState, error) {
354+ lock.RLock()
355+ defer lock.RUnlock()
356+ session := existingSession
357+ return &session, nil
358+ },
359+ SaveFunc: func(_ http.ResponseWriter, _ *http.Request, session *sessionsapi.SessionState) error {
360+ lock.Lock()
361+ defer lock.Unlock()
362+ existingSession = *session
363+ return nil
364+ },
365+ }
366+
367+ refreshedChan := make(chan bool, in.numConcReqs)
368+ for i := 0; i < in.numConcReqs; i++ {
369+ go func(refreshedChan chan bool, lockConc sessionsapi.Lock) {
370+ scope := &middlewareapi.RequestScope{
371+ Session: nil,
372+ }
373+
374+ // Set up the request with the request header and a request scope
375+ req := httptest.NewRequest("", "/", nil)
376+ req = middlewareapi.AddRequestScope(req, scope)
377+
378+ rw := httptest.NewRecorder()
379+
380+ sessionRefreshed := false
381+ opts := &StoredSessionLoaderOptions{
382+ SessionStore: store,
383+ RefreshPeriod: in.refreshPeriod,
384+ RefreshSession: func(ctx context.Context, s *sessionsapi.SessionState) (bool, error) {
385+ time.Sleep(10 * time.Millisecond)
386+ sessionRefreshed = true
387+ return true, nil
388+ },
389+ ValidateSession: func(context.Context, *sessionsapi.SessionState) bool {
390+ return true
391+ },
392+ }
393+
394+ handler := NewStoredSessionLoader(opts)(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {}))
395+ handler.ServeHTTP(rw, req)
396+
397+ refreshedChan <- sessionRefreshed
398+ }(refreshedChan, lockConc)
399+ }
400+ var refreshedSlice []bool
401+ for i := 0; i < in.numConcReqs; i++ {
402+ refreshedSlice = append(refreshedSlice, <-refreshedChan)
403+ }
404+ sessionRefreshedCount := 0
405+ for _, sessionRefreshed := range refreshedSlice {
406+ if sessionRefreshed {
407+ sessionRefreshedCount++
408+ }
409+ }
410+ Expect(sessionRefreshedCount).To(Equal(1))
411+ },
412+ Entry("with two concurrent requests", storedSessionLoaderConcurrentTableInput{
413+ existingSession: &sessionsapi.SessionState{
414+ RefreshToken: refresh,
415+ CreatedAt: &createdPast,
416+ },
417+ numConcReqs: 2,
418+ refreshPeriod: 1 * time.Minute,
419+ }),
420+ Entry("with 5 concurrent requests", storedSessionLoaderConcurrentTableInput{
421+ existingSession: &sessionsapi.SessionState{
422+ RefreshToken: refresh,
423+ CreatedAt: &createdPast,
424+ },
425+ numConcReqs: 5,
426+ refreshPeriod: 1 * time.Minute,
427+ }),
428+ Entry("with one request", storedSessionLoaderConcurrentTableInput{
429+ existingSession: &sessionsapi.SessionState{
430+ RefreshToken: refresh,
431+ CreatedAt: &createdPast,
432+ },
433+ numConcReqs: 1,
434+ refreshPeriod: 1 * time.Minute,
435+ }),
436+ )
269 })
437 })
270
438
271 Context("refreshSessionIfNeeded", func() {
439 Context("refreshSessionIfNeeded", func() {
272 type refreshSessionIfNeededTableInput struct {
440 type refreshSessionIfNeededTableInput struct {
273− refreshPeriod time.Duration
441+ refreshPeriod time.Duration
274− session *sessionsapi.SessionState
442+ session *sessionsapi.SessionState
443+ concurrentSessionRefresh bool
275− expectedErr error
444+ expectedErr error
276− expectRefreshed bool
445+ expectRefreshed bool
277− expectValidated bool
446+ expectValidated bool
447+ expectedLockObtained bool
278 }
448 }
279
449
280 createdPast := time.Now().Add(-5 * time.Minute)
450 createdPast := time.Now().Add(-5 * time.Minute)
@@ -285,9 +455,28 @@ var _ = Describe("Stored Session Suite", func() {
285 refreshed := false
455 refreshed := false
286 validated := false
456 validated := false
287
457
458+ session := &sessionsapi.SessionState{}
459+ *session = *in.session
460+ if in.concurrentSessionRefresh {
461+ // Update the session that Load returns.
462+ // This simulates a concurrent refresh in the background.
463+ session.CreatedAt = &createdFuture
464+ }
465+ store := &fakeSessionStore{
466+ LoadFunc: func(req *http.Request) (*sessionsapi.SessionState, error) {
467+ // Loading the session from the provider creates a new lock
468+ session.Lock = &testLock{}
469+ return session, nil
470+ },
471+ SaveFunc: func(_ http.ResponseWriter, _ *http.Request, s *sessionsapi.SessionState) error {
472+ *session = *s
473+ return nil
474+ },
475+ }
476+
288 s := &storedSessionLoader{
477 s := &storedSessionLoader{
289 refreshPeriod: in.refreshPeriod,
478 refreshPeriod: in.refreshPeriod,
290− store: &fakeSessionStore{},
479+ store: store,
291 sessionRefresher: func(_ context.Context, ss *sessionsapi.SessionState) (bool, error) {
480 sessionRefresher: func(_ context.Context, ss *sessionsapi.SessionState) (bool, error) {
292 refreshed = true
481 refreshed = true
293 switch ss.RefreshToken {
482 switch ss.RefreshToken {
@@ -316,89 +505,129 @@ var _ = Describe("Stored Session Suite", func() {
316 }
505 }
317 Expect(refreshed).To(Equal(in.expectRefreshed))
506 Expect(refreshed).To(Equal(in.expectRefreshed))
318 Expect(validated).To(Equal(in.expectValidated))
507 Expect(validated).To(Equal(in.expectValidated))
508+ testLock, ok := in.session.Lock.(*testLock)
509+ Expect(ok).To(Equal(true))
510+
511+ if in.expectedLockObtained {
512+ Expect(testLock.obtainAttempts).Should(BeNumerically(">", 0), "Expected at least one attempt at obtaining the session lock")
513+ }
514+ Expect(testLock.locked).To(BeFalse(), "Expected lock should always be released")
319 },
515 },
320 Entry("when the refresh period is 0, and the session does not need refreshing", refreshSessionIfNeededTableInput{
516 Entry("when the refresh period is 0, and the session does not need refreshing", refreshSessionIfNeededTableInput{
321 refreshPeriod: time.Duration(0),
517 refreshPeriod: time.Duration(0),
322 session: &sessionsapi.SessionState{
518 session: &sessionsapi.SessionState{
323 RefreshToken: refresh,
519 RefreshToken: refresh,
324 CreatedAt: &createdFuture,
520 CreatedAt: &createdFuture,
521+ Lock: &testLock{},
325 },
522 },
326− expectedErr: nil,
523+ expectedErr: nil,
327− expectRefreshed: false,
524+ expectRefreshed: false,
328− expectValidated: false,
525+ expectValidated: false,
526+ expectedLockObtained: false,
329 }),
527 }),
330 Entry("when the refresh period is 0, and the session needs refreshing", refreshSessionIfNeededTableInput{
528 Entry("when the refresh period is 0, and the session needs refreshing", refreshSessionIfNeededTableInput{
331 refreshPeriod: time.Duration(0),
529 refreshPeriod: time.Duration(0),
332 session: &sessionsapi.SessionState{
530 session: &sessionsapi.SessionState{
333 RefreshToken: refresh,
531 RefreshToken: refresh,
334 CreatedAt: &createdPast,
532 CreatedAt: &createdPast,
533+ Lock: &testLock{},
335 },
534 },
336− expectedErr: nil,
535+ expectedErr: nil,
337− expectRefreshed: false,
536+ expectRefreshed: false,
338− expectValidated: false,
537+ expectValidated: false,
538+ expectedLockObtained: false,
339 }),
539 }),
340 Entry("when the session does not need refreshing", refreshSessionIfNeededTableInput{
540 Entry("when the session does not need refreshing", refreshSessionIfNeededTableInput{
341 refreshPeriod: 1 * time.Minute,
541 refreshPeriod: 1 * time.Minute,
342 session: &sessionsapi.SessionState{
542 session: &sessionsapi.SessionState{
343 RefreshToken: refresh,
543 RefreshToken: refresh,
344 CreatedAt: &createdFuture,
544 CreatedAt: &createdFuture,
545+ Lock: &testLock{},
345 },
546 },
346− expectedErr: nil,
547+ expectedErr: nil,
347− expectRefreshed: false,
548+ expectRefreshed: false,
348− expectValidated: false,
549+ expectValidated: false,
550+ expectedLockObtained: false,
349 }),
551 }),
350 Entry("when the session is refreshed by the provider", refreshSessionIfNeededTableInput{
552 Entry("when the session is refreshed by the provider", refreshSessionIfNeededTableInput{
351 refreshPeriod: 1 * time.Minute,
553 refreshPeriod: 1 * time.Minute,
352 session: &sessionsapi.SessionState{
554 session: &sessionsapi.SessionState{
353 RefreshToken: refresh,
555 RefreshToken: refresh,
354 CreatedAt: &createdPast,
556 CreatedAt: &createdPast,
557+ Lock: &testLock{},
355 },
558 },
356− expectedErr: nil,
559+ expectedErr: nil,
357− expectRefreshed: true,
560+ expectRefreshed: true,
358− expectValidated: true,
561+ expectValidated: true,
562+ expectedLockObtained: true,
359 }),
563 }),
360− Entry("when the session is not refreshed by the provider", refreshSessionIfNeededTableInput{
564+ Entry("when obtaining lock failed, but concurrent request refreshed", refreshSessionIfNeededTableInput{
361 refreshPeriod: 1 * time.Minute,
565 refreshPeriod: 1 * time.Minute,
362 session: &sessionsapi.SessionState{
566 session: &sessionsapi.SessionState{
363 RefreshToken: noRefresh,
567 RefreshToken: noRefresh,
364 CreatedAt: &createdPast,
568 CreatedAt: &createdPast,
365− ExpiresOn: &createdFuture,
569+ Lock: &testLock{
570+ obtainOnAttempt: 4,
571+ },
366 },
572 },
573+ concurrentSessionRefresh: true,
367− expectedErr: nil,
574+ expectedErr: nil,
368− expectRefreshed: true,
575+ expectRefreshed: false,
369− expectValidated: true,
576+ expectValidated: false,
577+ expectedLockObtained: true,
370 }),
578 }),
371− Entry("when the provider doesn't implement refresh but validation succeeds", refreshSessionIfNeededTableInput{
579+ Entry("when obtaining lock failed with a valid session", refreshSessionIfNeededTableInput{
372 refreshPeriod: 1 * time.Minute,
580 refreshPeriod: 1 * time.Minute,
373 session: &sessionsapi.SessionState{
581 session: &sessionsapi.SessionState{
374− RefreshToken: notImplemented,
582+ RefreshToken: noRefresh,
583+ CreatedAt: &createdPast,
584+ Lock: &testLock{
585+ obtainError: sessionsapi.ErrLockNotObtained,
586+ },
587+ },
588+ expectedErr: errors.New("timeout obtaining session lock"),
589+ expectRefreshed: false,
590+ expectValidated: false,
591+ expectedLockObtained: true,
592+ }),
593+ Entry("when the session is not refreshed by the provider", refreshSessionIfNeededTableInput{
594+ refreshPeriod: 1 * time.Minute,
595+ session: &sessionsapi.SessionState{
596+ RefreshToken: noRefresh,
375 CreatedAt: &createdPast,
597 CreatedAt: &createdPast,
598+ ExpiresOn: &createdFuture,
599+ Lock: &testLock{},
376 },
600 },
377− expectedErr: nil,
601+ expectedErr: nil,
378− expectRefreshed: true,
602+ expectRefreshed: true,
379− expectValidated: true,
603+ expectValidated: true,
604+ expectedLockObtained: true,
380 }),
605 }),
381− Entry("when the provider refresh fails but validation succeeds", refreshSessionIfNeededTableInput{
606+ Entry("when the provider doesn't implement refresh", refreshSessionIfNeededTableInput{
382 refreshPeriod: 1 * time.Minute,
607 refreshPeriod: 1 * time.Minute,
383 session: &sessionsapi.SessionState{
608 session: &sessionsapi.SessionState{
384− RefreshToken: "RefreshError",
609+ RefreshToken: notImplemented,
385 CreatedAt: &createdPast,
610 CreatedAt: &createdPast,
611+ Lock: &testLock{},
386 },
612 },
387− expectedErr: nil,
613+ expectedErr: nil,
388− expectRefreshed: true,
614+ expectRefreshed: true,
389− expectValidated: true,
615+ expectValidated: true,
616+ expectedLockObtained: true,
390 }),
617 }),
391− Entry("when the session is not refreshed by the provider and validation fails", refreshSessionIfNeededTableInput{
618+ Entry("when the session is not refreshed by the provider", refreshSessionIfNeededTableInput{
392 refreshPeriod: 1 * time.Minute,
619 refreshPeriod: 1 * time.Minute,
393 session: &sessionsapi.SessionState{
620 session: &sessionsapi.SessionState{
394 AccessToken: "Invalid",
621 AccessToken: "Invalid",
395 RefreshToken: noRefresh,
622 RefreshToken: noRefresh,
396 CreatedAt: &createdPast,
623 CreatedAt: &createdPast,
397 ExpiresOn: &createdFuture,
624 ExpiresOn: &createdFuture,
625+ Lock: &testLock{},
398 },
626 },
399− expectedErr: errors.New("session is invalid"),
627+ expectedErr: errors.New("session is invalid"),
400− expectRefreshed: true,
628+ expectRefreshed: true,
401− expectValidated: true,
629+ expectValidated: true,
630+ expectedLockObtained: true,
402 }),
631 }),
403 )
632 )
404 })
633 })