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 1be3aee846 [#9270] : feat(iceberg): Refactor checkCurrentUser call to
the interceptor (#9268)
1be3aee846 is described below
commit 1be3aee8462b9de0070f9e7b22526d7c2469333d
Author: Bharath Krishna <[email protected]>
AuthorDate: Wed Nov 26 00:41:57 2025 -0800
[#9270] : feat(iceberg): Refactor checkCurrentUser call to the interceptor
(#9268)
### What changes were proposed in this pull request?
Moving the check to the interception service ensures the user is
validated **once per request** before authorization begins, improving
performance while maintaining the same security guarantees.
### Why are the changes needed?
Better refactor and minimize duplicate calls
Fix : #9270
### Does this PR introduce _any_ user-facing change?
No. The validation still occurs at the same point in the request flow
with identical behavior - non-existent users still receive
`ForbiddenException`.
### How was this patch tested?
Running all the tests
---
.../test/authorization/CheckCurrentUserIT.java | 9 +++++
.../gravitino/authorization/UserGroupManager.java | 11 +++++-
.../hook/AccessControlHookDispatcher.java | 3 --
.../gravitino/hook/CatalogHookDispatcher.java | 4 ---
.../gravitino/hook/FilesetHookDispatcher.java | 4 ---
.../apache/gravitino/hook/ModelHookDispatcher.java | 9 -----
.../gravitino/hook/SchemaHookDispatcher.java | 4 ---
.../apache/gravitino/hook/TableHookDispatcher.java | 5 ---
.../apache/gravitino/hook/TopicHookDispatcher.java | 4 ---
.../dispatcher/IcebergNamespaceHookDispatcher.java | 3 --
.../dispatcher/IcebergTableHookDispatcher.java | 5 ---
...BaseMetadataAuthorizationMethodInterceptor.java | 26 ++++++++++++++
.../authorization/jcasbin/JcasbinAuthorizer.java | 6 ++++
.../authorization/TestPassThroughAuthorizer.java | 42 +++++++++++++++++++---
.../web/filter/GravitinoInterceptionService.java | 42 +++++++++++++++++++---
15 files changed, 127 insertions(+), 50 deletions(-)
diff --git
a/clients/client-java/src/test/java/org/apache/gravitino/client/integration/test/authorization/CheckCurrentUserIT.java
b/clients/client-java/src/test/java/org/apache/gravitino/client/integration/test/authorization/CheckCurrentUserIT.java
index 517a92a99f..f829f91ceb 100644
---
a/clients/client-java/src/test/java/org/apache/gravitino/client/integration/test/authorization/CheckCurrentUserIT.java
+++
b/clients/client-java/src/test/java/org/apache/gravitino/client/integration/test/authorization/CheckCurrentUserIT.java
@@ -95,7 +95,9 @@ public class CheckCurrentUserIT extends BaseIT {
GravitinoAdminClient anotherClient =
GravitinoAdminClient.builder(uri).withSimpleAuth().build();
metalake = client.createMetalake(metalakeName, "metalake comment",
Collections.emptyMap());
+ metalake.addUser("test");
anotherMetalake = anotherClient.loadMetalake(metalakeName);
+ metalake.removeUser("test");
}
@AfterAll
@@ -167,7 +169,9 @@ public class CheckCurrentUserIT extends BaseIT {
catalogName, Catalog.Type.FILESET, "hadoop", "comment",
Collections.emptyMap());
// Test to create a schema with a not-existed user
+ metalake.addUser("test");
Catalog anotherCatalog = anotherMetalake.loadCatalog(catalogName);
+ metalake.removeUser("test");
Assertions.assertThrows(
ForbiddenException.class,
() -> anotherCatalog.asSchemas().createSchema("schema", "comment",
Collections.emptyMap()));
@@ -202,12 +206,15 @@ public class CheckCurrentUserIT extends BaseIT {
SecurableObject metalakeSecObject =
SecurableObjects.ofMetalake(
metalakeName,
Lists.newArrayList(Privileges.CreateCatalog.allow()));
+
+ // Test creating role with non-existent user fails
Assertions.assertThrows(
ForbiddenException.class,
() ->
anotherMetalake.createRole(
"role", Collections.emptyMap(),
Lists.newArrayList(metalakeSecObject)));
+ // Test creating role with admin user succeeds
Assertions.assertDoesNotThrow(
() ->
metalake.createRole(
@@ -232,7 +239,9 @@ public class CheckCurrentUserIT extends BaseIT {
catalogName, Catalog.Type.RELATIONAL, "hive", "catalog comment",
properties);
// Test to create schema with not-existed user
+ metalake.addUser("test");
Catalog anotherCatalog = anotherMetalake.loadCatalog(catalogName);
+ metalake.removeUser("test");
Assertions.assertThrows(
ForbiddenException.class,
() -> anotherCatalog.asSchemas().createSchema("schema", "comment",
Collections.emptyMap()));
diff --git
a/core/src/main/java/org/apache/gravitino/authorization/UserGroupManager.java
b/core/src/main/java/org/apache/gravitino/authorization/UserGroupManager.java
index 905b100a65..14f548cf6a 100644
---
a/core/src/main/java/org/apache/gravitino/authorization/UserGroupManager.java
+++
b/core/src/main/java/org/apache/gravitino/authorization/UserGroupManager.java
@@ -38,6 +38,7 @@ import
org.apache.gravitino.exceptions.NoSuchMetalakeException;
import org.apache.gravitino.exceptions.NoSuchUserException;
import org.apache.gravitino.exceptions.UserAlreadyExistsException;
import org.apache.gravitino.meta.AuditInfo;
+import org.apache.gravitino.meta.BaseMetalake;
import org.apache.gravitino.meta.GroupEntity;
import org.apache.gravitino.meta.UserEntity;
import org.apache.gravitino.storage.IdGenerator;
@@ -104,7 +105,15 @@ class UserGroupManager {
User getUser(String metalake, String user) throws NoSuchUserException {
try {
- checkMetalake(NameIdentifier.of(metalake), store);
+ // Only check metalake existence, not enabled status, to allow reading
user info
+ // on disabled metalakes for internal operations like checkCurrentUser().
+ try {
+ store.get(NameIdentifier.of(metalake), Entity.EntityType.METALAKE,
BaseMetalake.class);
+ } catch (NoSuchEntityException e) {
+ LOG.error("Metalake {} does not exist", metalake, e);
+ throw new NoSuchMetalakeException(METALAKE_DOES_NOT_EXIST_MSG,
metalake);
+ }
+
return store.get(
AuthorizationUtils.ofUser(metalake, user), Entity.EntityType.USER,
UserEntity.class);
diff --git
a/core/src/main/java/org/apache/gravitino/hook/AccessControlHookDispatcher.java
b/core/src/main/java/org/apache/gravitino/hook/AccessControlHookDispatcher.java
index 90f5b4cc98..df6847544c 100644
---
a/core/src/main/java/org/apache/gravitino/hook/AccessControlHookDispatcher.java
+++
b/core/src/main/java/org/apache/gravitino/hook/AccessControlHookDispatcher.java
@@ -159,9 +159,6 @@ public class AccessControlHookDispatcher implements
AccessControlDispatcher {
Map<String, String> properties,
List<SecurableObject> securableObjects)
throws RoleAlreadyExistsException, NoSuchMetalakeException {
- // Check whether the current user exists or not
- AuthorizationUtils.checkCurrentUser(metalake,
PrincipalUtils.getCurrentUserName());
-
Role createdRole = dispatcher.createRole(metalake, role, properties,
securableObjects);
// Set the creator as the owner of role.
diff --git
a/core/src/main/java/org/apache/gravitino/hook/CatalogHookDispatcher.java
b/core/src/main/java/org/apache/gravitino/hook/CatalogHookDispatcher.java
index 8284765b0a..ab643acb8a 100644
--- a/core/src/main/java/org/apache/gravitino/hook/CatalogHookDispatcher.java
+++ b/core/src/main/java/org/apache/gravitino/hook/CatalogHookDispatcher.java
@@ -79,10 +79,6 @@ public class CatalogHookDispatcher implements
CatalogDispatcher {
String comment,
Map<String, String> properties)
throws NoSuchMetalakeException, CatalogAlreadyExistsException {
- // Check whether the current user exists or not
- AuthorizationUtils.checkCurrentUser(
- ident.namespace().level(0), PrincipalUtils.getCurrentUserName());
-
Catalog catalog = dispatcher.createCatalog(ident, type, provider, comment,
properties);
try {
diff --git
a/core/src/main/java/org/apache/gravitino/hook/FilesetHookDispatcher.java
b/core/src/main/java/org/apache/gravitino/hook/FilesetHookDispatcher.java
index 0efb8a1198..800c6a66a2 100644
--- a/core/src/main/java/org/apache/gravitino/hook/FilesetHookDispatcher.java
+++ b/core/src/main/java/org/apache/gravitino/hook/FilesetHookDispatcher.java
@@ -75,10 +75,6 @@ public class FilesetHookDispatcher implements
FilesetDispatcher {
Map<String, String> storageLocations,
Map<String, String> properties)
throws NoSuchSchemaException, FilesetAlreadyExistsException {
- // Check whether the current user exists or not
- AuthorizationUtils.checkCurrentUser(
- ident.namespace().level(0), PrincipalUtils.getCurrentUserName());
-
Fileset fileset =
dispatcher.createMultipleLocationFileset(
ident, comment, type, storageLocations, properties);
diff --git
a/core/src/main/java/org/apache/gravitino/hook/ModelHookDispatcher.java
b/core/src/main/java/org/apache/gravitino/hook/ModelHookDispatcher.java
index 50651e9b00..97837f8103 100644
--- a/core/src/main/java/org/apache/gravitino/hook/ModelHookDispatcher.java
+++ b/core/src/main/java/org/apache/gravitino/hook/ModelHookDispatcher.java
@@ -24,7 +24,6 @@ import org.apache.gravitino.Entity;
import org.apache.gravitino.GravitinoEnv;
import org.apache.gravitino.NameIdentifier;
import org.apache.gravitino.Namespace;
-import org.apache.gravitino.authorization.AuthorizationUtils;
import org.apache.gravitino.authorization.Owner;
import org.apache.gravitino.authorization.OwnerDispatcher;
import org.apache.gravitino.catalog.ModelDispatcher;
@@ -67,10 +66,6 @@ public class ModelHookDispatcher implements ModelDispatcher {
@Override
public Model registerModel(NameIdentifier ident, String comment, Map<String,
String> properties)
throws NoSuchSchemaException, ModelAlreadyExistsException {
- // Check whether the current user exists or not
- AuthorizationUtils.checkCurrentUser(
- ident.namespace().level(0), PrincipalUtils.getCurrentUserName());
-
Model model = dispatcher.registerModel(ident, comment, properties);
// Set the creator as owner of the model.
@@ -159,10 +154,6 @@ public class ModelHookDispatcher implements
ModelDispatcher {
Map<String, String> properties)
throws NoSuchSchemaException, ModelAlreadyExistsException,
ModelVersionAliasesAlreadyExistException {
- // Check whether the current user exists or not
- AuthorizationUtils.checkCurrentUser(
- ident.namespace().level(0), PrincipalUtils.getCurrentUserName());
-
Model model = dispatcher.registerModel(ident, uris, aliases, comment,
properties);
// Set the creator as owner of the model.
diff --git
a/core/src/main/java/org/apache/gravitino/hook/SchemaHookDispatcher.java
b/core/src/main/java/org/apache/gravitino/hook/SchemaHookDispatcher.java
index 191c1e5859..a90be09111 100644
--- a/core/src/main/java/org/apache/gravitino/hook/SchemaHookDispatcher.java
+++ b/core/src/main/java/org/apache/gravitino/hook/SchemaHookDispatcher.java
@@ -57,10 +57,6 @@ public class SchemaHookDispatcher implements
SchemaDispatcher {
@Override
public Schema createSchema(NameIdentifier ident, String comment, Map<String,
String> properties)
throws NoSuchCatalogException, SchemaAlreadyExistsException {
- // Check whether the current user exists or not
- AuthorizationUtils.checkCurrentUser(
- ident.namespace().level(0), PrincipalUtils.getCurrentUserName());
-
Schema schema = dispatcher.createSchema(ident, comment, properties);
// Set the creator as the owner of the schema.
diff --git
a/core/src/main/java/org/apache/gravitino/hook/TableHookDispatcher.java
b/core/src/main/java/org/apache/gravitino/hook/TableHookDispatcher.java
index 55e9ad40a2..dc2bd0fb64 100644
--- a/core/src/main/java/org/apache/gravitino/hook/TableHookDispatcher.java
+++ b/core/src/main/java/org/apache/gravitino/hook/TableHookDispatcher.java
@@ -74,11 +74,6 @@ public class TableHookDispatcher implements TableDispatcher {
SortOrder[] sortOrders,
Index[] indexes)
throws NoSuchSchemaException, TableAlreadyExistsException {
- // Check whether the current user exists or not
- // TODO(bharos): Move this to PassThroughAuthorizer
- AuthorizationUtils.checkCurrentUser(
- ident.namespace().level(0), PrincipalUtils.getCurrentUserName());
-
Table table =
dispatcher.createTable(
ident, columns, comment, properties, partitions, distribution,
sortOrders, indexes);
diff --git
a/core/src/main/java/org/apache/gravitino/hook/TopicHookDispatcher.java
b/core/src/main/java/org/apache/gravitino/hook/TopicHookDispatcher.java
index dfd042b520..8926e01829 100644
--- a/core/src/main/java/org/apache/gravitino/hook/TopicHookDispatcher.java
+++ b/core/src/main/java/org/apache/gravitino/hook/TopicHookDispatcher.java
@@ -63,10 +63,6 @@ public class TopicHookDispatcher implements TopicDispatcher {
public Topic createTopic(
NameIdentifier ident, String comment, DataLayout dataLayout, Map<String,
String> properties)
throws NoSuchSchemaException, TopicAlreadyExistsException {
- // Check whether the current user exists or not
- AuthorizationUtils.checkCurrentUser(
- ident.namespace().level(0), PrincipalUtils.getCurrentUserName());
-
Topic topic = dispatcher.createTopic(ident, comment, dataLayout,
properties);
// Set the creator as the owner of the topic.
diff --git
a/iceberg/iceberg-rest-server/src/main/java/org/apache/gravitino/iceberg/service/dispatcher/IcebergNamespaceHookDispatcher.java
b/iceberg/iceberg-rest-server/src/main/java/org/apache/gravitino/iceberg/service/dispatcher/IcebergNamespaceHookDispatcher.java
index b474b9d16f..87932dea20 100644
---
a/iceberg/iceberg-rest-server/src/main/java/org/apache/gravitino/iceberg/service/dispatcher/IcebergNamespaceHookDispatcher.java
+++
b/iceberg/iceberg-rest-server/src/main/java/org/apache/gravitino/iceberg/service/dispatcher/IcebergNamespaceHookDispatcher.java
@@ -22,7 +22,6 @@ import java.io.IOException;
import org.apache.gravitino.Entity;
import org.apache.gravitino.EntityStore;
import org.apache.gravitino.GravitinoEnv;
-import org.apache.gravitino.authorization.AuthorizationUtils;
import org.apache.gravitino.catalog.SchemaDispatcher;
import org.apache.gravitino.catalog.TableDispatcher;
import org.apache.gravitino.exceptions.NoSuchEntityException;
@@ -58,8 +57,6 @@ public class IcebergNamespaceHookDispatcher implements
IcebergNamespaceOperation
@Override
public CreateNamespaceResponse createNamespace(
IcebergRequestContext context, CreateNamespaceRequest createRequest) {
- AuthorizationUtils.checkCurrentUser(metalake, context.userName());
-
CreateNamespaceResponse response = dispatcher.createNamespace(context,
createRequest);
importSchema(context.catalogName(), createRequest.namespace());
diff --git
a/iceberg/iceberg-rest-server/src/main/java/org/apache/gravitino/iceberg/service/dispatcher/IcebergTableHookDispatcher.java
b/iceberg/iceberg-rest-server/src/main/java/org/apache/gravitino/iceberg/service/dispatcher/IcebergTableHookDispatcher.java
index f96c38e6c2..5bdd7aedb8 100644
---
a/iceberg/iceberg-rest-server/src/main/java/org/apache/gravitino/iceberg/service/dispatcher/IcebergTableHookDispatcher.java
+++
b/iceberg/iceberg-rest-server/src/main/java/org/apache/gravitino/iceberg/service/dispatcher/IcebergTableHookDispatcher.java
@@ -24,7 +24,6 @@ import org.apache.gravitino.Entity;
import org.apache.gravitino.EntityStore;
import org.apache.gravitino.GravitinoEnv;
import org.apache.gravitino.NameIdentifier;
-import org.apache.gravitino.authorization.AuthorizationUtils;
import org.apache.gravitino.catalog.TableDispatcher;
import org.apache.gravitino.exceptions.NoSuchEntityException;
import org.apache.gravitino.iceberg.common.utils.IcebergIdentifierUtils;
@@ -55,8 +54,6 @@ public class IcebergTableHookDispatcher implements
IcebergTableOperationDispatch
@Override
public LoadTableResponse createTable(
IcebergRequestContext context, Namespace namespace, CreateTableRequest
createTableRequest) {
- AuthorizationUtils.checkCurrentUser(metalake, context.userName());
-
LoadTableResponse response = dispatcher.createTable(context, namespace,
createTableRequest);
importTable(context.catalogName(), namespace, createTableRequest.name());
IcebergOwnershipUtils.setTableOwner(
@@ -116,8 +113,6 @@ public class IcebergTableHookDispatcher implements
IcebergTableOperationDispatch
@Override
public void renameTable(IcebergRequestContext context, RenameTableRequest
renameTableRequest) {
- AuthorizationUtils.checkCurrentUser(metalake, context.userName());
-
dispatcher.renameTable(context, renameTableRequest);
NameIdentifier tableSource =
IcebergIdentifierUtils.toGravitinoTableIdentifier(
diff --git
a/iceberg/iceberg-rest-server/src/main/java/org/apache/gravitino/server/web/filter/BaseMetadataAuthorizationMethodInterceptor.java
b/iceberg/iceberg-rest-server/src/main/java/org/apache/gravitino/server/web/filter/BaseMetadataAuthorizationMethodInterceptor.java
index 7dc4931cf1..4632d70a68 100644
---
a/iceberg/iceberg-rest-server/src/main/java/org/apache/gravitino/server/web/filter/BaseMetadataAuthorizationMethodInterceptor.java
+++
b/iceberg/iceberg-rest-server/src/main/java/org/apache/gravitino/server/web/filter/BaseMetadataAuthorizationMethodInterceptor.java
@@ -29,6 +29,7 @@ import org.apache.gravitino.Entity;
import org.apache.gravitino.MetadataObject;
import org.apache.gravitino.NameIdentifier;
import org.apache.gravitino.authorization.AuthorizationRequestContext;
+import org.apache.gravitino.authorization.AuthorizationUtils;
import org.apache.gravitino.iceberg.service.IcebergExceptionMapper;
import
org.apache.gravitino.server.authorization.annotations.AuthorizationExpression;
import
org.apache.gravitino.server.authorization.expression.AuthorizationExpressionEvaluator;
@@ -123,6 +124,31 @@ public abstract class
BaseMetadataAuthorizationMethodInterceptor implements Meth
Map<Entity.EntityType, NameIdentifier> nameIdentifierMap =
extractNameIdentifierFromParameters(parameters, args);
+ // Check if current user exists in the metalake.
+ NameIdentifier metalakeIdent =
nameIdentifierMap.get(Entity.EntityType.METALAKE);
+
+ if (metalakeIdent != null) {
+ String currentUser = PrincipalUtils.getCurrentUserName();
+ try {
+ AuthorizationUtils.checkCurrentUser(metalakeIdent.name(),
currentUser);
+ } catch (org.apache.gravitino.exceptions.ForbiddenException ex) {
+ LOG.info(
+ "User validation failed - User: '{}', Metalake: '{}', Reason:
{}",
+ currentUser,
+ metalakeIdent.name(),
+ ex.getMessage());
+ return IcebergExceptionMapper.toRESTResponse(ex);
+ } catch (Exception ex) {
+ LOG.error(
+ "Unexpected error during user validation - User: '{}',
Metalake: '{}'",
+ currentUser,
+ metalakeIdent.name(),
+ ex);
+ return IcebergExceptionMapper.toRESTResponse(
+ new RuntimeException("Failed to validate user", ex));
+ }
+ }
+
// Process custom authorization if handler exists
Optional<AuthorizationHandler> handler =
createAuthorizationHandler(parameters, args);
boolean skipStandardCheck = false;
diff --git
a/server-common/src/main/java/org/apache/gravitino/server/authorization/jcasbin/JcasbinAuthorizer.java
b/server-common/src/main/java/org/apache/gravitino/server/authorization/jcasbin/JcasbinAuthorizer.java
index fe921adf5b..b7760c6b82 100644
---
a/server-common/src/main/java/org/apache/gravitino/server/authorization/jcasbin/JcasbinAuthorizer.java
+++
b/server-common/src/main/java/org/apache/gravitino/server/authorization/jcasbin/JcasbinAuthorizer.java
@@ -216,6 +216,12 @@ public class JcasbinAuthorizer implements
GravitinoAuthorizer {
if (StringUtils.isBlank(currentUserName)) {
return false;
}
+
+ // Service admins are considered users of all metalakes
+ if (isServiceAdmin()) {
+ return true;
+ }
+
try {
return GravitinoEnv.getInstance()
.entityStore()
diff --git
a/server-common/src/test/java/org/apache/gravitino/server/authorization/TestPassThroughAuthorizer.java
b/server-common/src/test/java/org/apache/gravitino/server/authorization/TestPassThroughAuthorizer.java
index 85540daff5..e8158ab3f7 100644
---
a/server-common/src/test/java/org/apache/gravitino/server/authorization/TestPassThroughAuthorizer.java
+++
b/server-common/src/test/java/org/apache/gravitino/server/authorization/TestPassThroughAuthorizer.java
@@ -17,8 +17,16 @@
package org.apache.gravitino.server.authorization;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.when;
+
import java.io.IOException;
+import java.security.Principal;
+import org.apache.gravitino.Entity;
+import org.apache.gravitino.MetadataObject;
+import org.apache.gravitino.MetadataObjects;
import org.apache.gravitino.authorization.AuthorizationRequestContext;
+import org.apache.gravitino.authorization.Privilege;
import org.junit.jupiter.api.Assertions;
import org.junit.jupiter.api.Test;
@@ -26,12 +34,38 @@ import org.junit.jupiter.api.Test;
public class TestPassThroughAuthorizer {
@Test
- public void testAuthorize() throws IOException {
+ public void testPassThroughAuthorizerDefaults() throws IOException {
try (PassThroughAuthorizer passThroughAuthorizer = new
PassThroughAuthorizer()) {
- boolean result =
+ Principal principal = mock(Principal.class);
+ when(principal.getName()).thenReturn("testUser");
+ MetadataObject metadataObject =
+ MetadataObjects.of("catalog", "schema", MetadataObject.Type.SCHEMA);
+ Assertions.assertTrue(
passThroughAuthorizer.authorize(
- null, null, null, null, new AuthorizationRequestContext());
- Assertions.assertTrue(result, "Logic error in PassThroughAuthorizer");
+ principal,
+ "metalake",
+ metadataObject,
+ Privilege.Name.CREATE_SCHEMA,
+ new AuthorizationRequestContext()));
+
+ Assertions.assertFalse(
+ passThroughAuthorizer.deny(
+ principal,
+ "metalake",
+ metadataObject,
+ Privilege.Name.CREATE_SCHEMA,
+ new AuthorizationRequestContext()));
+
+ Assertions.assertTrue(passThroughAuthorizer.isOwner(principal,
"metalake", metadataObject));
+ Assertions.assertTrue(passThroughAuthorizer.isServiceAdmin());
+ Assertions.assertTrue(passThroughAuthorizer.isMetalakeUser("metalake"));
+
Assertions.assertTrue(passThroughAuthorizer.isSelf(Entity.EntityType.USER,
null));
+ Assertions.assertTrue(
+ passThroughAuthorizer.hasSetOwnerPermission(
+ "metalake", "type", "fullName", new
AuthorizationRequestContext()));
+ Assertions.assertTrue(
+ passThroughAuthorizer.hasMetadataPrivilegePermission(
+ "metalake", "type", "fullName", new
AuthorizationRequestContext()));
}
}
}
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 23db4a1948..f49304d0b1 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
@@ -39,6 +39,7 @@ import org.apache.commons.lang3.StringUtils;
import org.apache.gravitino.Entity;
import org.apache.gravitino.MetadataObject;
import org.apache.gravitino.NameIdentifier;
+import org.apache.gravitino.authorization.AuthorizationUtils;
import
org.apache.gravitino.server.authorization.annotations.AuthorizationExpression;
import
org.apache.gravitino.server.authorization.annotations.AuthorizationRequest;
import
org.apache.gravitino.server.authorization.expression.AuthorizationExpressionEvaluator;
@@ -125,11 +126,43 @@ public class GravitinoInterceptionService implements
InterceptionService {
*/
@Override
public Object invoke(MethodInvocation methodInvocation) throws Throwable {
+ Method method = methodInvocation.getMethod();
+ Parameter[] parameters = method.getParameters();
+ 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 (org.apache.gravitino.exceptions.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 {
- Method method = methodInvocation.getMethod();
- Parameter[] parameters = method.getParameters();
- AuthorizationExpression expressionAnnotation =
- method.getAnnotation(AuthorizationExpression.class);
AuthorizationExecutor executor;
if (expressionAnnotation != null) {
String expression = expressionAnnotation.expression();
@@ -137,6 +170,7 @@ public class GravitinoInterceptionService implements
InterceptionService {
String entityType =
extractMetadataObjectTypeFromParameters(parameters, args);
Map<Entity.EntityType, NameIdentifier> metadataContext =
extractNameIdentifierFromParameters(parameters, args);
+
Map<String, Object> pathParams =
Utils.extractPathParamsFromParameters(parameters, args);
AuthorizationExpressionEvaluator authorizationExpressionEvaluator =
new AuthorizationExpressionEvaluator(expression);