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 c9f861cad7 [#12670] improve(authz): Show full NameIdentifier in denial 
messages (#12671)
c9f861cad7 is described below

commit c9f861cad79b5ff43848552ddded6eaefa619cb8
Author: jarred0214 <[email protected]>
AuthorDate: Thu Aug 27 21:06:48 2026 +0800

    [#12670] improve(authz): Show full NameIdentifier in denial messages 
(#12671)
    
    ### What changes were proposed in this pull request?
    
    This PR updates authorization denial messages generated by
    `GravitinoInterceptionService` to show the full `NameIdentifier` of the
    denied metadata object instead of only the last segment.
    
    For example, a table authorization failure now reports metadata like
    `metalake.catalog.schema.table` instead of `table`.
    
    A unit test is added to cover multi-segment `NameIdentifier` rendering
    in the no-auth response.
    
    ### Why are the changes needed?
    
    When different catalogs or schemas contain metadata objects with the
    same name, showing only `NameIdentifier.name()` makes authorization
    failures harder to diagnose.
    
    Showing the full `NameIdentifier` helps users identify the exact denied
    resource directly from the 403 error message.
    
    Fixes #12670
    
    ### Does this PR introduce _any_ user-facing change?
    
    Yes. The diagnostic text in 403 authorization denial messages changes
    from the last metadata name segment to the full `NameIdentifier`.
    
    This does not change authorization logic, HTTP status codes, error
    response structure, response fields, or user permissions.
    
    ### How was this patch tested?
    
    Added a unit test in `TestGravitinoInterceptionService` to verify that
    authorization denial messages include the full metadata
    `NameIdentifier`.
    
    Ran `JAVA_HOME=/opt/homebrew/opt/openjdk@17 ./gradlew :server:test
    --tests
    org.apache.gravitino.server.web.filter.TestGravitinoInterceptionService
    -PskipITs`.
---
 .../web/filter/GravitinoInterceptionService.java   |  2 +-
 .../filter/TestGravitinoInterceptionService.java   | 32 ++++++++++++++++++++++
 2 files changed, 33 insertions(+), 1 deletion(-)

diff --git 
a/server/src/main/java/org/apache/gravitino/server/web/filter/GravitinoInterceptionService.java
 
b/server/src/main/java/org/apache/gravitino/server/web/filter/GravitinoInterceptionService.java
index 11926facc3..8c5e24dac3 100644
--- 
a/server/src/main/java/org/apache/gravitino/server/web/filter/GravitinoInterceptionService.java
+++ 
b/server/src/main/java/org/apache/gravitino/server/web/filter/GravitinoInterceptionService.java
@@ -312,7 +312,7 @@ public class GravitinoInterceptionService implements 
InterceptionService {
       String contextualMessage;
       String accessMetadataMessage =
           accessMetadataName != null
-              ? String.format("on metadata '%s'", accessMetadataName.name())
+              ? String.format("on metadata '%s'", 
accessMetadataName.toString())
               : "";
       if (StringUtils.isNotBlank(errorMessage)) {
         contextualMessage =
diff --git 
a/server/src/test/java/org/apache/gravitino/server/web/filter/TestGravitinoInterceptionService.java
 
b/server/src/test/java/org/apache/gravitino/server/web/filter/TestGravitinoInterceptionService.java
index e3548d857c..615e0810d3 100644
--- 
a/server/src/test/java/org/apache/gravitino/server/web/filter/TestGravitinoInterceptionService.java
+++ 
b/server/src/test/java/org/apache/gravitino/server/web/filter/TestGravitinoInterceptionService.java
@@ -136,6 +136,38 @@ public class TestGravitinoInterceptionService {
     }
   }
 
+  @Test
+  public void testNoAuthResponseUsesFullMetadataNameIdentifier() throws 
Exception {
+    MethodInterceptor methodInterceptor =
+        new GravitinoInterceptionService()
+            .getMethodInterceptors(TestOperations.class.getMethods()[0])
+            .get(0);
+    Method buildNoAuthResponse =
+        methodInterceptor
+            .getClass()
+            .getDeclaredMethod(
+                "buildNoAuthResponse",
+                String.class,
+                NameIdentifier.class,
+                String.class,
+                String.class);
+    buildNoAuthResponse.setAccessible(true);
+
+    Response response =
+        (Response)
+            buildNoAuthResponse.invoke(
+                methodInterceptor,
+                "",
+                NameIdentifier.of("testMetalake", "testCatalog", "testSchema", 
"testTable"),
+                "tester",
+                "loadTable");
+
+    assertEquals(
+        "User 'tester' is not authorized to perform operation 'loadTable' on 
metadata "
+            + "'testMetalake.testCatalog.testSchema.testTable'",
+        ((ErrorResponse) response.getEntity()).getMessage());
+  }
+
   @Test
   public void testRejectsUnheldActiveRolesWith403() throws Throwable {
     try (MockedStatic<PrincipalUtils> principalUtilsMocked = 
mockStatic(PrincipalUtils.class);

Reply via email to