This is an automated email from the ASF dual-hosted git repository.
liuxun pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/gravitino.git
The following commit(s) were added to refs/heads/main by this push:
new 33bc3e3a80 [#7843] fix(authz): MetadataIdConverter can not get role id
(#7844)
33bc3e3a80 is described below
commit 33bc3e3a80f6d2578451895f32c1d9e524f19b00
Author: yangyang zhong <[email protected]>
AuthorDate: Fri Aug 1 10:46:51 2025 +0800
[#7843] fix(authz): MetadataIdConverter can not get role id (#7844)
### What changes were proposed in this pull request?
Fix MetadataIdConverter can not get role id
### Why are the changes needed?
Fix: #7843
### Does this PR introduce _any_ user-facing change?
None
### How was this patch tested?
org.apache.gravitino.client.integration.test.authorization.OwnerAuthorizationIT#testSetRoleOwner
---
.../test/authorization/OwnerAuthorizationIT.java | 26 ++++++++++++++++++++++
.../test/authorization/RoleAuthorizationIT.java | 17 ++------------
.../server/authorization/MetadataIdConverter.java | 9 +++-----
.../authorization/jcasbin/JcasbinAuthorizer.java | 9 +-------
.../gravitino/server/web/rest/OwnerOperations.java | 9 ++++++--
5 files changed, 39 insertions(+), 31 deletions(-)
diff --git
a/clients/client-java/src/test/java/org/apache/gravitino/client/integration/test/authorization/OwnerAuthorizationIT.java
b/clients/client-java/src/test/java/org/apache/gravitino/client/integration/test/authorization/OwnerAuthorizationIT.java
index 019c571072..11a18d734d 100644
---
a/clients/client-java/src/test/java/org/apache/gravitino/client/integration/test/authorization/OwnerAuthorizationIT.java
+++
b/clients/client-java/src/test/java/org/apache/gravitino/client/integration/test/authorization/OwnerAuthorizationIT.java
@@ -278,4 +278,30 @@ public class OwnerAuthorizationIT extends
BaseRestApiAuthorizationIT {
USER,
Owner.Type.USER);
}
+
+ @Test
+ public void testSetRoleOwner() {
+ GravitinoMetalake gravitinoMetalake = client.loadMetalake(METALAKE);
+ String tempRole = "tempRole";
+ String tempUser = "tempUser";
+ gravitinoMetalake.addUser(tempUser);
+ gravitinoMetalake.createRole(tempRole, new HashMap<>(),
Collections.emptyList());
+ // normal user can not set owner
+ GravitinoMetalake gravitinoMetalakeLoadByNormalUser =
normalUserClient.loadMetalake(METALAKE);
+ assertThrows(
+ "Current user can not set owner",
+ RuntimeException.class,
+ () -> {
+ gravitinoMetalakeLoadByNormalUser.setOwner(
+ MetadataObjects.of(ImmutableList.of(tempRole),
MetadataObject.Type.ROLE),
+ tempUser,
+ Owner.Type.USER);
+ });
+ gravitinoMetalake.setOwner(
+ MetadataObjects.of(ImmutableList.of(tempRole),
MetadataObject.Type.ROLE),
+ tempUser,
+ Owner.Type.USER);
+ // reset
+ gravitinoMetalake.deleteRole(tempRole);
+ }
}
diff --git
a/clients/client-java/src/test/java/org/apache/gravitino/client/integration/test/authorization/RoleAuthorizationIT.java
b/clients/client-java/src/test/java/org/apache/gravitino/client/integration/test/authorization/RoleAuthorizationIT.java
index 746545b852..542d3dc44a 100644
---
a/clients/client-java/src/test/java/org/apache/gravitino/client/integration/test/authorization/RoleAuthorizationIT.java
+++
b/clients/client-java/src/test/java/org/apache/gravitino/client/integration/test/authorization/RoleAuthorizationIT.java
@@ -49,7 +49,7 @@ public class RoleAuthorizationIT extends
BaseRestApiAuthorizationIT {
() -> {
normalUserClient
.loadMetalake(METALAKE)
- .createRole("role2", new HashMap<>(), Collections.emptyList());
+ .createRole("role4", new HashMap<>(), Collections.emptyList());
});
client.loadMetalake(METALAKE).grantRolesToUser(ImmutableList.of("role1"),
NORMAL_USER);
client
@@ -70,7 +70,7 @@ public class RoleAuthorizationIT extends
BaseRestApiAuthorizationIT {
String[] roleNames = client.loadMetalake(METALAKE).listRoleNames();
assertArrayEquals(new String[] {"role1", "role2", "role3", "role4"},
roleNames);
roleNames = normalUserClient.loadMetalake(METALAKE).listRoleNames();
- assertArrayEquals(new String[] {"role1"}, roleNames);
+ assertArrayEquals(new String[] {"role1", "role4"}, roleNames);
}
@Test
@@ -94,12 +94,6 @@ public class RoleAuthorizationIT extends
BaseRestApiAuthorizationIT {
() -> {
normalUserClient.loadMetalake(METALAKE).getRole("role3");
});
- assertThrows(
- "Current user can not create role.",
- RuntimeException.class,
- () -> {
- normalUserClient.loadMetalake(METALAKE).getRole("role4");
- });
}
@Test
@@ -124,17 +118,10 @@ public class RoleAuthorizationIT extends
BaseRestApiAuthorizationIT {
() -> {
normalUserClient.loadMetalake(METALAKE).deleteRole("role3");
});
- assertThrows(
- "Current user can not create role.",
- RuntimeException.class,
- () -> {
- normalUserClient.loadMetalake(METALAKE).deleteRole("role4");
- });
// owner can delete role
client.loadMetalake(METALAKE).deleteRole("role1");
client.loadMetalake(METALAKE).deleteRole("role2");
client.loadMetalake(METALAKE).deleteRole("role3");
- client.loadMetalake(METALAKE).deleteRole("role4");
// normal user can not create role after delete role
assertThrows(
"Current user can not create role.",
diff --git
a/server-common/src/main/java/org/apache/gravitino/server/authorization/MetadataIdConverter.java
b/server-common/src/main/java/org/apache/gravitino/server/authorization/MetadataIdConverter.java
index e7deaa15b0..4143477bd2 100644
---
a/server-common/src/main/java/org/apache/gravitino/server/authorization/MetadataIdConverter.java
+++
b/server-common/src/main/java/org/apache/gravitino/server/authorization/MetadataIdConverter.java
@@ -24,7 +24,6 @@ import com.google.common.base.Preconditions;
import com.google.common.collect.ImmutableMap;
import java.io.IOException;
import java.util.Map;
-import java.util.regex.Pattern;
import org.apache.gravitino.Entity;
import org.apache.gravitino.EntityStore;
import org.apache.gravitino.GravitinoEnv;
@@ -46,10 +45,11 @@ import org.apache.gravitino.meta.TableEntity;
import org.apache.gravitino.meta.TagEntity;
import org.apache.gravitino.meta.TopicEntity;
import org.apache.gravitino.meta.UserEntity;
+import org.apache.gravitino.utils.MetadataObjectUtil;
/** It is used to convert MetadataObject to MetadataId */
public class MetadataIdConverter {
- private static final Pattern DOT_PATTERN = Pattern.compile("\\.");
+
// Maps metadata type to entity type
private static final Map<MetadataObject.Type, Entity.EntityType>
METADATA_TO_ENTITY_TYPE_MAPPING =
ImmutableMap.of(
@@ -104,10 +104,7 @@ public class MetadataIdConverter {
CatalogManager catalogManager =
GravitinoEnv.getInstance().catalogManager();
MetadataObject.Type metadataType = metadataObject.type();
- NameIdentifier ident =
- (metadataType != MetadataObject.Type.METALAKE)
- ? NameIdentifier.of(DOT_PATTERN.split(metalake + "." +
metadataObject.fullName()))
- : NameIdentifier.of(metadataObject.fullName());
+ NameIdentifier ident = MetadataObjectUtil.toEntityIdent(metalake,
metadataObject);
NameIdentifier normalizedIdent =
normalizeCaseSensitive(ident,
METADATA_SCOPE_MAPPING.get(metadataType), catalogManager);
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 86a36b9897..8ec23469f5 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
@@ -154,14 +154,7 @@ public class JcasbinAuthorizer implements
GravitinoAuthorizer {
try {
Long roleId =
MetadataIdConverter.getID(
- MetadataObjects.of(
- String.join(
- ".",
- nameIdentifier.namespace().level(1),
- nameIdentifier.namespace().level(2)),
- nameIdentifier.name(),
- MetadataObject.Type.ROLE),
- metalake);
+ NameIdentifierUtil.toMetadataObject(nameIdentifier, type),
metalake);
UserEntity userEntity = getUserEntity(currentUserName, metalake);
Long userId = userEntity.id();
loadRolePrivilege(metalake, currentUserName, userId);
diff --git
a/server/src/main/java/org/apache/gravitino/server/web/rest/OwnerOperations.java
b/server/src/main/java/org/apache/gravitino/server/web/rest/OwnerOperations.java
index 67fe25ff21..264ba8a2b9 100644
---
a/server/src/main/java/org/apache/gravitino/server/web/rest/OwnerOperations.java
+++
b/server/src/main/java/org/apache/gravitino/server/web/rest/OwnerOperations.java
@@ -32,6 +32,7 @@ import javax.ws.rs.PathParam;
import javax.ws.rs.Produces;
import javax.ws.rs.core.Context;
import javax.ws.rs.core.Response;
+import org.apache.gravitino.Entity;
import org.apache.gravitino.GravitinoEnv;
import org.apache.gravitino.MetadataObject;
import org.apache.gravitino.MetadataObjects;
@@ -44,6 +45,7 @@ import org.apache.gravitino.dto.util.DTOConverters;
import org.apache.gravitino.metrics.MetricNames;
import org.apache.gravitino.server.authorization.NameBindings;
import
org.apache.gravitino.server.authorization.annotations.AuthorizationExpression;
+import
org.apache.gravitino.server.authorization.annotations.AuthorizationMetadata;
import org.apache.gravitino.server.web.Utils;
import org.apache.gravitino.utils.MetadataObjectUtil;
@@ -97,9 +99,12 @@ public class OwnerOperations {
@Produces("application/vnd.gravitino.v1+json")
@Timed(name = "set-object-owner." + MetricNames.HTTP_PROCESS_DURATION,
absolute = true)
@ResponseMetered(name = "set-object-owner", absolute = true)
- @AuthorizationExpression(expression = CAN_SET_OWNER)
+ @AuthorizationExpression(
+ expression = CAN_SET_OWNER,
+ errorMessage = "Current user can not set this objects's owner")
public Response setOwnerForObject(
- @PathParam("metalake") String metalake,
+ @PathParam("metalake") @AuthorizationMetadata(type =
Entity.EntityType.METALAKE)
+ String metalake,
@PathParam("metadataObjectType") String metadataObjectType,
@PathParam("fullName") String fullName,
OwnerSetRequest request) {
