From c4836f5341b63978db49f8c6fe8d6ba2ca2bf2bc Mon Sep 17 00:00:00 2001
From: flusflas <aflusflas@gmail.com>
Date: Mon, 23 May 2022 22:30:15 +0200
Subject: [PATCH] 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 <aflusflas@gmail.com>
---
 .../authenticator_oauth2_introspection.go     | 15 ++++++++++--
 ...authenticator_oauth2_introspection_test.go | 23 +++++++++++++++++++
 2 files changed, 36 insertions(+), 2 deletions(-)

diff --git a/pipeline/authn/authenticator_oauth2_introspection.go b/pipeline/authn/authenticator_oauth2_introspection.go
index 5b9f7fa759..072b7b8389 100644
--- 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
index 01ed11668c..d4366b05a3 100644
--- 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()
+			})
 		})
 	})