RANGER-5533:Verify JWT Issuer Claims if present#901
RANGER-5533:Verify JWT Issuer Claims if present#901ChinmayHegde24 wants to merge 1 commit intoapache:masterfrom
Conversation
| * @param jwtToken the JWT token from which the JWT issuer will be obtained | ||
| * @return true if an expected issuer is present, otherwise false | ||
| */ | ||
| protected boolean validateIssuer(final SignedJWT jwtToken) { |
There was a problem hiding this comment.
Instead of adding a standalone validateIssuer() method, consider using Nimbus's DefaultJWTClaimsVerifier, which would consolidate issuer, audience, and expiration checks in one place:
JWTClaimsSet.Builder exactMatchBuilder = new JWTClaimsSet.Builder();
String issuer = config.getProperty(KEY_JWT_ISSUER);
if (StringUtils.isNotBlank(issuer)) {
exactMatchBuilder.issuer(issuer);
}
Set<String> acceptedAudiences = null;
String audiencesStr = config.getProperty(KEY_JWT_AUDIENCES);
if (StringUtils.isNotBlank(audiencesStr)) {
acceptedAudiences = new HashSet<>(Arrays.asList(audiencesStr.split(",")));
}
claimsVerifier = new DefaultJWTClaimsVerifier<>(acceptedAudiences, exactMatchBuilder.build(), null, null);
claimsVerifier.verify(jwtToken.getJWTClaimsSet(), null);This would replace all three of validateExpiration(), validateAudiences(), and the proposed validateIssuer() with a single, well-tested library call. It also gets clock skew handling for free (the verifier has a configurable skew, defaulting to 60 seconds).
There was a problem hiding this comment.
Current validateIssuer method can accept multiple issuers, but if we use DefaultJWTClaimsVerifier acceptance of single-issuer limitation comes , is that okay?
There was a problem hiding this comment.
Multi-issuer support is a good follow-up (in another PR). For this PR, I suggest to scope it for single-issuer validation. Multi-issuer support would require issuer-specific config and processor selection, something like:
jwt.issuers=issuerA,issuerB
jwt.issuer.issuerA.iss=https://idp-a/...
jwt.issuer.issuerA.jwks-url=...
jwt.issuer.issuerA.audiences=...
jwt.issuer.issuerB.iss=https://idp-b/...
jwt.issuer.issuerB.jwks-url=...
jwt.issuer.issuerB.audiences=...
so it would be better handled in a separate PR.
Support for validating the iss (issuer) claim in JWT has been added as part of this PR.