Skip to content

RANGER-5533:Verify JWT Issuer Claims if present#901

Open
ChinmayHegde24 wants to merge 1 commit intoapache:masterfrom
ChinmayHegde24:RANGER-5533
Open

RANGER-5533:Verify JWT Issuer Claims if present#901
ChinmayHegde24 wants to merge 1 commit intoapache:masterfrom
ChinmayHegde24:RANGER-5533

Conversation

@ChinmayHegde24
Copy link
Copy Markdown
Contributor

Support for validating the iss (issuer) claim in JWT has been added as part of this PR.

* @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) {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants