ReactiveRemoteJWKSource caches invalid response status into jwkSetURL

[dongelci]

Describe the bug
ReactiveRemoteJWKSource receives a Mono for jwkSetURL, which will be lazy loaded. After invoking webClient in

private Mono<JWKSet> getJWKSet()

response will be parsed and cached separately in cachedJWKSet in success case and then cached again in line 111, which causes the trouble. If IDM is not reachable at that point of time, cache() call on jwkSetURL Mono caches an error state, which is never invalidated.

To Reproduce
It should be debugged inside ReactiveRemoteJWKSource.

1. Start a webflux app with resource server and spring security.
2. Set spring.security.oauth2.resourceserver.jwt.issuer-uri to any url, e.g. http://localhost:1, which is not working
3. Send a request with any jwt token
4. You will see exception logging in console like Connection refused: no further information
5. Set a breakpoint at line 103 in ReactiveRemoteJWKSource
6. Send the request again
7. An IllegalArgumentException with cause WebClientRequestException is cached inside jwkSetURL

Expected behavior
If jwkSetUrl should not cache error cases. There is no reason to cache a parsed object into jwkSetURL, since it is only for a url or url string depending on the name, not an object.

[sjohnr]

Thanks @dongelci, I see the issue and your explanation is a good summary! I haven't dug into it enough to know what the best solution is though. Are you interested in getting the process of a fix started by writing a failing test that demonstrates the issue?

[dongelci]

Hi @sjohnr , in my opinion best solution would be removing cache()-call on jwkSetURL Mono since it should deliver url to jwks, not cache any other object. There is another class instance for caching, cachedJWKSet

Yes I can provide test code within one week due to lack of time

[sjohnr]

Yes I can provide test code within one week due to lack of time

Thanks! And no problem, take your time. Feel free to create a fork and branch of spring-security to put the test in. No need to submit a PR right away (we can do that later).

[dongelci]

Hi again @sjohnr,

I finally found time to move on further on the issue.

I realized, I missed a point during analyzing. The well-known endpoint will be called lazily and uri in response will be passed to ReactiveRemoteJWKSource without validating if the response was successful. This is the cause of error case in ReactiveRemoteJWKSource. Cache invocation is not guilty. The fix should be implemented in NimbusReactiveJwtDecoder.

Imho, if jwks are loaded lazily, only the successful response should be passed as parameter to ReactiveRemoteJWKSource, not the error. I will text here again when I am done with the fix, which may take 2 weeks due to lack of time

[dongelci]

Hi @sjohnr,
Web fetches the jwks eagerly and fails startup if jwks are not fetched, but webflux fetches them lazily. Is there a reason why webflux has a different behaviour than web?

[sjohnr]

Is there a reason why webflux has a different behaviour than web?

I haven't looked closely at the code you're referring to just now, but generally it is not intentional. The two implementations are often developed separately at different times, and even occasionally evolve separately. That can cause discrepancies between them that aren't necessarily intended.

[dongelci]

@sjohnr thanks for your support with answers. I provided the pull request, which fixes the problem