fateh288 commented on code in PR #410:
URL: https://github.com/apache/ranger/pull/410#discussion_r1833414376


##########
agents-common/src/main/java/org/apache/ranger/plugin/policyevaluator/RangerDefaultPolicyEvaluator.java:
##########
@@ -1512,4 +1504,27 @@ private List<RangerConditionEvaluator> 
createPolicyConditionEvaluators(RangerPol
                return ret;
        }
 
+       private static boolean isMatchForResourceMatchingScope(final 
RangerAccessRequest.ResourceMatchingScope scope, final 
RangerPolicyResourceMatcher.MatchType matchType, boolean isAnyMatch) {
+               boolean ret = false;
+               if (isAnyMatch){
+                       ret = matchType !=  
RangerPolicyResourceMatcher.MatchType.NONE;
+               }
+               else if (scope!=null) {
+                               switch (scope) {
+                                               case SELF_OR_DESCENDANTS: {
+                                                       ret = matchType != 
RangerPolicyResourceMatcher.MatchType.NONE;
+                                                       break;
+                                               }
+                                               case SELF_AND_ALL_DESCENDANTS: {
+                                                       ret = matchType != 
RangerPolicyResourceMatcher.MatchType.NONE;

Review Comment:
   Yes, 'case' blocks for both SELF_OR_DESCENDANTS and SELF_AND_ALL_DESCENDANTS 
would be the same. 
   
   ResourceMatchingScope=SELF_OR_DESCENDANTS will result in isMatched = true 
for all cases of matchType in {SELF, DESCENDANT, ANCESTOR, 
SELF_AND_ALL_DESCENDANTS} --- I am not sure here if matchType is Ancestor then 
why isMatched=true makes sense. 
   For ResourceMatchingScope=SELF_OR_DESCENDANTS, I tried changing 
   `ret = matchType == RangerPolicyResourceMatcher.MatchType.SELF || matchType 
== RangerPolicyResourceMatcher.MatchType.SELF_AND_ALL_DESCENDANTS || matchType 
== RangerPolicyResourceMatcher.MatchType.DESCENDANT;`
   
   After this change, agents-common test cases pass successfully even after I 
change it to above (do you suggest changing it to this ?).
   
   The same works for SELF_AND_ALL_DESCENDANTS too.
    
   In the current implementation, the difference between SELF_OR_DESCENDANTS 
and SELF_AND_ALL_DESCENDANTS comes in implementation of updateAccessResult() 
https://github.com/apache/ranger/blob/c9ee929d025dd73689ba1f242e3efdd23d8c9c8a/agents-common/src/main/java/org/apache/ranger/plugin/policyevaluator/RangerDefaultPolicyEvaluator.java#L533
 
   



-- 
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: dev-unsubscr...@ranger.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org

Reply via email to