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 75f4bff96c [#9239] fix(authz): Return 403 instead of 404 for list
operations when metalake doesn't exist (#9271)
75f4bff96c is described below
commit 75f4bff96cf5a1c67153a2aa60d53a26af8d3db4
Author: Jerry Shao <[email protected]>
AuthorDate: Thu Nov 27 14:24:52 2025 +0800
[#9239] fix(authz): Return 403 instead of 404 for list operations when
metalake doesn't exist (#9271)
### What changes were proposed in this pull request?
This PR fixes the inconsistent HTTP response codes for tag list
operations when the metalake doesn't exist or is not in use. Previously,
`listTags()` and `listMetadataObjectsForTag()` returned 404 (Not Found)
while other tag operations like `createTag()`, `getTag()`, `alterTag()`,
and `deleteTag()` returned 403 (Forbidden).
The fix adds `@AuthorizationExpression` annotations with empty
expressions to the list tag methods, which triggers the authorization
interceptor to check metalake existence and return 403 consistently
across all tag operations.
The user, group, role, catalog have similar issues. This PR fixes them
together.
### Why are the changes needed?
Fix: #9239
When authorization is enabled, all tag operations should return
consistent error responses when the metalake doesn't exist or is not in
use. The current inconsistency confuses users and makes error handling
more complex for client applications.
### Does this PR introduce _any_ user-facing change?
Yes. The HTTP response code for `listTags()` and
`listMetadataObjectsForTag()` operations changes from 404 to 403 when
the metalake doesn't exist or is not in use. This makes the behavior
consistent with other tag operations.
### How was this patch tested?
- Added unit tests in `TestGravitinoInterceptionService`:
- `testMetalakeNotExistOrNotInUse()`: Verifies 403 response when
metalake doesn't exist
- `testEmptyExpressionSkipsAuthorization()`: Verifies empty expressions
work correctly
- Updated integration test
`TagOperationsAuthorizationIT.testTagOperationsWithNonExistentMetalake()`
to verify all tag operations return 403 consistently
- All tests pass successfully
---
.../test/authorization/CatalogAuthorizationIT.java | 47 ++++++++
.../test/authorization/GroupAuthorizationIT.java | 47 ++++++++
.../test/authorization/RoleAuthorizationIT.java | 45 +++++++
.../TagOperationsAuthorizationIT.java | 78 ++++++++++++
.../test/authorization/UserAuthorizationIT.java | 47 ++++++++
.../web/filter/GravitinoInterceptionService.java | 95 +++++++--------
.../server/web/rest/CatalogOperations.java | 4 +-
.../gravitino/server/web/rest/GroupOperations.java | 4 +-
.../gravitino/server/web/rest/RoleOperations.java | 5 +-
.../gravitino/server/web/rest/TagOperations.java | 9 +-
.../gravitino/server/web/rest/UserOperations.java | 4 +-
.../filter/TestGravitinoInterceptionService.java | 134 ++++++++++++++++++++-
12 files changed, 465 insertions(+), 54 deletions(-)
diff --git
a/clients/client-java/src/test/java/org/apache/gravitino/client/integration/test/authorization/CatalogAuthorizationIT.java
b/clients/client-java/src/test/java/org/apache/gravitino/client/integration/test/authorization/CatalogAuthorizationIT.java
index 3b62bbf229..f67f1168fc 100644
---
a/clients/client-java/src/test/java/org/apache/gravitino/client/integration/test/authorization/CatalogAuthorizationIT.java
+++
b/clients/client-java/src/test/java/org/apache/gravitino/client/integration/test/authorization/CatalogAuthorizationIT.java
@@ -22,8 +22,11 @@ import static
org.junit.jupiter.api.Assertions.assertArrayEquals;
import static org.junit.jupiter.api.Assertions.assertEquals;
import com.google.common.collect.Maps;
+import java.lang.reflect.Method;
import java.util.Map;
import org.apache.gravitino.Catalog;
+import org.apache.gravitino.client.GravitinoMetalake;
+import org.apache.gravitino.dto.MetalakeDTO;
import org.apache.gravitino.exceptions.ForbiddenException;
import org.apache.gravitino.integration.test.container.ContainerSuite;
import org.apache.gravitino.integration.test.container.HiveContainer;
@@ -146,4 +149,48 @@ public class CatalogAuthorizationIT extends
BaseRestApiAuthorizationIT {
catalogs = client.loadMetalake(METALAKE).listCatalogs();
assertEquals(0, catalogs.length);
}
+
+ @Test
+ @Order(4)
+ public void testListCatalogsWithNonExistentMetalake() throws Exception {
+ // Test that listCatalogs with @AuthorizationExpression returns 403
Forbidden
+ // when the metalake doesn't exist, instead of 404 response
+ String nonExistentMetalake = "nonExistentMetalake";
+
+ // Access the restClient from normalUserClient using reflection
+ Method restClientMethod =
+
normalUserClient.getClass().getSuperclass().getDeclaredMethod("restClient");
+ restClientMethod.setAccessible(true);
+ Object restClient = restClientMethod.invoke(normalUserClient);
+
+ // Create a MetalakeDTO for the non-existent metalake
+ MetalakeDTO metalakeDTO =
+ MetalakeDTO.builder()
+ .withName(nonExistentMetalake)
+ .withComment("test")
+ .withProperties(Maps.newHashMap())
+ .withAudit(
+ org.apache.gravitino.dto.AuditDTO.builder()
+ .withCreator("test")
+ .withCreateTime(java.time.Instant.now())
+ .build())
+ .build();
+
+ // Use DTOConverters.toMetaLake() via reflection to create
GravitinoMetalake
+ Class<?> dtoConvertersClass =
Class.forName("org.apache.gravitino.client.DTOConverters");
+ Method toMetaLakeMethod =
+ dtoConvertersClass.getDeclaredMethod(
+ "toMetaLake",
+ MetalakeDTO.class,
+ Class.forName("org.apache.gravitino.client.RESTClient"));
+ toMetaLakeMethod.setAccessible(true);
+ GravitinoMetalake nonExistentMetalakeObj =
+ (GravitinoMetalake) toMetaLakeMethod.invoke(null, metalakeDTO,
restClient);
+
+ // Test listCatalogs - should return 403 ForbiddenException
+ assertThrows(ForbiddenException.class,
nonExistentMetalakeObj::listCatalogs);
+
+ // Test listCatalogsInfo - should return 403 ForbiddenException
+ assertThrows(ForbiddenException.class,
nonExistentMetalakeObj::listCatalogsInfo);
+ }
}
diff --git
a/clients/client-java/src/test/java/org/apache/gravitino/client/integration/test/authorization/GroupAuthorizationIT.java
b/clients/client-java/src/test/java/org/apache/gravitino/client/integration/test/authorization/GroupAuthorizationIT.java
index deb5d8bf44..a2790eed74 100644
---
a/clients/client-java/src/test/java/org/apache/gravitino/client/integration/test/authorization/GroupAuthorizationIT.java
+++
b/clients/client-java/src/test/java/org/apache/gravitino/client/integration/test/authorization/GroupAuthorizationIT.java
@@ -20,12 +20,15 @@ package
org.apache.gravitino.client.integration.test.authorization;
import static org.junit.Assert.assertThrows;
import com.google.common.collect.ImmutableList;
+import com.google.common.collect.Maps;
+import java.lang.reflect.Method;
import java.util.Collections;
import java.util.HashMap;
import org.apache.gravitino.MetadataObject;
import org.apache.gravitino.MetadataObjects;
import org.apache.gravitino.authorization.Privileges;
import org.apache.gravitino.client.GravitinoMetalake;
+import org.apache.gravitino.dto.MetalakeDTO;
import org.apache.gravitino.exceptions.ForbiddenException;
import org.junit.jupiter.api.MethodOrderer;
import org.junit.jupiter.api.Order;
@@ -71,4 +74,48 @@ public class GroupAuthorizationIT extends
BaseRestApiAuthorizationIT {
gravitinoMetalake.grantRolesToUser(ImmutableList.of("role"), NORMAL_USER);
normalUserClient.loadMetalake(METALAKE).removeGroup("group2");
}
+
+ @Test
+ @Order(3)
+ public void testListGroupsWithNonExistentMetalake() throws Exception {
+ // Test that listGroups with @AuthorizationExpression returns 403 Forbidden
+ // when the metalake doesn't exist, instead of 404 response
+ String nonExistentMetalake = "nonExistentMetalake";
+
+ // Access the restClient from normalUserClient using reflection
+ Method restClientMethod =
+
normalUserClient.getClass().getSuperclass().getDeclaredMethod("restClient");
+ restClientMethod.setAccessible(true);
+ Object restClient = restClientMethod.invoke(normalUserClient);
+
+ // Create a MetalakeDTO for the non-existent metalake
+ MetalakeDTO metalakeDTO =
+ MetalakeDTO.builder()
+ .withName(nonExistentMetalake)
+ .withComment("test")
+ .withProperties(Maps.newHashMap())
+ .withAudit(
+ org.apache.gravitino.dto.AuditDTO.builder()
+ .withCreator("test")
+ .withCreateTime(java.time.Instant.now())
+ .build())
+ .build();
+
+ // Use DTOConverters.toMetaLake() via reflection to create
GravitinoMetalake
+ Class<?> dtoConvertersClass =
Class.forName("org.apache.gravitino.client.DTOConverters");
+ Method toMetaLakeMethod =
+ dtoConvertersClass.getDeclaredMethod(
+ "toMetaLake",
+ MetalakeDTO.class,
+ Class.forName("org.apache.gravitino.client.RESTClient"));
+ toMetaLakeMethod.setAccessible(true);
+ GravitinoMetalake nonExistentMetalakeObj =
+ (GravitinoMetalake) toMetaLakeMethod.invoke(null, metalakeDTO,
restClient);
+
+ // Test listGroups - should return 403 ForbiddenException
+ assertThrows(ForbiddenException.class, nonExistentMetalakeObj::listGroups);
+
+ // Test listGroupNames - should return 403 ForbiddenException
+ assertThrows(ForbiddenException.class,
nonExistentMetalakeObj::listGroupNames);
+ }
}
diff --git
a/clients/client-java/src/test/java/org/apache/gravitino/client/integration/test/authorization/RoleAuthorizationIT.java
b/clients/client-java/src/test/java/org/apache/gravitino/client/integration/test/authorization/RoleAuthorizationIT.java
index 73daa1ce72..e599651597 100644
---
a/clients/client-java/src/test/java/org/apache/gravitino/client/integration/test/authorization/RoleAuthorizationIT.java
+++
b/clients/client-java/src/test/java/org/apache/gravitino/client/integration/test/authorization/RoleAuthorizationIT.java
@@ -20,12 +20,16 @@ package
org.apache.gravitino.client.integration.test.authorization;
import static org.junit.Assert.assertArrayEquals;
import static org.junit.Assert.assertThrows;
+import com.google.common.collect.Maps;
+import java.lang.reflect.Method;
import java.util.Arrays;
import java.util.Collections;
import java.util.HashMap;
import org.apache.gravitino.MetadataObject;
import org.apache.gravitino.MetadataObjects;
import org.apache.gravitino.authorization.Privileges;
+import org.apache.gravitino.client.GravitinoMetalake;
+import org.apache.gravitino.dto.MetalakeDTO;
import org.apache.gravitino.exceptions.ForbiddenException;
import org.junit.jupiter.api.MethodOrderer;
import org.junit.jupiter.api.Order;
@@ -151,4 +155,45 @@ public class RoleAuthorizationIT extends
BaseRestApiAuthorizationIT {
.createRole("role2", new HashMap<>(), Collections.emptyList());
});
}
+
+ @Test
+ @Order(5)
+ public void testListRolesWithNonExistentMetalake() throws Exception {
+ // Test that listRoles with @AuthorizationExpression returns 403 Forbidden
+ // when the metalake doesn't exist, instead of 404 response
+ String nonExistentMetalake = "nonExistentMetalake";
+
+ // Access the restClient from normalUserClient using reflection
+ Method restClientMethod =
+
normalUserClient.getClass().getSuperclass().getDeclaredMethod("restClient");
+ restClientMethod.setAccessible(true);
+ Object restClient = restClientMethod.invoke(normalUserClient);
+
+ // Create a MetalakeDTO for the non-existent metalake
+ MetalakeDTO metalakeDTO =
+ MetalakeDTO.builder()
+ .withName(nonExistentMetalake)
+ .withComment("test")
+ .withProperties(Maps.newHashMap())
+ .withAudit(
+ org.apache.gravitino.dto.AuditDTO.builder()
+ .withCreator("test")
+ .withCreateTime(java.time.Instant.now())
+ .build())
+ .build();
+
+ // Use DTOConverters.toMetaLake() via reflection to create
GravitinoMetalake
+ Class<?> dtoConvertersClass =
Class.forName("org.apache.gravitino.client.DTOConverters");
+ Method toMetaLakeMethod =
+ dtoConvertersClass.getDeclaredMethod(
+ "toMetaLake",
+ MetalakeDTO.class,
+ Class.forName("org.apache.gravitino.client.RESTClient"));
+ toMetaLakeMethod.setAccessible(true);
+ GravitinoMetalake nonExistentMetalakeObj =
+ (GravitinoMetalake) toMetaLakeMethod.invoke(null, metalakeDTO,
restClient);
+
+ // Test listRoleNames - should return 403 ForbiddenException
+ assertThrows(ForbiddenException.class,
nonExistentMetalakeObj::listRoleNames);
+ }
}
diff --git
a/clients/client-java/src/test/java/org/apache/gravitino/client/integration/test/authorization/TagOperationsAuthorizationIT.java
b/clients/client-java/src/test/java/org/apache/gravitino/client/integration/test/authorization/TagOperationsAuthorizationIT.java
index 3d7d5eb961..247adb5a00 100644
---
a/clients/client-java/src/test/java/org/apache/gravitino/client/integration/test/authorization/TagOperationsAuthorizationIT.java
+++
b/clients/client-java/src/test/java/org/apache/gravitino/client/integration/test/authorization/TagOperationsAuthorizationIT.java
@@ -25,6 +25,7 @@ import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableSet;
import com.google.common.collect.Maps;
import java.io.IOException;
+import java.lang.reflect.Method;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.HashMap;
@@ -38,6 +39,7 @@ import org.apache.gravitino.authorization.Privileges;
import org.apache.gravitino.authorization.SecurableObject;
import org.apache.gravitino.authorization.SecurableObjects;
import org.apache.gravitino.client.GravitinoMetalake;
+import org.apache.gravitino.dto.MetalakeDTO;
import org.apache.gravitino.exceptions.ForbiddenException;
import org.apache.gravitino.integration.test.container.ContainerSuite;
import org.apache.gravitino.integration.test.container.HiveContainer;
@@ -294,6 +296,82 @@ public class TagOperationsAuthorizationIT extends
BaseRestApiAuthorizationIT {
gravitinoMetalake.deleteTag("tag1");
}
+ @Test
+ @Order(9)
+ public void testTagOperationsWithNonExistentMetalake() throws Exception {
+ // Test that all tag operations with @AuthorizationExpression return 403
Forbidden
+ // when the metalake doesn't exist, instead of inconsistent 404 responses
+ String nonExistentMetalake = "nonExistentMetalake";
+
+ // Access the restClient from normalUserClient using reflection
+ Method restClientMethod =
+
normalUserClient.getClass().getSuperclass().getDeclaredMethod("restClient");
+ restClientMethod.setAccessible(true);
+ Object restClient = restClientMethod.invoke(normalUserClient);
+
+ // Create a MetalakeDTO for the non-existent metalake
+ MetalakeDTO metalakeDTO =
+ MetalakeDTO.builder()
+ .withName(nonExistentMetalake)
+ .withComment("test")
+ .withProperties(Maps.newHashMap())
+ .withAudit(
+ org.apache.gravitino.dto.AuditDTO.builder()
+ .withCreator("test")
+ .withCreateTime(java.time.Instant.now())
+ .build())
+ .build();
+
+ // Use DTOConverters.toMetaLake() via reflection to create
GravitinoMetalake
+ Class<?> dtoConvertersClass =
Class.forName("org.apache.gravitino.client.DTOConverters");
+ Method toMetaLakeMethod =
+ dtoConvertersClass.getDeclaredMethod(
+ "toMetaLake",
+ MetalakeDTO.class,
+ Class.forName("org.apache.gravitino.client.RESTClient"));
+ toMetaLakeMethod.setAccessible(true);
+ GravitinoMetalake nonExistentMetalakeObj =
+ (GravitinoMetalake) toMetaLakeMethod.invoke(null, metalakeDTO,
restClient);
+
+ // Test createTag - should return 403 ForbiddenException
+ assertThrows(
+ ForbiddenException.class,
+ () -> nonExistentMetalakeObj.createTag("testTag", "comment",
Maps.newHashMap()));
+
+ // Test getTag - should return 403 ForbiddenException
+ assertThrows(ForbiddenException.class, () ->
nonExistentMetalakeObj.getTag("testTag"));
+
+ // Test alterTag - should return 403 ForbiddenException
+ assertThrows(
+ ForbiddenException.class,
+ () -> nonExistentMetalakeObj.alterTag("testTag",
TagChange.rename("newName")));
+
+ // Test deleteTag - should return 403 ForbiddenException
+ assertThrows(
+ ForbiddenException.class,
+ () -> {
+ nonExistentMetalakeObj.deleteTag("testTag");
+ });
+
+ // Test listTags - now has @AuthorizationExpression with empty expression,
+ // should return 403 ForbiddenException
+ assertThrows(ForbiddenException.class, nonExistentMetalakeObj::listTags);
+
+ // Test listTagsInfo - now has @AuthorizationExpression with empty
expression,
+ // should return 403 ForbiddenException
+ assertThrows(ForbiddenException.class,
nonExistentMetalakeObj::listTagsInfo);
+
+ // Test associateTags for metadata object - should return 403
ForbiddenException
+ // when trying to associate tags to a catalog in a non-existent metalake
+ assertThrows(
+ ForbiddenException.class,
+ () ->
+ nonExistentMetalakeObj
+ .loadCatalog(CATALOG)
+ .supportsTags()
+ .associateTags(new String[] {"testTag"}, null));
+ }
+
private Column[] createColumns() {
return new Column[] {Column.of("col1", Types.StringType.get())};
}
diff --git
a/clients/client-java/src/test/java/org/apache/gravitino/client/integration/test/authorization/UserAuthorizationIT.java
b/clients/client-java/src/test/java/org/apache/gravitino/client/integration/test/authorization/UserAuthorizationIT.java
index 4e39fe559b..f0b92daccc 100644
---
a/clients/client-java/src/test/java/org/apache/gravitino/client/integration/test/authorization/UserAuthorizationIT.java
+++
b/clients/client-java/src/test/java/org/apache/gravitino/client/integration/test/authorization/UserAuthorizationIT.java
@@ -21,6 +21,8 @@ import static org.junit.Assert.assertThrows;
import static org.junit.jupiter.api.Assertions.assertEquals;
import com.google.common.collect.ImmutableList;
+import com.google.common.collect.Maps;
+import java.lang.reflect.Method;
import java.util.Arrays;
import java.util.Collections;
import java.util.Comparator;
@@ -31,6 +33,7 @@ import org.apache.gravitino.authorization.Privileges;
import org.apache.gravitino.authorization.User;
import org.apache.gravitino.client.GravitinoAdminClient;
import org.apache.gravitino.client.GravitinoMetalake;
+import org.apache.gravitino.dto.MetalakeDTO;
import org.apache.gravitino.exceptions.ForbiddenException;
import org.junit.jupiter.api.Assertions;
import org.junit.jupiter.api.MethodOrderer;
@@ -126,6 +129,50 @@ public class UserAuthorizationIT extends
BaseRestApiAuthorizationIT {
assertUserEquals(new String[] {USER, NORMAL_USER, "user1"}, users);
}
+ @Test
+ @Order(4)
+ public void testListUsersWithNonExistentMetalake() throws Exception {
+ // Test that listUsers with @AuthorizationExpression returns 403 Forbidden
+ // when the metalake doesn't exist, instead of 404 response
+ String nonExistentMetalake = "nonExistentMetalake";
+
+ // Access the restClient from normalUserClient using reflection
+ Method restClientMethod =
+
normalUserClient.getClass().getSuperclass().getDeclaredMethod("restClient");
+ restClientMethod.setAccessible(true);
+ Object restClient = restClientMethod.invoke(normalUserClient);
+
+ // Create a MetalakeDTO for the non-existent metalake
+ MetalakeDTO metalakeDTO =
+ MetalakeDTO.builder()
+ .withName(nonExistentMetalake)
+ .withComment("test")
+ .withProperties(Maps.newHashMap())
+ .withAudit(
+ org.apache.gravitino.dto.AuditDTO.builder()
+ .withCreator("test")
+ .withCreateTime(java.time.Instant.now())
+ .build())
+ .build();
+
+ // Use DTOConverters.toMetaLake() via reflection to create
GravitinoMetalake
+ Class<?> dtoConvertersClass =
Class.forName("org.apache.gravitino.client.DTOConverters");
+ Method toMetaLakeMethod =
+ dtoConvertersClass.getDeclaredMethod(
+ "toMetaLake",
+ MetalakeDTO.class,
+ Class.forName("org.apache.gravitino.client.RESTClient"));
+ toMetaLakeMethod.setAccessible(true);
+ GravitinoMetalake nonExistentMetalakeObj =
+ (GravitinoMetalake) toMetaLakeMethod.invoke(null, metalakeDTO,
restClient);
+
+ // Test listUsers - should return 403 ForbiddenException
+ assertThrows(ForbiddenException.class, nonExistentMetalakeObj::listUsers);
+
+ // Test listUserNames - should return 403 ForbiddenException
+ assertThrows(ForbiddenException.class,
nonExistentMetalakeObj::listUserNames);
+ }
+
private void assertUserEquals(String[] exceptUsers, User[] actualUsers) {
Arrays.sort(exceptUsers);
Arrays.sort(actualUsers, Comparator.comparing(User::name));
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 6146781760..c0438a5354 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
@@ -41,6 +41,7 @@ import org.apache.gravitino.MetadataObject;
import org.apache.gravitino.NameIdentifier;
import org.apache.gravitino.authorization.AuthorizationUtils;
import org.apache.gravitino.exceptions.ForbiddenException;
+import org.apache.gravitino.exceptions.NoSuchMetalakeException;
import
org.apache.gravitino.server.authorization.annotations.AuthorizationExpression;
import
org.apache.gravitino.server.authorization.annotations.AuthorizationRequest;
import
org.apache.gravitino.server.authorization.expression.AuthorizationExpressionEvaluator;
@@ -132,37 +133,6 @@ public class GravitinoInterceptionService implements
InterceptionService {
AuthorizationExpression expressionAnnotation =
method.getAnnotation(AuthorizationExpression.class);
- // Check current user exists in metalake before authorization
- if (expressionAnnotation != null) {
- Object[] args = methodInvocation.getArguments();
- Map<Entity.EntityType, NameIdentifier> metadataContext =
- extractNameIdentifierFromParameters(parameters, args);
-
- // Check if current user exists in the metalake.
- NameIdentifier metalakeIdent =
metadataContext.get(Entity.EntityType.METALAKE);
-
- if (metalakeIdent != null) {
- String currentUser = PrincipalUtils.getCurrentUserName();
- try {
- AuthorizationUtils.checkCurrentUser(metalakeIdent.name(),
currentUser);
- } catch (ForbiddenException ex) {
- LOG.warn(
- "User validation failed - User: {}, Metalake: {}, Reason: {}",
- currentUser,
- metalakeIdent.name(),
- ex.getMessage());
- return Utils.forbidden(ex.getMessage(), ex);
- } catch (Exception ex) {
- LOG.error(
- "Unexpected error during user validation - User: {}, Metalake:
{}",
- currentUser,
- metalakeIdent.name(),
- ex);
- return Utils.internalError("Failed to validate user", ex);
- }
- }
- }
-
try {
AuthorizationExecutor executor;
if (expressionAnnotation != null) {
@@ -173,22 +143,53 @@ public class GravitinoInterceptionService implements
InterceptionService {
extractNameIdentifierFromParameters(parameters, args);
Map<String, Object> pathParams =
Utils.extractPathParamsFromParameters(parameters, args);
- AuthorizationExpressionEvaluator authorizationExpressionEvaluator =
- new AuthorizationExpressionEvaluator(expression);
- AuthorizationRequest.RequestType requestType =
- extractAuthorizationRequestTypeFromParameters(parameters);
- executor =
- AuthorizeExecutorFactory.create(
- requestType,
- metadataContext,
- authorizationExpressionEvaluator,
- pathParams,
- entityType,
- parameters,
- args);
- boolean authorizeResult = executor.execute();
- if (!authorizeResult) {
- return buildNoAuthResponse(expressionAnnotation, metadataContext,
method, expression);
+
+ // Check metalake and user existence before authorization
+ NameIdentifier metalakeIdent =
metadataContext.get(Entity.EntityType.METALAKE);
+ if (metalakeIdent != null) {
+ String currentUser = PrincipalUtils.getCurrentUserName();
+ try {
+ AuthorizationUtils.checkCurrentUser(metalakeIdent.name(),
currentUser);
+ } catch (NoSuchMetalakeException e) {
+ LOG.warn(
+ "Metalake {} does not exist when validating user {}",
metalakeIdent, currentUser);
+ return buildNoAuthResponse(expressionAnnotation,
metadataContext, method, expression);
+ } catch (ForbiddenException ex) {
+ LOG.warn(
+ "User validation failed - User: {}, Metalake: {}, Reason:
{}",
+ currentUser,
+ metalakeIdent.name(),
+ ex.getMessage());
+ return Utils.forbidden(ex.getMessage(), ex);
+ } catch (Exception ex) {
+ LOG.error(
+ "Unexpected error during user validation - User: {},
Metalake: {}",
+ currentUser,
+ metalakeIdent.name(),
+ ex);
+ return Utils.internalError("Failed to validate user", ex);
+ }
+ }
+
+ // If expression is empty, skip authorization check (method handles
its own filtering)
+ if (StringUtils.isNotBlank(expression)) {
+ AuthorizationExpressionEvaluator authorizationExpressionEvaluator =
+ new AuthorizationExpressionEvaluator(expression);
+ AuthorizationRequest.RequestType requestType =
+ extractAuthorizationRequestTypeFromParameters(parameters);
+ executor =
+ AuthorizeExecutorFactory.create(
+ requestType,
+ metadataContext,
+ authorizationExpressionEvaluator,
+ pathParams,
+ entityType,
+ parameters,
+ args);
+ boolean authorizeResult = executor.execute();
+ if (!authorizeResult) {
+ return buildNoAuthResponse(expressionAnnotation,
metadataContext, method, expression);
+ }
}
}
return methodInvocation.proceed();
diff --git
a/server/src/main/java/org/apache/gravitino/server/web/rest/CatalogOperations.java
b/server/src/main/java/org/apache/gravitino/server/web/rest/CatalogOperations.java
index 11b15809b2..43d7a6f779 100644
---
a/server/src/main/java/org/apache/gravitino/server/web/rest/CatalogOperations.java
+++
b/server/src/main/java/org/apache/gravitino/server/web/rest/CatalogOperations.java
@@ -84,8 +84,10 @@ public class CatalogOperations {
@Produces("application/vnd.gravitino.v1+json")
@Timed(name = "list-catalog." + MetricNames.HTTP_PROCESS_DURATION, absolute
= true)
@ResponseMetered(name = "list-catalog", absolute = true)
+ @AuthorizationExpression(expression = "")
public Response listCatalogs(
- @PathParam("metalake") String metalake,
+ @PathParam("metalake") @AuthorizationMetadata(type =
Entity.EntityType.METALAKE)
+ String metalake,
@QueryParam("details") @DefaultValue("false") boolean verbose) {
LOG.info(
"Received list catalog {} request for metalake: {}, ",
diff --git
a/server/src/main/java/org/apache/gravitino/server/web/rest/GroupOperations.java
b/server/src/main/java/org/apache/gravitino/server/web/rest/GroupOperations.java
index ae2292f26a..08ca7b7325 100644
---
a/server/src/main/java/org/apache/gravitino/server/web/rest/GroupOperations.java
+++
b/server/src/main/java/org/apache/gravitino/server/web/rest/GroupOperations.java
@@ -158,8 +158,10 @@ public class GroupOperations {
@Produces("application/vnd.gravitino.v1+json")
@Timed(name = "list-group." + MetricNames.HTTP_PROCESS_DURATION, absolute =
true)
@ResponseMetered(name = "list-group", absolute = true)
+ @AuthorizationExpression(expression = "")
public Response listGroups(
- @PathParam("metalake") String metalake,
+ @PathParam("metalake") @AuthorizationMetadata(type =
Entity.EntityType.METALAKE)
+ String metalake,
@QueryParam("details") @DefaultValue("false") boolean verbose) {
LOG.info("Received list groups request.");
try {
diff --git
a/server/src/main/java/org/apache/gravitino/server/web/rest/RoleOperations.java
b/server/src/main/java/org/apache/gravitino/server/web/rest/RoleOperations.java
index 4257819156..49f640b988 100644
---
a/server/src/main/java/org/apache/gravitino/server/web/rest/RoleOperations.java
+++
b/server/src/main/java/org/apache/gravitino/server/web/rest/RoleOperations.java
@@ -82,7 +82,10 @@ public class RoleOperations {
@Produces("application/vnd.gravitino.v1+json")
@Timed(name = "list-role." + MetricNames.HTTP_PROCESS_DURATION, absolute =
true)
@ResponseMetered(name = "list-role", absolute = true)
- public Response listRoles(@PathParam("metalake") String metalake) {
+ @AuthorizationExpression(expression = "")
+ public Response listRoles(
+ @PathParam("metalake") @AuthorizationMetadata(type =
Entity.EntityType.METALAKE)
+ String metalake) {
try {
return Utils.doAs(
httpRequest,
diff --git
a/server/src/main/java/org/apache/gravitino/server/web/rest/TagOperations.java
b/server/src/main/java/org/apache/gravitino/server/web/rest/TagOperations.java
index 0813ad4862..95a2973ddf 100644
---
a/server/src/main/java/org/apache/gravitino/server/web/rest/TagOperations.java
+++
b/server/src/main/java/org/apache/gravitino/server/web/rest/TagOperations.java
@@ -87,8 +87,10 @@ public class TagOperations {
@Produces("application/vnd.gravitino.v1+json")
@Timed(name = "list-tags." + MetricNames.HTTP_PROCESS_DURATION, absolute =
true)
@ResponseMetered(name = "list-tags", absolute = true)
+ @AuthorizationExpression(expression = "")
public Response listTags(
- @PathParam("metalake") String metalake,
+ @PathParam("metalake") @AuthorizationMetadata(type =
Entity.EntityType.METALAKE)
+ String metalake,
@QueryParam("details") @DefaultValue("false") boolean verbose) {
LOG.info(
"Received list tag {} request for metalake: {}", verbose ? "infos" :
"names", metalake);
@@ -261,8 +263,11 @@ public class TagOperations {
@Produces("application/vnd.gravitino.v1+json")
@Timed(name = "list-objects-for-tag." + MetricNames.HTTP_PROCESS_DURATION,
absolute = true)
@ResponseMetered(name = "list-objects-for-tag", absolute = true)
+ @AuthorizationExpression(expression = "")
public Response listMetadataObjectsForTag(
- @PathParam("metalake") String metalake, @PathParam("tag") String
tagName) {
+ @PathParam("metalake") @AuthorizationMetadata(type =
Entity.EntityType.METALAKE)
+ String metalake,
+ @PathParam("tag") @AuthorizationMetadata(type = Entity.EntityType.TAG)
String tagName) {
LOG.info("Received list objects for tag: {} under metalake: {}", tagName,
metalake);
try {
diff --git
a/server/src/main/java/org/apache/gravitino/server/web/rest/UserOperations.java
b/server/src/main/java/org/apache/gravitino/server/web/rest/UserOperations.java
index f35c4e5f49..2fc38033c8 100644
---
a/server/src/main/java/org/apache/gravitino/server/web/rest/UserOperations.java
+++
b/server/src/main/java/org/apache/gravitino/server/web/rest/UserOperations.java
@@ -103,8 +103,10 @@ public class UserOperations {
@Produces("application/vnd.gravitino.v1+json")
@Timed(name = "list-user." + MetricNames.HTTP_PROCESS_DURATION, absolute =
true)
@ResponseMetered(name = "list-user", absolute = true)
+ @AuthorizationExpression(expression = "")
public Response listUsers(
- @PathParam("metalake") String metalake,
+ @PathParam("metalake") @AuthorizationMetadata(type =
Entity.EntityType.METALAKE)
+ String metalake,
@QueryParam("details") @DefaultValue("false") boolean verbose) {
try {
return Utils.doAs(
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 f508d082e4..7a5826df7a 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
@@ -30,19 +30,25 @@ import javax.ws.rs.core.Response;
import org.aopalliance.intercept.MethodInterceptor;
import org.aopalliance.intercept.MethodInvocation;
import org.apache.gravitino.Entity;
+import org.apache.gravitino.EntityStore;
+import org.apache.gravitino.GravitinoEnv;
import org.apache.gravitino.MetadataObject;
import org.apache.gravitino.NameIdentifier;
import org.apache.gravitino.UserPrincipal;
import org.apache.gravitino.authorization.AuthorizationRequestContext;
+import org.apache.gravitino.authorization.AuthorizationUtils;
import org.apache.gravitino.authorization.GravitinoAuthorizer;
import org.apache.gravitino.authorization.Privilege;
import org.apache.gravitino.dto.responses.ErrorResponse;
+import org.apache.gravitino.exceptions.NoSuchMetalakeException;
+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.AuthorizationMetadata;
import org.apache.gravitino.server.web.Utils;
import org.apache.gravitino.utils.PrincipalUtils;
import org.junit.jupiter.api.Test;
+import org.mockito.ArgumentMatchers;
import org.mockito.MockedStatic;
/** Test for {@link GravitinoInterceptionService}. */
@@ -52,7 +58,9 @@ public class TestGravitinoInterceptionService {
public void testMetadataAuthorizationMethodInterceptor() throws Throwable {
try (MockedStatic<PrincipalUtils> principalUtilsMocked =
mockStatic(PrincipalUtils.class);
MockedStatic<GravitinoAuthorizerProvider> mockStatic =
- mockStatic(GravitinoAuthorizerProvider.class)) {
+ mockStatic(GravitinoAuthorizerProvider.class);
+ MockedStatic<GravitinoEnv> envMocked = mockStatic(GravitinoEnv.class);
+ MockedStatic<MetalakeManager> metalakeManagerMocked =
mockStatic(MetalakeManager.class)) {
principalUtilsMocked
.when(PrincipalUtils::getCurrentPrincipal)
.thenReturn(new UserPrincipal("tester"));
@@ -61,6 +69,18 @@ public class TestGravitinoInterceptionService {
GravitinoAuthorizerProvider mockedProvider =
mock(GravitinoAuthorizerProvider.class);
mockStatic.when(GravitinoAuthorizerProvider::getInstance).thenReturn(mockedProvider);
when(mockedProvider.getGravitinoAuthorizer()).thenReturn(new
MockGravitinoAuthorizer());
+
+ // Mock GravitinoEnv and EntityStore
+ GravitinoEnv mockEnv = mock(GravitinoEnv.class);
+ EntityStore mockStore = mock(EntityStore.class);
+ envMocked.when(GravitinoEnv::getInstance).thenReturn(mockEnv);
+ when(mockEnv.entityStore()).thenReturn(mockStore);
+
+ // Mock MetalakeManager.checkMetalake to do nothing (metalake exists)
+ metalakeManagerMocked
+ .when(() -> MetalakeManager.checkMetalake(ArgumentMatchers.any(),
ArgumentMatchers.any()))
+ .thenAnswer(invocation -> null);
+
GravitinoInterceptionService gravitinoInterceptionService =
new GravitinoInterceptionService();
Class<TestOperations> testOperationsClass = TestOperations.class;
@@ -127,6 +147,109 @@ public class TestGravitinoInterceptionService {
}
}
+ @Test
+ public void testMetalakeNotExist() 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");
+
+ MethodInvocation methodInvocation = mock(MethodInvocation.class);
+ GravitinoAuthorizerProvider mockedProvider =
mock(GravitinoAuthorizerProvider.class);
+
authorizerMocked.when(GravitinoAuthorizerProvider::getInstance).thenReturn(mockedProvider);
+ when(mockedProvider.getGravitinoAuthorizer()).thenReturn(new
MockGravitinoAuthorizer());
+
+ // Mock AuthorizationUtils.checkCurrentUser to throw
NoSuchMetalakeException
+ authorizationUtilsMocked
+ .when(
+ () ->
+ AuthorizationUtils.checkCurrentUser(
+ ArgumentMatchers.any(), ArgumentMatchers.any()))
+ .thenThrow(new NoSuchMetalakeException("Metalake nonExistentMetalake
does not exist"));
+
+ GravitinoInterceptionService gravitinoInterceptionService =
+ new GravitinoInterceptionService();
+ Class<TestOperations> testOperationsClass = TestOperations.class;
+ Method[] methods = testOperationsClass.getMethods();
+ Method testMethod = methods[0];
+ List<MethodInterceptor> methodInterceptors =
+ gravitinoInterceptionService.getMethodInterceptors(testMethod);
+ MethodInterceptor methodInterceptor = methodInterceptors.get(0);
+
+ // Test with non-existent metalake
+ when(methodInvocation.getMethod()).thenReturn(testMethod);
+ when(methodInvocation.getArguments()).thenReturn(new Object[]
{"nonExistentMetalake"});
+ Response response = (Response)
methodInterceptor.invoke(methodInvocation);
+
+ // Verify that a 403 Forbidden response is returned
+ assertEquals(Response.Status.FORBIDDEN.getStatusCode(),
response.getStatus());
+ ErrorResponse errorResponse = (ErrorResponse) response.getEntity();
+ assertEquals(
+ "User 'tester' is not authorized to perform operation 'testMethod'
on "
+ + "metadata 'nonExistentMetalake'",
+ errorResponse.getMessage());
+ }
+ }
+
+ @Test
+ public void testEmptyExpressionSkipsAuthorization() throws Throwable {
+ try (MockedStatic<PrincipalUtils> principalUtilsMocked =
mockStatic(PrincipalUtils.class);
+ MockedStatic<GravitinoAuthorizerProvider> authorizerMocked =
+ mockStatic(GravitinoAuthorizerProvider.class);
+ MockedStatic<org.apache.gravitino.GravitinoEnv> envMocked =
+ mockStatic(org.apache.gravitino.GravitinoEnv.class);
+ MockedStatic<org.apache.gravitino.metalake.MetalakeManager>
metalakeManagerMocked =
+ mockStatic(org.apache.gravitino.metalake.MetalakeManager.class)) {
+
+ principalUtilsMocked
+ .when(PrincipalUtils::getCurrentPrincipal)
+ .thenReturn(new UserPrincipal("tester"));
+
principalUtilsMocked.when(PrincipalUtils::getCurrentUserName).thenReturn("tester");
+
+ GravitinoAuthorizerProvider mockedProvider =
mock(GravitinoAuthorizerProvider.class);
+
authorizerMocked.when(GravitinoAuthorizerProvider::getInstance).thenReturn(mockedProvider);
+ when(mockedProvider.getGravitinoAuthorizer()).thenReturn(new
MockGravitinoAuthorizer());
+
+ GravitinoEnv mockEnv = mock(GravitinoEnv.class);
+ EntityStore mockStore = mock(EntityStore.class);
+ envMocked.when(GravitinoEnv::getInstance).thenReturn(mockEnv);
+ when(mockEnv.entityStore()).thenReturn(mockStore);
+
+ // Mock MetalakeManager.checkMetalake to do nothing (metalake exists)
+ metalakeManagerMocked
+ .when(() -> MetalakeManager.checkMetalake(ArgumentMatchers.any(),
ArgumentMatchers.any()))
+ .thenAnswer(invocation -> null);
+
+ GravitinoInterceptionService gravitinoInterceptionService =
+ new GravitinoInterceptionService();
+ Class<TestOperationsWithEmptyExpression> testOperationsClass =
+ TestOperationsWithEmptyExpression.class;
+ Method[] methods = testOperationsClass.getMethods();
+ Method testMethod = methods[0];
+ List<MethodInterceptor> methodInterceptors =
+ gravitinoInterceptionService.getMethodInterceptors(testMethod);
+ MethodInterceptor methodInterceptor = methodInterceptors.get(0);
+
+ MethodInvocation methodInvocation = mock(MethodInvocation.class);
+ when(methodInvocation.getMethod()).thenReturn(testMethod);
+ when(methodInvocation.getArguments()).thenReturn(new Object[]
{"testMetalake"});
+ when(methodInvocation.proceed()).thenReturn(Utils.ok("success"));
+
+ // Test with empty expression - should skip authorization and proceed
+ Response response = (Response)
methodInterceptor.invoke(methodInvocation);
+
+ // Verify that the method was allowed to proceed without authorization
check
+ assertEquals(Response.Status.OK.getStatusCode(), response.getStatus());
+ assertEquals("success", response.getEntity());
+ }
+ }
+
public static class TestOperations {
@AuthorizationExpression(
@@ -138,6 +261,15 @@ public class TestGravitinoInterceptionService {
}
}
+ public static class TestOperationsWithEmptyExpression {
+
+ @AuthorizationExpression(expression = "", accessMetadataType =
MetadataObject.Type.METALAKE)
+ public Response testMethodWithEmptyExpression(
+ @AuthorizationMetadata(type = Entity.EntityType.METALAKE) String
metalake) {
+ return Utils.ok("success");
+ }
+ }
+
private static class MockGravitinoAuthorizer implements GravitinoAuthorizer {
@Override
