okumin commented on code in PR #6086:
URL: https://github.com/apache/hive/pull/6086#discussion_r2357384903
##########
standalone-metastore/metastore-rest-catalog/src/main/java/org/apache/iceberg/rest/HMSCatalogAdapter.java:
##########
@@ -127,8 +117,6 @@ public HMSCatalogAdapter(Catalog catalog) {
}
enum Route {
- TOKENS(HTTPMethod.POST, "v1/oauth/tokens", null),
Review Comment:
The initial specification of Iceberg REST allows the Iceberg REST Catalog to
act as an Authorization Server, which is why the list contains this endpoint.
As this does not follow security best practices, [it will be
removed](https://github.com/apache/iceberg/blob/7eef46da0d00edb79b20d24cb7806f6ea8da4000/open-api/rest-catalog-open-api.yaml#L180-L181).
##########
standalone-metastore/metastore-server/src/test/java/org/apache/hadoop/hive/metastore/auth/oauth2/TestTokenIntrospectionAuthenticator.java:
##########
@@ -0,0 +1,307 @@
+/*
+ * 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
+ * <p>
+ * http://www.apache.org/licenses/LICENSE-2.0
+ * <p>
+ * 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.oauth2;
+
+import com.nimbusds.oauth2.sdk.auth.ClientSecretBasic;
+import com.nimbusds.oauth2.sdk.auth.Secret;
+import com.nimbusds.oauth2.sdk.id.Audience;
+import com.nimbusds.oauth2.sdk.id.ClientID;
+import java.net.URI;
+import java.net.URISyntaxException;
+import java.time.Duration;
+import java.util.Collections;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+import java.util.Optional;
+import java.util.concurrent.TimeUnit;
+import java.util.regex.Pattern;
+import org.apache.hadoop.hive.metastore.annotation.MetastoreUnitTest;
+import org.apache.hadoop.hive.metastore.auth.HttpAuthenticationException;
+import org.junit.AfterClass;
+import org.junit.Assert;
+import org.junit.BeforeClass;
+import org.junit.Test;
+import org.junit.experimental.categories.Category;
+import org.keycloak.OAuth2Constants;
+import org.keycloak.admin.client.Keycloak;
+import org.keycloak.admin.client.KeycloakBuilder;
+import org.keycloak.admin.client.resource.RealmResource;
+import org.keycloak.representations.idm.ClientRepresentation;
+import org.keycloak.representations.idm.ClientScopeRepresentation;
+import org.keycloak.representations.idm.ProtocolMapperRepresentation;
+import org.keycloak.representations.idm.RealmRepresentation;
+import org.testcontainers.containers.GenericContainer;
+import org.testcontainers.utility.DockerImageName;
+
+@Category(MetastoreUnitTest.class)
+public class TestTokenIntrospectionAuthenticator {
+ private static final String AUDIENCE = "http://localhost:8081/hive";;
+ private static final String USERNAME = "test-user";
+ private static final List<String> SCOPES = List.of("read", "update");
+ private static final ClientSecretBasic RESOURCE_SERVER_CREDENTIAL = new
ClientSecretBasic(
+ new ClientID("hive-resource-server-id"), new
Secret("hive-resource-server-secret"));
+ private static final OAuth2PrincipalMapper PRINCIPAL_MAPPER = new
RegexOAuth2PrincipalMapper("email",
+ Pattern.compile("(.*)@example.com"));
+
+ private static GenericContainer<?> container;
+ private static URI INTROSPECTION_ENDPOINT;
+ private static String ACCESS_TOKEN;
+ private static String ACCESS_TOKEN_EXPIRED;
+ private static String ACCESS_TOKEN_WITH_WRONG_ISSUER;
+ private static String ACCESS_TOKEN_WITH_WRONG_AUDIENCE;
+ private static String ACCESS_TOKEN_WITH_MISSING_SCOPE;
+ private static String ACCESS_TOKEN_WITH_INSUFFICIENT_SCOPE;
+
+ private static RealmResource createRealm(Keycloak keycloak, String
realmName) {
+ var realm = new RealmRepresentation();
+ realm.setRealm(realmName);
+ realm.setEnabled(true);
+ keycloak.realms().create(realm);
+ return keycloak.realm(realmName);
+ }
+
+ private static void createResourceServer(RealmResource realm) {
+ var resourceServer = new ClientRepresentation();
+
resourceServer.setClientId(RESOURCE_SERVER_CREDENTIAL.getClientID().getValue());
+
resourceServer.setSecret(RESOURCE_SERVER_CREDENTIAL.getClientSecret().getValue());
+ resourceServer.setEnabled(true);
+ resourceServer.setProtocol("openid-connect");
+ resourceServer.setPublicClient(false);
+ resourceServer.setServiceAccountsEnabled(true);
+ resourceServer.setAuthorizationServicesEnabled(true);
+ realm.clients().create(resourceServer).close();
+ }
+
+ private static void createScope(RealmResource realm, String name) {
+ var scope = new ClientScopeRepresentation();
+ scope.setName(name);
+ scope.setProtocol("openid-connect");
+ realm.clientScopes().create(scope).close();
+ }
+
+ private static ProtocolMapperRepresentation createAudience(String audience) {
+ var aud = new ProtocolMapperRepresentation();
+ aud.setName("audience");
+ aud.setProtocol("openid-connect");
+ aud.setProtocolMapper("oidc-audience-mapper");
+ aud.setConfig(Map.of(
+ "included.custom.audience", audience,
+ "access.token.claim", "true"
+ ));
+ return aud;
+ }
+
+ private static ProtocolMapperRepresentation createEmailClaim() {
+ var mapper = new ProtocolMapperRepresentation();
+ mapper.setName("email");
+ mapper.setProtocol("openid-connect");
+ mapper.setProtocolMapper("oidc-hardcoded-claim-mapper");
+ mapper.setConfig(Map.of(
+ "claim.name", "email",
+ "claim.value", USERNAME + "@example.com",
+ "jsonType.label", "String",
+ "access.token.claim", "true"
+ ));
+ return mapper;
+ }
+
+ private static void createClient(RealmResource realm, String clientId,
String clientSecret, List<String> scopes,
+ List<ProtocolMapperRepresentation> protocolMappers) {
+ createClient(realm, clientId, clientSecret, scopes, protocolMappers,
Collections.emptyMap());
+ }
+
+ private static void createClient(RealmResource realm, String clientId,
String clientSecret, List<String> scopes,
+ List<ProtocolMapperRepresentation> protocolMappers, Map<String, String>
additionalAttributes) {
+ var client = new ClientRepresentation();
+ client.setClientId(clientId);
+ client.setSecret(clientSecret);
+ client.setEnabled(true);
+ client.setProtocol("openid-connect");
+ client.setPublicClient(false);
+ client.setServiceAccountsEnabled(true);
+ client.setOptionalClientScopes(scopes);
+ var attributes = new HashMap<>(additionalAttributes);
+ attributes.put("access.token.header.type.rfc9068", "true");
+ client.setAttributes(attributes);
+ client.setProtocolMappers(protocolMappers);
+ realm.clients().create(client).close();
+ }
+
+ private static String getAccessToken(String url, String realm, String
clientId, String clientSecret,
+ List<String> scopes) {
+ try (var keycloak = KeycloakBuilder.builder()
+ .serverUrl(url)
+ .realm(realm)
+ .clientId(clientId)
+ .clientSecret(clientSecret)
+ .scope(scopes == null ? null : String.join(" ", scopes))
+ .grantType(OAuth2Constants.CLIENT_CREDENTIALS)
+ .build()) {
+ return keycloak.tokenManager().getAccessTokenString();
+ }
+ }
+
+ @BeforeClass
+ public static void setup() throws URISyntaxException, InterruptedException {
+ container = new
GenericContainer<>(DockerImageName.parse("quay.io/keycloak/keycloak:26.3.4"))
+ .withEnv("KEYCLOAK_ADMIN","admin")
+ .withEnv("KEYCLOAK_ADMIN_PASSWORD","admin")
+ .withCommand("start-dev")
+ .withExposedPorts(8080);
Review Comment:
I initially tried to use `spring-security-oauth2-authorization-server`, but
gave it up because it has a hard dependency conflict on spring-core derived
from Apache Atlas, and Nimbus we're using.
##########
standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/auth/jwt/JWTValidator.java:
##########
@@ -46,64 +47,35 @@
* This is cloned from JWTValidator in HS2 so as to NOT have any dependency on
HS2 code.
*/
public class JWTValidator {
- private static final Logger LOG =
LoggerFactory.getLogger(JWTValidator.class.getName());
- private static final DefaultJWSVerifierFactory JWS_VERIFIER_FACTORY = new
DefaultJWSVerifierFactory();
- private final URLBasedJWKSProvider jwksProvider;
- public JWTValidator(Configuration conf) throws IOException, ParseException {
- this.jwksProvider = new URLBasedJWKSProvider(conf);
- }
-
- public String validateJWTAndExtractUser(String signedJwt) throws
ParseException, AuthenticationException {
- Preconditions.checkNotNull(jwksProvider);
- Preconditions.checkNotNull(signedJwt, "No token found");
- final SignedJWT parsedJwt = SignedJWT.parse(signedJwt);
- List<JWK> matchedJWKS = jwksProvider.getJWKs(parsedJwt.getHeader());
- if (matchedJWKS.isEmpty()) {
- throw new AuthenticationException("Failed to find matched JWKs with the
JWT header: " + parsedJwt.getHeader());
- }
+ // Accept asymmetric cryptography based algorithms only
+ private static final Set<JWSAlgorithm> ACCEPTABLE_ALGORITHMS = new
HashSet<>(Family.SIGNATURE);
- // verify signature
- Exception lastException = null;
- for (JWK matchedJWK : matchedJWKS) {
- String keyID = matchedJWK.getKeyID() == null ? "null" :
matchedJWK.getKeyID();
- try {
- JWSVerifier verifier = getVerifier(parsedJwt.getHeader(), matchedJWK);
- if (parsedJwt.verify(verifier)) {
- LOG.debug("Verified JWT {} by JWK {}", parsedJwt.getPayload(),
keyID);
- break;
- }
- } catch (Exception e) {
- lastException = e;
- LOG.warn("Failed to verify JWT {} by JWK {}", parsedJwt.getPayload(),
keyID, e);
- }
- }
- // We use only the last seven characters to let a user can differentiate
exceptions for different JWT
- int startIndex = Math.max(0, signedJwt.length() - 7);
- String lastSevenChars = signedJwt.substring(startIndex);
- if (parsedJwt.getState() != JWSObject.State.VERIFIED) {
- throw new AuthenticationException("Failed to verify the JWT signature
(ends with " + lastSevenChars + ")",
- lastException);
- }
+ private final ConfigurableJWTProcessor<SecurityContext> jwtProcessor;
- // verify claims
- JWTClaimsSet claimsSet = parsedJwt.getJWTClaimsSet();
- Date expirationTime = claimsSet.getExpirationTime();
- if (expirationTime != null) {
- Date now = new Date();
- if (now.after(expirationTime)) {
- LOG.warn("Rejecting an expired JWT: {}", parsedJwt.getPayload());
- throw new AuthenticationException("JWT (ends with " + lastSevenChars +
") has been expired");
- }
+ public JWTValidator(Set<JOSEObjectType> acceptableTypes, List<URL> jwksURLs,
String expectedIssuer,
Review Comment:
I replaced our custom implementation, following the article below.
https://connect2id.com/products/nimbus-jose-jwt/examples/validating-jwt-access-tokens
--
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]
