fix: JWKS Request should not be duplicated headers added by filters #1924
Conversation
sdelamo
changed the title
| .cacheInvalidateIf(JwksCacheEntry::isExpired); | ||
| .defaultIfEmpty(new JWKSet()) | ||
| .map(jwksSet -> instantiateCacheEntry(cacheKey, jwksSet)) | ||
| .doOnNext(entry -> { |
There was a problem hiding this comment.
i dont think this works, it will only be called once. you should put it into the cacheInvalidateIf parameter instead
There was a problem hiding this comment.
If remove the doOnNext and I do
.cacheInvalidateIf(entry -> {
if (entry.isExpired()) {
cache.remove(cacheKey);
}
return entry.isExpired();
});The test fails 7 headers instead of the expected five.
There was a problem hiding this comment.
The patch doesn't work either, though. As soon as the first expires, you're going to have multiple JWKSets for the same key, defeating the point of the caching.
This class is deprecated, does it need to be fixed? A proper cache implementation with eviction here would be more complicated than this PR
There was a problem hiding this comment.
there. i found a simple solution after all
| .cacheInvalidateIf(JwksCacheEntry::isExpired); | ||
| .defaultIfEmpty(new JWKSet()) | ||
| .map(jwksSet -> instantiateCacheEntry(cacheKey, jwksSet)) | ||
| .doOnNext(entry -> { |
There was a problem hiding this comment.
there. i found a simple solution after all
|
Thanks.
Yes. it needs to be fixed.
@yawkat a proper implementation already exists based on Micronaut Cache. |
Close: #1881
We have two implementations of
JwkSetFetcher:CacheableJwkSetFetcherandReactorJwkSetFetcher.JwkSetFetcherdefines a bean responsible for sending an HTTP request to a remote authorization server to fetch the public keys, the JSON Web Key Set (JWKS).The former is not affected, but the latter, which uses Project Reactor
Mono::cacheInvalidateIfto cache requests, had a bug that manifested if a client filter added extra HTTP headers to a request sent to the authorization server. For example, if you were using Micronaut Open Telemetry integration. The HTTP Client filter in Micronaut Open Telemetry adds HTTP Headers. Before this fix, the header value would keep getting added.