This is an automated email from the ASF dual-hosted git repository.
roryqi 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 405fcdad3e [#14610] fix(authz): Return 400 for invalid requests on
branch-1.3 (#14613)
405fcdad3e is described below
commit 405fcdad3e0c1eb9f5cf0245007ca89bffa02fc7
Author: Jerry Shao <[email protected]>
AuthorDate: Thu Nov 5 09:41:48 2026 +0800
[#14610] fix(authz): Return 400 for invalid requests on branch-1.3 (#14613)
### What changes were proposed in this pull request?
Adapt the authorization validation fixes from #14612 to branch-1.3:
- Return HTTP 400 for invalid metadata object names and privileges.
- Validate all privileges without short-circuiting.
- Return HTTP 400 for null or invalid tag/policy association bodies.
Preserve HTTP 500 for unexpected internal errors.
### Why are the changes needed?
Invalid client inputs are incorrectly reported as authorization internal
errors. Null tag/policy association bodies return 403 instead of 400.
Fixes #14610
### Does this PR introduce _any_ user-facing change?
Affected invalid requests return HTTP 400 with the validation error
instead of HTTP 500 or 403.
### How was this patch tested?
- Added regression coverage for tag/policy association bodies, invalid
object names, and invalid privileges.
- Confirmed empty association bodies returned 500 before the fix.
- Passed 103 targeted authorization and REST tests.
- Ran `./gradlew spotlessApply` and `git diff --check`.
---
.../web/filter/GravitinoInterceptionService.java | 6 +-
.../gravitino/server/web/filter/ParameterUtil.java | 7 +-
.../server/web/filter/RequestValidation.java | 75 ++++++++
.../AssociatePolicyAuthorizationExecutor.java | 6 +-
.../AssociateTagAuthorizationExecutor.java | 6 +-
.../LineageAuthorizationExecutor.java | 8 +-
.../LoadTableAuthorizationExecutor.java | 11 +-
.../authorization/RunJobAuthorizationExecutor.java | 9 +-
.../filter/TestGravitinoInterceptionService.java | 200 +++++++++++++++++++++
9 files changed, 304 insertions(+), 24 deletions(-)
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 e5440fdd70..47fa28e110 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
@@ -256,9 +256,6 @@ public class GravitinoInterceptionService implements
InterceptionService {
viewIdent.name(),
NameIdentifierUtil.getSchemaIdentifier(viewIdent).name(),
exception);
- } catch (BadRequestException exception) {
- LOG.warn("Invalid authorization request", exception);
- return Utils.illegalArguments(exception.getMessage(), exception);
}
if (!authorizeResult) {
MetadataObject.Type type =
expressionAnnotation.accessMetadataType();
@@ -280,6 +277,9 @@ public class GravitinoInterceptionService implements
InterceptionService {
}
}
return methodInvocation.proceed();
+ } catch (BadRequestException ex) {
+ LOG.warn("Invalid authorization request", ex);
+ return Utils.illegalArguments(ex.getMessage(), ex);
} catch (IllegalMetadataObjectException ex) {
LOG.warn("Invalid metadata object type during authorization", ex);
return Utils.illegalArguments(
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 026a5a47f3..eede162869 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
@@ -90,8 +90,11 @@ public class ParameterUtil {
} catch (IllegalArgumentException e) {
throw new IllegalMetadataObjectException(e, "Invalid metadata object
type: %s", rawType);
}
- NameIdentifier nameIdentifier =
- MetadataObjectUtil.toEntityIdent(metalake,
MetadataObjects.parse(fullName.get(), type));
+ MetadataObject object =
+ RequestValidation.parse(
+ () -> MetadataObjects.parse(fullName.get(), type),
+ e -> new IllegalMetadataObjectException(e, "%s",
e.getMessage()));
+ NameIdentifier nameIdentifier =
MetadataObjectUtil.toEntityIdent(metalake, object);
nameIdentifierMap.putAll(
NameIdentifierUtil.splitNameIdentifier(
metalake, MetadataObjectUtil.toEntityType(type),
nameIdentifier));
diff --git
a/server/src/main/java/org/apache/gravitino/server/web/filter/RequestValidation.java
b/server/src/main/java/org/apache/gravitino/server/web/filter/RequestValidation.java
new file mode 100644
index 0000000000..418091f924
--- /dev/null
+++
b/server/src/main/java/org/apache/gravitino/server/web/filter/RequestValidation.java
@@ -0,0 +1,75 @@
+/*
+ * 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.server.web.filter;
+
+import java.util.function.Function;
+import java.util.function.Supplier;
+import org.apache.gravitino.exceptions.BadRequestException;
+
+/** Converts failures from explicit client-input validation into request
errors. */
+public final class RequestValidation {
+
+ private RequestValidation() {}
+
+ /**
+ * Validates client input without including authorization or backend
operations.
+ *
+ * @param validation the client-input validation to perform
+ * @throws BadRequestException if validation rejects the input
+ */
+ public static void validate(Runnable validation) {
+ parse(
+ () -> {
+ validation.run();
+ return null;
+ });
+ }
+
+ /**
+ * Parses client input and reports invalid arguments as a bad request.
+ *
+ * @param parser the client-input parser, excluding configuration and
authorization operations
+ * @param <T> the parsed result type
+ * @return the parsed result
+ * @throws BadRequestException if parsing rejects the input
+ */
+ public static <T> T parse(Supplier<T> parser) {
+ return parse(parser, e -> new BadRequestException(e, "%s",
e.getMessage()));
+ }
+
+ /**
+ * Parses client input while preserving the caller's request-error type and
message.
+ *
+ * <p>Only explicit client-input parsing belongs in this scope. Authorizer
construction,
+ * configuration loading, expression evaluation, and backend calls must
remain outside it.
+ *
+ * @param parser the client-input parser
+ * @param errorMapper converts an input validation failure into the
appropriate request error
+ * @param <T> the parsed result type
+ * @return the parsed result
+ */
+ public static <T> T parse(
+ Supplier<T> parser,
+ Function<IllegalArgumentException, ? extends RuntimeException>
errorMapper) {
+ try {
+ return parser.get();
+ } catch (IllegalArgumentException e) {
+ throw errorMapper.apply(e);
+ }
+ }
+}
diff --git
a/server/src/main/java/org/apache/gravitino/server/web/filter/authorization/AssociatePolicyAuthorizationExecutor.java
b/server/src/main/java/org/apache/gravitino/server/web/filter/authorization/AssociatePolicyAuthorizationExecutor.java
index 59b2e83305..6e32c7c891 100644
---
a/server/src/main/java/org/apache/gravitino/server/web/filter/authorization/AssociatePolicyAuthorizationExecutor.java
+++
b/server/src/main/java/org/apache/gravitino/server/web/filter/authorization/AssociatePolicyAuthorizationExecutor.java
@@ -29,6 +29,8 @@ import org.apache.gravitino.Entity;
import org.apache.gravitino.NameIdentifier;
import org.apache.gravitino.authorization.AuthorizationRequestContext;
import org.apache.gravitino.dto.requests.PoliciesAssociateRequest;
+import org.apache.gravitino.exceptions.BadRequestException;
+import org.apache.gravitino.server.web.filter.RequestValidation;
/**
* Metadata object authorization for {@link
@@ -57,7 +59,7 @@ public class AssociatePolicyAuthorizationExecutor extends
CommonAuthorizerExecut
public boolean execute(AuthorizationRequestContext context) throws Exception
{
Object request = extractFromParameters(parameters, args);
if (request == null) {
- return false;
+ throw new BadRequestException("Request body cannot be null");
}
context.setOriginalAuthorizationExpression(expression);
@@ -67,7 +69,7 @@ public class AssociatePolicyAuthorizationExecutor extends
CommonAuthorizerExecut
request instanceof PoliciesAssociateRequest,
"Only policy can use AssociatePolicyAuthorizationExecutor, please
contact the administrator.");
PoliciesAssociateRequest policiesAssociateRequest =
(PoliciesAssociateRequest) request;
- policiesAssociateRequest.validate();
+ RequestValidation.validate(policiesAssociateRequest::validate);
// Authorize both 'policiesToAdd' and 'policiesToRemove' fields.
return authorizePolicy(policiesAssociateRequest.getPoliciesToAdd(),
context, targetType)
&& authorizePolicy(policiesAssociateRequest.getPoliciesToRemove(),
context, targetType);
diff --git
a/server/src/main/java/org/apache/gravitino/server/web/filter/authorization/AssociateTagAuthorizationExecutor.java
b/server/src/main/java/org/apache/gravitino/server/web/filter/authorization/AssociateTagAuthorizationExecutor.java
index 2fef4e9ee3..0535889f11 100644
---
a/server/src/main/java/org/apache/gravitino/server/web/filter/authorization/AssociateTagAuthorizationExecutor.java
+++
b/server/src/main/java/org/apache/gravitino/server/web/filter/authorization/AssociateTagAuthorizationExecutor.java
@@ -29,6 +29,8 @@ import org.apache.gravitino.Entity;
import org.apache.gravitino.NameIdentifier;
import org.apache.gravitino.authorization.AuthorizationRequestContext;
import org.apache.gravitino.dto.requests.TagsAssociateRequest;
+import org.apache.gravitino.exceptions.BadRequestException;
+import org.apache.gravitino.server.web.filter.RequestValidation;
import org.apache.gravitino.server.web.rest.MetadataObjectTagOperations;
/**
@@ -57,7 +59,7 @@ public class AssociateTagAuthorizationExecutor extends
CommonAuthorizerExecutor
public boolean execute(AuthorizationRequestContext context) throws Exception
{
Object request = extractFromParameters(parameters, args);
if (request == null) {
- return false;
+ throw new BadRequestException("Request body cannot be null");
}
context.setOriginalAuthorizationExpression(expression);
@@ -67,7 +69,7 @@ public class AssociateTagAuthorizationExecutor extends
CommonAuthorizerExecutor
request instanceof TagsAssociateRequest,
"Only tag can use AssociateTagAuthorizationExecutor, please contact
the administrator.");
TagsAssociateRequest tagsAssociateRequest = (TagsAssociateRequest) request;
- tagsAssociateRequest.validate();
+ RequestValidation.validate(tagsAssociateRequest::validate);
// Authorize both 'tagsToAdd' and 'tagsToRemove' fields.
return authorizeTag(tagsAssociateRequest.getTagsToAdd(), context,
targetType)
&& authorizeTag(tagsAssociateRequest.getTagsToRemove(), context,
targetType);
diff --git
a/server/src/main/java/org/apache/gravitino/server/web/filter/authorization/LineageAuthorizationExecutor.java
b/server/src/main/java/org/apache/gravitino/server/web/filter/authorization/LineageAuthorizationExecutor.java
index 5af3a60d62..2ef2dafb9e 100644
---
a/server/src/main/java/org/apache/gravitino/server/web/filter/authorization/LineageAuthorizationExecutor.java
+++
b/server/src/main/java/org/apache/gravitino/server/web/filter/authorization/LineageAuthorizationExecutor.java
@@ -38,9 +38,9 @@ import org.apache.gravitino.MetadataObject;
import org.apache.gravitino.MetadataObjects;
import org.apache.gravitino.NameIdentifier;
import org.apache.gravitino.authorization.AuthorizationRequestContext;
-import org.apache.gravitino.exceptions.BadRequestException;
import org.apache.gravitino.lineage.source.rest.LineageEventValidator;
import
org.apache.gravitino.server.authorization.expression.AuthorizationExpressionEvaluator;
+import org.apache.gravitino.server.web.filter.RequestValidation;
import org.apache.gravitino.utils.MetadataObjectUtil;
import org.apache.gravitino.utils.NameIdentifierUtil;
@@ -80,11 +80,7 @@ public class LineageAuthorizationExecutor implements
AuthorizationExecutor {
@Override
public boolean execute(AuthorizationRequestContext context) {
if (!authorizationTargetsResolved) {
- try {
- resolveAuthorizationTargets(extractRunEvent());
- } catch (IllegalArgumentException e) {
- throw new BadRequestException(e, "%s", e.getMessage());
- }
+ RequestValidation.validate(() ->
resolveAuthorizationTargets(extractRunEvent()));
}
AuthorizationExpressionEvaluator evaluator = new
AuthorizationExpressionEvaluator(expression);
diff --git
a/server/src/main/java/org/apache/gravitino/server/web/filter/authorization/LoadTableAuthorizationExecutor.java
b/server/src/main/java/org/apache/gravitino/server/web/filter/authorization/LoadTableAuthorizationExecutor.java
index 36b49e43e1..d94febf8dc 100644
---
a/server/src/main/java/org/apache/gravitino/server/web/filter/authorization/LoadTableAuthorizationExecutor.java
+++
b/server/src/main/java/org/apache/gravitino/server/web/filter/authorization/LoadTableAuthorizationExecutor.java
@@ -26,9 +26,11 @@ import org.apache.gravitino.GravitinoEnv;
import org.apache.gravitino.NameIdentifier;
import org.apache.gravitino.authorization.AuthorizationRequestContext;
import org.apache.gravitino.authorization.Privilege;
+import org.apache.gravitino.exceptions.BadRequestException;
import
org.apache.gravitino.server.authorization.annotations.ExpressionCondition;
import
org.apache.gravitino.server.authorization.expression.AuthorizationExpressionEvaluator;
import org.apache.gravitino.server.web.filter.ParameterUtil;
+import org.apache.gravitino.server.web.filter.RequestValidation;
/**
* Authorization executor for load table operations.
@@ -107,7 +109,12 @@ public class LoadTableAuthorizationExecutor extends
CommonAuthorizerExecutor {
return Arrays.stream(privileges.split(","))
.map(String::trim)
.filter(StringUtils::isNotBlank)
- .map(Privilege.Name::valueOf)
- .anyMatch(Privilege.Name.MODIFY_TABLE::equals);
+ .map(
+ privilege ->
+ RequestValidation.parse(
+ () -> Privilege.Name.valueOf(privilege),
+ e -> new BadRequestException(e, "Invalid privilege: %s",
privilege)))
+ .toList()
+ .contains(Privilege.Name.MODIFY_TABLE);
}
}
diff --git
a/server/src/main/java/org/apache/gravitino/server/web/filter/authorization/RunJobAuthorizationExecutor.java
b/server/src/main/java/org/apache/gravitino/server/web/filter/authorization/RunJobAuthorizationExecutor.java
index aaadc129b9..dec8e1eb4e 100644
---
a/server/src/main/java/org/apache/gravitino/server/web/filter/authorization/RunJobAuthorizationExecutor.java
+++
b/server/src/main/java/org/apache/gravitino/server/web/filter/authorization/RunJobAuthorizationExecutor.java
@@ -30,6 +30,7 @@ import
org.apache.gravitino.authorization.AuthorizationRequestContext;
import org.apache.gravitino.dto.requests.JobRunRequest;
import org.apache.gravitino.exceptions.BadRequestException;
import
org.apache.gravitino.server.authorization.expression.AuthorizationExpressionEvaluator;
+import org.apache.gravitino.server.web.filter.RequestValidation;
import org.apache.gravitino.utils.NameIdentifierUtil;
public class RunJobAuthorizationExecutor implements AuthorizationExecutor {
@@ -71,13 +72,7 @@ public class RunJobAuthorizationExecutor implements
AuthorizationExecutor {
"Expected JobRunRequest but found %s",
request.getClass().getSimpleName());
JobRunRequest jobRunRequest = (JobRunRequest) request;
- try {
- jobRunRequest.validate();
- } catch (IllegalArgumentException e) {
- // validate() throws IllegalArgumentException; map to BadRequest so the
interceptor returns
- // 400 instead of the catch-all 500 authorization internal error.
- throw new BadRequestException(e, "%s", e.getMessage());
- }
+ RequestValidation.validate(jobRunRequest::validate);
String jobTemplateName = jobRunRequest.getJobTemplateName();
NameIdentifier metalake = metadataContext.get(Entity.EntityType.METALAKE);
NameIdentifier jobTemplateNameIdentifier =
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 33ffa50053..63175f93cc 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
@@ -28,6 +28,7 @@ import static org.mockito.Mockito.mockStatic;
import static org.mockito.Mockito.never;
import static org.mockito.Mockito.spy;
import static org.mockito.Mockito.verify;
+import static org.mockito.Mockito.verifyNoInteractions;
import static org.mockito.Mockito.when;
import com.google.common.collect.ImmutableList;
@@ -58,6 +59,8 @@ import org.apache.gravitino.catalog.TableDispatcher;
import org.apache.gravitino.catalog.ViewDispatcher;
import org.apache.gravitino.dto.requests.CatalogUpdateRequest;
import org.apache.gravitino.dto.requests.CatalogUpdatesRequest;
+import org.apache.gravitino.dto.requests.JobRunRequest;
+import org.apache.gravitino.dto.requests.PoliciesAssociateRequest;
import org.apache.gravitino.dto.requests.SchemaCreateRequest;
import org.apache.gravitino.dto.requests.TagsAssociateRequest;
import org.apache.gravitino.dto.responses.ErrorConstants;
@@ -66,6 +69,7 @@ import org.apache.gravitino.exceptions.ForbiddenException;
import org.apache.gravitino.exceptions.NoSuchMetalakeException;
import org.apache.gravitino.exceptions.NoSuchTableException;
import org.apache.gravitino.exceptions.NoSuchViewException;
+import org.apache.gravitino.json.JsonUtils;
import org.apache.gravitino.listener.EventBus;
import
org.apache.gravitino.listener.api.event.server.AuthorizationDenialFailureEvent;
import org.apache.gravitino.meta.AuditInfo;
@@ -87,6 +91,9 @@ import
org.apache.gravitino.server.authorization.annotations.AuthorizationObject
import
org.apache.gravitino.server.authorization.annotations.AuthorizationRequest;
import org.apache.gravitino.server.web.Utils;
import org.apache.gravitino.server.web.rest.CatalogOperations;
+import org.apache.gravitino.server.web.rest.JobOperations;
+import org.apache.gravitino.server.web.rest.MetadataObjectPolicyOperations;
+import org.apache.gravitino.server.web.rest.MetadataObjectTagOperations;
import org.apache.gravitino.server.web.rest.SchemaOperations;
import org.apache.gravitino.server.web.rest.TableOperations;
import org.apache.gravitino.server.web.rest.ViewOperations;
@@ -96,6 +103,8 @@ import org.glassfish.hk2.api.Descriptor;
import org.junit.jupiter.api.AfterEach;
import org.junit.jupiter.api.Assertions;
import org.junit.jupiter.api.Test;
+import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.ValueSource;
import org.mockito.ArgumentCaptor;
import org.mockito.ArgumentMatchers;
import org.mockito.MockedStatic;
@@ -109,6 +118,158 @@ public class TestGravitinoInterceptionService {
RequestContext.clear();
}
+ /** Verifies invalid tag and policy association bodies are rejected before
resource dispatch. */
+ @ParameterizedTest
+ @ValueSource(strings = {"tags", "policies"})
+ public void testInvalidAssociationBodyReturnsBadRequest(String kind) throws
Throwable {
+ boolean tags = kind.equals("tags");
+ Method method =
+ tags
+ ? MetadataObjectTagOperations.class.getMethod(
+ "associateTagsForObject",
+ String.class,
+ String.class,
+ String.class,
+ TagsAssociateRequest.class)
+ : MetadataObjectPolicyOperations.class.getMethod(
+ "associatePoliciesForObject",
+ String.class,
+ String.class,
+ String.class,
+ PoliciesAssociateRequest.class);
+ Class<?> requestClass = tags ? TagsAssociateRequest.class :
PoliciesAssociateRequest.class;
+ try (MockedStatic<PrincipalUtils> principals =
mockStatic(PrincipalUtils.class);
+ MockedStatic<GravitinoAuthorizerProvider> providers =
+ mockStatic(GravitinoAuthorizerProvider.class);
+ MockedStatic<AuthorizationUtils> authUtils =
mockStatic(AuthorizationUtils.class)) {
+ principals.when(PrincipalUtils::getCurrentPrincipal).thenReturn(new
UserPrincipal("tester"));
+ principals.when(PrincipalUtils::getCurrentUserName).thenReturn("tester");
+ GravitinoAuthorizerProvider provider =
mock(GravitinoAuthorizerProvider.class);
+
providers.when(GravitinoAuthorizerProvider::getInstance).thenReturn(provider);
+ GravitinoAuthorizer authorizer = mock(GravitinoAuthorizer.class);
+ when(provider.getGravitinoAuthorizer()).thenReturn(authorizer);
+ String add = kind + "ToAdd";
+ String remove = kind + "ToRemove";
+ String bothNull = add + " and " + remove + " cannot both be null";
+ String[][] cases = {
+ {"{}", bothNull},
+ {"{\"" + add + "\":null,\"" + remove + "\":null}", bothNull},
+ {"null", "Request body cannot be null"},
+ {"{\"" + add + "\":[null]}", add},
+ {"{\"" + add + "\":[\"\"]}", add},
+ {"{\"" + add + "\":[\" \"]}", add},
+ {"{\"" + remove + "\":[null]}", remove},
+ {"{\"" + remove + "\":[\"\"]}", remove},
+ {"{\"" + remove + "\":[\" \"]}", remove}
+ };
+ for (String[] testCase : cases) {
+ Object request = JsonUtils.objectMapper().readValue(testCase[0],
requestClass);
+ MethodInvocation invocation = mock(MethodInvocation.class);
+ when(invocation.getMethod()).thenReturn(method);
+ when(invocation.getArguments())
+ .thenReturn(new Object[] {"metalake", "schema", "catalog.schema",
request});
+ Response response =
+ (Response)
+ new GravitinoInterceptionService()
+ .getMethodInterceptors(method)
+ .get(0)
+ .invoke(invocation);
+ assertEquals(400, response.getStatus(), testCase[0]);
+ ErrorResponse error = (ErrorResponse) response.getEntity();
+ assertEquals(ErrorConstants.ILLEGAL_ARGUMENTS_CODE, error.getCode());
+ Assertions.assertTrue(error.getMessage().contains(testCase[1]),
error.getMessage());
+ Assertions.assertFalse(error.getMessage().contains("Authorization
failed"));
+ verify(invocation, never()).proceed();
+ }
+ verifyNoInteractions(authorizer);
+ }
+ }
+
+ /** Verifies that malformed object names return client errors for tag and
policy endpoints. */
+ @Test
+ public void testInvalidMetadataObjectFullNameReturnsBadRequest() throws
Throwable {
+ Method tags =
+ MetadataObjectTagOperations.class.getMethod(
+ "associateTagsForObject",
+ String.class,
+ String.class,
+ String.class,
+ TagsAssociateRequest.class);
+ Method policies =
+ MetadataObjectPolicyOperations.class.getMethod(
+ "listPoliciesForMetadataObject",
+ String.class,
+ String.class,
+ String.class,
+ boolean.class);
+ for (Method method : new Method[] {tags, policies}) {
+ for (String fullName : new String[] {"catalog", "catalog.schema.extra",
"catalog.*", " "}) {
+ MethodInvocation invocation = mock(MethodInvocation.class);
+ when(invocation.getMethod()).thenReturn(method);
+ Object request =
+ method.equals(tags) ? new TagsAssociateRequest(new String[]
{"tag"}, null) : false;
+ when(invocation.getArguments())
+ .thenReturn(new Object[] {"metalake", "schema", fullName,
request});
+ Response response =
+ (Response)
+ new GravitinoInterceptionService()
+ .getMethodInterceptors(method)
+ .get(0)
+ .invoke(invocation);
+ assertEquals(400, response.getStatus(), fullName);
+ ErrorResponse error = (ErrorResponse) response.getEntity();
+ assertEquals(ErrorConstants.ILLEGAL_ARGUMENTS_CODE, error.getCode());
+ assertEquals(IllegalArgumentException.class.getSimpleName(),
error.getType());
+ Assertions.assertFalse(error.getMessage().contains("Authorization
failed"));
+ verify(invocation, never()).proceed();
+ }
+ }
+ }
+
+ /** Verifies that invalid privileges return 400 even after a valid
MODIFY_TABLE privilege. */
+ @Test
+ public void testInvalidLoadTablePrivilegesReturnsBadRequest() throws
Throwable {
+ try (MockedStatic<PrincipalUtils> principals =
mockStatic(PrincipalUtils.class);
+ MockedStatic<GravitinoAuthorizerProvider> providers =
+ mockStatic(GravitinoAuthorizerProvider.class);
+ MockedStatic<AuthorizationUtils> authUtils =
mockStatic(AuthorizationUtils.class)) {
+ principals.when(PrincipalUtils::getCurrentPrincipal).thenReturn(new
UserPrincipal("tester"));
+ principals.when(PrincipalUtils::getCurrentUserName).thenReturn("tester");
+ GravitinoAuthorizerProvider provider =
mock(GravitinoAuthorizerProvider.class);
+
providers.when(GravitinoAuthorizerProvider::getInstance).thenReturn(provider);
+
when(provider.getGravitinoAuthorizer()).thenReturn(mock(GravitinoAuthorizer.class));
+ Method method =
+ TableOperations.class.getMethod(
+ "loadTable", String.class, String.class, String.class,
String.class, String.class);
+ for (String privileges :
+ new String[] {
+ "BOGUS",
+ "select_table",
+ "SELECT_TABLE,BOGUS",
+ "MODIFY_TABLE,BOGUS",
+ "BOGUS,MODIFY_TABLE"
+ }) {
+ MethodInvocation invocation = mock(MethodInvocation.class);
+ when(invocation.getMethod()).thenReturn(method);
+ when(invocation.getArguments())
+ .thenReturn(new Object[] {"metalake", "catalog", "schema",
"table", privileges});
+ Response response =
+ (Response)
+ new GravitinoInterceptionService()
+ .getMethodInterceptors(method)
+ .get(0)
+ .invoke(invocation);
+ assertEquals(400, response.getStatus(), privileges);
+ ErrorResponse error = (ErrorResponse) response.getEntity();
+ assertEquals(ErrorConstants.ILLEGAL_ARGUMENTS_CODE, error.getCode());
+ assertEquals(
+ "Invalid privilege: " + (privileges.equals("select_table") ?
"select_table" : "BOGUS"),
+ error.getMessage());
+ verify(invocation, never()).proceed();
+ }
+ }
+ }
+
@Test
public void testPathMetalakeValidationPrecedesExecutorConstruction() throws
Throwable {
Method method =
@@ -355,6 +516,45 @@ public class TestGravitinoInterceptionService {
}
}
+ /** Verifies that authorizer failures after successful input validation
remain server errors. */
+ @Test
+ public void testAuthorizerIllegalArgumentExceptionRemainsInternalError()
throws Throwable {
+ try (MockedStatic<PrincipalUtils> principals =
mockStatic(PrincipalUtils.class);
+ MockedStatic<GravitinoAuthorizerProvider> providers =
+ mockStatic(GravitinoAuthorizerProvider.class);
+ MockedStatic<AuthorizationUtils> authUtils =
mockStatic(AuthorizationUtils.class)) {
+ principals.when(PrincipalUtils::getCurrentPrincipal).thenReturn(new
UserPrincipal("tester"));
+ principals.when(PrincipalUtils::getCurrentUserName).thenReturn("tester");
+ GravitinoAuthorizerProvider provider =
mock(GravitinoAuthorizerProvider.class);
+ GravitinoAuthorizer authorizer = mock(GravitinoAuthorizer.class);
+
providers.when(GravitinoAuthorizerProvider::getInstance).thenReturn(provider);
+ when(provider.getGravitinoAuthorizer()).thenReturn(authorizer);
+ when(authorizer.isOwner(any(), any(), any(), any()))
+ .thenThrow(new IllegalArgumentException("Invalid authorizer
policy"));
+ Method method = JobOperations.class.getMethod("runJob", String.class,
JobRunRequest.class);
+ MethodInvocation invocation = mock(MethodInvocation.class);
+ when(invocation.getMethod()).thenReturn(method);
+ when(invocation.getArguments())
+ .thenReturn(
+ new Object[] {"metalake", new JobRunRequest("template",
Collections.emptyMap())});
+
+ Response response =
+ (Response)
+ new GravitinoInterceptionService()
+ .getMethodInterceptors(method)
+ .get(0)
+ .invoke(invocation);
+
+ assertEquals(500, response.getStatus());
+ Assertions.assertTrue(
+ ((ErrorResponse) response.getEntity())
+ .getMessage()
+ .contains("Authorization failed due to system internal error"));
+ verify(authorizer).isOwner(any(), any(), any(), any());
+ verify(invocation, never()).proceed();
+ }
+ }
+
@Test
public void testUnexpectedIllegalArgumentExceptionRemainsInternalError()
throws Throwable {
try (MockedStatic<PrincipalUtils> principalUtilsMocked =
mockStatic(PrincipalUtils.class);
