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);