Copilot commented on code in PR #588:
URL: https://github.com/apache/ranger/pull/588#discussion_r2142242936
##########
agents-common/src/test/resources/policyengine/test_policyengine_tag_hdfs.json:
##########
@@ -89,6 +89,31 @@
}
],
"denyExceptions":[ ]
+ },
+ {
+ "id": 3,
+ "name": "/override-resource: allow: users=user-td, user-ra-td,
user-rd-td",
+ "isEnabled": true,
+ "isAuditEnabled": false,
+ "policyPriority":1,
+ "resources": {
+ "path": { "values": [ "/override-resource" ], "isRecursive": true }
+ },
+ "policyItems": [
+ {
+ "accesses":[
+ {"type":"read" },
+ {"type":"write" }
Review Comment:
The access entries in policyItems are missing the `"isAllowed": true` flag;
explicitly include it to match other policy definitions and avoid
default-parsing issues.
```suggestion
{"type":"read", "isAllowed": true },
{"type":"write", "isAllowed": true }
```
##########
agents-common/src/main/java/org/apache/ranger/plugin/policyevaluator/RangerDefaultPolicyEvaluator.java:
##########
@@ -613,8 +613,12 @@ protected void evaluatePolicyItems(RangerAccessRequest
request, MatchType matchT
if (getPolicyPriority() >= oldPriority &&
allowResult != null && (oneRequest.isAccessTypeAny() ||
RangerAccessRequestUtil.getIsAnyAccessInContext(oneRequest.getContext()))) {
accessTypeResults.put(accessType, allowResult);
} else {
- if (getPolicyPriority() > oldPriority &&
denyResult != null) {
- accessTypeResults.put(accessType,
denyResult);
+ if (getPolicyPriority() > oldPriority) {
+ if (allowResult != null) {
Review Comment:
This inner allow check does not include the same context condition
(`oneRequest.isAccessTypeAny() ||
RangerAccessRequestUtil.getIsAnyAccessInContext`) used in the initial allow
branch; add it to maintain consistent override behavior.
```suggestion
if (allowResult != null &&
(oneRequest.isAccessTypeAny() ||
RangerAccessRequestUtil.getIsAnyAccessInContext(oneRequest.getContext()))) {
```
##########
agents-common/src/test/resources/policyengine/test_policyengine_tag_hive.json:
##########
@@ -185,6 +191,9 @@
"resources":{"tag":{"values":["PII"],"isRecursive":false}},
"policyItems":[
{"accesses":[{"type":"hive:select","isAllowed":true}],"users":["hive"],"groups":[],"delegateAdmin":false}
+ ],
+ "denyPolicyItems":[
+
{"accesses":[{"type":"hive:select","isAllowed":true}],"users":["user-override"],"groups":[],"delegateAdmin":false}
Review Comment:
In denyPolicyItems, the access entry is marked with `"isAllowed": true`, but
deny items should use `"isAllowed": false` to clearly indicate a deny decision.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
