hsnusonic commented on a change in pull request #3006:
URL: https://github.com/apache/hive/pull/3006#discussion_r817058633
##########
File path:
service/src/java/org/apache/hive/service/cli/thrift/ThriftHttpServlet.java
##########
@@ -213,6 +221,8 @@ protected void doPost(HttpServletRequest request,
HttpServletResponse response)
} else {
clientUserName = doKerberosAuth(request);
}
+ } else if (authType.isEnabled(HiveAuthConstants.AuthTypes.JWT) &&
hasJWT(request)) {
Review comment:
We now support multi authentication at a time. If SAML and JWT are both
enabled, the bearer token is possible for either of them so we need to
distinguish which one should be used.
##########
File path: common/src/java/org/apache/hadoop/hive/conf/HiveConf.java
##########
@@ -4127,7 +4127,8 @@ private static void populateLlapDaemonVarsSet(Set<String>
llapDaemonVarsSetLocal
" (Use with property
hive.server2.custom.authentication.class)\n" +
" PAM: Pluggable authentication module\n" +
" NOSASL: Raw transport\n" +
- " SAML: SAML 2.0 compliant authentication. This is only supported in
http transport mode."),
+ " SAML: SAML 2.0 compliant authentication. This is only supported in
http transport mode\n" +
+ " JWT: JWT based authentication, JWT needs to contain the user as
subject"),
Review comment:
Yes, it is. I will add a supplement here.
##########
File path:
service/src/java/org/apache/hive/service/cli/thrift/ThriftHttpServlet.java
##########
@@ -264,8 +274,7 @@ protected void doPost(HttpServletRequest request,
HttpServletResponse response)
LOG.info("Cookie added for clientUserName " + clientUserName);
Review comment:
This is interesting. It seems the same scenario is applicable to all
other authentications. HS2 will keep a authenticated user to have access for
the max age of cookie, one day by default. I'm not sure if we need to change
how we set the max age of cookie. If we need, maybe change it for all the
authentications in another ticket?
##########
File path: jdbc/src/java/org/apache/hive/jdbc/HiveConnection.java
##########
@@ -804,6 +811,34 @@ protected boolean requestIsAborted(final HttpRequest
request) {
return httpClientBuilder.build();
}
+ private String getJWT() {
+ String jwtCredential = getJWTStringFromSession();
+ if (jwtCredential == null) {
+ jwtCredential = getJWTStringFromEnv();
+ }
+ return jwtCredential;
+ }
+
+ private String getJWTStringFromEnv() {
+ String jwtCredential = System.getenv(JdbcConnectionParams.AUTH_JWT_ENV);
+ if (jwtCredential == null) {
+ LOG.debug("No JWT is specified in env variable {}",
JdbcConnectionParams.AUTH_JWT_ENV);
+ } else {
+ LOG.debug("Fetched JWT from the env.");
+ }
+ return jwtCredential;
+ }
+
+ private String getJWTStringFromSession() {
+ String jwtCredential =
sessConfMap.get(JdbcConnectionParams.AUTH_TYPE_JWT_KEY);
+ if (jwtCredential == null) {
+ LOG.debug("No JWT is specified in connection string.");
+ } else {
+ LOG.debug("Fetched JWT from the session.");
Review comment:
I assumed we should treat token like password, so I chose not to print
it out. Thoughts?
##########
File path: jdbc/src/java/org/apache/hive/jdbc/HiveConnection.java
##########
@@ -804,6 +811,34 @@ protected boolean requestIsAborted(final HttpRequest
request) {
return httpClientBuilder.build();
}
+ private String getJWT() {
+ String jwtCredential = getJWTStringFromSession();
+ if (jwtCredential == null) {
Review comment:
If the JWT is passed explicitly to the connection string, we should just
use it no matter what it looks like, right? Do you think we should check it in
driver side? For example,
`jdbc:hive2://<hostname>/default;transportMode=http;httpPath=cliservice;jwt=;`.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
