The selectJwk method of NimbusJwtEncoder class should not throw Exception when jwks size great than one

[douxiaofeng99]

Describe the bug
I implemented a rotating JWKS using Redis, where a new JWK is generated at regular intervals, and the old JWKs are also retained for a certain period. In this scenario, the selectJwk method of NimbusJwtEncoder retrieves multiple JWKs when selecting the JWKs. This happens because the jwkSelector only sets the algorithm but does not provide any kid, leading to the exception being thrown below.

To Reproduce
Steps to reproduce the behavior.

Expected behavior
remove the block:
if (jwks.size() > 1) {
throw new JwtEncodingException(String.format(ENCODING_ERROR_MESSAGE_TEMPLATE,
"Found multiple JWK signing keys for algorithm '" + headers.getAlgorithm().getName() + "'"));
}
because in the last the method, return jwks.get(0); already use first jwk.

Sample

A link to a GitHub repository with a minimal, reproducible sample.

Reports that include a sample will take priority over reports that do not.
At times, we may require a sample, so it is good to try and include a sample up front.

[jzheaux]

Thanks for reaching out, @douxiaofeng99.

Note that while you are able to specify a kid or a thumbprint in the JwsHeader, I agree that NimbusJwtEncoder could be improved when it comes to supporting key rotation when those are not specified.

I think a good start would be to introduce a setter that allows applications to indicate which JWK to pick. Something like:

setJwkSelector(Converter<List<JWK>, JWK> selector);

Then applications could do:

setJwkSelector(List::getFirst); // or
setJwkSelector(List::getLast);  // or
setJwkSelector((jwks) -> jwks.stream().max(Comparator.comparing(JWK::getIssueTime)).orElseThrow())

Given the following from the RFC:

The value of the "keys" parameter is an array of JWK values. By
default, the order of the JWK values within the array does not imply
an order of preference among them, although applications of JWK Sets
can choose to assign a meaning to the order for their purposes, if
desired.

and to stay passive, the default should continue to be an exception. That said, I think it would also be helpful to change the error message to be something like:

Failed to select a key since there are multiple for the signing algorithm [%s]; please specify a selector in NimbusJwsEncoder#setJwkSelector

How well would this work for you and are you able to submit a PR that adds this?

[douxiaofeng99]

Hello, @jzheaux: I understand your point above as adding a selector for the JWKs list to choose which JWK to use after fetching them. The selectJwk method has been updated as follows:

private JWK selectJwk(JwsHeader headers) {
    List<JWK> jwks;
    try {
        JWKSelector jwkSelector = new JWKSelector(createJwkMatcher(headers));
        jwks = this.jwkSource.get(jwkSelector, null);
    } catch (Exception ex) {
        throw new JwtEncodingException(String.format(ENCODING_ERROR_MESSAGE_TEMPLATE,
                "Failed to select a JWK signing key -> " + ex.getMessage()), ex);
    }
    if (jwks.isEmpty()) {
        throw new JwtEncodingException(
                String.format(ENCODING_ERROR_MESSAGE_TEMPLATE, "Failed to select a JWK signing key"));
    }
    if (this.jwkSelector != null) {
        return this.jwkSelector.convert(jwks);
    }
    if (jwks.size() > 1) {
        throw new JwtEncodingException(String.format(ENCODING_ERROR_MESSAGE_TEMPLATE,
                "Found multiple JWK signing keys for algorithm '" + headers.getAlgorithm().getName() + "'"));
    }
    return jwks.get(0);
}

The main change involves moving the null-check earlier in the logic. If a converter is set, it selects and returns the JWK using the converter. If no converter is present, the method continues with the original logic.

If that’s the case, I will submit a PR.