----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/73094/#review222369 -----------------------------------------------------------
agents-common/src/main/java/org/apache/ranger/plugin/policyevaluator/RangerOptimizedPolicyEvaluator.java Lines 297 (patched) <https://reviews.apache.org/r/73094/#comment311430> isAdminAccess won't be true if isAccessTypeAny is. For better readability, consider following: ret = isAccessTypeAny || accessPerms.contains(accessType); if (!ret) { if (StringUtils.equals(accessType, RangerPolicyEngine.ADMIN_ACCESS)) { ret = delegateAdmin; } } security-admin/src/main/java/org/apache/ranger/biz/RangerPolicyAdminImpl.java Line 294 (original), 306 (patched) <https://reviews.apache.org/r/73094/#comment311431> revert this change - since the method name hasn't changed? security-admin/src/main/java/org/apache/ranger/rest/ServiceREST.java Line 3654 (original), 3706 (patched) <https://reviews.apache.org/r/73094/#comment311429> Consider using serviceDef from policyAdmin.policyEngine. retrieving in each call to hasAdminAccess() will be expensive and unnecessary. Perhaps the logic to get access-types from policy (getAllAccessTypes()) can be moved inside implementation of RangerPolicyAdmin.isDelegatedAdminAccessAllowed() security-admin/src/test/java/org/apache/ranger/biz/TestPolicyDb.java Line 141 (original), 141 (patched) <https://reviews.apache.org/r/73094/#comment311428> revert this change in message, since no change to the method called above at #139? - Madhan Neethiraj On Dec. 22, 2020, 1:07 a.m., Abhay Kulkarni wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/73094/ > ----------------------------------------------------------- > > (Updated Dec. 22, 2020, 1:07 a.m.) > > > Review request for ranger, Madhan Neethiraj, Mehul Parikh, Ramesh Mani, > Sailaja Polavarapu, and Velmurugan Periasamy. > > > Bugs: RANGER-3122 > https://issues.apache.org/jira/browse/RANGER-3122 > > > Repository: ranger > > > Description > ------- > > Currently delegate-admin cannot be marked for specific permissions. It is > all-or-nothing for the permissions defined in resource policy. Ranger should > have ability for granting delegate-admin for specific permissions. > > > Diffs > ----- > > > agents-common/src/main/java/org/apache/ranger/plugin/policyevaluator/RangerDefaultPolicyEvaluator.java > d64d226a6 > > agents-common/src/main/java/org/apache/ranger/plugin/policyevaluator/RangerOptimizedPolicyEvaluator.java > bac076c29 > > agents-common/src/main/java/org/apache/ranger/plugin/policyevaluator/RangerPolicyEvaluator.java > 236f99820 > > agents-common/src/main/java/org/apache/ranger/plugin/service/RangerBasePlugin.java > 873553a60 > > agents-common/src/main/java/org/apache/ranger/plugin/util/ServiceDefUtil.java > cd6c18ba7 > security-admin/src/main/java/org/apache/ranger/biz/RangerPolicyAdmin.java > 891c800fe > > security-admin/src/main/java/org/apache/ranger/biz/RangerPolicyAdminImpl.java > cd566bc34 > security-admin/src/main/java/org/apache/ranger/rest/ServiceREST.java > 802abac68 > security-admin/src/test/java/org/apache/ranger/biz/TestPolicyDb.java > 7416fe45d > > > Diff: https://reviews.apache.org/r/73094/diff/2/ > > > Testing > ------- > > Passes all unit tests. > Tested in cluster with HDFS policies: > 1. There is a delegate-admin policy giving user1 'read' permission on /tmp, > and another delegate-admin policy giving user1 'write' permission on /tmp/a > a. user1 can create policy on /tmp/b with permission 'read', but cannot > create policy on /tmp/c with permission 'write' > b. user1 can create policy on /tmp/a/d with permissions 'read' and > 'write' but cannot create policy on /tmp/a/e with permission 'execute'. > > > Thanks, > > Abhay Kulkarni > >
