Add discoverJwsAlgorithms() in NimbusJwtDecoder

[therepanic]

Closes: gh-17785

[jzheaux]

@therepanic are you able to add a unit test that shows this feature working? I imagine there is a similar test for withIssuerLocation that you could borrow.

[therepanic]

Hi, @jzheaux! The problem turned out to be a little deeper than I expected. Because I didn't take a few things into account, it turns out that when the entire project is compiled, the test runs indefinitely. Let's go through everything in order.

First, in my original change, the test withJwkSetUriWhenNullOrEmptyThenThrowsException fails. This is because the initial implementation uses the JwkSetUriJwtDecoderBuilder constructor with one argument and checks that jwkSetUri != null. The constructor with the function only checks that the function itself is not null. Because of this, we can conclude that we can check for null when creating the function.

The problem with jwsKeySelectorWhenNoAlgorithmThenReturnsRS256Selector is that our current behavior includes only RS256 available, in addition to mocking jwkSource. I understand that this test becomes simply unnecessary due to our changes? Of course, we can take a different approach and if JwtDecoderProviderConfigurationUtils.getJWSAlgorithms returns an error, we can return RS256, but this does not seem flexible to me. What do you think?

Now it is worth discussing the fact that some tests can run indefinitely, and let's take decodeWhenJwkEndpointIsUnresponsiveAndCacheIsConfiguredThenReturnsJwtException as an example. The main problem is in the specifics of the test and in the fact that, as far as I understand, JwtDecoderProviderConfigurationUtils::getJWSAlgorithms can wait indefinitely. In tests like this, we can correctly mock RestOptions as in the case of withIssuerLocation tests:

RestOperations restOperations = mock(RestOperations.class);
given(restOperations.exchange(any(RequestEntity.class), any(ParameterizedTypeReference.class)))
	.willReturn(new ResponseEntity<>(Map.of(“issuer”, issuer, ‘jwks_uri’, issuer + “/jwks”), HttpStatus.OK));
given(restOperations.exchange(any(RequestEntity.class), eq(String.class)))

However, there are tests such as decodeWhenJwkEndpointIsUnresponsiveAndCacheIsConfiguredThenReturnsJwtException that check for endpoint unavailability. As far as I understand, the problem is that when the endpoint is unavailable, we wait for the result indefinitely.

What do you think?

[jzheaux]

@therepanic, thanks for looking into this. Given that withJwkSetUri has been around for some time now, I think it would be best to preserve its default behavior and then consider moving in Spring Security 8.

For now, we can enable this behavior by adding .discoverJwsAlgorithms() to the builder. By default, withIssuerLocation will use this method to activate the discovery behavior. It can be opted into when using withJwkSetUri in the following way:

NimbusJwtDecoder.withJwkSetUri("https://jwks.example.org").discoverJwsAlgorithms().build();

In Spring Security 8, we can make this the default setting. I believe this will preserve the behavior of the existing tests as well, correct?

[therepanic]

Hi, @jzheaux!

I think this is a good solution, especially considering that a bunch of tests are failing and this will be a breaking change at the very least. I pushed a new commit that implements this idea.

I also added a test that verifies the idea behind this method.

[jzheaux]

Thanks, @therepanic! This is now merged into main.