sourabh912 commented on code in PR #3233:
URL: https://github.com/apache/hive/pull/3233#discussion_r866242342
##########
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)
Review Comment:
The servlet response is required in `extractBearerToken()` which is called
from extractUserName().
--
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]
