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?
--
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.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
