Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -52,11 +52,13 @@ public abstract class RangerJwtAuthHandler implements RangerAuthHandler {
public static final String KEY_JWT_PUBLIC_KEY = "jwt.public-key"; // JWT token provider public key
public static final String KEY_JWT_COOKIE_NAME = "jwt.cookie-name"; // JWT cookie name
public static final String KEY_JWT_AUDIENCES = "jwt.audiences";
public static final String KEY_JWT_ISS = "jwt.issuers";
public static final String JWT_AUTHZ_PREFIX = "Bearer ";

protected static String cookieName = "hadoop-jwt";

protected List<String> audiences;
protected List<String> issuers;
protected JWKSource<SecurityContext> keySource;
private JWSVerifier verifier;
private String jwksProviderUrl;
Expand Down Expand Up @@ -99,6 +101,12 @@ public void initialize(final Properties config) throws Exception {
audiences = Arrays.asList(audiencesStr.split(","));
}

// setup issuers if configured
String issuersStr = config.getProperty(KEY_JWT_ISS);
if (StringUtils.isNotBlank(issuersStr)) {
issuers = Arrays.asList(issuersStr.split(","));
}

if (LOG.isDebugEnabled()) {
LOG.debug("<<<=== RangerJwtAuthHandler.initialize()");
}
Expand Down Expand Up @@ -182,20 +190,25 @@ protected boolean validateToken(final SignedJWT jwtToken) {
boolean expValid = validateExpiration(jwtToken);
boolean sigValid = false;
boolean audValid = false;
boolean issValid = false;

if (expValid) {
sigValid = validateSignature(jwtToken);

if (sigValid) {
audValid = validateAudiences(jwtToken);

if (audValid) {
issValid = validateIssuer(jwtToken);
}
}
}

if (LOG.isDebugEnabled()) {
LOG.debug("expValid={}, sigValid={}, audValid={}", expValid, sigValid, audValid);
LOG.debug("expValid={}, sigValid={}, audValid={}, issValid={}", expValid, sigValid, audValid, issValid);
}

return sigValid && audValid && expValid;
return sigValid && audValid && expValid && issValid;
}

/**
Expand Down Expand Up @@ -290,6 +303,38 @@ protected boolean validateAudiences(final SignedJWT jwtToken) {
return valid;
}

/**
* Validate whether any of the accepted issuer claims is present in the issued
* token claims list for issuer. Override this method in subclasses in order
* to customize the audience validation behavior.
*
* @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).

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Current validateIssuer method can accept multiple issuers, but if we use DefaultJWTClaimsVerifier acceptance of single-issuer limitation comes , is that okay?

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.

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.

boolean valid = false;
try {
String tokenIssuer = jwtToken.getJWTClaimsSet().getIssuer();
// if there were no expected issuers configured then just consider any issuer acceptable
if (issuers == null) {
valid = true;
} else {
// if any of the configured issuers is found then consider it acceptable
if (issuers.contains(tokenIssuer)) {
if (LOG.isDebugEnabled()) {
LOG.debug("JWT token issuer has been successfully validated.");
}
valid = true;
} else {
LOG.warn("JWT issuer validation failed.");
}
}
} catch (ParseException pe) {
LOG.warn("Unable to parse the JWT token.", pe);
}
return valid;
}

/**
* Validate that the expiration time of the JWT has not been violated. If
* it has, then throw an AuthenticationException. Override this method in
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,7 @@ public void initialize() {
config.setProperty(RangerJwtAuthHandler.KEY_JWT_PUBLIC_KEY, PropertiesUtil.getProperty(RangerSSOAuthenticationFilter.JWT_PUBLIC_KEY, ""));
config.setProperty(RangerJwtAuthHandler.KEY_JWT_COOKIE_NAME, PropertiesUtil.getProperty(RangerSSOAuthenticationFilter.JWT_COOKIE_NAME, RangerSSOAuthenticationFilter.JWT_COOKIE_NAME_DEFAULT));
config.setProperty(RangerJwtAuthHandler.KEY_JWT_AUDIENCES, PropertiesUtil.getProperty(RangerSSOAuthenticationFilter.JWT_AUDIENCES, ""));
config.setProperty(RangerJwtAuthHandler.KEY_JWT_ISS, PropertiesUtil.getProperty(RangerSSOAuthenticationFilter.JWT_ISSUERS, ""));

super.initialize(config);
} catch (Exception e) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,7 @@ public class RangerSSOAuthenticationFilter implements Filter {
public static final String JWT_PUBLIC_KEY = "ranger.sso.publicKey";
public static final String JWT_COOKIE_NAME = "ranger.sso.cookiename";
public static final String JWT_AUDIENCES = "ranger.sso.audiences";
public static final String JWT_ISSUERS = "ranger.sso.issuers";
public static final String JWT_ORIGINAL_URL_QUERY_PARAM = "ranger.sso.query.param.originalurl";
public static final String JWT_COOKIE_NAME_DEFAULT = "hadoop-jwt";
public static final String JWT_ORIGINAL_URL_QUERY_PARAM_DEFAULT = "originalUrl";
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -498,4 +498,9 @@ public void testDoFilter_ssoDisabled_locallogin_redirectsToLoginJsp() throws Exc
verify(res, times(1)).sendRedirect("/login.jsp");
verify(chain, times(0)).doFilter(any(ServletRequest.class), any(ServletResponse.class));
}

@Test
void testJwtIssuersConstant() {
assertEquals("ranger.sso.issuers", RangerSSOAuthenticationFilter.JWT_ISSUERS);
}
}
Loading