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 5aea36da5d [#12977] fix(server): Report dotted metadata names clearly
(#12980)
5aea36da5d is described below
commit 5aea36da5d7a9460a9b2e35e94e938c1161de4d9
Author: roryqi <[email protected]>
AuthorDate: Tue Sep 8 18:28:27 2026 +0800
[#12977] fix(server): Report dotted metadata names clearly (#12980)
### What changes were proposed in this pull request?
- Validate dotted metadata object names before authorization converts
structured identifiers into qualified names.
- Return HTTP 400 for unsupported dotted metadata names instead of
wrapping the validation failure as an internal authorization error.
- Report dotted list results explicitly when authorization requires
per-object metadata conversion.
- Preserve connector-compatible dotted names when authorization is
disabled.
- Add regression coverage for dotted table and topic names, list
filtering, and authorization interception.
### Why are the changes needed?
External systems such as Kafka and PostgreSQL may contain objects whose
names include dots. Gravitino reserves `.` as the qualified-name
separator, so authorization metadata conversion cannot represent these
names.
The existing behavior is misleading and inconsistent:
- Loading an affected object with authorization enabled may return HTTP
500 with an authorization failure message.
- Per-object list authorization may silently remove the object, making
the catalog appear complete.
The proposed validation returns a clear client error at the
authorization boundary without changing existing connector behavior when
authorization is disabled.
Fix: #12977
### Does this PR introduce _any_ user-facing change?
Yes.
When authorization metadata conversion encounters an unsupported dotted
object name, the request now returns HTTP 400 with an explicit message
such as:
```text
The TOPIC name 'orders.created.v1' is unsupported because '.' is reserved
as the qualified-name separator.
```
List operations with authorization enabled report the unsupported name
instead of silently omitting it. With authorization disabled,
connector-supported dotted names remain available.
### How was this patch tested?
```bash
./gradlew \
:core:test --tests org.apache.gravitino.utils.TestNameIdentifierUtil \
:server-common:test --tests
org.apache.gravitino.server.authorization.TestMetadataAuthzHelper \
:server:test --tests
org.apache.gravitino.server.web.filter.TestGravitinoInterceptionService \
-PskipITs -PskipWeb=true
./gradlew :catalogs:catalog-jdbc-mysql:test \
--tests
org.apache.gravitino.catalog.mysql.integration.test.CatalogMysqlIT.testListSchemaWithDotInMysqlUnderlyingDatabaseName
\
--tests
org.apache.gravitino.catalog.mysql.integration.test.CatalogMysqlIT.testObjectNamesWithDots
\
-PskipDockerTests=false
```
All tests and Spotless checks passed.
---
.../apache/gravitino/utils/NameIdentifierUtil.java | 20 ++++++++++
.../gravitino/utils/TestNameIdentifierUtil.java | 24 ++++++++++++
.../server/authorization/MetadataAuthzHelper.java | 12 ++++++
.../authorization/TestMetadataAuthzHelper.java | 44 +++++++++++++++++++++
.../web/filter/GravitinoInterceptionService.java | 4 ++
.../filter/TestGravitinoInterceptionService.java | 45 ++++++++++++++++++++++
6 files changed, 149 insertions(+)
diff --git
a/core/src/main/java/org/apache/gravitino/utils/NameIdentifierUtil.java
b/core/src/main/java/org/apache/gravitino/utils/NameIdentifierUtil.java
index 19b664adad..8efb9aab7b 100644
--- a/core/src/main/java/org/apache/gravitino/utils/NameIdentifierUtil.java
+++ b/core/src/main/java/org/apache/gravitino/utils/NameIdentifierUtil.java
@@ -618,6 +618,25 @@ public class NameIdentifierUtil {
NamespaceUtil.checkJobTemplate(ident.namespace());
}
+ /**
+ * Check whether the metadata object name can be represented in a qualified
metadata object name.
+ *
+ * @param ident The metadata object identifier to check
+ * @param entityType The metadata object entity type
+ * @throws IllegalNameIdentifierException If the object name contains the
qualified-name separator
+ */
+ public static void checkMetadataObjectName(NameIdentifier ident,
Entity.EntityType entityType) {
+ Preconditions.checkArgument(
+ ident != null && entityType != null, "The identifier and entity type
must not be null");
+
+ if (ident.name().contains(".")) {
+ throw new IllegalNameIdentifierException(
+ "The %s name '%s' is unsupported because '.' is reserved as the
qualified-name "
+ + "separator.",
+ entityType, ident.name());
+ }
+ }
+
/**
* Convert the given {@link NameIdentifier} and {@link Entity.EntityType} to
{@link
* MetadataObject}.
@@ -630,6 +649,7 @@ public class NameIdentifierUtil {
NameIdentifier ident, Entity.EntityType entityType) {
Preconditions.checkArgument(
ident != null && entityType != null, "The identifier and entity type
must not be null");
+ checkMetadataObjectName(ident, entityType);
Joiner dot = Joiner.on(".");
diff --git
a/core/src/test/java/org/apache/gravitino/utils/TestNameIdentifierUtil.java
b/core/src/test/java/org/apache/gravitino/utils/TestNameIdentifierUtil.java
index 0567651247..01ec887408 100644
--- a/core/src/test/java/org/apache/gravitino/utils/TestNameIdentifierUtil.java
+++ b/core/src/test/java/org/apache/gravitino/utils/TestNameIdentifierUtil.java
@@ -173,6 +173,30 @@ public class TestNameIdentifierUtil {
assertTrue(e3.getMessage().contains("Entity type MODEL_VERSION is not
supported"));
}
+ @Test
+ public void testRejectDottedMetadataObjectName() {
+ NameIdentifier table = NameIdentifier.of("metalake1", "catalog1",
"schema1", "sales.2024");
+ IllegalNameIdentifierException tableException =
+ assertThrows(
+ IllegalNameIdentifierException.class,
+ () -> NameIdentifierUtil.toMetadataObject(table,
Entity.EntityType.TABLE));
+ assertEquals(
+ "The TABLE name 'sales.2024' is unsupported because '.' is reserved as
the "
+ + "qualified-name separator.",
+ tableException.getMessage());
+
+ NameIdentifier topic =
+ NameIdentifier.of("metalake1", "catalog1", "schema1",
"orders.created.v1");
+ IllegalNameIdentifierException topicException =
+ assertThrows(
+ IllegalNameIdentifierException.class,
+ () -> NameIdentifierUtil.toMetadataObject(topic,
Entity.EntityType.TOPIC));
+ assertEquals(
+ "The TOPIC name 'orders.created.v1' is unsupported because '.' is
reserved as the "
+ + "qualified-name separator.",
+ topicException.getMessage());
+ }
+
@Test
void testOfUser() {
String userName = "userA";
diff --git
a/server-common/src/main/java/org/apache/gravitino/server/authorization/MetadataAuthzHelper.java
b/server-common/src/main/java/org/apache/gravitino/server/authorization/MetadataAuthzHelper.java
index c3ac8dbef3..02506e97d4 100644
---
a/server-common/src/main/java/org/apache/gravitino/server/authorization/MetadataAuthzHelper.java
+++
b/server-common/src/main/java/org/apache/gravitino/server/authorization/MetadataAuthzHelper.java
@@ -31,6 +31,7 @@ import java.util.concurrent.CompletableFuture;
import java.util.concurrent.Executor;
import java.util.concurrent.Executors;
import java.util.function.Function;
+import java.util.stream.Collectors;
import org.apache.gravitino.Config;
import org.apache.gravitino.Configs;
import org.apache.gravitino.Entity;
@@ -89,6 +90,11 @@ public class MetadataAuthzHelper {
private static final List<Entity.EntityType> REQUIRE_SCHEMA_EXISTS =
Arrays.asList(Entity.EntityType.TABLE, Entity.EntityType.TOPIC);
+ private static final Set<Entity.EntityType> METADATA_OBJECT_ENTITY_TYPES =
+ Arrays.stream(MetadataObject.Type.values())
+ .map(type -> Entity.EntityType.valueOf(type.name()))
+ .collect(Collectors.toUnmodifiableSet());
+
private static final String TABLE_PARENT_SCOPES = "METALAKE, CATALOG,
SCHEMA";
private static final String SCHEMA_PARENT_SCOPES = "METALAKE, CATALOG";
private static final String CATALOG_PARENT_SCOPES = "METALAKE";
@@ -376,6 +382,12 @@ public class MetadataAuthzHelper {
NameIdentifier[] nameIdentifiers =
Arrays.stream(entities).map(toNameIdentifier).toArray(NameIdentifier[]::new);
if (enableAuthorization() && nameIdentifiers.length > 0) {
+ if (METADATA_OBJECT_ENTITY_TYPES.contains(entityType)) {
+ Arrays.stream(nameIdentifiers)
+ .forEach(
+ identifier ->
NameIdentifierUtil.checkMetadataObjectName(identifier, entityType));
+ }
+
String principalName = PrincipalUtils.getCurrentPrincipal().getName();
if (allVisibleViaParentScope(metalake, expression, entityType,
nameIdentifiers)) {
// A privilege granted at a parent scope (metalake/catalog/schema)
makes every object in
diff --git
a/server-common/src/test/java/org/apache/gravitino/server/authorization/TestMetadataAuthzHelper.java
b/server-common/src/test/java/org/apache/gravitino/server/authorization/TestMetadataAuthzHelper.java
index cec329b30c..3fe7fc6e89 100644
---
a/server-common/src/test/java/org/apache/gravitino/server/authorization/TestMetadataAuthzHelper.java
+++
b/server-common/src/test/java/org/apache/gravitino/server/authorization/TestMetadataAuthzHelper.java
@@ -44,6 +44,7 @@ import org.apache.gravitino.authorization.GravitinoAuthorizer;
import org.apache.gravitino.authorization.Privilege;
import org.apache.gravitino.catalog.SchemaDispatcher;
import org.apache.gravitino.dto.tag.MetadataObjectDTO;
+import org.apache.gravitino.exceptions.IllegalNameIdentifierException;
import
org.apache.gravitino.server.authorization.expression.AuthorizationExpressionConstants;
import org.apache.gravitino.utils.NameIdentifierUtil;
import org.apache.gravitino.utils.PrincipalUtils;
@@ -115,6 +116,49 @@ public class TestMetadataAuthzHelper {
}
}
+ @ParameterizedTest
+ @EnumSource(
+ value = Entity.EntityType.class,
+ names = {"TABLE", "TOPIC"})
+ public void testFilterRejectsDottedExternalObjectName(Entity.EntityType
entityType) {
+ NameIdentifier[] identifiers = {
+ NameIdentifier.of("testMetalake", "testCatalog", "testSchema",
"object.with.dot")
+ };
+
+ IllegalNameIdentifierException exception =
+ Assertions.assertThrows(
+ IllegalNameIdentifierException.class,
+ () ->
+ MetadataAuthzHelper.filterByExpression(
+ "testMetalake", "", entityType, identifiers));
+
+ Assertions.assertEquals(
+ "The "
+ + entityType
+ + " name 'object.with.dot' is unsupported because '.' is reserved
as the "
+ + "qualified-name separator.",
+ exception.getMessage());
+ }
+
+ @Test
+ public void
testFilterPreservesDottedExternalObjectNameWithoutAuthorization() {
+ Config config = gravitinoEnv.config();
+ when(config.get(eq(Configs.ENABLE_AUTHORIZATION))).thenReturn(false);
+ NameIdentifier[] identifiers = {
+ NameIdentifier.of("testMetalake", "testCatalog", "testSchema",
"object.with.dot")
+ };
+
+ try {
+ NameIdentifier[] filtered =
+ MetadataAuthzHelper.filterByExpression(
+ "testMetalake", "", Entity.EntityType.TABLE, identifiers);
+
+ Assertions.assertSame(identifiers, filtered);
+ } finally {
+ when(config.get(eq(Configs.ENABLE_AUTHORIZATION))).thenReturn(true);
+ }
+ }
+
@Test
public void testPreloadUsesInternalDispatchers() throws Exception {
AccessControlDispatcher accessControlDispatcher =
mock(AccessControlDispatcher.class);
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 462c8ed6ff..4220a79f20 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
@@ -46,6 +46,7 @@ import
org.apache.gravitino.authorization.AuthorizationRequestContext;
import org.apache.gravitino.authorization.AuthorizationUtils;
import org.apache.gravitino.exceptions.BadRequestException;
import org.apache.gravitino.exceptions.ForbiddenException;
+import org.apache.gravitino.exceptions.IllegalNameIdentifierException;
import org.apache.gravitino.exceptions.NoSuchMetalakeException;
import org.apache.gravitino.lineage.source.rest.LineageOperations;
import
org.apache.gravitino.listener.api.event.server.AuthorizationDenialFailureEvent;
@@ -262,6 +263,9 @@ public class GravitinoInterceptionService implements
InterceptionService {
}
}
return methodInvocation.proceed();
+ } catch (IllegalNameIdentifierException ex) {
+ LOG.warn("Invalid metadata object identifier during authorization",
ex);
+ return Utils.illegalArguments(ex.getMessage(), ex);
} catch (Exception ex) {
String currentUser = PrincipalUtils.getCurrentUserName();
String methodName = methodInvocation.getMethod().getName();
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 2d9e83857c..0e06ba92dd 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
@@ -358,6 +358,51 @@ public class TestGravitinoInterceptionService {
}
}
+ @Test
+ public void testDottedMetadataNameReturnsBadRequest() throws Throwable {
+ try (MockedStatic<PrincipalUtils> principalUtilsMocked =
mockStatic(PrincipalUtils.class);
+ MockedStatic<GravitinoAuthorizerProvider> authorizerMocked =
+ mockStatic(GravitinoAuthorizerProvider.class);
+ MockedStatic<AuthorizationUtils> authorizationUtilsMocked =
+ mockStatic(AuthorizationUtils.class)) {
+ principalUtilsMocked
+ .when(PrincipalUtils::getCurrentPrincipal)
+ .thenReturn(new UserPrincipal("tester"));
+
principalUtilsMocked.when(PrincipalUtils::getCurrentUserName).thenReturn("tester");
+ authorizationUtilsMocked
+ .when(
+ () ->
+ AuthorizationUtils.checkCurrentUser(
+ ArgumentMatchers.any(), ArgumentMatchers.any(),
ArgumentMatchers.any()))
+ .thenAnswer(invocation -> null);
+
+ GravitinoAuthorizerProvider provider =
mock(GravitinoAuthorizerProvider.class);
+ GravitinoAuthorizer authorizer = tableProbeAuthorizer();
+
authorizerMocked.when(GravitinoAuthorizerProvider::getInstance).thenReturn(provider);
+ when(provider.getGravitinoAuthorizer()).thenReturn(authorizer);
+
+ Method method =
+ TestTableLoadOperations.class.getMethod(
+ "loadTable", String.class, String.class, String.class,
String.class, String.class);
+ MethodInvocation invocation = mock(MethodInvocation.class);
+ when(invocation.getMethod()).thenReturn(method);
+ when(invocation.getArguments())
+ .thenReturn(
+ new Object[] {"testMetalake", "testCatalog", "testSchema",
"sales.2024", null});
+
+ MethodInterceptor interceptor =
+ new
GravitinoInterceptionService().getMethodInterceptors(method).get(0);
+ Response response = (Response) interceptor.invoke(invocation);
+
+ assertEquals(Response.Status.BAD_REQUEST.getStatusCode(),
response.getStatus());
+ assertEquals(
+ "The TABLE name 'sales.2024' is unsupported because '.' is reserved
as the "
+ + "qualified-name separator.",
+ ((ErrorResponse) response.getEntity()).getMessage());
+ verify(invocation, never()).proceed();
+ }
+ }
+
@Test
public void testMetalakeNotExist() throws Throwable {
try (MockedStatic<PrincipalUtils> principalUtilsMocked =
mockStatic(PrincipalUtils.class);
