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 3b78a5dde7 [#9270][followup] refactor(authz): Clean up redundant
checkCurrentUser check in TagHookDispatcher (#9275)
3b78a5dde7 is described below
commit 3b78a5dde733350ae07b0e342de452832a2b3afd
Author: Bharath Krishna <[email protected]>
AuthorDate: Wed Nov 26 18:05:34 2025 -0800
[#9270][followup] refactor(authz): Clean up redundant checkCurrentUser
check in TagHookDispatcher (#9275)
### What changes were proposed in this pull request?
1. Remove redundant isServiceAdmin check from isMetalakeUser()
- Service admins bypass via SERVICE_ADMIN in authorization expressions
- Regular users are validated by checkCurrentUser() before authorization
2. Use simple ForbiddenException name instead of fully qualified
- Add import and use simple name in catch block
3. Remove checkCurrentUser from TagHookDispatcher
- Check now handled by interceptor, consistent with other dispatchers
Related to PR #9268
### Why are the changes needed?
Refactor
Fix: #9270
### Does this PR introduce _any_ user-facing change?
no
### How was this patch tested?
Run all tests
---
core/src/main/java/org/apache/gravitino/hook/TagHookDispatcher.java | 2 --
.../gravitino/server/authorization/jcasbin/JcasbinAuthorizer.java | 5 -----
.../gravitino/server/web/filter/GravitinoInterceptionService.java | 3 ++-
3 files changed, 2 insertions(+), 8 deletions(-)
diff --git
a/core/src/main/java/org/apache/gravitino/hook/TagHookDispatcher.java
b/core/src/main/java/org/apache/gravitino/hook/TagHookDispatcher.java
index bc5dffd581..1db84fdff8 100644
--- a/core/src/main/java/org/apache/gravitino/hook/TagHookDispatcher.java
+++ b/core/src/main/java/org/apache/gravitino/hook/TagHookDispatcher.java
@@ -21,7 +21,6 @@ import java.util.Map;
import org.apache.gravitino.Entity;
import org.apache.gravitino.GravitinoEnv;
import org.apache.gravitino.MetadataObject;
-import org.apache.gravitino.authorization.AuthorizationUtils;
import org.apache.gravitino.authorization.Owner;
import org.apache.gravitino.authorization.OwnerDispatcher;
import org.apache.gravitino.exceptions.NoSuchTagException;
@@ -57,7 +56,6 @@ public class TagHookDispatcher implements TagDispatcher {
@Override
public Tag createTag(
String metalake, String name, String comment, Map<String, String>
properties) {
- AuthorizationUtils.checkCurrentUser(metalake,
PrincipalUtils.getCurrentUserName());
Tag tag = dispatcher.createTag(metalake, name, comment, properties);
// Set the creator as the owner of the catalog.
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 b7760c6b82..14f2fd04c9 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
@@ -217,11 +217,6 @@ public class JcasbinAuthorizer implements
GravitinoAuthorizer {
return false;
}
- // Service admins are considered users of all metalakes
- if (isServiceAdmin()) {
- return true;
- }
-
try {
return GravitinoEnv.getInstance()
.entityStore()
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 f49304d0b1..6146781760 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
@@ -40,6 +40,7 @@ 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.exceptions.ForbiddenException;
import
org.apache.gravitino.server.authorization.annotations.AuthorizationExpression;
import
org.apache.gravitino.server.authorization.annotations.AuthorizationRequest;
import
org.apache.gravitino.server.authorization.expression.AuthorizationExpressionEvaluator;
@@ -144,7 +145,7 @@ public class GravitinoInterceptionService implements
InterceptionService {
String currentUser = PrincipalUtils.getCurrentUserName();
try {
AuthorizationUtils.checkCurrentUser(metalakeIdent.name(),
currentUser);
- } catch (org.apache.gravitino.exceptions.ForbiddenException ex) {
+ } catch (ForbiddenException ex) {
LOG.warn(
"User validation failed - User: {}, Metalake: {}, Reason: {}",
currentUser,
