Fix cache behavior with TTL #968
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
aeneasr
force-pushed
the
feature/fix-cache-ttl
branch
from
fb3539c to
fee70df
Compare
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]>
When configuring an AuthenticatorOAuth2Introspection, the token cache is cleared if a cache TTL is set. Signed-off-by: flusflas <[email protected]>
This will prevent keeping a cached token alive if it is authenticated with a period less than tha cache TTL. Signed-off-by: flusflas <[email protected]>
aeneasr
force-pushed
the
feature/fix-cache-ttl
branch
from
fee70df to
95f0cb2
Compare
Codecov Report
@@ Coverage Diff @@
## master #968 +/- ##
==========================================
+ Coverage 65.98% 66.01% +0.03%
==========================================
Files 106 106
Lines 4718 4723 +5
==========================================
+ Hits 3113 3118 +5
Misses 1324 1324
Partials 281 281
Continue to review full report at Codecov.
|
aeneasr
approved these changes
May 23, 2022
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This pull requests fixes a possible issue in the behavior of the oauth2 introspection authenticator cache with TTL.
The problem is any token successfully authenticated is pushed to the cache, even if it the token was already cached. When using cache TTL, this makes possible to keep a token in the cache by forcing the token authentication with a period less than the TTL.
This fix doesn't introduce breaking changes.
Checklist
introduces a new feature.
contributing code guidelines.
vulnerability. If this pull request addresses a security. vulnerability, I
confirm that I got green light (please contact
[email protected]) from the maintainers to push
the changes.
works.
Further Comments
Although this PR fixes the problem above, maybe I'm missing details of the cache implementation or whether stopping re-caching already cached tokens is a desired behavior (e.g. my fix would probably break a LRU cache), so please check that this does not adversely affect the behavior of the cache.