jon-wei commented on a change in pull request #7685: Remove unnecessary
principal handling in KerberosAuthenticator
URL: https://github.com/apache/incubator-druid/pull/7685#discussion_r287115003
##########
File path:
extensions-core/druid-kerberos/src/main/java/org/apache/druid/security/kerberos/KerberosAuthenticator.java
##########
@@ -230,56 +218,36 @@ protected AuthenticationToken
getToken(HttpServletRequest request) throws Authen
public void doFilter(ServletRequest request, ServletResponse response,
FilterChain filterChain)
throws IOException, ServletException
{
- HttpServletRequest httpReq = (HttpServletRequest) request;
-
// If there's already an auth result, then we have authenticated
already, skip this.
if (request.getAttribute(AuthConfig.DRUID_AUTHENTICATION_RESULT) !=
null) {
filterChain.doFilter(request, response);
return;
}
+ // In the hadoop-auth 2.7.3 code that this was adapted from, the login
would've occurred during init() of
+ // the AuthenticationFilter via
`initializeAuthHandler(authHandlerClassName, filterConfig)`.
+ // Since we co-exist with other authentication schemes, don't login
until we've checked that
+ // some other Authenticator didn't already validate this request.
if (loginContext == null) {
initializeKerberosLogin();
}
+ // Checking for excluded paths is Druid-specific, not from hadoop-auth
String path = ((HttpServletRequest) request).getRequestURI();
if (isExcluded(path)) {
filterChain.doFilter(request, response);
} else {
- String clientPrincipal;
- try {
- Cookie[] cookies = httpReq.getCookies();
- if (cookies == null) {
- clientPrincipal =
getPrincipalFromRequestNew((HttpServletRequest) request);
- } else {
- clientPrincipal = null;
- for (Cookie cookie : cookies) {
- if ("hadoop.auth".equals(cookie.getName())) {
- Matcher matcher =
HADOOP_AUTH_COOKIE_REGEX.matcher(cookie.getValue());
- if (matcher.matches()) {
- clientPrincipal = matcher.group(1);
- break;
- }
- }
- }
- }
- }
- catch (Exception ex) {
- clientPrincipal = null;
- }
-
- if (clientPrincipal != null) {
- request.setAttribute(
- AuthConfig.DRUID_AUTHENTICATION_RESULT,
- new AuthenticationResult(clientPrincipal, authorizerName,
name, null)
- );
- }
+ // Run the original doFilter method, but with modifications to error
handling
+ doFilterSuper(request, response, filterChain);
}
-
- doFilterSuper(request, response, filterChain);
Review comment:
Hm, this `isExcluded` parameter should actually be removed, it was part of
the original Kerberos authentication extension before the larger auth system
was added.
This path exclusion functionality should now be handled by setting
`druid.auth.unsecuredPaths`, a filter at the start of the filterChain would
authenticate+authorize requests made to paths specified in this config.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscr...@druid.apache.org
For additional commands, e-mail: commits-h...@druid.apache.org
