This is an automated email from the ASF dual-hosted git repository.
yuqi4733 pushed a commit to branch branch-1.1
in repository https://gitbox.apache.org/repos/asf/gravitino.git
The following commit(s) were added to refs/heads/branch-1.1 by this push:
new 237cc3edb7 [#9616] fix(authz): Fix the `PassThroughAuthorizer` user
verification logic (#9703)
237cc3edb7 is described below
commit 237cc3edb72cc08b58d26aff66a4a96d6b5666ab
Author: github-actions[bot]
<41898282+github-actions[bot]@users.noreply.github.com>
AuthorDate: Tue Jan 13 20:39:11 2026 +0800
[#9616] fix(authz): Fix the `PassThroughAuthorizer` user verification
logic (#9703)
### What changes were proposed in this pull request?
Fix the `PassThroughAuthorizer` user verification logic
### Why are the changes needed?
Fix: #9616
### Does this PR introduce _any_ user-facing change?
No.
### How was this patch tested?
Added IT.
Co-authored-by: roryqi <[email protected]>
---
.../test/authorization/CheckCurrentUserIT.java | 285 ---------------------
.../authorization/PassTroughAuthorizationIT.java | 47 ++++
.../authorization/AuthorizationUtils.java | 10 +-
.../apache/gravitino/hook/JobHookDispatcher.java | 7 -
docs/security/access-control.md | 68 +++++
.../authorization/PassThroughAuthorizer.java | 12 +
.../authorization/jcasbin/JcasbinAuthorizer.java | 7 +-
7 files changed, 130 insertions(+), 306 deletions(-)
diff --git
a/clients/client-java/src/test/java/org/apache/gravitino/client/integration/test/authorization/CheckCurrentUserIT.java
b/clients/client-java/src/test/java/org/apache/gravitino/client/integration/test/authorization/CheckCurrentUserIT.java
deleted file mode 100644
index f829f91ceb..0000000000
---
a/clients/client-java/src/test/java/org/apache/gravitino/client/integration/test/authorization/CheckCurrentUserIT.java
+++ /dev/null
@@ -1,285 +0,0 @@
-/*
- * 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.gravitino.client.integration.test.authorization;
-
-import static
org.apache.gravitino.server.GravitinoServer.WEBSERVER_CONF_PREFIX;
-
-import com.google.common.collect.Lists;
-import com.google.common.collect.Maps;
-import java.util.Collections;
-import java.util.Map;
-import org.apache.gravitino.Catalog;
-import org.apache.gravitino.Configs;
-import org.apache.gravitino.NameIdentifier;
-import org.apache.gravitino.auth.AuthConstants;
-import org.apache.gravitino.authorization.Privileges;
-import org.apache.gravitino.authorization.SecurableObject;
-import org.apache.gravitino.authorization.SecurableObjects;
-import org.apache.gravitino.client.GravitinoAdminClient;
-import org.apache.gravitino.client.GravitinoMetalake;
-import org.apache.gravitino.exceptions.ForbiddenException;
-import org.apache.gravitino.file.Fileset;
-import org.apache.gravitino.integration.test.container.ContainerSuite;
-import org.apache.gravitino.integration.test.container.HiveContainer;
-import org.apache.gravitino.integration.test.container.KafkaContainer;
-import org.apache.gravitino.integration.test.util.BaseIT;
-import org.apache.gravitino.rel.Column;
-import org.apache.gravitino.rel.types.Types;
-import org.apache.gravitino.server.web.JettyServerConfig;
-import org.apache.gravitino.utils.RandomNameUtils;
-import org.junit.jupiter.api.AfterAll;
-import org.junit.jupiter.api.Assertions;
-import org.junit.jupiter.api.BeforeAll;
-import org.junit.jupiter.api.Tag;
-import org.junit.jupiter.api.Test;
-import org.slf4j.Logger;
-import org.slf4j.LoggerFactory;
-
-@Tag("gravitino-docker-test")
-public class CheckCurrentUserIT extends BaseIT {
-
- private static final Logger LOG =
LoggerFactory.getLogger(CheckCurrentUserIT.class);
- private static final ContainerSuite containerSuite =
ContainerSuite.getInstance();
- private static String hmsUri;
- private static String kafkaBootstrapServers;
- private static GravitinoMetalake metalake;
- private static GravitinoMetalake anotherMetalake;
- private static String metalakeName =
RandomNameUtils.genRandomName("metalake");
-
- @BeforeAll
- public void startIntegrationTest() throws Exception {
- Map<String, String> configs = Maps.newHashMap();
- configs.put(Configs.ENABLE_AUTHORIZATION.getKey(), String.valueOf(true));
- configs.put(Configs.SERVICE_ADMINS.getKey(), AuthConstants.ANONYMOUS_USER);
- configs.put(
- Configs.AUTHORIZATION_IMPL.getKey(),
- "org.apache.gravitino.server.authorization.PassThroughAuthorizer");
- registerCustomConfigs(configs);
- super.startIntegrationTest();
-
- containerSuite.startHiveContainer();
- hmsUri =
- String.format(
- "thrift://%s:%d",
- containerSuite.getHiveContainer().getContainerIpAddress(),
- HiveContainer.HIVE_METASTORE_PORT);
-
- containerSuite.startKafkaContainer();
- kafkaBootstrapServers =
- String.format(
- "%s:%d",
- containerSuite.getKafkaContainer().getContainerIpAddress(),
- KafkaContainer.DEFAULT_BROKER_PORT);
-
- JettyServerConfig jettyServerConfig =
- JettyServerConfig.fromConfig(serverConfig, WEBSERVER_CONF_PREFIX);
-
- String uri = "http://"; + jettyServerConfig.getHost() + ":" +
jettyServerConfig.getHttpPort();
- System.setProperty("user.name", "test");
- GravitinoAdminClient anotherClient =
GravitinoAdminClient.builder(uri).withSimpleAuth().build();
-
- metalake = client.createMetalake(metalakeName, "metalake comment",
Collections.emptyMap());
- metalake.addUser("test");
- anotherMetalake = anotherClient.loadMetalake(metalakeName);
- metalake.removeUser("test");
- }
-
- @AfterAll
- public void tearDown() {
- if (client != null) {
- client.dropMetalake(metalakeName, true);
- client.close();
- client = null;
- }
-
- try {
- closer.close();
- } catch (Exception e) {
- LOG.error("Exception in closing CloseableGroup", e);
- }
- }
-
- @Test
- public void testCreateTopic() {
- String catalogName = RandomNameUtils.genRandomName("catalogA");
-
- Map<String, String> properties = Maps.newHashMap();
- properties.put("bootstrap.servers", kafkaBootstrapServers);
-
- // Test to create catalog with not-existed user
- Assertions.assertThrows(
- ForbiddenException.class,
- () ->
- anotherMetalake.createCatalog(
- catalogName, Catalog.Type.MESSAGING, "kafka", "comment",
properties));
-
- Catalog catalog =
- metalake.createCatalog(catalogName, Catalog.Type.MESSAGING, "kafka",
"comment", properties);
-
- // Test to create topic with not-existed user
- metalake.addUser("test");
- Catalog anotherCatalog = anotherMetalake.loadCatalog(catalogName);
- metalake.removeUser("test");
- NameIdentifier topicIdent = NameIdentifier.of("default", "topic");
- Assertions.assertThrows(
- ForbiddenException.class,
- () ->
- anotherCatalog
- .asTopicCatalog()
- .createTopic(topicIdent, "comment", null,
Collections.emptyMap()));
-
- Assertions.assertDoesNotThrow(
- () ->
- catalog
- .asTopicCatalog()
- .createTopic(topicIdent, "comment", null,
Collections.emptyMap()));
- catalog.asTopicCatalog().dropTopic(topicIdent);
-
- metalake.dropCatalog(catalogName, true);
- }
-
- @Test
- public void testCreateFileset() {
- String catalogName = RandomNameUtils.genRandomName("catalog");
- // Test to create a fileset with a not-existed user
- Assertions.assertThrows(
- ForbiddenException.class,
- () ->
- anotherMetalake.createCatalog(
- catalogName, Catalog.Type.FILESET, "hadoop", "comment",
Collections.emptyMap()));
-
- Catalog catalog =
- metalake.createCatalog(
- catalogName, Catalog.Type.FILESET, "hadoop", "comment",
Collections.emptyMap());
-
- // Test to create a schema with a not-existed user
- metalake.addUser("test");
- Catalog anotherCatalog = anotherMetalake.loadCatalog(catalogName);
- metalake.removeUser("test");
- Assertions.assertThrows(
- ForbiddenException.class,
- () -> anotherCatalog.asSchemas().createSchema("schema", "comment",
Collections.emptyMap()));
-
- catalog.asSchemas().createSchema("schema", "comment",
Collections.emptyMap());
-
- // Test to create a fileset with a not-existed user
- NameIdentifier fileIdent = NameIdentifier.of("schema", "fileset");
- Assertions.assertThrows(
- ForbiddenException.class,
- () ->
- anotherCatalog
- .asFilesetCatalog()
- .createFileset(
- fileIdent, "comment", Fileset.Type.EXTERNAL, "tmp",
Collections.emptyMap()));
-
- Assertions.assertDoesNotThrow(
- () ->
- catalog
- .asFilesetCatalog()
- .createFileset(
- fileIdent, "comment", Fileset.Type.EXTERNAL, "tmp",
Collections.emptyMap()));
-
- // Clean up
- catalog.asFilesetCatalog().dropFileset(fileIdent);
- catalog.asSchemas().dropSchema("schema", true);
- metalake.dropCatalog(catalogName, true);
- }
-
- @Test
- public void testCreateRole() {
- SecurableObject metalakeSecObject =
- SecurableObjects.ofMetalake(
- metalakeName,
Lists.newArrayList(Privileges.CreateCatalog.allow()));
-
- // Test creating role with non-existent user fails
- Assertions.assertThrows(
- ForbiddenException.class,
- () ->
- anotherMetalake.createRole(
- "role", Collections.emptyMap(),
Lists.newArrayList(metalakeSecObject)));
-
- // Test creating role with admin user succeeds
- Assertions.assertDoesNotThrow(
- () ->
- metalake.createRole(
- "role", Collections.emptyMap(),
Lists.newArrayList(metalakeSecObject)));
- metalake.deleteRole("role");
- }
-
- @Test
- public void testCreateTable() {
- String catalogName = RandomNameUtils.genRandomName("catalog");
- Map<String, String> properties = Maps.newHashMap();
- properties.put("metastore.uris", hmsUri);
-
- // Test to create catalog with not-existed user
- Assertions.assertThrows(
- ForbiddenException.class,
- () ->
- anotherMetalake.createCatalog(
- catalogName, Catalog.Type.RELATIONAL, "hive", "catalog
comment", properties));
- Catalog catalog =
- metalake.createCatalog(
- catalogName, Catalog.Type.RELATIONAL, "hive", "catalog comment",
properties);
-
- // Test to create schema with not-existed user
- metalake.addUser("test");
- Catalog anotherCatalog = anotherMetalake.loadCatalog(catalogName);
- metalake.removeUser("test");
- Assertions.assertThrows(
- ForbiddenException.class,
- () -> anotherCatalog.asSchemas().createSchema("schema", "comment",
Collections.emptyMap()));
-
- catalog.asSchemas().createSchema("schema", "comment",
Collections.emptyMap());
-
- // Test to create table with not-existed user
- NameIdentifier tableIdent = NameIdentifier.of("schema", "table");
- Assertions.assertThrows(
- ForbiddenException.class,
- () ->
- anotherCatalog
- .asTableCatalog()
- .createTable(
- tableIdent,
- new Column[] {
- Column.of("col1", Types.IntegerType.get()),
- Column.of("col2", Types.StringType.get())
- },
- "comment",
- Collections.emptyMap()));
-
- Assertions.assertDoesNotThrow(
- () ->
- catalog
- .asTableCatalog()
- .createTable(
- tableIdent,
- new Column[] {
- Column.of("col1", Types.IntegerType.get()),
- Column.of("col2", Types.StringType.get())
- },
- "comment",
- Collections.emptyMap()));
-
- // Clean up
- catalog.asTableCatalog().dropTable(tableIdent);
- catalog.asSchemas().dropSchema("schema", true);
- metalake.dropCatalog(catalogName, true);
- }
-}
diff --git
a/clients/client-java/src/test/java/org/apache/gravitino/client/integration/test/authorization/PassTroughAuthorizationIT.java
b/clients/client-java/src/test/java/org/apache/gravitino/client/integration/test/authorization/PassTroughAuthorizationIT.java
new file mode 100644
index 0000000000..39494e1e48
--- /dev/null
+++
b/clients/client-java/src/test/java/org/apache/gravitino/client/integration/test/authorization/PassTroughAuthorizationIT.java
@@ -0,0 +1,47 @@
+/*
+ * 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.gravitino.client.integration.test.authorization;
+
+import org.apache.gravitino.Configs;
+import org.apache.gravitino.server.authorization.PassThroughAuthorizer;
+import org.junit.jupiter.api.BeforeAll;
+import org.junit.jupiter.api.MethodOrderer;
+import org.junit.jupiter.api.Order;
+import org.junit.jupiter.api.Tag;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.TestMethodOrder;
+
+@Tag("gravitino-docker-test")
+@TestMethodOrder(MethodOrderer.OrderAnnotation.class)
+public class PassTroughAuthorizationIT extends BaseRestApiAuthorizationIT {
+ @BeforeAll
+ @Override
+ public void startIntegrationTest() throws Exception {
+ customConfigs.put(Configs.AUTHORIZATION_IMPL.getKey(),
PassThroughAuthorizer.class.getName());
+ super.startIntegrationTest();
+ }
+
+ @Test
+ @Order(1)
+ public void testCreateUser() {
+ normalUserClient.loadMetalake(METALAKE).addUser("user0");
+ client.loadMetalake(METALAKE).addUser("user1");
+ client.loadMetalake(METALAKE).addUser("user2");
+ }
+}
diff --git
a/core/src/main/java/org/apache/gravitino/authorization/AuthorizationUtils.java
b/core/src/main/java/org/apache/gravitino/authorization/AuthorizationUtils.java
index d1db956d20..387a618165 100644
---
a/core/src/main/java/org/apache/gravitino/authorization/AuthorizationUtils.java
+++
b/core/src/main/java/org/apache/gravitino/authorization/AuthorizationUtils.java
@@ -49,7 +49,6 @@ import org.apache.gravitino.exceptions.ForbiddenException;
import org.apache.gravitino.exceptions.IllegalPrivilegeException;
import org.apache.gravitino.exceptions.NoSuchCatalogException;
import org.apache.gravitino.exceptions.NoSuchMetadataObjectException;
-import org.apache.gravitino.exceptions.NoSuchUserException;
import org.apache.gravitino.file.Fileset;
import org.apache.gravitino.meta.RoleEntity;
import org.apache.gravitino.rel.Table;
@@ -117,13 +116,8 @@ public class AuthorizationUtils {
private AuthorizationUtils() {}
public static void checkCurrentUser(String metalake, String user) {
- try {
- AccessControlDispatcher dispatcher =
GravitinoEnv.getInstance().accessControlDispatcher();
- // Only when we enable authorization, we need to check the current user
- if (dispatcher != null) {
- dispatcher.getUser(metalake, user);
- }
- } catch (NoSuchUserException nsu) {
+ GravitinoAuthorizer authorizer =
GravitinoEnv.getInstance().gravitinoAuthorizer();
+ if (authorizer != null && !authorizer.isMetalakeUser(metalake)) {
throw new ForbiddenException(
"Current user %s doesn't exist in the metalake %s, you should add
the user to the metalake first",
user, metalake);
diff --git
a/core/src/main/java/org/apache/gravitino/hook/JobHookDispatcher.java
b/core/src/main/java/org/apache/gravitino/hook/JobHookDispatcher.java
index a6dac9bbd9..99135ec5dc 100644
--- a/core/src/main/java/org/apache/gravitino/hook/JobHookDispatcher.java
+++ b/core/src/main/java/org/apache/gravitino/hook/JobHookDispatcher.java
@@ -24,7 +24,6 @@ import java.util.Map;
import java.util.Optional;
import org.apache.gravitino.Entity;
import org.apache.gravitino.GravitinoEnv;
-import org.apache.gravitino.authorization.AuthorizationUtils;
import org.apache.gravitino.authorization.Owner;
import org.apache.gravitino.authorization.OwnerDispatcher;
import org.apache.gravitino.exceptions.InUseException;
@@ -53,9 +52,6 @@ public class JobHookDispatcher implements
JobOperationDispatcher {
@Override
public void registerJobTemplate(String metalake, JobTemplateEntity
jobTemplateEntity)
throws JobTemplateAlreadyExistsException {
- // Check whether the current user exists or not
- AuthorizationUtils.checkCurrentUser(metalake,
PrincipalUtils.getCurrentUserName());
-
jobOperationDispatcher.registerJobTemplate(metalake, jobTemplateEntity);
// Set the creator as the owner of the job template.
@@ -102,9 +98,6 @@ public class JobHookDispatcher implements
JobOperationDispatcher {
@Override
public JobEntity runJob(String metalake, String jobTemplateName, Map<String,
String> jobConf)
throws NoSuchJobTemplateException {
- // Check whether the current user exists or not
- AuthorizationUtils.checkCurrentUser(metalake,
PrincipalUtils.getCurrentUserName());
-
JobEntity jobEntity = jobOperationDispatcher.runJob(metalake,
jobTemplateName, jobConf);
// Set the creator as the owner of the job.
diff --git a/docs/security/access-control.md b/docs/security/access-control.md
index 5ff5397a58..613d75630c 100644
--- a/docs/security/access-control.md
+++ b/docs/security/access-control.md
@@ -455,6 +455,73 @@ gravitino.authorization.enable = true
gravitino.authorization.serviceAdmins = admin1,admin2
```
+## Migration Guide
+
+If you have metalakes that were created before authorization was enabled, you
need to perform a migration to ensure proper access control.
+
+### Migrating Existing Metalakes
+
+When you created metalakes with `gravitino.authorization.enable = false`,
those metalakes don't have owners assigned. To enable authorization for these
existing metalakes:
+
+**Step 1: Configure PassThrough Authorization**
+
+Temporarily set the authorization implementation to pass-through mode:
+
+```properties
+gravitino.authorization.enable = true
+gravitino.authorization.serviceAdmins = admin1,admin2
+gravitino.authorization.impl =
org.apache.gravitino.server.authorization.PassThroughAuthorizer
+```
+
+**Step 2: Set Metalake Owners**
+
+Set the owner for each existing metalake using the Gravitino API:
+
+<Tabs groupId='language' queryString>
+<TabItem value="shell" label="Shell">
+
+```shell
+curl -X PUT -H "Accept: application/vnd.gravitino.v1+json" \
+-H "Content-Type: application/json" -d '{
+ "owner": "admin1",
+ "ownerType": "USER"
+}' http://localhost:8090/api/metalakes/{metalake}/owners
+```
+
+</TabItem>
+<TabItem value="java" label="Java">
+
+```java
+GravitinoClient client = GravitinoClient.builder("http://localhost:8090";)
+ .build();
+
+MetadataObject metalake = MetadataObjects.of(null, "metalake_name",
MetadataObject.Type.METALAKE);
+client.setOwner(metalake, "admin1", Owner.Type.USER);
+```
+
+</TabItem>
+</Tabs>
+
+**Step 3: Enable Full Authorization**
+
+After setting owners for all metalakes, remove the
`gravitino.authorization.impl` configuration to enable full authorization:
+
+```properties
+gravitino.authorization.enable = true
+gravitino.authorization.serviceAdmins = admin1,admin2
+# Remove: gravitino.authorization.impl =
org.apache.gravitino.server.authorization.PassThroughAuthorizer
+```
+
+**Step 4: Restart Gravitino**
+
+Restart the Gravitino server for the configuration changes to take effect.
+
+:::caution
+**Important Migration Notes:**
+- Set owners for **all** existing metalakes before removing the
`gravitino.authorization.impl` configuration
+- Without assigned owners, operations on these metalakes will fail after full
authorization is enabled
+:::
+
## Operations
The following sections demonstrate how to perform common access control
operations using both the REST API (Shell) and Java client.
@@ -1206,3 +1273,4 @@ The following table lists the required privileges for
each API.
| get a job | The owner of the metalake or the job.
|
| cancel a job | The owner of the metalake or the job.
|
| get credential | If you can load the metadata object, you
can get its credential.
|
+
diff --git
a/server-common/src/main/java/org/apache/gravitino/server/authorization/PassThroughAuthorizer.java
b/server-common/src/main/java/org/apache/gravitino/server/authorization/PassThroughAuthorizer.java
index 50b88c00aa..06e20b8ada 100644
---
a/server-common/src/main/java/org/apache/gravitino/server/authorization/PassThroughAuthorizer.java
+++
b/server-common/src/main/java/org/apache/gravitino/server/authorization/PassThroughAuthorizer.java
@@ -20,11 +20,15 @@ package org.apache.gravitino.server.authorization;
import java.io.IOException;
import java.security.Principal;
import org.apache.gravitino.Entity;
+import org.apache.gravitino.GravitinoEnv;
import org.apache.gravitino.MetadataObject;
import org.apache.gravitino.NameIdentifier;
+import org.apache.gravitino.authorization.AccessControlDispatcher;
import org.apache.gravitino.authorization.AuthorizationRequestContext;
import org.apache.gravitino.authorization.GravitinoAuthorizer;
import org.apache.gravitino.authorization.Privilege;
+import org.apache.gravitino.exceptions.NoSuchUserException;
+import org.apache.gravitino.utils.PrincipalUtils;
/**
* The default implementation of GravitinoAuthorizer, indicating that metadata
permission control is
@@ -76,6 +80,14 @@ public class PassThroughAuthorizer implements
GravitinoAuthorizer {
@Override
public boolean isMetalakeUser(String metalake) {
+ AccessControlDispatcher dispatcher =
GravitinoEnv.getInstance().accessControlDispatcher();
+ if (dispatcher != null) {
+ try {
+ dispatcher.getUser(metalake, PrincipalUtils.getCurrentUserName());
+ } catch (NoSuchUserException e) {
+ dispatcher.addUser(metalake, PrincipalUtils.getCurrentUserName());
+ }
+ }
return true;
}
diff --git
a/server-common/src/main/java/org/apache/gravitino/server/authorization/jcasbin/JcasbinAuthorizer.java
b/server-common/src/main/java/org/apache/gravitino/server/authorization/jcasbin/JcasbinAuthorizer.java
index 68125aec56..54f6241a94 100644
---
a/server-common/src/main/java/org/apache/gravitino/server/authorization/jcasbin/JcasbinAuthorizer.java
+++
b/server-common/src/main/java/org/apache/gravitino/server/authorization/jcasbin/JcasbinAuthorizer.java
@@ -226,12 +226,7 @@ public class JcasbinAuthorizer implements
GravitinoAuthorizer {
}
try {
- return GravitinoEnv.getInstance()
- .entityStore()
- .get(
- NameIdentifierUtil.ofUser(metalake, currentUserName),
- Entity.EntityType.USER,
- UserEntity.class)
+ return
GravitinoEnv.getInstance().accessControlDispatcher().getUser(metalake,
currentUserName)
!= null;
} catch (Exception e) {
LOG.warn("Can not get user {} in metalake {}", currentUserName,
metalake, e);
