[ https://issues.apache.org/jira/browse/HIVE-26071?focusedWorklogId=763306&page=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-763306 ]
ASF GitHub Bot logged work on HIVE-26071: ----------------------------------------- Author: ASF GitHub Bot Created on: 28/Apr/22 06:07 Start Date: 28/Apr/22 06:07 Worklog Time Spent: 10m Work Description: dengzhhu653 commented on code in PR #3233: URL: https://github.com/apache/hive/pull/3233#discussion_r860516284 ########## standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/HmsThriftHttpServlet.java: ########## @@ -39,75 +48,119 @@ public class HmsThriftHttpServlet extends TServlet { .getLogger(HmsThriftHttpServlet.class); private static final String X_USER = MetaStoreUtils.USER_NAME_HTTP_HEADER; - private final boolean isSecurityEnabled; + private final boolean jwtAuthEnabled; + public static final String AUTHORIZATION = "Authorization"; + private JWTValidator jwtValidator; + private Configuration conf; public HmsThriftHttpServlet(TProcessor processor, - TProtocolFactory inProtocolFactory, TProtocolFactory outProtocolFactory) { - super(processor, inProtocolFactory, outProtocolFactory); - // This should ideally be reveiving an instance of the Configuration which is used for the check + TProtocolFactory protocolFactory, Configuration conf) { + super(processor, protocolFactory); + this.conf = conf; isSecurityEnabled = UserGroupInformation.isSecurityEnabled(); + if (MetastoreConf.getVar(conf, + ConfVars.THRIFT_METASTORE_AUTHENTICATION).equalsIgnoreCase("jwt")) { + jwtAuthEnabled = true; + } else { + jwtAuthEnabled = false; + jwtValidator = null; + } } - public HmsThriftHttpServlet(TProcessor processor, - TProtocolFactory protocolFactory) { - super(processor, protocolFactory); - isSecurityEnabled = UserGroupInformation.isSecurityEnabled(); + public void init() throws ServletException { + super.init(); + if (jwtAuthEnabled) { + try { + jwtValidator = new JWTValidator(this.conf); + } catch (Exception e) { + throw new ServletException("Failed to initialize HmsThriftHttpServlet." + + " Error: " + e); + } + } } @Override protected void doPost(HttpServletRequest request, HttpServletResponse response) throws ServletException, IOException { - - Enumeration<String> headerNames = request.getHeaderNames(); if (LOG.isDebugEnabled()) { - LOG.debug("Logging headers in request"); + LOG.debug(" Logging headers in doPost request"); + Enumeration<String> headerNames = request.getHeaderNames(); while (headerNames.hasMoreElements()) { String headerName = headerNames.nextElement(); LOG.debug("Header: [{}], Value: [{}]", headerName, request.getHeader(headerName)); } } - String userFromHeader = request.getHeader(X_USER); - if (userFromHeader == null || userFromHeader.isEmpty()) { - LOG.error("No user header: {} found", X_USER); - response.sendError(HttpServletResponse.SC_FORBIDDEN, - "Header: " + X_USER + " missing in the request"); - return; - } - - // TODO: These should ideally be in some kind of a Cache with Weak referencse. - // If HMS were to set up some kind of a session, this would go into the session by having - // this filter work with a custom Processor / or set the username into the session - // as is done for HS2. - // In case of HMS, it looks like each request is independent, and there is no session - // information, so the UGI needs to be set up in the Connection layer itself. - UserGroupInformation clientUgi; - // Temporary, and useless for now. Here only to allow this to work on an otherwise kerberized - // server. - if (isSecurityEnabled) { - LOG.info("Creating proxy user for: {}", userFromHeader); - clientUgi = UserGroupInformation.createProxyUser(userFromHeader, UserGroupInformation.getLoginUser()); - } else { - LOG.info("Creating remote user for: {}", userFromHeader); - clientUgi = UserGroupInformation.createRemoteUser(userFromHeader); + try { + String userFromHeader = extractUserName(request, response); + UserGroupInformation clientUgi; + // Temporary, and useless for now. Here only to allow this to work on an otherwise kerberized + // server. + if (isSecurityEnabled) { + LOG.info("Creating proxy user for: {}", userFromHeader); + clientUgi = UserGroupInformation.createProxyUser(userFromHeader, UserGroupInformation.getLoginUser()); + } else { + LOG.info("Creating remote user for: {}", userFromHeader); + clientUgi = UserGroupInformation.createRemoteUser(userFromHeader); + } + PrivilegedExceptionAction<Void> action = new PrivilegedExceptionAction<Void>() { + @Override + public Void run() throws Exception { + HmsThriftHttpServlet.super.doPost(request, response); + return null; + } + }; + try { + clientUgi.doAs(action); + } catch (InterruptedException | RuntimeException e) { + LOG.error("Exception when executing http request as user: " + clientUgi.getUserName(), + e); + throw new ServletException(e); + } + } catch (HttpAuthenticationException e) { + response.setStatus(HttpServletResponse.SC_UNAUTHORIZED); + response.getWriter().println("Authentication error: " + e.getMessage()); + // Also log the error message on server side + LOG.error("Authentication error: ", e); } - - - PrivilegedExceptionAction<Void> action = new PrivilegedExceptionAction<Void>() { - @Override - public Void run() throws Exception { - HmsThriftHttpServlet.super.doPost(request, response); - return null; + } + private String extractUserName(HttpServletRequest request, HttpServletResponse response) + throws HttpAuthenticationException { + if (!jwtAuthEnabled) { + String userFromHeader = request.getHeader(X_USER); + if (userFromHeader == null || userFromHeader.isEmpty()) { + throw new HttpAuthenticationException("User header " + X_USER + " missing in request"); } - }; - + return userFromHeader; + } + String signedJwt = extractBearerToken(request, response); + if (signedJwt == null) { + throw new HttpAuthenticationException("Couldn't find bearer token in the auth header in the request"); + } + String user; try { - clientUgi.doAs(action); - } catch (InterruptedException | RuntimeException e) { - LOG.error("Exception when executing http request as user: " + clientUgi.getUserName(), - e); - throw new ServletException(e); + user = jwtValidator.validateJWTAndExtractUser(signedJwt); + Preconditions.checkNotNull(user, "JWT needs to contain the user name as subject"); + Preconditions.checkState(!user.isEmpty(), "User name should not be empty in JWT"); + LOG.info("Successfully validated and extracted user name {} from JWT in Auth " + + "header in the request", user); + } catch (Exception e) { + throw new HttpAuthenticationException("Failed to validate JWT from Bearer token in " Review Comment: could we append the message of `e` to the message of the `HttpAuthenticationException`, so the client can tell what happens in the server side? Issue Time Tracking ------------------- Worklog Id: (was: 763306) Time Spent: 1h 20m (was: 1h 10m) > JWT authentication for Thrift over HTTP in HiveMetaStore > -------------------------------------------------------- > > Key: HIVE-26071 > URL: https://issues.apache.org/jira/browse/HIVE-26071 > Project: Hive > Issue Type: New Feature > Components: Standalone Metastore > Reporter: Sourabh Goyal > Assignee: Sourabh Goyal > Priority: Major > Labels: pull-request-available > Time Spent: 1h 20m > Remaining Estimate: 0h > > HIVE-25575 recently added a support for JWT authentication in HS2. This Jira > aims to add the same feature in HMS -- This message was sent by Atlassian Jira (v8.20.7#820007)