You are viewing a plain text version of this content. The canonical link for it is here.
Posted to gitbox@hive.apache.org by GitBox <gi...@apache.org> on 2022/04/21 23:02:57 UTC
[GitHub] [hive] sourabh912 opened a new pull request, #3233: [WIP]HIVE-26071: JWT authentication mechanism for Thrift over HTTP in HiveMetastore
sourabh912 opened a new pull request, #3233:
URL: https://github.com/apache/hive/pull/3233
<!--
Thanks for sending a pull request! Here are some tips for you:
1. If this is your first time, please read our contributor guidelines: https://cwiki.apache.org/confluence/display/Hive/HowToContribute
2. Ensure that you have created an issue on the Hive project JIRA: https://issues.apache.org/jira/projects/HIVE/summary
3. Ensure you have added or run the appropriate tests for your PR:
4. If the PR is unfinished, add '[WIP]' in your PR title, e.g., '[WIP]HIVE-XXXXX: Your PR title ...'.
5. Be sure to keep the PR description updated to reflect all changes.
6. Please write your PR title to summarize what this PR proposes.
7. If possible, provide a concise example to reproduce the issue for a faster review.
-->
### What changes were proposed in this pull request?
<!--
Please clarify what changes you are proposing. The purpose of this section is to outline the changes and how this PR fixes the issue.
If possible, please consider writing useful notes for better and faster reviews in your PR. See the examples below.
1. If you refactor some codes with changing classes, showing the class hierarchy will help reviewers.
2. If you fix some SQL features, you can provide some references of other DBMSes.
3. If there is design documentation, please add the link.
4. If there is a discussion in the mailing list, please add the link.
-->
### Why are the changes needed?
<!--
Please clarify why the changes are needed. For instance,
1. If you propose a new API, clarify the use case for a new API.
2. If you fix a bug, you can clarify why it is a bug.
-->
### Does this PR introduce _any_ user-facing change?
<!--
Note that it means *any* user-facing change including all aspects such as the documentation fix.
If yes, please clarify the previous behavior and the change this PR proposes - provide the console output, description, screenshot and/or a reproducable example to show the behavior difference if possible.
If possible, please also clarify if this is a user-facing change compared to the released Hive versions or within the unreleased branches such as master.
If no, write 'No'.
-->
### How was this patch tested?
<!--
If tests were added, say they were added here. Please make sure to add some test cases that check the changes thoroughly including negative and positive cases if possible.
If it was tested in a way different from regular unit tests, please clarify how you tested step by step, ideally copy and paste-able, so that other reviewers can test and check, and descendants can verify in the future.
If tests were not added, please describe why they were not added and/or why it was difficult to add.
-->
--
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: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
[GitHub] [hive] dengzhhu653 commented on a diff in pull request #3233: [WIP]HIVE-26071: JWT authentication mechanism for Thrift over HTTP in HiveMetastore
Posted by GitBox <gi...@apache.org>.
dengzhhu653 commented on code in PR #3233:
URL: https://github.com/apache/hive/pull/3233#discussion_r862206299
##########
standalone-metastore/metastore-common/src/main/java/org/apache/hadoop/hive/metastore/HiveMetaStoreClient.java:
##########
@@ -605,27 +608,44 @@ private THttpClient createHttpClient(URI store, boolean useSSL) throws MetaExcep
String path = MetaStoreUtils.getHttpPath(MetastoreConf.getVar(conf, ConfVars.THRIFT_HTTP_PATH));
String httpUrl = (useSSL ? "https://" : "http://") + store.getHost() + ":" + store.getPort() + path;
- String user = MetastoreConf.getVar(conf, ConfVars.METASTORE_CLIENT_PLAIN_USERNAME);
- if (user == null || user.equals("")) {
- try {
- LOG.debug("No username passed in config " + ConfVars.METASTORE_CLIENT_PLAIN_USERNAME.getHiveName() +
- ". Trying to get the current user from UGI" );
- user = UserGroupInformation.getCurrentUser().getShortUserName();
- } catch (IOException e) {
- throw new MetaException("Failed to get client username from UGI");
+ HttpClientBuilder httpClientBuilder = HttpClientBuilder.create();
+ String authType = MetastoreConf.getAsString(conf, ConfVars.METASTORE_CLIENT_AUTH_MODE).toLowerCase(
Review Comment:
sorry, I mean the the `METASTORE_CLIENT_AUTH_MODE` has been restricted to `JWT`,..., there is no need to make the value be lower-cased by Local.ROOT, `equalsIgnoreCase` may be better for this case? Just my opinion, thanks.
--
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: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
[GitHub] [hive] sourabh912 commented on pull request #3233: HIVE-26071: JWT authentication mechanism for Thrift over HTTP in HiveMetastore
Posted by GitBox <gi...@apache.org>.
sourabh912 commented on PR #3233:
URL: https://github.com/apache/hive/pull/3233#issuecomment-1125314717
Thanks @nrg4878 @dengzhhu653 @saihemanth-cloudera @hsnusonic for the review and @yongzhi for the merge.
--
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: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
[GitHub] [hive] sourabh912 commented on a diff in pull request #3233: [WIP]HIVE-26071: JWT authentication mechanism for Thrift over HTTP in HiveMetastore
Posted by GitBox <gi...@apache.org>.
sourabh912 commented on code in PR #3233:
URL: https://github.com/apache/hive/pull/3233#discussion_r866209034
##########
standalone-metastore/metastore-common/src/main/java/org/apache/hadoop/hive/metastore/HiveMetaStoreClient.java:
##########
@@ -605,27 +608,44 @@ private THttpClient createHttpClient(URI store, boolean useSSL) throws MetaExcep
String path = MetaStoreUtils.getHttpPath(MetastoreConf.getVar(conf, ConfVars.THRIFT_HTTP_PATH));
String httpUrl = (useSSL ? "https://" : "http://") + store.getHost() + ":" + store.getPort() + path;
- String user = MetastoreConf.getVar(conf, ConfVars.METASTORE_CLIENT_PLAIN_USERNAME);
- if (user == null || user.equals("")) {
- try {
- LOG.debug("No username passed in config " + ConfVars.METASTORE_CLIENT_PLAIN_USERNAME.getHiveName() +
- ". Trying to get the current user from UGI" );
- user = UserGroupInformation.getCurrentUser().getShortUserName();
- } catch (IOException e) {
- throw new MetaException("Failed to get client username from UGI");
+ HttpClientBuilder httpClientBuilder = HttpClientBuilder.create();
+ String authType = MetastoreConf.getAsString(conf, ConfVars.METASTORE_CLIENT_AUTH_MODE).toLowerCase(
+ Locale.ROOT);
+ String user = null;
Review Comment:
Not really. It was left out during refactoring. Will fix it.
--
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: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
[GitHub] [hive] dengzhhu653 commented on a diff in pull request #3233: [WIP]HIVE-26071: JWT authentication mechanism for Thrift over HTTP in HiveMetastore
Posted by GitBox <gi...@apache.org>.
dengzhhu653 commented on code in PR #3233:
URL: https://github.com/apache/hive/pull/3233#discussion_r864399754
##########
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:
Fine, that makes sense to me!
--
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: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
[GitHub] [hive] sourabh912 commented on a diff in pull request #3233: [WIP]HIVE-26071: JWT authentication mechanism for Thrift over HTTP in HiveMetastore
Posted by GitBox <gi...@apache.org>.
sourabh912 commented on code in PR #3233:
URL: https://github.com/apache/hive/pull/3233#discussion_r866206252
##########
standalone-metastore/metastore-common/src/main/java/org/apache/hadoop/hive/metastore/HiveMetaStoreClient.java:
##########
@@ -605,27 +608,44 @@ private THttpClient createHttpClient(URI store, boolean useSSL) throws MetaExcep
String path = MetaStoreUtils.getHttpPath(MetastoreConf.getVar(conf, ConfVars.THRIFT_HTTP_PATH));
String httpUrl = (useSSL ? "https://" : "http://") + store.getHost() + ":" + store.getPort() + path;
- String user = MetastoreConf.getVar(conf, ConfVars.METASTORE_CLIENT_PLAIN_USERNAME);
- if (user == null || user.equals("")) {
- try {
- LOG.debug("No username passed in config " + ConfVars.METASTORE_CLIENT_PLAIN_USERNAME.getHiveName() +
- ". Trying to get the current user from UGI" );
- user = UserGroupInformation.getCurrentUser().getShortUserName();
- } catch (IOException e) {
- throw new MetaException("Failed to get client username from UGI");
+ HttpClientBuilder httpClientBuilder = HttpClientBuilder.create();
+ String authType = MetastoreConf.getAsString(conf, ConfVars.METASTORE_CLIENT_AUTH_MODE).toLowerCase(
Review Comment:
@dengzhhu653 : Makes sense. I will make the change.
--
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: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
[GitHub] [hive] dengzhhu653 commented on a diff in pull request #3233: [WIP]HIVE-26071: JWT authentication mechanism for Thrift over HTTP in HiveMetastore
Posted by GitBox <gi...@apache.org>.
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: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
[GitHub] [hive] saihemanth-cloudera commented on a diff in pull request #3233: [WIP]HIVE-26071: JWT authentication mechanism for Thrift over HTTP in HiveMetastore
Posted by GitBox <gi...@apache.org>.
saihemanth-cloudera commented on code in PR #3233:
URL: https://github.com/apache/hive/pull/3233#discussion_r863250675
##########
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) {
Review Comment:
Adding a new argument to the constructor would break the compatibility/inter-operability between HMS and clients. Instead, wouldn't it be ideal to add a new constructor with an added argument?
--
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: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
[GitHub] [hive] sourabh912 commented on a diff in pull request #3233: [WIP]HIVE-26071: JWT authentication mechanism for Thrift over HTTP in HiveMetastore
Posted by GitBox <gi...@apache.org>.
sourabh912 commented on code in PR #3233:
URL: https://github.com/apache/hive/pull/3233#discussion_r862089037
##########
standalone-metastore/metastore-common/src/main/java/org/apache/hadoop/hive/metastore/HiveMetaStoreClient.java:
##########
@@ -605,27 +608,44 @@ private THttpClient createHttpClient(URI store, boolean useSSL) throws MetaExcep
String path = MetaStoreUtils.getHttpPath(MetastoreConf.getVar(conf, ConfVars.THRIFT_HTTP_PATH));
String httpUrl = (useSSL ? "https://" : "http://") + store.getHost() + ":" + store.getPort() + path;
- String user = MetastoreConf.getVar(conf, ConfVars.METASTORE_CLIENT_PLAIN_USERNAME);
- if (user == null || user.equals("")) {
- try {
- LOG.debug("No username passed in config " + ConfVars.METASTORE_CLIENT_PLAIN_USERNAME.getHiveName() +
- ". Trying to get the current user from UGI" );
- user = UserGroupInformation.getCurrentUser().getShortUserName();
- } catch (IOException e) {
- throw new MetaException("Failed to get client username from UGI");
+ HttpClientBuilder httpClientBuilder = HttpClientBuilder.create();
+ String authType = MetastoreConf.getAsString(conf, ConfVars.METASTORE_CLIENT_AUTH_MODE).toLowerCase(
Review Comment:
@dengzhhu653 : Sorry I didn't understand your question. Can you please elaborate?
@hsnusonic : For this config, getAsString and getVar work the same way. But its better to use `getVar` here since it enforces that defaultVal of this config should be of String type.
--
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: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
[GitHub] [hive] saihemanth-cloudera commented on a diff in pull request #3233: [WIP]HIVE-26071: JWT authentication mechanism for Thrift over HTTP in HiveMetastore
Posted by GitBox <gi...@apache.org>.
saihemanth-cloudera commented on code in PR #3233:
URL: https://github.com/apache/hive/pull/3233#discussion_r863264338
##########
standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/auth/jwt/JWTValidator.java:
##########
@@ -0,0 +1,112 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.hadoop.hive.metastore.auth.jwt;
+
+import com.google.common.base.Preconditions;
+import com.nimbusds.jose.JOSEException;
+import com.nimbusds.jose.JWSHeader;
+import com.nimbusds.jose.JWSObject;
+import com.nimbusds.jose.JWSVerifier;
+import com.nimbusds.jose.crypto.factories.DefaultJWSVerifierFactory;
+import com.nimbusds.jose.jwk.AsymmetricJWK;
+import com.nimbusds.jose.jwk.JWK;
+import com.nimbusds.jwt.JWTClaimsSet;
+import com.nimbusds.jwt.SignedJWT;
+import org.apache.hadoop.conf.Configuration;
+import org.apache.hadoop.hive.metastore.conf.MetastoreConf;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import javax.security.sasl.AuthenticationException;
+import java.io.IOException;
+import java.security.Key;
+import java.text.ParseException;
+import java.util.Date;
+import java.util.List;
+
+/**
+ * This class is used to validate JWT. JWKS is fetched during instantiation and kept in the memory.
+ * We disallow JWT signature verification with symmetric key, because that means anyone can get the same key
+ * and use it to sign a JWT.
+ */
+public class JWTValidator {
Review Comment:
JWTValidatorUtils makes more sense than JWTValidator?
--
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: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
[GitHub] [hive] nrg4878 commented on a diff in pull request #3233: [WIP]HIVE-26071: JWT authentication mechanism for Thrift over HTTP in HiveMetastore
Posted by GitBox <gi...@apache.org>.
nrg4878 commented on code in PR #3233:
URL: https://github.com/apache/hive/pull/3233#discussion_r859330555
##########
standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/HmsThriftHttpServlet.java:
##########
@@ -17,14 +17,23 @@
package org.apache.hadoop.hive.metastore;
+import com.google.common.base.Preconditions;
import java.io.IOException;
import java.security.PrivilegedExceptionAction;
import java.util.Enumeration;
Review Comment:
nit: empty line
##########
standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/HttpAuthenticationException.java:
##########
@@ -0,0 +1,43 @@
+/*
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License. See accompanying LICENSE file.
+ */
+
+package org.apache.hadoop.hive.metastore;
Review Comment:
nit: would it be better if this was in the metastore.auth package because it is Authentication specific exception?
##########
standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/auth/jwt/JWTValidator.java:
##########
@@ -0,0 +1,112 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.hadoop.hive.metastore.auth.jwt;
+
+import com.google.common.base.Preconditions;
+import com.nimbusds.jose.JOSEException;
+import com.nimbusds.jose.JWSHeader;
+import com.nimbusds.jose.JWSObject;
+import com.nimbusds.jose.JWSVerifier;
+import com.nimbusds.jose.crypto.factories.DefaultJWSVerifierFactory;
+import com.nimbusds.jose.jwk.AsymmetricJWK;
+import com.nimbusds.jose.jwk.JWK;
+import com.nimbusds.jwt.JWTClaimsSet;
+import com.nimbusds.jwt.SignedJWT;
+import org.apache.hadoop.conf.Configuration;
+import org.apache.hadoop.hive.metastore.conf.MetastoreConf;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import javax.security.sasl.AuthenticationException;
Review Comment:
nit: typically imports are ordered alphabetically. Seems like this should be above
##########
standalone-metastore/metastore-server/src/test/java/org/apache/hadoop/hive/metastore/TestRemoteHiveMetastoreWithHttpJwt.java:
##########
@@ -0,0 +1,269 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.hadoop.hive.metastore;
+
+import com.github.tomakehurst.wiremock.junit.WireMockRule;
+import com.nimbusds.jose.JWSAlgorithm;
+import com.nimbusds.jose.JWSHeader;
+import com.nimbusds.jose.JWSSigner;
+import com.nimbusds.jose.crypto.RSASSASigner;
+import com.nimbusds.jose.jwk.RSAKey;
+import com.nimbusds.jwt.JWTClaimsSet;
+import com.nimbusds.jwt.SignedJWT;
+
+import java.io.File;
Review Comment:
nit: unordered imports (alphabetical)
--
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: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
[GitHub] [hive] dengzhhu653 commented on a diff in pull request #3233: [WIP]HIVE-26071: JWT authentication mechanism for Thrift over HTTP in HiveMetastore
Posted by GitBox <gi...@apache.org>.
dengzhhu653 commented on code in PR #3233:
URL: https://github.com/apache/hive/pull/3233#discussion_r862198179
##########
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:
oops, seems that the http client would ignore the details of exception throwing by the servlet, not sure it's intended or not.
or could we add a `HttpResponseInterceptor` to the client and throw authentication exception if meet , which i think will make the exception be friendly to the user?
--
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: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
[GitHub] [hive] sourabh912 commented on a diff in pull request #3233: [WIP]HIVE-26071: JWT authentication mechanism for Thrift over HTTP in HiveMetastore
Posted by GitBox <gi...@apache.org>.
sourabh912 commented on code in PR #3233:
URL: https://github.com/apache/hive/pull/3233#discussion_r862048737
##########
standalone-metastore/metastore-server/src/test/java/org/apache/hadoop/hive/metastore/TestRemoteHiveMetastoreWithHttpJwt.java:
##########
@@ -0,0 +1,269 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.hadoop.hive.metastore;
+
+import com.github.tomakehurst.wiremock.junit.WireMockRule;
+import com.nimbusds.jose.JWSAlgorithm;
+import com.nimbusds.jose.JWSHeader;
+import com.nimbusds.jose.JWSSigner;
+import com.nimbusds.jose.crypto.RSASSASigner;
+import com.nimbusds.jose.jwk.RSAKey;
+import com.nimbusds.jwt.JWTClaimsSet;
+import com.nimbusds.jwt.SignedJWT;
+
+import java.io.File;
+import java.lang.reflect.Field;
+import java.lang.reflect.Modifier;
+import java.nio.file.Files;
+import java.nio.file.Path;
+import java.util.HashMap;
+import java.util.Map;
+import org.apache.hadoop.conf.Configuration;
+import org.apache.hadoop.hive.metastore.annotation.MetastoreUnitTest;
+import org.apache.hadoop.hive.metastore.api.Database;
+import org.apache.hadoop.hive.metastore.conf.MetastoreConf;
+import org.apache.hadoop.hive.metastore.conf.MetastoreConf.ConfVars;
+import org.apache.hadoop.hive.metastore.security.HadoopThriftAuthBridge;
+import org.apache.thrift.transport.TTransportException;
+import org.junit.AfterClass;
+import org.junit.Before;
+import org.junit.BeforeClass;
+import org.junit.ClassRule;
+import java.nio.charset.StandardCharsets;
+import java.util.Date;
+import java.util.UUID;
+import java.util.concurrent.TimeUnit;
+import org.junit.Ignore;
+import org.junit.Test;
+import org.junit.experimental.categories.Category;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import static com.github.tomakehurst.wiremock.client.WireMock.get;
+import static com.github.tomakehurst.wiremock.client.WireMock.ok;
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertTrue;
+
+@Category(MetastoreUnitTest.class)
+public class TestRemoteHiveMetastoreWithHttpJwt {
+ private static final Map<String, String> DEFAULTS = new HashMap<>(System.getenv());
+ private static Map<String, String> envMap;
+
+ private static String baseDir = System.getProperty("basedir");
+ private static final File jwtAuthorizedKeyFile =
+ new File(baseDir,"src/test/resources/auth/jwt/jwt-authorized-key.json");
+ private static final File jwtUnauthorizedKeyFile =
+ new File(baseDir,"src/test/resources/auth/jwt/jwt-unauthorized-key.json");
+ private static final File jwtVerificationJWKSFile =
+ new File(baseDir,"src/test/resources/auth/jwt/jwt-verification-jwks.json");
+
+ private static final String USER_1 = "HMS_TEST_USER_1";
+ private static final String TEST_DB_NAME_PREFIX = "HMS_JWT_AUTH_DB";
+ private static final Logger LOG = LoggerFactory.getLogger(TestRemoteHiveMetastoreWithHttpJwt.class);
+ //private static MiniHS2 miniHS2;
+
+ private static final int MOCK_JWKS_SERVER_PORT = 8089;
+ @ClassRule
+ public static final WireMockRule MOCK_JWKS_SERVER = new WireMockRule(MOCK_JWKS_SERVER_PORT);
+
+ /**
+ * This is a hack to make environment variables modifiable.
+ * Ref: https://stackoverflow.com/questions/318239/how-do-i-set-environment-variables-from-java.
+ */
+ @BeforeClass
+ public static void makeEnvModifiable() throws Exception {
+ envMap = new HashMap<>();
+ Class<?> envClass = Class.forName("java.lang.ProcessEnvironment");
+ Field theEnvironmentField = envClass.getDeclaredField("theEnvironment");
+ Field theUnmodifiableEnvironmentField = envClass.getDeclaredField("theUnmodifiableEnvironment");
+ removeStaticFinalAndSetValue(theEnvironmentField, envMap);
+ removeStaticFinalAndSetValue(theUnmodifiableEnvironmentField, envMap);
+ }
+
+ private static void removeStaticFinalAndSetValue(Field field, Object value) throws Exception {
+ field.setAccessible(true);
+ Field modifiersField = Field.class.getDeclaredField("modifiers");
+ modifiersField.setAccessible(true);
+ modifiersField.setInt(field, field.getModifiers() & ~Modifier.FINAL);
+ field.set(null, value);
+ }
+ private static int port;
+ private static Configuration conf = null;
+
+ public TestRemoteHiveMetastoreWithHttpJwt() {
+ // default constructor
+ }
+
+ @AfterClass
+ public static void stopServices() throws Exception {
+ System.getenv().remove("HMS_JWT");
+ }
+
+ @BeforeClass
+ public static void setUp() throws Exception {
+ conf = MetastoreConf.newMetastoreConf();
+
+ // set some values to use for getting conf. vars
+ MetastoreConf.setBoolVar(conf, ConfVars.METRICS_ENABLED, true);
+ conf.set("datanucleus.autoCreateTables", "false");
+ conf.set("hive.in.test", "true");
+ MetastoreConf.setVar(conf, ConfVars.METASTORE_METADATA_TRANSFORMER_CLASS, " ");
+
+ MetaStoreTestUtils.setConfForStandloneMode(conf);
+ MetastoreConf.setLongVar(conf, ConfVars.BATCH_RETRIEVE_MAX, 2);
+ MetastoreConf.setLongVar(conf, ConfVars.LIMIT_PARTITION_REQUEST, 100);
+ MetastoreConf.setVar(conf, ConfVars.STORAGE_SCHEMA_READER_IMPL, "no.such.class");
+
+ setupMockServer();
+ MetastoreConf.setBoolVar(conf, ConfVars.EXECUTE_SET_UGI, false);
+ MetastoreConf.setVar(conf, ConfVars.THRIFT_TRANSPORT_MODE, "http");
+ MetastoreConf.setVar(conf, ConfVars.METASTORE_CLIENT_THRIFT_TRANSPORT_MODE, "http");
+
+ MetastoreConf.setVar(conf, ConfVars.METASTORE_CLIENT_AUTH_MODE, "JWT");
+ MetastoreConf.setVar(conf, ConfVars.THRIFT_METASTORE_AUTHENTICATION, "JWT");
+ startMetastoreServer();
+ }
+
+ private static void startMetastoreServer() throws Exception {
+ port = MetaStoreTestUtils.startMetaStoreWithRetry(HadoopThriftAuthBridge.getBridge(),
+ conf);
+ MetastoreConf.setVar(conf, ConfVars.THRIFT_URIS, "thrift://localhost:" + port);
+ System.out.println("Starting MetaStore Server on port " + port);
+ }
+
+ @Before
+ public void initEnvMap() {
+ envMap.clear();
+ envMap.putAll(DEFAULTS);
+ }
+
+ private static void setupMockServer() throws Exception {
+ MOCK_JWKS_SERVER.stubFor(get("/jwks")
+ .willReturn(ok()
+ .withBody(Files.readAllBytes(jwtVerificationJWKSFile.toPath()))));
+ MetastoreConf.setVar(conf, ConfVars.THRIFT_METASTORE_AUTHENTICATION_JWT_JWKS_URL,
+ "http://localhost:" + MOCK_JWKS_SERVER_PORT + "/jwks");
+ }
+
+ @Test
+ public void testValidJWT() throws Exception {
+ String validJwtToken = generateJWT(USER_1, jwtAuthorizedKeyFile.toPath(),
+ TimeUnit.MINUTES.toMillis(5));
+ System.getenv().put("HMS_JWT", validJwtToken);
+ String dbName = ("valid_jwt_" + TEST_DB_NAME_PREFIX + "_" + UUID.randomUUID()).toLowerCase();
+ HiveMetaStoreClient client = new HiveMetaStoreClient(conf);
+ try {
+ Database createdDb = new Database();
+ createdDb.setName(dbName);
+ client.createDatabase(createdDb);
+ Database dbFromServer = client.getDatabase(dbName);
+ assertEquals(dbName, dbFromServer.getName());
+ } finally {
+ try {
+ client.dropDatabase(dbName);
+ } catch (Exception e) {
+ LOG.warn("Failed to drop database: " + dbName + ". Error message: " + e);
+ }
+ try {
+ client.close();
+ } catch (Exception e) {
+ LOG.error("Failed to close metastore client");
+ }
+ }
+ }
+
+ @Test(expected = TTransportException.class)
+ public void testExpiredJWT() throws Exception {
+ String validJwtToken = generateJWT(USER_1, jwtAuthorizedKeyFile.toPath(),
+ TimeUnit.MILLISECONDS.toMillis(2));
+ System.getenv().put("HMS_JWT", validJwtToken);
+ String dbName = ("expired_jwt_" + TEST_DB_NAME_PREFIX + "_" + UUID.randomUUID()).toLowerCase();
+ HiveMetaStoreClient client = new HiveMetaStoreClient(conf);
+ try {
+ Thread.sleep(TimeUnit.MILLISECONDS.toMillis(2));
+ Database createdDb = new Database();
+ createdDb.setName(dbName);
+ client.createDatabase(createdDb);
Review Comment:
Yes, that is what I wanted to check initially. However, it looks like THttpClient class (which the above client use as underlying transport) catches the exception from the server and throws TTransportException instead.
--
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: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
[GitHub] [hive] dengzhhu653 commented on a diff in pull request #3233: [WIP]HIVE-26071: JWT authentication mechanism for Thrift over HTTP in HiveMetastore
Posted by GitBox <gi...@apache.org>.
dengzhhu653 commented on code in PR #3233:
URL: https://github.com/apache/hive/pull/3233#discussion_r860498124
##########
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");
Review Comment:
nit: extra empty space
--
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: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
[GitHub] [hive] saihemanth-cloudera commented on a diff in pull request #3233: [WIP]HIVE-26071: JWT authentication mechanism for Thrift over HTTP in HiveMetastore
Posted by GitBox <gi...@apache.org>.
saihemanth-cloudera commented on code in PR #3233:
URL: https://github.com/apache/hive/pull/3233#discussion_r863251645
##########
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;
Review Comment:
How about initializing it to null in L54?
--
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: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
[GitHub] [hive] hsnusonic commented on a diff in pull request #3233: HIVE-26071: JWT authentication mechanism for Thrift over HTTP in HiveMetastore
Posted by GitBox <gi...@apache.org>.
hsnusonic commented on code in PR #3233:
URL: https://github.com/apache/hive/pull/3233#discussion_r868525627
##########
standalone-metastore/metastore-common/src/main/java/org/apache/hadoop/hive/metastore/HiveMetaStoreClient.java:
##########
@@ -79,8 +79,10 @@
import org.apache.hadoop.util.ReflectionUtils;
import org.apache.hadoop.util.StringUtils;
import org.apache.http.HttpException;
+import org.apache.http.HttpHeaders;
import org.apache.http.HttpRequest;
import org.apache.http.HttpRequestInterceptor;
+import org.apache.http.client.utils.HttpClientUtils;
Review Comment:
nit: unused import?
##########
standalone-metastore/metastore-server/pom.xml:
##########
@@ -311,6 +311,22 @@
<artifactId>curator-test</artifactId>
<scope>test</scope>
</dependency>
+ <dependency>
+ <groupId>com.nimbusds</groupId>
+ <artifactId>nimbus-jose-jwt</artifactId>
+ <version>9.20</version>
+ </dependency>
+ <dependency>
+ <groupId>org.pac4j</groupId>
+ <artifactId>pac4j-core</artifactId>
+ <version>4.5.5</version>
+ </dependency>
+ <dependency>
+ <groupId>com.github.tomakehurst</groupId>
+ <artifactId>wiremock-jre8-standalone</artifactId>
+ <version>2.32.0</version>
Review Comment:
Never mind. This is only used in test, so I am OK with it.
--
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: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
[GitHub] [hive] saihemanth-cloudera commented on a diff in pull request #3233: [WIP]HIVE-26071: JWT authentication mechanism for Thrift over HTTP in HiveMetastore
Posted by GitBox <gi...@apache.org>.
saihemanth-cloudera commented on code in PR #3233:
URL: https://github.com/apache/hive/pull/3233#discussion_r863262348
##########
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:
We would only need the servlet header right to extract the username? Can you take out the servlet response argument?
--
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: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
[GitHub] [hive] dengzhhu653 commented on a diff in pull request #3233: [WIP]HIVE-26071: JWT authentication mechanism for Thrift over HTTP in HiveMetastore
Posted by GitBox <gi...@apache.org>.
dengzhhu653 commented on code in PR #3233:
URL: https://github.com/apache/hive/pull/3233#discussion_r867290587
##########
standalone-metastore/metastore-common/src/main/java/org/apache/hadoop/hive/metastore/HiveMetaStoreClient.java:
##########
@@ -42,6 +42,7 @@
import java.util.Iterator;
import java.util.LinkedHashMap;
import java.util.List;
+import java.util.Locale;
Review Comment:
nit: unused import
--
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: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
[GitHub] [hive] sourabh912 commented on a diff in pull request #3233: [WIP]HIVE-26071: JWT authentication mechanism for Thrift over HTTP in HiveMetastore
Posted by GitBox <gi...@apache.org>.
sourabh912 commented on code in PR #3233:
URL: https://github.com/apache/hive/pull/3233#discussion_r866239180
##########
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) {
Review Comment:
@saihemanth-cloudera: HmsThriftHttpServlet is only used by HiveMetastoreServer if HTTP mode is enabled. Therefore HMSClients don't have anything to do with this class.
However I totally agree with the point that we could have added a new constructor. I deliberately choose not to do so because this class was added very recently in Thrift over HTTP patch and since the apis are still evolving, changing the signature of existing constructor made more sense to me.
Let me know your thoughts.
--
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: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
[GitHub] [hive] sourabh912 commented on a diff in pull request #3233: [WIP]HIVE-26071: JWT authentication mechanism for Thrift over HTTP in HiveMetastore
Posted by GitBox <gi...@apache.org>.
sourabh912 commented on code in PR #3233:
URL: https://github.com/apache/hive/pull/3233#discussion_r866222510
##########
standalone-metastore/metastore-server/pom.xml:
##########
@@ -311,6 +311,22 @@
<artifactId>curator-test</artifactId>
<scope>test</scope>
</dependency>
+ <dependency>
+ <groupId>com.nimbusds</groupId>
+ <artifactId>nimbus-jose-jwt</artifactId>
+ <version>9.20</version>
+ </dependency>
+ <dependency>
+ <groupId>org.pac4j</groupId>
+ <artifactId>pac4j-core</artifactId>
+ <version>4.5.5</version>
Review Comment:
Sure. Thanks for the suggestion.
--
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: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
[GitHub] [hive] dengzhhu653 commented on a diff in pull request #3233: [WIP]HIVE-26071: JWT authentication mechanism for Thrift over HTTP in HiveMetastore
Posted by GitBox <gi...@apache.org>.
dengzhhu653 commented on code in PR #3233:
URL: https://github.com/apache/hive/pull/3233#discussion_r860513871
##########
standalone-metastore/metastore-server/src/test/java/org/apache/hadoop/hive/metastore/TestRemoteHiveMetastoreWithHttpJwt.java:
##########
@@ -0,0 +1,269 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.hadoop.hive.metastore;
+
+import com.github.tomakehurst.wiremock.junit.WireMockRule;
+import com.nimbusds.jose.JWSAlgorithm;
+import com.nimbusds.jose.JWSHeader;
+import com.nimbusds.jose.JWSSigner;
+import com.nimbusds.jose.crypto.RSASSASigner;
+import com.nimbusds.jose.jwk.RSAKey;
+import com.nimbusds.jwt.JWTClaimsSet;
+import com.nimbusds.jwt.SignedJWT;
+
+import java.io.File;
+import java.lang.reflect.Field;
+import java.lang.reflect.Modifier;
+import java.nio.file.Files;
+import java.nio.file.Path;
+import java.util.HashMap;
+import java.util.Map;
+import org.apache.hadoop.conf.Configuration;
+import org.apache.hadoop.hive.metastore.annotation.MetastoreUnitTest;
+import org.apache.hadoop.hive.metastore.api.Database;
+import org.apache.hadoop.hive.metastore.conf.MetastoreConf;
+import org.apache.hadoop.hive.metastore.conf.MetastoreConf.ConfVars;
+import org.apache.hadoop.hive.metastore.security.HadoopThriftAuthBridge;
+import org.apache.thrift.transport.TTransportException;
+import org.junit.AfterClass;
+import org.junit.Before;
+import org.junit.BeforeClass;
+import org.junit.ClassRule;
+import java.nio.charset.StandardCharsets;
+import java.util.Date;
+import java.util.UUID;
+import java.util.concurrent.TimeUnit;
+import org.junit.Ignore;
+import org.junit.Test;
+import org.junit.experimental.categories.Category;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import static com.github.tomakehurst.wiremock.client.WireMock.get;
+import static com.github.tomakehurst.wiremock.client.WireMock.ok;
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertTrue;
+
+@Category(MetastoreUnitTest.class)
+public class TestRemoteHiveMetastoreWithHttpJwt {
+ private static final Map<String, String> DEFAULTS = new HashMap<>(System.getenv());
+ private static Map<String, String> envMap;
+
+ private static String baseDir = System.getProperty("basedir");
+ private static final File jwtAuthorizedKeyFile =
+ new File(baseDir,"src/test/resources/auth/jwt/jwt-authorized-key.json");
+ private static final File jwtUnauthorizedKeyFile =
+ new File(baseDir,"src/test/resources/auth/jwt/jwt-unauthorized-key.json");
+ private static final File jwtVerificationJWKSFile =
+ new File(baseDir,"src/test/resources/auth/jwt/jwt-verification-jwks.json");
+
+ private static final String USER_1 = "HMS_TEST_USER_1";
+ private static final String TEST_DB_NAME_PREFIX = "HMS_JWT_AUTH_DB";
+ private static final Logger LOG = LoggerFactory.getLogger(TestRemoteHiveMetastoreWithHttpJwt.class);
+ //private static MiniHS2 miniHS2;
+
+ private static final int MOCK_JWKS_SERVER_PORT = 8089;
+ @ClassRule
+ public static final WireMockRule MOCK_JWKS_SERVER = new WireMockRule(MOCK_JWKS_SERVER_PORT);
+
+ /**
+ * This is a hack to make environment variables modifiable.
+ * Ref: https://stackoverflow.com/questions/318239/how-do-i-set-environment-variables-from-java.
+ */
+ @BeforeClass
+ public static void makeEnvModifiable() throws Exception {
+ envMap = new HashMap<>();
+ Class<?> envClass = Class.forName("java.lang.ProcessEnvironment");
+ Field theEnvironmentField = envClass.getDeclaredField("theEnvironment");
+ Field theUnmodifiableEnvironmentField = envClass.getDeclaredField("theUnmodifiableEnvironment");
+ removeStaticFinalAndSetValue(theEnvironmentField, envMap);
+ removeStaticFinalAndSetValue(theUnmodifiableEnvironmentField, envMap);
+ }
+
+ private static void removeStaticFinalAndSetValue(Field field, Object value) throws Exception {
+ field.setAccessible(true);
+ Field modifiersField = Field.class.getDeclaredField("modifiers");
+ modifiersField.setAccessible(true);
+ modifiersField.setInt(field, field.getModifiers() & ~Modifier.FINAL);
+ field.set(null, value);
+ }
+ private static int port;
+ private static Configuration conf = null;
+
+ public TestRemoteHiveMetastoreWithHttpJwt() {
+ // default constructor
+ }
+
+ @AfterClass
+ public static void stopServices() throws Exception {
+ System.getenv().remove("HMS_JWT");
+ }
+
+ @BeforeClass
+ public static void setUp() throws Exception {
+ conf = MetastoreConf.newMetastoreConf();
+
+ // set some values to use for getting conf. vars
+ MetastoreConf.setBoolVar(conf, ConfVars.METRICS_ENABLED, true);
+ conf.set("datanucleus.autoCreateTables", "false");
+ conf.set("hive.in.test", "true");
+ MetastoreConf.setVar(conf, ConfVars.METASTORE_METADATA_TRANSFORMER_CLASS, " ");
+
+ MetaStoreTestUtils.setConfForStandloneMode(conf);
+ MetastoreConf.setLongVar(conf, ConfVars.BATCH_RETRIEVE_MAX, 2);
+ MetastoreConf.setLongVar(conf, ConfVars.LIMIT_PARTITION_REQUEST, 100);
+ MetastoreConf.setVar(conf, ConfVars.STORAGE_SCHEMA_READER_IMPL, "no.such.class");
+
+ setupMockServer();
+ MetastoreConf.setBoolVar(conf, ConfVars.EXECUTE_SET_UGI, false);
+ MetastoreConf.setVar(conf, ConfVars.THRIFT_TRANSPORT_MODE, "http");
+ MetastoreConf.setVar(conf, ConfVars.METASTORE_CLIENT_THRIFT_TRANSPORT_MODE, "http");
+
+ MetastoreConf.setVar(conf, ConfVars.METASTORE_CLIENT_AUTH_MODE, "JWT");
+ MetastoreConf.setVar(conf, ConfVars.THRIFT_METASTORE_AUTHENTICATION, "JWT");
+ startMetastoreServer();
+ }
+
+ private static void startMetastoreServer() throws Exception {
+ port = MetaStoreTestUtils.startMetaStoreWithRetry(HadoopThriftAuthBridge.getBridge(),
+ conf);
+ MetastoreConf.setVar(conf, ConfVars.THRIFT_URIS, "thrift://localhost:" + port);
+ System.out.println("Starting MetaStore Server on port " + port);
+ }
+
+ @Before
+ public void initEnvMap() {
+ envMap.clear();
+ envMap.putAll(DEFAULTS);
+ }
+
+ private static void setupMockServer() throws Exception {
+ MOCK_JWKS_SERVER.stubFor(get("/jwks")
+ .willReturn(ok()
+ .withBody(Files.readAllBytes(jwtVerificationJWKSFile.toPath()))));
+ MetastoreConf.setVar(conf, ConfVars.THRIFT_METASTORE_AUTHENTICATION_JWT_JWKS_URL,
+ "http://localhost:" + MOCK_JWKS_SERVER_PORT + "/jwks");
+ }
+
+ @Test
+ public void testValidJWT() throws Exception {
+ String validJwtToken = generateJWT(USER_1, jwtAuthorizedKeyFile.toPath(),
+ TimeUnit.MINUTES.toMillis(5));
+ System.getenv().put("HMS_JWT", validJwtToken);
+ String dbName = ("valid_jwt_" + TEST_DB_NAME_PREFIX + "_" + UUID.randomUUID()).toLowerCase();
+ HiveMetaStoreClient client = new HiveMetaStoreClient(conf);
+ try {
+ Database createdDb = new Database();
+ createdDb.setName(dbName);
+ client.createDatabase(createdDb);
+ Database dbFromServer = client.getDatabase(dbName);
+ assertEquals(dbName, dbFromServer.getName());
+ } finally {
+ try {
+ client.dropDatabase(dbName);
+ } catch (Exception e) {
+ LOG.warn("Failed to drop database: " + dbName + ". Error message: " + e);
+ }
+ try {
+ client.close();
+ } catch (Exception e) {
+ LOG.error("Failed to close metastore client");
+ }
+ }
+ }
+
+ @Test(expected = TTransportException.class)
+ public void testExpiredJWT() throws Exception {
+ String validJwtToken = generateJWT(USER_1, jwtAuthorizedKeyFile.toPath(),
+ TimeUnit.MILLISECONDS.toMillis(2));
+ System.getenv().put("HMS_JWT", validJwtToken);
+ String dbName = ("expired_jwt_" + TEST_DB_NAME_PREFIX + "_" + UUID.randomUUID()).toLowerCase();
+ HiveMetaStoreClient client = new HiveMetaStoreClient(conf);
+ try {
+ Thread.sleep(TimeUnit.MILLISECONDS.toMillis(2));
+ Database createdDb = new Database();
+ createdDb.setName(dbName);
+ client.createDatabase(createdDb);
Review Comment:
maybe we should check the exception which is caused by expired token here
--
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: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
[GitHub] [hive] dengzhhu653 commented on a diff in pull request #3233: [WIP]HIVE-26071: JWT authentication mechanism for Thrift over HTTP in HiveMetastore
Posted by GitBox <gi...@apache.org>.
dengzhhu653 commented on code in PR #3233:
URL: https://github.com/apache/hive/pull/3233#discussion_r860532560
##########
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:
how about extractUserName -> validateAndExtractUser?
--
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: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
[GitHub] [hive] dengzhhu653 commented on a diff in pull request #3233: [WIP]HIVE-26071: JWT authentication mechanism for Thrift over HTTP in HiveMetastore
Posted by GitBox <gi...@apache.org>.
dengzhhu653 commented on code in PR #3233:
URL: https://github.com/apache/hive/pull/3233#discussion_r860526502
##########
standalone-metastore/metastore-common/src/main/java/org/apache/hadoop/hive/metastore/HiveMetaStoreClient.java:
##########
@@ -605,27 +608,44 @@ private THttpClient createHttpClient(URI store, boolean useSSL) throws MetaExcep
String path = MetaStoreUtils.getHttpPath(MetastoreConf.getVar(conf, ConfVars.THRIFT_HTTP_PATH));
String httpUrl = (useSSL ? "https://" : "http://") + store.getHost() + ":" + store.getPort() + path;
- String user = MetastoreConf.getVar(conf, ConfVars.METASTORE_CLIENT_PLAIN_USERNAME);
- if (user == null || user.equals("")) {
- try {
- LOG.debug("No username passed in config " + ConfVars.METASTORE_CLIENT_PLAIN_USERNAME.getHiveName() +
- ". Trying to get the current user from UGI" );
- user = UserGroupInformation.getCurrentUser().getShortUserName();
- } catch (IOException e) {
- throw new MetaException("Failed to get client username from UGI");
+ HttpClientBuilder httpClientBuilder = HttpClientBuilder.create();
+ String authType = MetastoreConf.getAsString(conf, ConfVars.METASTORE_CLIENT_AUTH_MODE).toLowerCase(
Review Comment:
nit: the var `METASTORE_CLIENT_AUTH_MODE` is strictly allowed to be some modes, maybe we do not need to localize here?
--
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: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
[GitHub] [hive] sourabh912 commented on a diff in pull request #3233: [WIP]HIVE-26071: JWT authentication mechanism for Thrift over HTTP in HiveMetastore
Posted by GitBox <gi...@apache.org>.
sourabh912 commented on code in PR #3233:
URL: https://github.com/apache/hive/pull/3233#discussion_r866217178
##########
standalone-metastore/metastore-server/pom.xml:
##########
@@ -311,6 +311,22 @@
<artifactId>curator-test</artifactId>
<scope>test</scope>
</dependency>
+ <dependency>
+ <groupId>com.nimbusds</groupId>
+ <artifactId>nimbus-jose-jwt</artifactId>
+ <version>9.20</version>
Review Comment:
@hsnusonic : standalone-metastore's pom does not define Hive pom as its root pom. Are you suggesting to move this version in standalone-metastore's pom.xml ?
--
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: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
[GitHub] [hive] dengzhhu653 commented on a diff in pull request #3233: [WIP]HIVE-26071: JWT authentication mechanism for Thrift over HTTP in HiveMetastore
Posted by GitBox <gi...@apache.org>.
dengzhhu653 commented on code in PR #3233:
URL: https://github.com/apache/hive/pull/3233#discussion_r860514305
##########
standalone-metastore/metastore-server/src/test/java/org/apache/hadoop/hive/metastore/TestRemoteHiveMetastoreWithHttpJwt.java:
##########
@@ -0,0 +1,269 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.hadoop.hive.metastore;
+
+import com.github.tomakehurst.wiremock.junit.WireMockRule;
+import com.nimbusds.jose.JWSAlgorithm;
+import com.nimbusds.jose.JWSHeader;
+import com.nimbusds.jose.JWSSigner;
+import com.nimbusds.jose.crypto.RSASSASigner;
+import com.nimbusds.jose.jwk.RSAKey;
+import com.nimbusds.jwt.JWTClaimsSet;
+import com.nimbusds.jwt.SignedJWT;
+
+import java.io.File;
+import java.lang.reflect.Field;
+import java.lang.reflect.Modifier;
+import java.nio.file.Files;
+import java.nio.file.Path;
+import java.util.HashMap;
+import java.util.Map;
+import org.apache.hadoop.conf.Configuration;
+import org.apache.hadoop.hive.metastore.annotation.MetastoreUnitTest;
+import org.apache.hadoop.hive.metastore.api.Database;
+import org.apache.hadoop.hive.metastore.conf.MetastoreConf;
+import org.apache.hadoop.hive.metastore.conf.MetastoreConf.ConfVars;
+import org.apache.hadoop.hive.metastore.security.HadoopThriftAuthBridge;
+import org.apache.thrift.transport.TTransportException;
+import org.junit.AfterClass;
+import org.junit.Before;
+import org.junit.BeforeClass;
+import org.junit.ClassRule;
+import java.nio.charset.StandardCharsets;
+import java.util.Date;
+import java.util.UUID;
+import java.util.concurrent.TimeUnit;
+import org.junit.Ignore;
+import org.junit.Test;
+import org.junit.experimental.categories.Category;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import static com.github.tomakehurst.wiremock.client.WireMock.get;
+import static com.github.tomakehurst.wiremock.client.WireMock.ok;
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertTrue;
+
+@Category(MetastoreUnitTest.class)
+public class TestRemoteHiveMetastoreWithHttpJwt {
+ private static final Map<String, String> DEFAULTS = new HashMap<>(System.getenv());
+ private static Map<String, String> envMap;
+
+ private static String baseDir = System.getProperty("basedir");
+ private static final File jwtAuthorizedKeyFile =
+ new File(baseDir,"src/test/resources/auth/jwt/jwt-authorized-key.json");
+ private static final File jwtUnauthorizedKeyFile =
+ new File(baseDir,"src/test/resources/auth/jwt/jwt-unauthorized-key.json");
+ private static final File jwtVerificationJWKSFile =
+ new File(baseDir,"src/test/resources/auth/jwt/jwt-verification-jwks.json");
+
+ private static final String USER_1 = "HMS_TEST_USER_1";
+ private static final String TEST_DB_NAME_PREFIX = "HMS_JWT_AUTH_DB";
+ private static final Logger LOG = LoggerFactory.getLogger(TestRemoteHiveMetastoreWithHttpJwt.class);
+ //private static MiniHS2 miniHS2;
+
+ private static final int MOCK_JWKS_SERVER_PORT = 8089;
+ @ClassRule
+ public static final WireMockRule MOCK_JWKS_SERVER = new WireMockRule(MOCK_JWKS_SERVER_PORT);
+
+ /**
+ * This is a hack to make environment variables modifiable.
+ * Ref: https://stackoverflow.com/questions/318239/how-do-i-set-environment-variables-from-java.
+ */
+ @BeforeClass
+ public static void makeEnvModifiable() throws Exception {
+ envMap = new HashMap<>();
+ Class<?> envClass = Class.forName("java.lang.ProcessEnvironment");
+ Field theEnvironmentField = envClass.getDeclaredField("theEnvironment");
+ Field theUnmodifiableEnvironmentField = envClass.getDeclaredField("theUnmodifiableEnvironment");
+ removeStaticFinalAndSetValue(theEnvironmentField, envMap);
+ removeStaticFinalAndSetValue(theUnmodifiableEnvironmentField, envMap);
+ }
+
+ private static void removeStaticFinalAndSetValue(Field field, Object value) throws Exception {
+ field.setAccessible(true);
+ Field modifiersField = Field.class.getDeclaredField("modifiers");
+ modifiersField.setAccessible(true);
+ modifiersField.setInt(field, field.getModifiers() & ~Modifier.FINAL);
+ field.set(null, value);
+ }
+ private static int port;
+ private static Configuration conf = null;
+
+ public TestRemoteHiveMetastoreWithHttpJwt() {
+ // default constructor
+ }
+
+ @AfterClass
+ public static void stopServices() throws Exception {
+ System.getenv().remove("HMS_JWT");
+ }
+
+ @BeforeClass
+ public static void setUp() throws Exception {
+ conf = MetastoreConf.newMetastoreConf();
+
+ // set some values to use for getting conf. vars
+ MetastoreConf.setBoolVar(conf, ConfVars.METRICS_ENABLED, true);
+ conf.set("datanucleus.autoCreateTables", "false");
+ conf.set("hive.in.test", "true");
+ MetastoreConf.setVar(conf, ConfVars.METASTORE_METADATA_TRANSFORMER_CLASS, " ");
+
+ MetaStoreTestUtils.setConfForStandloneMode(conf);
+ MetastoreConf.setLongVar(conf, ConfVars.BATCH_RETRIEVE_MAX, 2);
+ MetastoreConf.setLongVar(conf, ConfVars.LIMIT_PARTITION_REQUEST, 100);
+ MetastoreConf.setVar(conf, ConfVars.STORAGE_SCHEMA_READER_IMPL, "no.such.class");
+
+ setupMockServer();
+ MetastoreConf.setBoolVar(conf, ConfVars.EXECUTE_SET_UGI, false);
+ MetastoreConf.setVar(conf, ConfVars.THRIFT_TRANSPORT_MODE, "http");
+ MetastoreConf.setVar(conf, ConfVars.METASTORE_CLIENT_THRIFT_TRANSPORT_MODE, "http");
+
+ MetastoreConf.setVar(conf, ConfVars.METASTORE_CLIENT_AUTH_MODE, "JWT");
+ MetastoreConf.setVar(conf, ConfVars.THRIFT_METASTORE_AUTHENTICATION, "JWT");
+ startMetastoreServer();
+ }
+
+ private static void startMetastoreServer() throws Exception {
+ port = MetaStoreTestUtils.startMetaStoreWithRetry(HadoopThriftAuthBridge.getBridge(),
+ conf);
+ MetastoreConf.setVar(conf, ConfVars.THRIFT_URIS, "thrift://localhost:" + port);
+ System.out.println("Starting MetaStore Server on port " + port);
+ }
+
+ @Before
+ public void initEnvMap() {
+ envMap.clear();
+ envMap.putAll(DEFAULTS);
+ }
+
+ private static void setupMockServer() throws Exception {
+ MOCK_JWKS_SERVER.stubFor(get("/jwks")
+ .willReturn(ok()
+ .withBody(Files.readAllBytes(jwtVerificationJWKSFile.toPath()))));
+ MetastoreConf.setVar(conf, ConfVars.THRIFT_METASTORE_AUTHENTICATION_JWT_JWKS_URL,
+ "http://localhost:" + MOCK_JWKS_SERVER_PORT + "/jwks");
+ }
+
+ @Test
+ public void testValidJWT() throws Exception {
+ String validJwtToken = generateJWT(USER_1, jwtAuthorizedKeyFile.toPath(),
+ TimeUnit.MINUTES.toMillis(5));
+ System.getenv().put("HMS_JWT", validJwtToken);
+ String dbName = ("valid_jwt_" + TEST_DB_NAME_PREFIX + "_" + UUID.randomUUID()).toLowerCase();
+ HiveMetaStoreClient client = new HiveMetaStoreClient(conf);
+ try {
+ Database createdDb = new Database();
+ createdDb.setName(dbName);
+ client.createDatabase(createdDb);
+ Database dbFromServer = client.getDatabase(dbName);
+ assertEquals(dbName, dbFromServer.getName());
+ } finally {
+ try {
+ client.dropDatabase(dbName);
+ } catch (Exception e) {
+ LOG.warn("Failed to drop database: " + dbName + ". Error message: " + e);
+ }
+ try {
+ client.close();
+ } catch (Exception e) {
+ LOG.error("Failed to close metastore client");
+ }
+ }
+ }
+
+ @Test(expected = TTransportException.class)
+ public void testExpiredJWT() throws Exception {
+ String validJwtToken = generateJWT(USER_1, jwtAuthorizedKeyFile.toPath(),
+ TimeUnit.MILLISECONDS.toMillis(2));
+ System.getenv().put("HMS_JWT", validJwtToken);
+ String dbName = ("expired_jwt_" + TEST_DB_NAME_PREFIX + "_" + UUID.randomUUID()).toLowerCase();
+ HiveMetaStoreClient client = new HiveMetaStoreClient(conf);
+ try {
+ Thread.sleep(TimeUnit.MILLISECONDS.toMillis(2));
+ Database createdDb = new Database();
+ createdDb.setName(dbName);
+ client.createDatabase(createdDb);
+ } catch (InterruptedException e) {
+ // ignore
+ } finally {
+ try {
+ client.close();
+ } catch (Exception e) {
+ LOG.error("Failed to close metastore client");
+ }
+ }
+ }
+
+ @Test(expected = TTransportException.class)
+ public void testInvalidJWT() throws Exception {
+ String jwtToken = generateJWT(USER_1, jwtUnauthorizedKeyFile.toPath(),
+ TimeUnit.MINUTES.toMillis(2));
+ System.getenv().put("HMS_JWT", jwtToken);
+ String dbName = ("invalid_jwt_" + TEST_DB_NAME_PREFIX + "_" + UUID.randomUUID()).toLowerCase();
+ HiveMetaStoreClient client = new HiveMetaStoreClient(conf);
+ try {
+ Thread.sleep(TimeUnit.MILLISECONDS.toMillis(2));
+ Database createdDb = new Database();
+ createdDb.setName(dbName);
+ client.createDatabase(createdDb);
Review Comment:
maybe we should check the exception which is caused by the invalid token here
--
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: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
[GitHub] [hive] hsnusonic commented on a diff in pull request #3233: [WIP]HIVE-26071: JWT authentication mechanism for Thrift over HTTP in HiveMetastore
Posted by GitBox <gi...@apache.org>.
hsnusonic commented on code in PR #3233:
URL: https://github.com/apache/hive/pull/3233#discussion_r861974843
##########
standalone-metastore/metastore-common/src/main/java/org/apache/hadoop/hive/metastore/HiveMetaStoreClient.java:
##########
@@ -605,27 +608,44 @@ private THttpClient createHttpClient(URI store, boolean useSSL) throws MetaExcep
String path = MetaStoreUtils.getHttpPath(MetastoreConf.getVar(conf, ConfVars.THRIFT_HTTP_PATH));
String httpUrl = (useSSL ? "https://" : "http://") + store.getHost() + ":" + store.getPort() + path;
- String user = MetastoreConf.getVar(conf, ConfVars.METASTORE_CLIENT_PLAIN_USERNAME);
- if (user == null || user.equals("")) {
- try {
- LOG.debug("No username passed in config " + ConfVars.METASTORE_CLIENT_PLAIN_USERNAME.getHiveName() +
- ". Trying to get the current user from UGI" );
- user = UserGroupInformation.getCurrentUser().getShortUserName();
- } catch (IOException e) {
- throw new MetaException("Failed to get client username from UGI");
+ HttpClientBuilder httpClientBuilder = HttpClientBuilder.create();
+ String authType = MetastoreConf.getAsString(conf, ConfVars.METASTORE_CLIENT_AUTH_MODE).toLowerCase(
+ Locale.ROOT);
+ String user = null;
Review Comment:
nit: do we need to declare `user` here?
##########
standalone-metastore/metastore-common/src/main/java/org/apache/hadoop/hive/metastore/HiveMetaStoreClient.java:
##########
@@ -605,27 +608,44 @@ private THttpClient createHttpClient(URI store, boolean useSSL) throws MetaExcep
String path = MetaStoreUtils.getHttpPath(MetastoreConf.getVar(conf, ConfVars.THRIFT_HTTP_PATH));
String httpUrl = (useSSL ? "https://" : "http://") + store.getHost() + ":" + store.getPort() + path;
- String user = MetastoreConf.getVar(conf, ConfVars.METASTORE_CLIENT_PLAIN_USERNAME);
- if (user == null || user.equals("")) {
- try {
- LOG.debug("No username passed in config " + ConfVars.METASTORE_CLIENT_PLAIN_USERNAME.getHiveName() +
- ". Trying to get the current user from UGI" );
- user = UserGroupInformation.getCurrentUser().getShortUserName();
- } catch (IOException e) {
- throw new MetaException("Failed to get client username from UGI");
+ HttpClientBuilder httpClientBuilder = HttpClientBuilder.create();
+ String authType = MetastoreConf.getAsString(conf, ConfVars.METASTORE_CLIENT_AUTH_MODE).toLowerCase(
Review Comment:
Why we need `getAsString` instead of `getVar`?
##########
standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/auth/jwt/JWTValidator.java:
##########
@@ -0,0 +1,112 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.hadoop.hive.metastore.auth.jwt;
+
+import com.google.common.base.Preconditions;
+import com.nimbusds.jose.JOSEException;
+import com.nimbusds.jose.JWSHeader;
+import com.nimbusds.jose.JWSObject;
+import com.nimbusds.jose.JWSVerifier;
+import com.nimbusds.jose.crypto.factories.DefaultJWSVerifierFactory;
+import com.nimbusds.jose.jwk.AsymmetricJWK;
+import com.nimbusds.jose.jwk.JWK;
+import com.nimbusds.jwt.JWTClaimsSet;
+import com.nimbusds.jwt.SignedJWT;
+import org.apache.hadoop.conf.Configuration;
+import org.apache.hadoop.hive.metastore.conf.MetastoreConf;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import javax.security.sasl.AuthenticationException;
+import java.io.IOException;
+import java.security.Key;
+import java.text.ParseException;
+import java.util.Date;
+import java.util.List;
+
+/**
+ * This class is used to validate JWT. JWKS is fetched during instantiation and kept in the memory.
+ * We disallow JWT signature verification with symmetric key, because that means anyone can get the same key
+ * and use it to sign a JWT.
+ */
+public class JWTValidator {
Review Comment:
Is it possible to have an unified class `JWTValidator`? If that needs too much effort, can we at least add TODO or create a Jira to track it?
##########
standalone-metastore/metastore-server/pom.xml:
##########
@@ -311,6 +311,22 @@
<artifactId>curator-test</artifactId>
<scope>test</scope>
</dependency>
+ <dependency>
+ <groupId>com.nimbusds</groupId>
+ <artifactId>nimbus-jose-jwt</artifactId>
+ <version>9.20</version>
Review Comment:
We now use this in two packages, can we define the version in root pom's `<dependencyManagement>`?
##########
standalone-metastore/metastore-server/pom.xml:
##########
@@ -311,6 +311,22 @@
<artifactId>curator-test</artifactId>
<scope>test</scope>
</dependency>
+ <dependency>
+ <groupId>com.nimbusds</groupId>
+ <artifactId>nimbus-jose-jwt</artifactId>
+ <version>9.20</version>
+ </dependency>
+ <dependency>
+ <groupId>org.pac4j</groupId>
+ <artifactId>pac4j-core</artifactId>
+ <version>4.5.5</version>
+ </dependency>
+ <dependency>
+ <groupId>com.github.tomakehurst</groupId>
+ <artifactId>wiremock-jre8-standalone</artifactId>
+ <version>2.32.0</version>
Review Comment:
Same as nimbus-jose-jwt, can we define the version in root pom?
--
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: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
[GitHub] [hive] dengzhhu653 commented on a diff in pull request #3233: [WIP]HIVE-26071: JWT authentication mechanism for Thrift over HTTP in HiveMetastore
Posted by GitBox <gi...@apache.org>.
dengzhhu653 commented on code in PR #3233:
URL: https://github.com/apache/hive/pull/3233#discussion_r862198179
##########
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:
oops, seems that the http client would ignore the message of the exception throwing by the servlet, not sure it's intended or not.
or could we add a `HttpResponseInterceptor` to the client and throw authentication exception if meet , which i think will make the exception be friendly to the user?
--
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: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
[GitHub] [hive] dengzhhu653 commented on a diff in pull request #3233: [WIP]HIVE-26071: JWT authentication mechanism for Thrift over HTTP in HiveMetastore
Posted by GitBox <gi...@apache.org>.
dengzhhu653 commented on code in PR #3233:
URL: https://github.com/apache/hive/pull/3233#discussion_r862198179
##########
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:
oops, seems that the http client would ignore the exception throwing by the servlet, not sure it's intended or not.
or could we add a `HttpResponseInterceptor` to the client and throw authentication exception if meet , which i think will make the exception be friendly to the user?
--
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: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
[GitHub] [hive] saihemanth-cloudera commented on a diff in pull request #3233: [WIP]HIVE-26071: JWT authentication mechanism for Thrift over HTTP in HiveMetastore
Posted by GitBox <gi...@apache.org>.
saihemanth-cloudera commented on code in PR #3233:
URL: https://github.com/apache/hive/pull/3233#discussion_r863247389
##########
standalone-metastore/metastore-server/pom.xml:
##########
@@ -311,6 +311,22 @@
<artifactId>curator-test</artifactId>
<scope>test</scope>
</dependency>
+ <dependency>
+ <groupId>com.nimbusds</groupId>
+ <artifactId>nimbus-jose-jwt</artifactId>
+ <version>9.20</version>
+ </dependency>
+ <dependency>
+ <groupId>org.pac4j</groupId>
+ <artifactId>pac4j-core</artifactId>
+ <version>4.5.5</version>
Review Comment:
Can you define the versions at the top instead of hard-coding them here?
--
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: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
[GitHub] [hive] sourabh912 commented on a diff in pull request #3233: [WIP]HIVE-26071: JWT authentication mechanism for Thrift over HTTP in HiveMetastore
Posted by GitBox <gi...@apache.org>.
sourabh912 commented on code in PR #3233:
URL: https://github.com/apache/hive/pull/3233#discussion_r862085584
##########
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:
THttpClient hides all the error messages received from server and throws TTransportException. Sharing the code snippet from THttpClient
```
if (responseCode != 200) {
throw new TTransportException("HTTP Response code: " + responseCode);
}
```
--
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: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
[GitHub] [hive] nrg4878 commented on pull request #3233: [WIP]HIVE-26071: JWT authentication mechanism for Thrift over HTTP in HiveMetastore
Posted by GitBox <gi...@apache.org>.
nrg4878 commented on PR #3233:
URL: https://github.com/apache/hive/pull/3233#issuecomment-1111028342
@dengzhhu653 @yongzhi @harishjp Could you please review this PR please? Thank you
--
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: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
[GitHub] [hive] saihemanth-cloudera commented on a diff in pull request #3233: [WIP]HIVE-26071: JWT authentication mechanism for Thrift over HTTP in HiveMetastore
Posted by GitBox <gi...@apache.org>.
saihemanth-cloudera commented on code in PR #3233:
URL: https://github.com/apache/hive/pull/3233#discussion_r863248572
##########
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) {
Review Comment:
Adding a new argument would break the compatibility/interoperability between HMS and clients. Instead, wouldn't it be ideal to add a new constructor with an added argument?
--
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: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
[GitHub] [hive] sourabh912 commented on a diff in pull request #3233: [WIP]HIVE-26071: JWT authentication mechanism for Thrift over HTTP in HiveMetastore
Posted by GitBox <gi...@apache.org>.
sourabh912 commented on code in PR #3233:
URL: https://github.com/apache/hive/pull/3233#discussion_r862015821
##########
standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/HttpAuthenticationException.java:
##########
@@ -0,0 +1,43 @@
+/*
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License. See accompanying LICENSE file.
+ */
+
+package org.apache.hadoop.hive.metastore;
Review Comment:
Yes, that is better. Will move this class into auth package.
--
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: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
[GitHub] [hive] sourabh912 commented on a diff in pull request #3233: [WIP]HIVE-26071: JWT authentication mechanism for Thrift over HTTP in HiveMetastore
Posted by GitBox <gi...@apache.org>.
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: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
[GitHub] [hive] saihemanth-cloudera commented on a diff in pull request #3233: [WIP]HIVE-26071: JWT authentication mechanism for Thrift over HTTP in HiveMetastore
Posted by GitBox <gi...@apache.org>.
saihemanth-cloudera commented on code in PR #3233:
URL: https://github.com/apache/hive/pull/3233#discussion_r866317420
##########
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) {
Review Comment:
@sourabh912 - I didn't know that this class was recently introduced in the thrift over http patch. In that case, it would be ideal to leave this as such with the existing constructor. We can always do constructor overloading in the future if any use case arises.
--
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: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
[GitHub] [hive] sourabh912 commented on a diff in pull request #3233: [WIP]HIVE-26071: JWT authentication mechanism for Thrift over HTTP in HiveMetastore
Posted by GitBox <gi...@apache.org>.
sourabh912 commented on code in PR #3233:
URL: https://github.com/apache/hive/pull/3233#discussion_r864369423
##########
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:
@dengzhhu653 : It is a good idea. I tried the approach you suggested. By defining HttpResponseInterceptor I was able to get the error message from the response input stream. If we want to surface it in `TTransportException` thrown by THttpClient, there does not seem to be an easy way. We would probably have to extend THttpClient class and override some of its methods (eg flush()) to capture the error message in the inputstream. Is it ok if we can explore it in a follow up JIRA? Let me know and I will create one.
--
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: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
[GitHub] [hive] sourabh912 commented on a diff in pull request #3233: [WIP]HIVE-26071: JWT authentication mechanism for Thrift over HTTP in HiveMetastore
Posted by GitBox <gi...@apache.org>.
sourabh912 commented on code in PR #3233:
URL: https://github.com/apache/hive/pull/3233#discussion_r866217881
##########
standalone-metastore/metastore-server/pom.xml:
##########
@@ -311,6 +311,22 @@
<artifactId>curator-test</artifactId>
<scope>test</scope>
</dependency>
+ <dependency>
+ <groupId>com.nimbusds</groupId>
+ <artifactId>nimbus-jose-jwt</artifactId>
+ <version>9.20</version>
+ </dependency>
+ <dependency>
+ <groupId>org.pac4j</groupId>
+ <artifactId>pac4j-core</artifactId>
+ <version>4.5.5</version>
+ </dependency>
+ <dependency>
+ <groupId>com.github.tomakehurst</groupId>
+ <artifactId>wiremock-jre8-standalone</artifactId>
+ <version>2.32.0</version>
Review Comment:
Same question as [above](https://github.com/apache/hive/pull/3233#discussion_r866217178) .
--
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: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
[GitHub] [hive] saihemanth-cloudera commented on a diff in pull request #3233: [WIP]HIVE-26071: JWT authentication mechanism for Thrift over HTTP in HiveMetastore
Posted by GitBox <gi...@apache.org>.
saihemanth-cloudera commented on code in PR #3233:
URL: https://github.com/apache/hive/pull/3233#discussion_r863250675
##########
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) {
Review Comment:
Adding a new argument to the constructor would break the compatibility/inter-operability between HMS and clients. Instead, wouldn't it be ideal to add a new constructor with an added argument?
--
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: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
[GitHub] [hive] yongzhi merged pull request #3233: HIVE-26071: JWT authentication mechanism for Thrift over HTTP in HiveMetastore
Posted by GitBox <gi...@apache.org>.
yongzhi merged PR #3233:
URL: https://github.com/apache/hive/pull/3233
--
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: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
[GitHub] [hive] dengzhhu653 commented on a diff in pull request #3233: [WIP]HIVE-26071: JWT authentication mechanism for Thrift over HTTP in HiveMetastore
Posted by GitBox <gi...@apache.org>.
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 end user 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: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
[GitHub] [hive] dengzhhu653 commented on pull request #3233: [WIP]HIVE-26071: JWT authentication mechanism for Thrift over HTTP in HiveMetastore
Posted by GitBox <gi...@apache.org>.
dengzhhu653 commented on PR #3233:
URL: https://github.com/apache/hive/pull/3233#issuecomment-1111796249
> @dengzhhu653 @yongzhi @harishjp Could you please review this PR please? Thank you
Thank you for informing me. Some minor comments, others look good to me!
--
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: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
[GitHub] [hive] sourabh912 commented on a diff in pull request #3233: [WIP]HIVE-26071: JWT authentication mechanism for Thrift over HTTP in HiveMetastore
Posted by GitBox <gi...@apache.org>.
sourabh912 commented on code in PR #3233:
URL: https://github.com/apache/hive/pull/3233#discussion_r862038799
##########
standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/auth/jwt/JWTValidator.java:
##########
@@ -0,0 +1,112 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.hadoop.hive.metastore.auth.jwt;
+
+import com.google.common.base.Preconditions;
+import com.nimbusds.jose.JOSEException;
+import com.nimbusds.jose.JWSHeader;
+import com.nimbusds.jose.JWSObject;
+import com.nimbusds.jose.JWSVerifier;
+import com.nimbusds.jose.crypto.factories.DefaultJWSVerifierFactory;
+import com.nimbusds.jose.jwk.AsymmetricJWK;
+import com.nimbusds.jose.jwk.JWK;
+import com.nimbusds.jwt.JWTClaimsSet;
+import com.nimbusds.jwt.SignedJWT;
+import org.apache.hadoop.conf.Configuration;
+import org.apache.hadoop.hive.metastore.conf.MetastoreConf;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import javax.security.sasl.AuthenticationException;
Review Comment:
Yes, thanks for catching it. Will fix it.
--
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: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org