(gravitino) branch main updated: [#12269] fix(authz): Allow catalog owners to grant schema privileges (#12673)

[roryqi]

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

roryqi 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 c79996e7a8 [#12269] fix(authz): Allow catalog owners to grant schema 
privileges (#12673)
c79996e7a8 is described below

commit c79996e7a882147bbe93660edc2bc771a2fb2292
Author: Stefan Wang <1fan...@gmail.com>
AuthorDate: Thu Aug 27 22:23:38 2026 -0400

    [#12269] fix(authz): Allow catalog owners to grant schema privileges 
(#12673)
    
    ### What changes were proposed in this pull request?
    
    A user who owns a catalog and one of its schemas can currently get `403
    Forbidden` when granting schema privileges unless they also have an
    explicit `USE_CATALOG` grant. This change allows the catalog owner to
    perform the operation and adds a regression for that case.
    
    The authorizer now continues checking ancestors when ownership of the
    current object does not satisfy its parent usage requirement.
    
    ### Why are the changes needed?
    
    The schema ownership check returned its failed `USE_CATALOG` result
    immediately, so the authorizer never reached the catalog ownership
    check. A user who owned both objects received `403 Forbidden: "Current
    user can not grant privilege to role."`, while owning only the catalog
    allowed the same operation.
    
    Fix: #12269
    
    ### Does this PR introduce _any_ user-facing change?
    
    Yes. A catalog owner who also owns a child schema can grant privileges
    on that schema without a redundant `USE_CATALOG` grant.
    
    ### How was this patch tested?
    
    ```shell
    ./gradlew :server-common:test --tests 
'org.apache.gravitino.server.authorization.jcasbin.TestJcasbinAuthorizer'
    ```
    
    <details>
    <summary>Raw logs</summary>
    
    ```text
    Before, on upstream main:
    TestJcasbinAuthorizer.testHasSetOwnerPermissionAllowsSchemaAndCatalogOwner 
FAILED
    expected: <true> but was: <false>
    Tests: 1, failures: 1
    
    After:
    Tests: 59, skipped: 0, failures: 0, errors: 0
    BUILD SUCCESSFUL
    ```
    
    </details>
    
    Signed-off-by: 1fanwang <1fan...@gmail.com>
---
 .../authorization/jcasbin/JcasbinAuthorizer.java   |  7 ++++---
 .../jcasbin/TestJcasbinAuthorizer.java             | 24 ++++++++++++++++++++++
 2 files changed, 28 insertions(+), 3 deletions(-)

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 eb3c2ab766..4e82289318 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
@@ -585,9 +585,10 @@ public class JcasbinAuthorizer implements 
GravitinoAuthorizer {
 
     MetadataObject metadataObject = MetadataObjects.parse(fullName, 
metadataType);
     do {
-      if (isOwner(currentPrincipal, metalake, metadataObject, requestContext)) 
{
-        return hasParentUsagePermission(
-            currentPrincipal, metalake, metadataObject, metalakeObject, 
requestContext);
+      if (isOwner(currentPrincipal, metalake, metadataObject, requestContext)
+          && hasParentUsagePermission(
+              currentPrincipal, metalake, metadataObject, metalakeObject, 
requestContext)) {
+        return true;
       }
     } while ((metadataObject = MetadataObjects.parent(metadataObject)) != 
null);
     return false;
diff --git 
a/server-common/src/test/java/org/apache/gravitino/server/authorization/jcasbin/TestJcasbinAuthorizer.java
 
b/server-common/src/test/java/org/apache/gravitino/server/authorization/jcasbin/TestJcasbinAuthorizer.java
index 80297eda17..c5339e1a18 100644
--- 
a/server-common/src/test/java/org/apache/gravitino/server/authorization/jcasbin/TestJcasbinAuthorizer.java
+++ 
b/server-common/src/test/java/org/apache/gravitino/server/authorization/jcasbin/TestJcasbinAuthorizer.java
@@ -2428,6 +2428,30 @@ public class TestJcasbinAuthorizer {
         "Owner should be able to manage privileges without checking DENY 
MANAGE_GRANTS");
   }
 
+  @Test
+  public void testHasSetOwnerPermissionAllowsSchemaAndCatalogOwner() throws 
Exception {
+    MetadataObject metalakeObject =
+        MetadataObjects.of(ImmutableList.of(METALAKE), 
MetadataObject.Type.METALAKE);
+    metadataIdConverterMockedStatic
+        .when(() -> MetadataIdConverter.getID(eq(metalakeObject), 
eq(METALAKE)))
+        .thenReturn(Optional.of(USER_METALAKE_ID));
+    when(ownerMetaMapper.selectOwnerByMetadataObjectIdAndType(eq(CATALOG_ID), 
eq("SCHEMA")))
+        .thenReturn(new OwnerInfo(USER_ID, "USER"));
+    when(ownerMetaMapper.selectOwnerByMetadataObjectIdAndType(eq(CATALOG_ID), 
eq("CATALOG")))
+        .thenReturn(new OwnerInfo(USER_ID, "USER"));
+    getOwnerRelCache(jcasbinAuthorizer).invalidateAll();
+
+    try {
+      assertTrue(
+          jcasbinAuthorizer.hasSetOwnerPermission(
+              METALAKE, "SCHEMA", "testCatalog.testSchema", new 
AuthorizationRequestContext()));
+    } finally {
+      metadataIdConverterMockedStatic
+          .when(() -> MetadataIdConverter.getID(eq(metalakeObject), 
eq(METALAKE)))
+          .thenReturn(Optional.of(CATALOG_ID));
+    }
+  }
+
   @Test
   public void testHasSetOwnerPermissionRejectsDenyUseCatalogForTableOwner() 
throws Exception {
     makeCompletableFutureUseCurrentThread(jcasbinAuthorizer);