amichair commented on PR #2744: URL: https://github.com/apache/james-project/pull/2744#issuecomment-2979286911
The kid changes are not related to low-level processing in the JWKS library itself, but to our partial, inconsistent and less intuitive use of it in our own providers. The basic idea is just that if a token contains a standard kid header, it references one specific key that was used to sign the token, and only that key should be used in verifying the token. Potentially better for both performance and security, and standard practice in JWT. Before the change: - We have two PublicKeyProviders (both in our own code) - the JWKS one and the Default one (where we load keys from pem files). The PublicKeyProvider interface had only one method to return all keys. - Our JWKS provider had a thing where (only in one flow) we first check for a kid header without verification, then if it exists we pre-set the kid in the provider, and then the "get all" method internally filters the keys and returns only the one with the matching kid. If there is no kid header it returns all keys as usual. So the same method is used both for "get all" and "get by kid", based on how the constructor was called. btw in the new jjwt api it no longer works - there is no way to extract the kid header without signature verification before setting up the provider/verifier. The changes in this PR simplify and consolidate the behavior: - Our PublicKeyProvider interface now has two separate methods, one for returning all keys and one for returning a single key by kid, simple and intuitive. Both providers now implement them. The JWKS provider no longer needs to be told at construction time which of the two will be called. - The verifier now internally (regardless of which provider is used) tries to first check for the kid (via the key locator in the jjwt api that exists for this purpose) and lookup the key. If found - only that key is used. If the kid header exists but the key is not found - it fails (it shouldn't try other keys). If the kid header is missing - then it falls back to trying all keys one by one (this is for backwards compatibility with existing tokens - we can decide some day to always require kid and easily get rid of the fallback). So the end result is now: 1. using the new jjwt api 2. using a simple and intuitive provider interface 3. the default provider now also supports kid (as will future providers - they only need to implement the get(kid) method of the provider interface) 4. the logic of first trying the kid and then falling back to try all keys is now in the verifier, agnostic to the specific provider in use. I added some comments in the code explaining the relevant parts of this - if you think it's still not enough I'd be happy to try to explain it better here or in the code. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: notifications-unsubscr...@james.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org --------------------------------------------------------------------- To unsubscribe, e-mail: notifications-unsubscr...@james.apache.org For additional commands, e-mail: notifications-h...@james.apache.org