(gravitino) branch main updated: [#8906] improvement(authz): Add the check for setting owner operations (#9359)

[jshao]

This is an automated email from the ASF dual-hosted git repository.

jshao 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 9ca13515fe [#8906] improvement(authz): Add the check for setting owner 
operations (#9359)
9ca13515fe is described below

commit 9ca13515fed8358d6e314110280d91d13a22c7cb
Author: roryqi <h...@datastrato.com>
AuthorDate: Thu Dec 4 19:54:50 2025 +0800

    [#8906] improvement(authz): Add the check for setting owner operations 
(#9359)
    
    ### What changes were proposed in this pull request?
    
    Add the check for setting owner operations
    
    ### Why are the changes needed?
    
    Fix: #8906
    
    ### Does this PR introduce _any_ user-facing change?
    
    No.
    
    ### How was this patch tested?
    
    Add new test cases.
    
    ---------
    
    Co-authored-by: Copilot <175728472+copi...@users.noreply.github.com>
---
 .../integration/test/authorization/OwnerIT.java    | 10 ++++----
 .../gravitino/authorization/OwnerManager.java      |  4 ++++
 .../gravitino/authorization/TestOwnerManager.java  | 27 ++++++++++++++--------
 3 files changed, 25 insertions(+), 16 deletions(-)

diff --git 
a/clients/client-java/src/test/java/org/apache/gravitino/client/integration/test/authorization/OwnerIT.java
 
b/clients/client-java/src/test/java/org/apache/gravitino/client/integration/test/authorization/OwnerIT.java
index 0f59d46c8b..e76356f1e6 100644
--- 
a/clients/client-java/src/test/java/org/apache/gravitino/client/integration/test/authorization/OwnerIT.java
+++ 
b/clients/client-java/src/test/java/org/apache/gravitino/client/integration/test/authorization/OwnerIT.java
@@ -445,16 +445,14 @@ public class OwnerIT extends BaseIT {
       // create a Group
       String groupName = "ownerGroup";
       metalake.addGroup(groupName);
+
       // Now set the owner of catalog to the group
-      metalake.setOwner(catalogObject, groupName, Owner.Type.GROUP);
-      owner = metalake.getOwner(catalogObject).get();
-      Assertions.assertEquals(groupName, owner.name());
+      Assertions.assertThrows(
+          IllegalArgumentException.class,
+          () -> metalake.setOwner(catalogObject, groupName, Owner.Type.GROUP));
 
       // Drop the group
       ordinaryClient.removeGroup(groupName);
-      // The owner should be removed as well
-      ownerAfterDrop = metalake.getOwner(catalogObject).orElse(null);
-      Assertions.assertNull(ownerAfterDrop);
     }
     // Cleanup
     client.disableMetalake(metalakeNameE);
diff --git 
a/core/src/main/java/org/apache/gravitino/authorization/OwnerManager.java 
b/core/src/main/java/org/apache/gravitino/authorization/OwnerManager.java
index c6e1c0315f..5b7ce28c98 100644
--- a/core/src/main/java/org/apache/gravitino/authorization/OwnerManager.java
+++ b/core/src/main/java/org/apache/gravitino/authorization/OwnerManager.java
@@ -18,6 +18,7 @@
  */
 package org.apache.gravitino.authorization;
 
+import com.google.common.base.Preconditions;
 import java.io.IOException;
 import java.util.List;
 import java.util.Optional;
@@ -64,6 +65,9 @@ public class OwnerManager implements OwnerDispatcher {
   @Override
   public void setOwner(
       String metalake, MetadataObject metadataObject, String ownerName, 
Owner.Type ownerType) {
+    // TODO(ISSUE-9375): Support GROUP type owner in the future.
+    Preconditions.checkArgument(
+        ownerType == Owner.Type.USER, "Only USER type is supported as owner 
currently.");
     NameIdentifier objectIdent = MetadataObjectUtil.toEntityIdent(metalake, 
metadataObject);
     try {
       Optional<Owner> originOwner = getOwner(metalake, metadataObject);
diff --git 
a/core/src/test/java/org/apache/gravitino/authorization/TestOwnerManager.java 
b/core/src/test/java/org/apache/gravitino/authorization/TestOwnerManager.java
index 184449d2ec..1840a44251 100644
--- 
a/core/src/test/java/org/apache/gravitino/authorization/TestOwnerManager.java
+++ 
b/core/src/test/java/org/apache/gravitino/authorization/TestOwnerManager.java
@@ -198,18 +198,25 @@ public class TestOwnerManager {
     Assertions.assertEquals(USER, owner.name());
     Assertions.assertEquals(Owner.Type.USER, owner.type());
 
-    // Test to set the group as the owner
-    Mockito.reset(authorizationPlugin);
-    ownerManager.setOwner(METALAKE, metalakeObject, GROUP, Owner.Type.GROUP);
-    Mockito.verify(authorizationPlugin).onOwnerSet(Mockito.any(), 
Mockito.any(), Mockito.any());
-
-    // Test not-existed metadata object
+    // Test not-existed metadata object when setting owner
     Assertions.assertThrows(
         NotFoundException.class,
-        () -> ownerManager.setOwner(METALAKE, notExistObject, GROUP, 
Owner.Type.GROUP));
+        () -> ownerManager.setOwner(METALAKE, notExistObject, USER, 
Owner.Type.USER));
+  }
+
+  @Test
+  public void testGroupTypeOwnerNotSupported() {
+    // Test that GROUP type owner is not supported and throws 
IllegalArgumentException
+    MetadataObject metalakeObject =
+        MetadataObjects.of(Lists.newArrayList(METALAKE), 
MetadataObject.Type.METALAKE);
+
+    // Verify that setting GROUP type owner throws IllegalArgumentException
+    IllegalArgumentException exception =
+        Assertions.assertThrows(
+            IllegalArgumentException.class,
+            () -> ownerManager.setOwner(METALAKE, metalakeObject, GROUP, 
Owner.Type.GROUP));
 
-    owner = ownerManager.getOwner(METALAKE, metalakeObject).get();
-    Assertions.assertEquals(GROUP, owner.name());
-    Assertions.assertEquals(Owner.Type.GROUP, owner.type());
+    Assertions.assertEquals(
+        "Only USER type is supported as owner currently.", 
exception.getMessage());
   }
 }