SOLR-13701: Fixed JWTAuthPlugin to update metrics prior to continuing w/other filters or returning error

This commit is contained in:
Chris Hostetter 2019-08-18 12:20:51 -07:00
parent 251259d5ab
commit f5856ef404
3 changed files with 16 additions and 17 deletions

View File

@ -153,6 +153,8 @@ Bug Fixes
* SOLR-13700: Fixed a race condition when initializing metrics for new security plugins on security.json change (hossman) * SOLR-13700: Fixed a race condition when initializing metrics for new security plugins on security.json change (hossman)
* SOLR-13701: Fixed JWTAuthPlugin to update metrics prior to continuing w/other filters or returning error (hossman)
Other Changes Other Changes
---------------------- ----------------------

View File

@ -302,8 +302,8 @@ public class JWTAuthPlugin extends AuthenticationPlugin implements SpecProvider,
if (jwtConsumer == null) { if (jwtConsumer == null) {
if (header == null && !blockUnknown) { if (header == null && !blockUnknown) {
log.info("JWTAuth not configured, but allowing anonymous access since {}==false", PARAM_BLOCK_UNKNOWN); log.info("JWTAuth not configured, but allowing anonymous access since {}==false", PARAM_BLOCK_UNKNOWN);
filterChain.doFilter(request, response);
numPassThrough.inc(); numPassThrough.inc();
filterChain.doFilter(request, response);
return true; return true;
} }
// Retry config // Retry config
@ -342,22 +342,22 @@ public class JWTAuthPlugin extends AuthenticationPlugin implements SpecProvider,
} }
if (log.isDebugEnabled()) if (log.isDebugEnabled())
log.debug("Authentication SUCCESS"); log.debug("Authentication SUCCESS");
filterChain.doFilter(wrapper, response);
numAuthenticated.inc(); numAuthenticated.inc();
filterChain.doFilter(wrapper, response);
return true; return true;
case PASS_THROUGH: case PASS_THROUGH:
if (log.isDebugEnabled()) if (log.isDebugEnabled())
log.debug("Unknown user, but allow due to {}=false", PARAM_BLOCK_UNKNOWN); log.debug("Unknown user, but allow due to {}=false", PARAM_BLOCK_UNKNOWN);
filterChain.doFilter(request, response);
numPassThrough.inc(); numPassThrough.inc();
filterChain.doFilter(request, response);
return true; return true;
case AUTZ_HEADER_PROBLEM: case AUTZ_HEADER_PROBLEM:
case JWT_PARSE_ERROR: case JWT_PARSE_ERROR:
log.warn("Authentication failed. {}, {}", authResponse.getAuthCode(), authResponse.getAuthCode().getMsg()); log.warn("Authentication failed. {}, {}", authResponse.getAuthCode(), authResponse.getAuthCode().getMsg());
authenticationFailure(response, authResponse.getAuthCode().getMsg(), HttpServletResponse.SC_BAD_REQUEST, BearerWwwAuthErrorCode.invalid_request);
numErrors.mark(); numErrors.mark();
authenticationFailure(response, authResponse.getAuthCode().getMsg(), HttpServletResponse.SC_BAD_REQUEST, BearerWwwAuthErrorCode.invalid_request);
return false; return false;
case CLAIM_MISMATCH: case CLAIM_MISMATCH:
@ -365,25 +365,25 @@ public class JWTAuthPlugin extends AuthenticationPlugin implements SpecProvider,
case JWT_VALIDATION_EXCEPTION: case JWT_VALIDATION_EXCEPTION:
case PRINCIPAL_MISSING: case PRINCIPAL_MISSING:
log.warn("Authentication failed. {}, {}", authResponse.getAuthCode(), exceptionMessage); log.warn("Authentication failed. {}, {}", authResponse.getAuthCode(), exceptionMessage);
authenticationFailure(response, authResponse.getAuthCode().getMsg(), HttpServletResponse.SC_UNAUTHORIZED, BearerWwwAuthErrorCode.invalid_token);
numWrongCredentials.inc(); numWrongCredentials.inc();
authenticationFailure(response, authResponse.getAuthCode().getMsg(), HttpServletResponse.SC_UNAUTHORIZED, BearerWwwAuthErrorCode.invalid_token);
return false; return false;
case SIGNATURE_INVALID: case SIGNATURE_INVALID:
log.warn("Signature validation failed: {}", exceptionMessage); log.warn("Signature validation failed: {}", exceptionMessage);
authenticationFailure(response, authResponse.getAuthCode().getMsg(), HttpServletResponse.SC_UNAUTHORIZED, BearerWwwAuthErrorCode.invalid_token);
numWrongCredentials.inc(); numWrongCredentials.inc();
authenticationFailure(response, authResponse.getAuthCode().getMsg(), HttpServletResponse.SC_UNAUTHORIZED, BearerWwwAuthErrorCode.invalid_token);
return false; return false;
case SCOPE_MISSING: case SCOPE_MISSING:
authenticationFailure(response, authResponse.getAuthCode().getMsg(), HttpServletResponse.SC_UNAUTHORIZED, BearerWwwAuthErrorCode.insufficient_scope);
numWrongCredentials.inc(); numWrongCredentials.inc();
authenticationFailure(response, authResponse.getAuthCode().getMsg(), HttpServletResponse.SC_UNAUTHORIZED, BearerWwwAuthErrorCode.insufficient_scope);
return false; return false;
case NO_AUTZ_HEADER: case NO_AUTZ_HEADER:
default: default:
authenticationFailure(response, authResponse.getAuthCode().getMsg(), HttpServletResponse.SC_UNAUTHORIZED, null);
numMissingCredentials.inc(); numMissingCredentials.inc();
authenticationFailure(response, authResponse.getAuthCode().getMsg(), HttpServletResponse.SC_UNAUTHORIZED, null);
return false; return false;
} }
} }

View File

@ -119,16 +119,13 @@ public class SolrCloudAuthTestCase extends SolrCloudTestCase {
expectedCounts.put("failMissingCredentials", (long) failMissingCredentials); expectedCounts.put("failMissingCredentials", (long) failMissingCredentials);
expectedCounts.put("errors", (long) errors); expectedCounts.put("errors", (long) errors);
Map<String, Long> counts = countSecurityMetrics(cluster, prefix, AUTH_METRICS_KEYS); final Map<String, Long> counts = countSecurityMetrics(cluster, prefix, AUTH_METRICS_KEYS);
boolean success = isMetricsEqualOrLarger(AUTH_METRICS_TO_COMPARE, expectedCounts, counts); final boolean success = isMetricsEqualOrLarger(AUTH_METRICS_TO_COMPARE, expectedCounts, counts);
if (!success) {
log.info("First metrics count assert failed, pausing 2s before re-attempt");
Thread.sleep(2000);
counts = countSecurityMetrics(cluster, prefix, AUTH_METRICS_KEYS);
success = isMetricsEqualOrLarger(AUTH_METRICS_TO_COMPARE, expectedCounts, counts);
}
assertTrue("Expected metric minimums for prefix " + prefix + ": " + expectedCounts + ", but got: " + counts, success); assertTrue("Expected metric minimums for prefix " + prefix + ": " + expectedCounts +
", but got: " + counts + "(Possible cause is delay in loading modified " +
"security.json; see SOLR-13464 for test work around)",
success);
if (counts.get("requests") > 0) { if (counts.get("requests") > 0) {
assertTrue("requestTimes count not > 1", counts.get("requestTimes") > 1); assertTrue("requestTimes count not > 1", counts.get("requestTimes") > 1);