fix: cache behavior with TTL (#968)

This test will fail since everytime Authenticate() succeeds the token is cached, even if it was already cached. This behavior makes it possible to keep a token in cache if it is authenticated in a period less than the TTL. Signed-off-by: flusflas <[email protected]>
ory · May 23, 2022 · c4836f5 · c4836f5
1 parent 7f370a1
commit c4836f5
Show file tree

Hide file tree

Showing 2 changed files with 36 additions and 2 deletions.
diff --git a/pipeline/authn/authenticator_oauth2_introspection.go b/pipeline/authn/authenticator_oauth2_introspection.go
@@ -183,10 +183,11 @@ func (a *AuthenticatorOAuth2Introspection) Authenticate(r *http.Request, session
 	ss := a.c.ToScopeStrategy(cf.ScopeStrategy, "authenticators.oauth2_introspection.config.scope_strategy")
 
 	i := a.tokenFromCache(cf, token, ss)
+	inCache := i != nil
 
 	// If the token can not be found, and the scope strategy is nil, and the required scope list
 	// is not empty, then we can not use the cache.
-	if i == nil {
+	if !inCache {
 		body := url.Values{"token": {token}}
 		if ss == nil {
 			body.Add("scope", strings.Join(cf.Scopes, " "))
@@ -256,7 +257,9 @@ func (a *AuthenticatorOAuth2Introspection) Authenticate(r *http.Request, session
 		}
 	}
 
-	a.tokenToCache(cf, i, token, ss)
+	if !inCache {
+		a.tokenToCache(cf, i, token, ss)
+	}
 
 	if len(i.Extra) == 0 {
 		i.Extra = map[string]interface{}{}
@@ -352,6 +355,14 @@ func (a *AuthenticatorOAuth2Introspection) Config(config json.RawMessage) (*Auth
 		if err != nil {
 			return nil, nil, err
 		}
+
+		// clear cache if previous ttl was longer (or none)
+		if a.tokenCache != nil {
+			if a.cacheTTL == nil || (a.cacheTTL != nil && a.cacheTTL.Seconds() > cacheTTL.Seconds()) {
+				a.tokenCache.Clear()
+			}
+		}
+
 		a.cacheTTL = &cacheTTL
 	}
 

diff --git a/pipeline/authn/authenticator_oauth2_introspection_test.go b/pipeline/authn/authenticator_oauth2_introspection_test.go
@@ -768,6 +768,29 @@ func TestAuthenticatorOAuth2Introspection(t *testing.T) {
 				require.Error(t, a.Authenticate(r, new(AuthenticationSession), config, nil))
 				didNotUseCache.Wait()
 			})
+
+			t.Run("case=cache cleared after ttl", func(t *testing.T) {
+				//time.Sleep(time.Second)
+				config := setup(t, `{ "required_scope": ["scope-a"], "trusted_issuers": ["foo", "bar"], "target_audience": ["audience"], "cache": { "ttl": "100ms" } }`)
+
+				didNotUseCache.Add(1)
+				require.NoError(t, a.Authenticate(r, expected, config, nil))
+				didNotUseCache.Wait()
+
+				// wait cache to save value
+				time.Sleep(time.Millisecond * 10)
+
+				require.NoError(t, a.Authenticate(r, new(AuthenticationSession), config, nil))
+				time.Sleep(50 * time.Millisecond)
+
+				require.NoError(t, a.Authenticate(r, new(AuthenticationSession), config, nil))
+				time.Sleep(50 * time.Millisecond)
+
+				// cache should have been cleared
+				didNotUseCache.Add(1)
+				require.NoError(t, a.Authenticate(r, new(AuthenticationSession), config, nil))
+				didNotUseCache.Wait()
+			})
 		})
 	})