This is an automated email from the ASF dual-hosted git repository.
jerryshao pushed a commit to branch branch-1.3
in repository https://gitbox.apache.org/repos/asf/gravitino.git
The following commit(s) were added to refs/heads/branch-1.3 by this push:
new 8014cfa30c [Cherry-pick to branch-1.3] [#12727] fix(server): Return
400 for invalid metadata object types (#13059) (#13073)
8014cfa30c is described below
commit 8014cfa30cb24453885e9d696dc5194c803053cf
Author: github-actions[bot]
<41898282+github-actions[bot]@users.noreply.github.com>
AuthorDate: Thu Sep 10 20:07:41 2026 +0800
[Cherry-pick to branch-1.3] [#12727] fix(server): Return 400 for invalid
metadata object types (#13059) (#13073)
**Cherry-pick Information:**
- Original commit: b6b0f24f9af095950f7d2bd6ebdadd688032d249
- Target branch: `branch-1.3`
- Status: ✅ **Conflicts resolved**
**Conflict resolution:**
Conflicts were limited to `TestGravitinoInterceptionService.java`, which
was adapted to `branch-1.3`:
- Dropped `testRejectsUnheldActiveRolesWith403`, which depends on the
active-roles feature not present on `branch-1.3`.
- Kept `testInvalidMetadataObjectTypeReturnsBadRequest`, using
`TagsAssociateRequest` in place of the `main`-only
`TagValuesAssociateRequest`.
`TestGravitinoInterceptionService` passes on `branch-1.3` (14 tests).
---------
Co-authored-by: Nevin Zheng <[email protected]>
Co-authored-by: Jerry Shao <[email protected]>
Co-authored-by: Claude Opus 5 <[email protected]>
Co-authored-by: Jerry Shao <[email protected]>
---
.../InvalidMetadataObjectTypeAuthorizationIT.java | 67 ++++++++++++++++++
.../web/filter/GravitinoInterceptionService.java | 5 ++
.../gravitino/server/web/filter/ParameterUtil.java | 10 ++-
.../filter/TestGravitinoInterceptionService.java | 81 ++++++++++++++++++++++
4 files changed, 161 insertions(+), 2 deletions(-)
diff --git
a/clients/client-java/src/test/java/org/apache/gravitino/client/integration/test/authorization/InvalidMetadataObjectTypeAuthorizationIT.java
b/clients/client-java/src/test/java/org/apache/gravitino/client/integration/test/authorization/InvalidMetadataObjectTypeAuthorizationIT.java
new file mode 100644
index 0000000000..a8530caba3
--- /dev/null
+++
b/clients/client-java/src/test/java/org/apache/gravitino/client/integration/test/authorization/InvalidMetadataObjectTypeAuthorizationIT.java
@@ -0,0 +1,67 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.gravitino.client.integration.test.authorization;
+
+import java.net.URI;
+import java.net.http.HttpClient;
+import java.net.http.HttpRequest;
+import java.net.http.HttpResponse;
+import java.nio.charset.StandardCharsets;
+import java.util.Base64;
+import org.apache.gravitino.auth.AuthConstants;
+import org.apache.gravitino.dto.responses.ErrorConstants;
+import org.apache.gravitino.dto.responses.ErrorResponse;
+import org.apache.gravitino.server.web.ObjectMapperProvider;
+import org.junit.jupiter.api.Assertions;
+import org.junit.jupiter.api.Test;
+
+/** Integration tests for invalid metadata object types in
authorization-protected REST paths. */
+public class InvalidMetadataObjectTypeAuthorizationIT extends
BaseRestApiAuthorizationIT {
+
+ /** Verifies that an invalid metadata object type is reported as malformed
client input. */
+ @Test
+ public void testInvalidMetadataObjectTypeReturnsBadRequest() throws
Exception {
+ String authorization =
+ AuthConstants.AUTHORIZATION_BASIC_HEADER
+ + Base64.getEncoder()
+ .encodeToString((USER +
":dummy").getBytes(StandardCharsets.UTF_8));
+ HttpRequest request =
+ HttpRequest.newBuilder()
+ .uri(new URI(serverUri +
"/api/metalakes/zz/objects/bogusType/a.b.c/tags"))
+ .header(AuthConstants.HTTP_HEADER_AUTHORIZATION, authorization)
+ .GET()
+ .build();
+
+ HttpResponse<String> response =
+ HttpClient.newHttpClient().send(request,
HttpResponse.BodyHandlers.ofString());
+
+ Assertions.assertEquals(400, response.statusCode(), "Unexpected body: " +
response.body());
+ ErrorResponse errorResponse =
+ ObjectMapperProvider.objectMapper().readValue(response.body(),
ErrorResponse.class);
+ Assertions.assertEquals(ErrorConstants.ILLEGAL_ARGUMENTS_CODE,
errorResponse.getCode());
+ Assertions.assertEquals(
+ IllegalArgumentException.class.getSimpleName(),
errorResponse.getType());
+ Assertions.assertTrue(
+ errorResponse.getMessage().contains("bogusType"),
+ "Unexpected message: " + errorResponse.getMessage());
+ Assertions.assertFalse(
+ errorResponse.getMessage().contains("Authorization failed due to
system internal error"),
+ "Unexpected message: " + errorResponse.getMessage());
+ }
+}
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 202e3f6d87..25ebd402b1 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
@@ -45,6 +45,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.IllegalMetadataObjectException;
import org.apache.gravitino.exceptions.IllegalNameIdentifierException;
import org.apache.gravitino.exceptions.NoSuchMetalakeException;
import org.apache.gravitino.lineage.source.rest.LineageOperations;
@@ -256,6 +257,10 @@ public class GravitinoInterceptionService implements
InterceptionService {
}
}
return methodInvocation.proceed();
+ } catch (IllegalMetadataObjectException ex) {
+ LOG.warn("Invalid metadata object type during authorization", ex);
+ return Utils.illegalArguments(
+ IllegalArgumentException.class.getSimpleName(), ex.getMessage(),
ex);
} catch (IllegalNameIdentifierException ex) {
LOG.warn("Invalid metadata object identifier during authorization",
ex);
return Utils.illegalArguments(ex.getMessage(), ex);
diff --git
a/server/src/main/java/org/apache/gravitino/server/web/filter/ParameterUtil.java
b/server/src/main/java/org/apache/gravitino/server/web/filter/ParameterUtil.java
index 70c0731d64..026a5a47f3 100644
---
a/server/src/main/java/org/apache/gravitino/server/web/filter/ParameterUtil.java
+++
b/server/src/main/java/org/apache/gravitino/server/web/filter/ParameterUtil.java
@@ -28,6 +28,7 @@ import org.apache.gravitino.Entity;
import org.apache.gravitino.MetadataObject;
import org.apache.gravitino.MetadataObjects;
import org.apache.gravitino.NameIdentifier;
+import org.apache.gravitino.exceptions.IllegalMetadataObjectException;
import
org.apache.gravitino.server.authorization.annotations.AuthorizationFullName;
import
org.apache.gravitino.server.authorization.annotations.AuthorizationMetadata;
import
org.apache.gravitino.server.authorization.annotations.AuthorizationObjectType;
@@ -82,8 +83,13 @@ public class ParameterUtil {
if (fullName.isPresent() && metadataObjectType.isPresent()) {
String metalake = entities.get(Entity.EntityType.METALAKE);
if (metalake != null) {
- MetadataObject.Type type =
-
MetadataObject.Type.valueOf(metadataObjectType.get().toUpperCase(Locale.ROOT));
+ String rawType = metadataObjectType.get();
+ MetadataObject.Type type;
+ try {
+ type = MetadataObject.Type.valueOf(rawType.toUpperCase(Locale.ROOT));
+ } catch (IllegalArgumentException e) {
+ throw new IllegalMetadataObjectException(e, "Invalid metadata object
type: %s", rawType);
+ }
NameIdentifier nameIdentifier =
MetadataObjectUtil.toEntityIdent(metalake,
MetadataObjects.parse(fullName.get(), type));
nameIdentifierMap.putAll(
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 6ff61d736b..3000fa752f 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
@@ -17,6 +17,7 @@
package org.apache.gravitino.server.web.filter;
+import static
org.apache.gravitino.server.authorization.expression.AuthorizationExpressionConstants.CAN_ACCESS_METADATA_AND_TAG;
import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.mockito.ArgumentMatchers.any;
import static org.mockito.Mockito.mock;
@@ -48,6 +49,8 @@ import org.apache.gravitino.authorization.GravitinoAuthorizer;
import org.apache.gravitino.authorization.Privilege;
import org.apache.gravitino.catalog.ViewDispatcher;
import org.apache.gravitino.dto.requests.SchemaCreateRequest;
+import org.apache.gravitino.dto.requests.TagsAssociateRequest;
+import org.apache.gravitino.dto.responses.ErrorConstants;
import org.apache.gravitino.dto.responses.ErrorResponse;
import org.apache.gravitino.exceptions.ForbiddenException;
import org.apache.gravitino.exceptions.NoSuchMetalakeException;
@@ -56,7 +59,10 @@ import
org.apache.gravitino.listener.api.event.server.AuthorizationDenialFailure
import org.apache.gravitino.metalake.MetalakeManager;
import org.apache.gravitino.server.authorization.GravitinoAuthorizerProvider;
import
org.apache.gravitino.server.authorization.annotations.AuthorizationExpression;
+import
org.apache.gravitino.server.authorization.annotations.AuthorizationFullName;
import
org.apache.gravitino.server.authorization.annotations.AuthorizationMetadata;
+import
org.apache.gravitino.server.authorization.annotations.AuthorizationObjectType;
+import
org.apache.gravitino.server.authorization.annotations.AuthorizationRequest;
import org.apache.gravitino.server.web.Utils;
import org.apache.gravitino.server.web.rest.SchemaOperations;
import org.apache.gravitino.server.web.rest.TableOperations;
@@ -255,6 +261,34 @@ public class TestGravitinoInterceptionService {
((ErrorResponse) response.getEntity()).getMessage());
}
+ @Test
+ public void testInvalidMetadataObjectTypeReturnsBadRequest() throws
Throwable {
+ Method method =
+ TestMetadataObjectTagAssociationOperations.class.getMethod(
+ "associateTagsForObject",
+ String.class,
+ String.class,
+ String.class,
+ TagsAssociateRequest.class);
+ MethodInvocation invocation = mock(MethodInvocation.class);
+ when(invocation.getMethod()).thenReturn(method);
+ when(invocation.getArguments())
+ .thenReturn(new Object[] {"testMetalake", "bogusType", "a.b.c", null});
+
+ MethodInterceptor interceptor =
+ new
GravitinoInterceptionService().getMethodInterceptors(method).get(0);
+ Response response = (Response) interceptor.invoke(invocation);
+
+ assertEquals(Response.Status.BAD_REQUEST.getStatusCode(),
response.getStatus());
+ ErrorResponse errorResponse = (ErrorResponse) response.getEntity();
+ assertEquals(ErrorConstants.ILLEGAL_ARGUMENTS_CODE,
errorResponse.getCode());
+ assertEquals(IllegalArgumentException.class.getSimpleName(),
errorResponse.getType());
+ Assertions.assertTrue(errorResponse.getMessage().contains("bogusType"));
+ Assertions.assertFalse(
+ errorResponse.getMessage().contains("Authorization failed due to
system internal error"));
+ verify(invocation, never()).proceed();
+ }
+
@Test
public void testSystemInternalErrorHandling() throws Throwable {
try (MockedStatic<PrincipalUtils> principalUtilsMocked =
mockStatic(PrincipalUtils.class);
@@ -298,6 +332,40 @@ public class TestGravitinoInterceptionService {
}
}
+ @Test
+ public void testUnexpectedIllegalArgumentExceptionRemainsInternalError()
throws Throwable {
+ try (MockedStatic<PrincipalUtils> principalUtilsMocked =
mockStatic(PrincipalUtils.class);
+ MockedStatic<GravitinoAuthorizerProvider> mockStatic =
+ mockStatic(GravitinoAuthorizerProvider.class)) {
+ principalUtilsMocked
+ .when(PrincipalUtils::getCurrentPrincipal)
+ .thenReturn(new UserPrincipal("tester"));
+
principalUtilsMocked.when(PrincipalUtils::getCurrentUserName).thenReturn("tester");
+
+ MethodInvocation methodInvocation = mock(MethodInvocation.class);
+ GravitinoAuthorizerProvider mockedProvider =
mock(GravitinoAuthorizerProvider.class);
+
mockStatic.when(GravitinoAuthorizerProvider::getInstance).thenReturn(mockedProvider);
+ when(mockedProvider.getGravitinoAuthorizer())
+ .thenThrow(new IllegalArgumentException("Invalid authorizer
configuration"));
+
+ GravitinoInterceptionService gravitinoInterceptionService =
+ new GravitinoInterceptionService();
+ Method testMethod = TestOperations.class.getMethods()[0];
+ MethodInterceptor methodInterceptor =
+
gravitinoInterceptionService.getMethodInterceptors(testMethod).get(0);
+ when(methodInvocation.getMethod()).thenReturn(testMethod);
+ when(methodInvocation.getArguments()).thenReturn(new Object[]
{"testMetalake"});
+
+ Response response = (Response)
methodInterceptor.invoke(methodInvocation);
+
+ ErrorResponse errorResponse = (ErrorResponse) response.getEntity();
+ assertEquals(
+ "Authorization failed due to system internal error. Please contact
administrator.",
+ errorResponse.getMessage());
+ assertEquals(Response.Status.INTERNAL_SERVER_ERROR.getStatusCode(),
response.getStatus());
+ }
+ }
+
@Test
public void testDottedMetadataNameReturnsBadRequest() throws Throwable {
try (MockedStatic<PrincipalUtils> principalUtilsMocked =
mockStatic(PrincipalUtils.class);
@@ -619,6 +687,19 @@ public class TestGravitinoInterceptionService {
}
}
+ public static class TestMetadataObjectTagAssociationOperations {
+
+ @AuthorizationExpression(expression = CAN_ACCESS_METADATA_AND_TAG)
+ public Response associateTagsForObject(
+ @AuthorizationMetadata(type = Entity.EntityType.METALAKE) String
metalake,
+ @AuthorizationObjectType String type,
+ @AuthorizationFullName String fullName,
+ @AuthorizationRequest(type =
AuthorizationRequest.RequestType.ASSOCIATE_TAG)
+ TagsAssociateRequest request) {
+ return Utils.ok("unused");
+ }
+ }
+
public static class TestOperations {
@AuthorizationExpression(
