----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/73443/#review223224 -----------------------------------------------------------
agents-common/src/main/java/org/apache/ranger/plugin/policyengine/RangerAccessRequest.java Lines 70 (patched) <https://reviews.apache.org/r/73443/#comment312318> New method getIsAnyRequest() appears to overlap with isAccessTypeAny(). Please review and consider avoiding this method. agents-common/src/main/java/org/apache/ranger/plugin/policyengine/RangerPolicyEngineImpl.java Lines 645 (patched) <https://reviews.apache.org/r/73443/#comment312319> Are isAccessDeniedForOneAccessType and isAccessAllowedForOneAccessType needed, given the result can be determined from allowResult and denyResult? if (allowResult != null) { ret = allowResult; } else if (denyResult != null) { ret = denyResult; } agents-common/src/main/java/org/apache/ranger/plugin/policyengine/RangerPolicyEngineImpl.java Lines 654 (patched) <https://reviews.apache.org/r/73443/#comment312320> Consider moving #654 - #668 to a constructor in RangerAccessRequestImpl - for better readability and potenial reuse. agents-common/src/main/java/org/apache/ranger/plugin/policyengine/RangerPolicyEngineImpl.java Lines 676 (patched) <https://reviews.apache.org/r/73443/#comment312321> Is this special handling of isSuperUser needed here, given above call to evaluatePoliciesForOneAccessTypeNoAudit() will return allowed=true, which in turn will cause this loop to break at #683. - Madhan Neethiraj On July 9, 2021, 6:17 p.m., Abhay Kulkarni wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/73443/ > ----------------------------------------------------------- > > (Updated July 9, 2021, 6:17 p.m.) > > > Review request for ranger, Madhan Neethiraj, Ramesh Mani, Sailaja Polavarapu, > and Velmurugan Periasamy. > > > Bugs: RANGER-3329 > https://issues.apache.org/jira/browse/RANGER-3329 > > > Repository: ranger > > > Description > ------- > > Currently a request for _any access-type is denied only if all access-types > in the service-def are denied by policies. Instead of this, the policy-engine > should deny _any access if there are no allowed accesses, and at least one of > the access-type is denied. This will help address following usecase: > > when accessTypeRestrictions is defined on a resource i.e. only a subset of > access-types are shown in policy-UI, it will not be possible to create > policies that deny all accesses. In such cases, the proposed change will > enable denying _any access-type with only subset of access-types denied. > > The fix is to deny the access with type _any only if all of access-types > "specified in the denying policy" are explicitly denied by policies. > > > Diffs > ----- > > > agents-common/src/main/java/org/apache/ranger/plugin/policyengine/RangerAccessRequest.java > 6a38747f4 > > agents-common/src/main/java/org/apache/ranger/plugin/policyengine/RangerAccessRequestImpl.java > 74a7a2615 > > agents-common/src/main/java/org/apache/ranger/plugin/policyengine/RangerAccessRequestReadOnly.java > 4887c0112 > > agents-common/src/main/java/org/apache/ranger/plugin/policyengine/RangerPolicyEngineImpl.java > 5ffd38f98 > > agents-common/src/main/java/org/apache/ranger/plugin/policyengine/RangerTagAccessRequest.java > ebe85e9a2 > > agents-common/src/main/java/org/apache/ranger/plugin/policyevaluator/RangerDefaultPolicyEvaluator.java > 03e37fe3d > > agents-common/src/test/resources/policyengine/test_policyengine_tag_hive.json > a8ec02733 > > > Diff: https://reviews.apache.org/r/73443/diff/2/ > > > Testing > ------- > > Passed all existing test cases. > Created a unit test for the use-case outlined in the JIRA, and ensured that > it passes. > > > Thanks, > > Abhay Kulkarni > >
