joshelser commented on a change in pull request #4064:
URL: https://github.com/apache/hbase/pull/4064#discussion_r805997186
##########
File path:
hbase-client/src/main/java/org/apache/hadoop/hbase/security/token/OAuthBearerTokenUtil.java
##########
@@ -68,8 +74,44 @@ public static void addTokenForUser(User user, String
encodedToken, long lifetime
}
};
subject.getPrivateCredentials().add(jwt);
+ if (LOG.isDebugEnabled()) {
+ LOG.debug("JWT token has been added to user credentials with expiry
{}",
+ lifetimeMs == 0 ? "0" :
Instant.ofEpochMilli(lifetimeMs).toString());
+ }
return null;
}
});
}
+
+ /**
+ * Check whether an OAuth Beaerer token is provided in environment variable
HADOOP_JWT.
Review comment:
```suggestion
* Check whether an OAuth Bearer token is provided in environment variable
HBASE_JWT.
```
##########
File path:
hbase-common/src/main/java/org/apache/hadoop/hbase/security/oauthbearer/OAuthBearerUtils.java
##########
@@ -25,6 +25,7 @@
@InterfaceAudience.Private
public final class OAuthBearerUtils {
public static final String OAUTHBEARER_MECHANISM = "OAUTHBEARER";
+ public static final String TOKEN_KIND = "OAUTHBEARER_AUTH_TOKEN";
Review comment:
Let's put "HBASE" in this variable too. It can be helpful if we're
diagnosing an HBase mapreduce job (or other) and we can tell that a job
obviously has an HBase token ("OAUTHBEARER" wouldn't tell us for sure where it
was coming from). I'd suggest `HBASE_OAUTHBEARER_TOKEN` or just
`HBASE_JWT_TOKEN`
##########
File path:
hbase-client/src/main/java/org/apache/hadoop/hbase/client/ConnectionFactory.java
##########
@@ -70,7 +70,11 @@
@InterfaceAudience.Public
public class ConnectionFactory {
- public static final String HBASE_CLIENT_ASYNC_CONNECTION_IMPL =
"hbase.client.async.connection.impl";
+ public static final String HBASE_CLIENT_ASYNC_CONNECTION_IMPL =
+ "hbase.client.async.connection.impl";
+
+ /** Environment variable for OAuth Bearer token */
+ public static final String ENV_OAUTHBEARER_TOKEN = "HADOOP_JWT";
Review comment:
Should be `HBASE_JWT`, no? :)
##########
File path:
hbase-client/src/main/java/org/apache/hadoop/hbase/security/token/OAuthBearerTokenUtil.java
##########
@@ -68,8 +74,44 @@ public static void addTokenForUser(User user, String
encodedToken, long lifetime
}
};
subject.getPrivateCredentials().add(jwt);
+ if (LOG.isDebugEnabled()) {
+ LOG.debug("JWT token has been added to user credentials with expiry
{}",
+ lifetimeMs == 0 ? "0" :
Instant.ofEpochMilli(lifetimeMs).toString());
+ }
return null;
}
});
}
+
+ /**
+ * Check whether an OAuth Beaerer token is provided in environment variable
HADOOP_JWT.
+ * Parse and add it to user private credentials, but only if another token
is not already present.
+ */
+ public static void addTokenFromEnvironmentVar(User user, String token) {
+ Optional<Token<?>> oauthBearerToken = user.getTokens().stream()
+ .filter((t) -> new Text(OAuthBearerUtils.TOKEN_KIND).equals(t.getKind()))
+ .findFirst();
+
+ if (oauthBearerToken.isPresent()) {
+ return;
+ }
+
+ String[] tokens = token.split(",");
+ if (StringUtils.isEmpty(tokens[0])) {
+ return;
+ }
+ long lifetimeMs = 0;
+ if (tokens.length > 1) {
+ try {
+ ZonedDateTime lifetime = ZonedDateTime.parse(tokens[1]);
+ lifetimeMs = lifetime.toInstant().toEpochMilli();
+ } catch (DateTimeParseException e) {
+ LOG.warn("Unable to parse JWT expiry: {}", tokens[1]);
Review comment:
Reading `addTokenForUser(String, Token, long)`, it seems like we don't
_need_ to have a lifetime, which matches the warning (not failure) here. Lazy
question: do we have an "expectation" on how it should work? Like, does Knox
always give us a lifetime? What about the nimbus test server?
If the jwt server doesn't give us a lifetime, do we have any kind of loss of
functionality on the HBase client side?
##########
File path:
hbase-client/src/main/java/org/apache/hadoop/hbase/security/token/OAuthBearerTokenUtil.java
##########
@@ -68,8 +74,44 @@ public static void addTokenForUser(User user, String
encodedToken, long lifetime
}
};
subject.getPrivateCredentials().add(jwt);
+ if (LOG.isDebugEnabled()) {
+ LOG.debug("JWT token has been added to user credentials with expiry
{}",
+ lifetimeMs == 0 ? "0" :
Instant.ofEpochMilli(lifetimeMs).toString());
+ }
return null;
}
});
}
+
+ /**
+ * Check whether an OAuth Beaerer token is provided in environment variable
HADOOP_JWT.
+ * Parse and add it to user private credentials, but only if another token
is not already present.
+ */
+ public static void addTokenFromEnvironmentVar(User user, String token) {
+ Optional<Token<?>> oauthBearerToken = user.getTokens().stream()
+ .filter((t) -> new Text(OAuthBearerUtils.TOKEN_KIND).equals(t.getKind()))
+ .findFirst();
+
+ if (oauthBearerToken.isPresent()) {
+ return;
Review comment:
Let's add a `LOG.warn()` here
##########
File path:
hbase-client/src/test/java/org/apache/hadoop/hbase/security/token/TestOAuthBearerTokenUtil.java
##########
@@ -0,0 +1,145 @@
+/**
+ * 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.hbase.security.token;
+
+import static
org.apache.hadoop.hbase.security.oauthbearer.OAuthBearerUtils.TOKEN_KIND;
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertTrue;
+import java.security.AccessController;
+import java.security.PrivilegedAction;
+import java.time.Instant;
+import java.util.Optional;
+import java.util.Set;
+import javax.security.auth.Subject;
+import org.apache.hadoop.hbase.HBaseConfiguration;
+import org.apache.hadoop.hbase.security.User;
+import org.apache.hadoop.hbase.security.oauthbearer.OAuthBearerToken;
+import org.apache.hadoop.io.Text;
+import org.apache.hadoop.security.token.Token;
+import org.junit.Test;
+
+public class TestOAuthBearerTokenUtil {
+
+ @Test
+ public void testAddTokenFromEnvVar() {
+ // Arrange
+ User user = User.createUserForTesting(new HBaseConfiguration(),
"testuser", new String[] {});
+ String testToken =
"some_base64_encoded_stuff,2022-01-25T16:59:48.614000+00:00";
+
+ // Act
+ OAuthBearerTokenUtil.addTokenFromEnvironmentVar(user, testToken);
+
+ // Assert
+ Optional<Token<?>> oauthBearerToken = user.getTokens().stream()
+ .filter((t) -> new Text(TOKEN_KIND).equals(t.getKind()))
+ .findFirst();
+ assertTrue("Token cannot be found in user tokens",
oauthBearerToken.isPresent());
+ user.runAs(new PrivilegedAction<Object>() {
+ @Override public Object run() {
+ Subject subject = Subject.getSubject(AccessController.getContext());
+ Set<OAuthBearerToken> tokens =
subject.getPrivateCredentials(OAuthBearerToken.class);
+ assertFalse("Token cannot be found in subject's private credentials",
tokens.isEmpty());
+ OAuthBearerToken jwt = tokens.iterator().next();
+ assertEquals("Invalid encoded JWT value", "some_base64_encoded_stuff",
jwt.value());
+ assertEquals("Invalid JWT expiry", "2022-01-25T16:59:48.614Z",
+ Instant.ofEpochMilli(jwt.lifetimeMs()).toString());
+ return null;
+ }
+ });
+ }
+
+ @Test
+ public void testAddTokenEnvVarWithoutExpiry() {
+ // Arrange
+ User user = User.createUserForTesting(new HBaseConfiguration(),
"testuser", new String[] {});
+ String testToken = "some_base64_encoded_stuff";
+
+ // Act
+ OAuthBearerTokenUtil.addTokenFromEnvironmentVar(user, testToken);
+
+ // Assert
+ Optional<Token<?>> oauthBearerToken = user.getTokens().stream()
+ .filter((t) -> new Text(TOKEN_KIND).equals(t.getKind()))
+ .findFirst();
+ assertTrue("Token cannot be found in user tokens",
oauthBearerToken.isPresent());
+ user.runAs(new PrivilegedAction<Object>() {
+ @Override public Object run() {
+ Subject subject = Subject.getSubject(AccessController.getContext());
+ Set<OAuthBearerToken> tokens =
subject.getPrivateCredentials(OAuthBearerToken.class);
+ assertFalse("Token cannot be found in subject's private credentials",
tokens.isEmpty());
+ OAuthBearerToken jwt = tokens.iterator().next();
+ assertEquals("Invalid encoded JWT value", "some_base64_encoded_stuff",
jwt.value());
+ assertEquals("Invalid JWT expiry", 0, jwt.lifetimeMs());
+ return null;
+ }
+ });
+ }
+
+ @Test
+ public void testAddTokenEnvVarWithInvalidExpiry() {
+ // Arrange
+ User user = User.createUserForTesting(new HBaseConfiguration(),
"testuser", new String[] {});
+ String testToken = "some_base64_encoded_stuff,foobarblahblah328742";
+
+ // Act
+ OAuthBearerTokenUtil.addTokenFromEnvironmentVar(user, testToken);
+
+ // Assert
+ Optional<Token<?>> oauthBearerToken = user.getTokens().stream()
+ .filter((t) -> new Text(TOKEN_KIND).equals(t.getKind()))
+ .findFirst();
+ assertTrue("Token cannot be found in user tokens",
oauthBearerToken.isPresent());
+ user.runAs(new PrivilegedAction<Object>() {
+ @Override public Object run() {
+ Subject subject = Subject.getSubject(AccessController.getContext());
+ Set<OAuthBearerToken> tokens =
subject.getPrivateCredentials(OAuthBearerToken.class);
+ assertFalse("Token cannot be found in subject's private credentials",
tokens.isEmpty());
+ OAuthBearerToken jwt = tokens.iterator().next();
+ assertEquals("Invalid encoded JWT value", "some_base64_encoded_stuff",
jwt.value());
+ assertEquals("Invalid JWT expiry", 0, jwt.lifetimeMs());
+ return null;
+ }
+ });
+ }
+
+ @Test
+ public void testAddTokenEnvVarTokenAlreadyPresent() {
+ // Arrange
+ User user = User.createUserForTesting(new HBaseConfiguration(),
"testuser", new String[] {});
+ user.addToken(new Token<>(null, null, new Text(TOKEN_KIND), null));
+ String testToken = "some_base64_encoded_stuff,foobarblahblah328742";
+
+ // Act
+ OAuthBearerTokenUtil.addTokenFromEnvironmentVar(user, testToken);
+
+ // Assert
+ long numberOfTokens = user.getTokens().stream()
+ .filter((t) -> new Text(TOKEN_KIND).equals(t.getKind()))
+ .count();
+ assertEquals("Invalid number of tokens on User",1, numberOfTokens);
Review comment:
```suggestion
assertEquals("Invalid number of tokens on User", 1, numberOfTokens);
```
##########
File path:
hbase-client/src/test/java/org/apache/hadoop/hbase/security/token/TestOAuthBearerTokenUtil.java
##########
@@ -0,0 +1,145 @@
+/**
+ * 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.hbase.security.token;
+
+import static
org.apache.hadoop.hbase.security.oauthbearer.OAuthBearerUtils.TOKEN_KIND;
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertTrue;
+import java.security.AccessController;
+import java.security.PrivilegedAction;
+import java.time.Instant;
+import java.util.Optional;
+import java.util.Set;
+import javax.security.auth.Subject;
+import org.apache.hadoop.hbase.HBaseConfiguration;
+import org.apache.hadoop.hbase.security.User;
+import org.apache.hadoop.hbase.security.oauthbearer.OAuthBearerToken;
+import org.apache.hadoop.io.Text;
+import org.apache.hadoop.security.token.Token;
+import org.junit.Test;
+
+public class TestOAuthBearerTokenUtil {
+
+ @Test
+ public void testAddTokenFromEnvVar() {
+ // Arrange
+ User user = User.createUserForTesting(new HBaseConfiguration(),
"testuser", new String[] {});
Review comment:
```suggestion
User user = User.createUserForTesting(HBaseConfiguration.create(),
"testuser", new String[] {});
```
--
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]