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

Reply via email to