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 d37426426f [#8182] fix(authz): Fix list model version bug (#8223)
d37426426f is described below

commit d37426426f9228adc417315c9cd1ad2f98781496
Author: yangyang zhong <[email protected]>
AuthorDate: Thu Aug 21 19:11:31 2025 +0800

    [#8182] fix(authz): Fix list model version bug (#8223)
    
    ### What changes were proposed in this pull request?
    
    fix list model version bug
    
    ### Why are the changes needed?
    
    Fix: #8182
    
    ### Does this PR introduce _any_ user-facing change?
    
    None
    
    ### How was this patch tested?
    
    
    
org.apache.gravitino.client.integration.test.authorization.ModelAuthorizationIT
---
 .../test/authorization/ModelAuthorizationIT.java   | 40 +++++++++++++++-------
 .../apache/gravitino/utils/NameIdentifierUtil.java | 29 ++++++++++++++++
 .../server/authorization/MetadataFilterHelper.java | 19 +++++++++-
 3 files changed, 74 insertions(+), 14 deletions(-)

diff --git 
a/clients/client-java/src/test/java/org/apache/gravitino/client/integration/test/authorization/ModelAuthorizationIT.java
 
b/clients/client-java/src/test/java/org/apache/gravitino/client/integration/test/authorization/ModelAuthorizationIT.java
index e75e4f514c..43c490e039 100644
--- 
a/clients/client-java/src/test/java/org/apache/gravitino/client/integration/test/authorization/ModelAuthorizationIT.java
+++ 
b/clients/client-java/src/test/java/org/apache/gravitino/client/integration/test/authorization/ModelAuthorizationIT.java
@@ -239,29 +239,32 @@ public class ModelAuthorizationIT extends 
BaseRestApiAuthorizationIT {
   @Test
   @Order(7)
   public void testListModelVersion() {
-    ModelCatalog modelCatalog = 
client.loadMetalake(METALAKE).loadCatalog(CATALOG).asModelCatalog();
+    GravitinoMetalake gravitinoMetalake = client.loadMetalake(METALAKE);
+    ModelCatalog modelCatalog = 
gravitinoMetalake.loadCatalog(CATALOG).asModelCatalog();
     Catalog catalogEntityLoadByNormalUser =
         normalUserClient.loadMetalake(METALAKE).loadCatalog(CATALOG);
     ModelCatalog modelCatalogLoadByNormalUser = 
catalogEntityLoadByNormalUser.asModelCatalog();
     int[] versions = modelCatalog.listModelVersions(NameIdentifier.of(SCHEMA, 
"model1"));
     assertEquals(2, versions.length);
-    assertThrows(
-        "Can not access metadata {" + METALAKE + "," + CATALOG + "." + SCHEMA 
+ "model1" + "}.",
-        ForbiddenException.class,
-        () -> {
-          modelCatalogLoadByNormalUser.linkModelVersion(
-              NameIdentifier.of(SCHEMA, "model1"),
-              "uri1",
-              new String[] {"alias2"},
-              "comment2",
-              null);
-        });
+    versions = 
modelCatalogLoadByNormalUser.listModelVersions(NameIdentifier.of(SCHEMA, 
"model1"));
+    assertEquals(0, versions.length);
+    gravitinoMetalake.grantPrivilegesToRole(
+        role,
+        MetadataObjects.of(ImmutableList.of(CATALOG, SCHEMA, "model1"), 
MetadataObject.Type.MODEL),
+        ImmutableSet.of(Privileges.UseModel.allow()));
+    versions = 
modelCatalogLoadByNormalUser.listModelVersions(NameIdentifier.of(SCHEMA, 
"model1"));
+    assertEquals(2, versions.length);
+    gravitinoMetalake.revokePrivilegesFromRole(
+        role,
+        MetadataObjects.of(ImmutableList.of(CATALOG, SCHEMA, "model1"), 
MetadataObject.Type.MODEL),
+        ImmutableSet.of(Privileges.UseModel.allow()));
   }
 
   @Test
   @Order(8)
   public void testLoadModelVersion() {
-    ModelCatalog modelCatalog = 
client.loadMetalake(METALAKE).loadCatalog(CATALOG).asModelCatalog();
+    GravitinoMetalake gravitinoMetalake = client.loadMetalake(METALAKE);
+    ModelCatalog modelCatalog = 
gravitinoMetalake.loadCatalog(CATALOG).asModelCatalog();
     Catalog catalogEntityLoadByNormalUser =
         normalUserClient.loadMetalake(METALAKE).loadCatalog(CATALOG);
     ModelCatalog modelCatalogLoadByNormalUser = 
catalogEntityLoadByNormalUser.asModelCatalog();
@@ -273,6 +276,17 @@ public class ModelAuthorizationIT extends 
BaseRestApiAuthorizationIT {
         () -> {
           
modelCatalogLoadByNormalUser.getModelVersion(NameIdentifier.of(SCHEMA, 
"model1"), 1);
         });
+
+    gravitinoMetalake.grantPrivilegesToRole(
+        role,
+        MetadataObjects.of(ImmutableList.of(CATALOG, SCHEMA, "model1"), 
MetadataObject.Type.MODEL),
+        ImmutableSet.of(Privileges.UseModel.allow()));
+    version = 
modelCatalogLoadByNormalUser.getModelVersion(NameIdentifier.of(SCHEMA, 
"model1"), 1);
+    assertEquals(1, version.version());
+    gravitinoMetalake.revokePrivilegesFromRole(
+        role,
+        MetadataObjects.of(ImmutableList.of(CATALOG, SCHEMA, "model1"), 
MetadataObject.Type.MODEL),
+        ImmutableSet.of(Privileges.UseModel.allow()));
   }
 
   @Test
diff --git 
a/core/src/main/java/org/apache/gravitino/utils/NameIdentifierUtil.java 
b/core/src/main/java/org/apache/gravitino/utils/NameIdentifierUtil.java
index c1fd511807..095fe6259b 100644
--- a/core/src/main/java/org/apache/gravitino/utils/NameIdentifierUtil.java
+++ b/core/src/main/java/org/apache/gravitino/utils/NameIdentifierUtil.java
@@ -560,4 +560,33 @@ public class NameIdentifierUtil {
   public static NameIdentifier ofStatistic(NameIdentifier entityIdent, String 
name) {
     return NameIdentifier.of(Namespace.fromString(entityIdent.toString()), 
name);
   }
+
+  /**
+   * Try to get the model {@link NameIdentifier} from the given {@link 
NameIdentifier}.
+   *
+   * @param ident The {@link NameIdentifier} to check.
+   * @return The model {@link NameIdentifier}
+   * @throws IllegalNameIdentifierException If the given {@link 
NameIdentifier} does not include
+   *     model name
+   */
+  public static NameIdentifier getModelIdentifier(NameIdentifier ident) {
+    NameIdentifier.check(
+        ident.name() != null && !ident.name().isEmpty(),
+        "The name variable in the NameIdentifier must have value.");
+    Namespace.check(
+        ident.namespace() != null
+            && !ident.namespace().isEmpty()
+            && ident.namespace().length() >= 4,
+        "ModelVersion namespace must be non-null and at least 4 level, the 
input namespace is %s",
+        ident.namespace());
+
+    List<String> allElems =
+        Stream.concat(Arrays.stream(ident.namespace().levels()), 
Stream.of(ident.name()))
+            .collect(Collectors.toList());
+    if (allElems.size() < 4) {
+      throw new IllegalNameIdentifierException(
+          "Cannot create a model NameIdentifier less than four elements.");
+    }
+    return NameIdentifier.of(allElems.get(0), allElems.get(1), 
allElems.get(2), allElems.get(3));
+  }
 }
diff --git 
a/server-common/src/main/java/org/apache/gravitino/server/authorization/MetadataFilterHelper.java
 
b/server-common/src/main/java/org/apache/gravitino/server/authorization/MetadataFilterHelper.java
index 3f7ade56a6..1a002daafc 100644
--- 
a/server-common/src/main/java/org/apache/gravitino/server/authorization/MetadataFilterHelper.java
+++ 
b/server-common/src/main/java/org/apache/gravitino/server/authorization/MetadataFilterHelper.java
@@ -172,6 +172,15 @@ public class MetadataFilterHelper {
         nameIdentifierMap.put(
             Entity.EntityType.CATALOG, 
NameIdentifierUtil.getCatalogIdentifier(nameIdentifier));
         break;
+      case MODEL_VERSION:
+        nameIdentifierMap.put(Entity.EntityType.MODEL_VERSION, nameIdentifier);
+        nameIdentifierMap.put(
+            Entity.EntityType.MODEL, 
NameIdentifierUtil.getModelIdentifier(nameIdentifier));
+        nameIdentifierMap.put(
+            Entity.EntityType.SCHEMA, 
NameIdentifierUtil.getSchemaIdentifier(nameIdentifier));
+        nameIdentifierMap.put(
+            Entity.EntityType.CATALOG, 
NameIdentifierUtil.getCatalogIdentifier(nameIdentifier));
+        break;
       case TOPIC:
         nameIdentifierMap.put(Entity.EntityType.TOPIC, nameIdentifier);
         nameIdentifierMap.put(
@@ -186,9 +195,17 @@ public class MetadataFilterHelper {
         nameIdentifierMap.put(
             Entity.EntityType.CATALOG, 
NameIdentifierUtil.getCatalogIdentifier(nameIdentifier));
         break;
-      default:
+      case METALAKE:
+        nameIdentifierMap.put(entityType, nameIdentifier);
+        break;
+      case ROLE:
         nameIdentifierMap.put(entityType, nameIdentifier);
         break;
+      case USER:
+        nameIdentifierMap.put(entityType, nameIdentifier);
+        break;
+      default:
+        throw new IllegalArgumentException("Unsupported entity type: " + 
entityType);
     }
     return nameIdentifierMap;
   }

Reply via email to