This is an automated email from the ASF dual-hosted git repository. github-actions[bot] pushed a commit to branch cherry-pick-c9f861ca-to-branch-1.3 in repository https://gitbox.apache.org/repos/asf/gravitino.git
commit 42689ffe1a25cb85730d5f0d7e3e162340ed8bb9 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`. # Conflicts: # server/src/test/java/org/apache/gravitino/server/web/filter/TestGravitinoInterceptionService.java --- .../web/filter/GravitinoInterceptionService.java | 2 +- .../filter/TestGravitinoInterceptionService.java | 84 ++++++++++++++++++++++ 2 files changed, 85 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 82135aa463..d55273a2d5 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 @@ -283,7 +283,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 3439287d8a..62cfed29d8 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 @@ -122,6 +122,90 @@ public class TestGravitinoInterceptionService { } @Test +<<<<<<< HEAD +======= + 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); + MockedStatic<GravitinoAuthorizerProvider> mockStatic = + mockStatic(GravitinoAuthorizerProvider.class); + MockedStatic<GravitinoEnv> envMocked = mockStatic(GravitinoEnv.class); + MockedStatic<MetalakeManager> metalakeManagerMocked = mockStatic(MetalakeManager.class)) { + // The caller declares an active role via the header; the authorizer reports it as unheld. + UserPrincipal principal = + new UserPrincipal("tester") + .withActiveRoles(ActiveRoles.of(Collections.singletonList("ghostRole"))); + principalUtilsMocked.when(PrincipalUtils::getCurrentPrincipal).thenReturn(principal); + principalUtilsMocked.when(PrincipalUtils::getCurrentUserName).thenReturn("tester"); + + MethodInvocation methodInvocation = mock(MethodInvocation.class); + GravitinoAuthorizerProvider mockedProvider = mock(GravitinoAuthorizerProvider.class); + mockStatic.when(GravitinoAuthorizerProvider::getInstance).thenReturn(mockedProvider); + GravitinoAuthorizer authorizer = mock(GravitinoAuthorizer.class); + when(mockedProvider.getGravitinoAuthorizer()).thenReturn(authorizer); + when(authorizer.findUnheldRoles( + ArgumentMatchers.any(), + ArgumentMatchers.eq("testMetalake"), + ArgumentMatchers.any(), + ArgumentMatchers.any())) + .thenReturn(Collections.singleton("ghostRole")); + + GravitinoEnv mockEnv = mock(GravitinoEnv.class); + EntityStore mockStore = mock(EntityStore.class); + envMocked.when(GravitinoEnv::getInstance).thenReturn(mockEnv); + when(mockEnv.entityStore()).thenReturn(mockStore); + metalakeManagerMocked + .when(() -> MetalakeManager.checkMetalake(ArgumentMatchers.any(), ArgumentMatchers.any())) + .thenAnswer(invocation -> null); + + GravitinoInterceptionService service = new GravitinoInterceptionService(); + Method testMethod = TestOperations.class.getMethods()[0]; + MethodInterceptor interceptor = service.getMethodInterceptors(testMethod).get(0); + when(methodInvocation.getMethod()).thenReturn(testMethod); + when(methodInvocation.getArguments()).thenReturn(new Object[] {"testMetalake"}); + + Response response = (Response) interceptor.invoke(methodInvocation); + + assertEquals(Response.Status.FORBIDDEN.getStatusCode(), response.getStatus()); + Assertions.assertTrue( + ((ErrorResponse) response.getEntity()).getMessage().contains("ghostRole")); + verify(methodInvocation, never()).proceed(); + } + } + + @Test +>>>>>>> c9f861cad ([#12670] improve(authz): Show full NameIdentifier in denial messages (#12671)) public void testSystemInternalErrorHandling() throws Throwable { try (MockedStatic<PrincipalUtils> principalUtilsMocked = mockStatic(PrincipalUtils.class); MockedStatic<GravitinoAuthorizerProvider> mockStatic =
