This is an automated email from the ASF dual-hosted git repository.
roryqi pushed a commit to branch branch-1.1
in repository https://gitbox.apache.org/repos/asf/gravitino.git
The following commit(s) were added to refs/heads/branch-1.1 by this push:
new 0ac2295299 [#9291] improvement(authz): Add comment explaining why deny
privileges are injected into allowEnforcer (#9455)
0ac2295299 is described below
commit 0ac22952999fdf5dae8c59b71a973318f64cfd98
Author: github-actions[bot]
<41898282+github-actions[bot]@users.noreply.github.com>
AuthorDate: Thu Dec 11 15:35:39 2025 +0800
[#9291] improvement(authz): Add comment explaining why deny privileges are
injected into allowEnforcer (#9455)
### What changes were proposed in this pull request?
Add comment explaining why deny privileges are injected into
allowEnforcer
### Why are the changes needed?
Fix: #9291
### Does this PR introduce _any_ user-facing change?
None
### How was this patch tested?
None
Co-authored-by: yangyang zhong <[email protected]>
---
.../expression/AuthorizationExpressionConverter.java | 15 +++++++++++++++
.../server/authorization/jcasbin/JcasbinAuthorizer.java | 8 ++++++++
2 files changed, 23 insertions(+)
diff --git
a/server-common/src/main/java/org/apache/gravitino/server/authorization/expression/AuthorizationExpressionConverter.java
b/server-common/src/main/java/org/apache/gravitino/server/authorization/expression/AuthorizationExpressionConverter.java
index 160f22069f..34648782b4 100644
---
a/server-common/src/main/java/org/apache/gravitino/server/authorization/expression/AuthorizationExpressionConverter.java
+++
b/server-common/src/main/java/org/apache/gravitino/server/authorization/expression/AuthorizationExpressionConverter.java
@@ -207,6 +207,21 @@ public class AuthorizationExpressionConverter {
public static String replaceAnyPrivilege(String expression) {
expression = expression.replaceAll("SERVICE_ADMIN",
"authorizer.isServiceAdmin()");
expression = expression.replaceAll("METALAKE_USER",
"authorizer.isMetalakeUser(METALAKE_NAME)");
+
+ // A single privilege (e.g., SELECT_TABLE) can be granted or denied at
multiple namespace
+ // levels: metalake, catalog, schema, and table.
+ //
+ // Deny takes precedence over allow: if deny is set for the privilege at
any level in the
+ // hierarchy,
+ // the user is not considered to have that privilege—even if an allow
exists at a more specific
+ // level.
+ //
+ // Examples:
+ // - If role1 is allowed SELECT_TABLE on metalake1 but denied on catalog1,
+ // then SELECT_TABLE is denied for all objects under catalog1.
+ // - If role1 is denied SELECT_TABLE on metalake1, any allow on catalog1
(or deeper) is
+ // overridden,
+ // and SELECT_TABLE remains denied for catalog1 and its descendants.
expression =
expression.replaceAll(
"ANY_USE_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 034a742c48..68125aec56 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
@@ -535,6 +535,14 @@ public class JcasbinAuthorizer implements
GravitinoAuthorizer {
.toUpperCase(java.util.Locale.ROOT),
AuthConstants.ALLOW);
}
+ // Since different roles of a user may simultaneously hold both
"allow" and "deny"
+ // permissions
+ // for the same privilege on a given MetadataObject, the allowEnforcer
must also incorporate
+ // the "deny" privilege to ensure that the authorize method correctly
returns false in such
+ // cases. For example, if role1 has an "allow" privilege for
SELECT_TABLE on table1, while
+ // role2 has a "deny" privilege for the same action on table1, then a
user assigned both
+ // roles should receive a false result when calling the authorize
method.
+
allowEnforcer.addPolicy(
String.valueOf(roleEntity.id()),
securableObject.type().name(),
