----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/74229/#review224943 -----------------------------------------------------------
security-admin/src/main/java/org/apache/ranger/rest/ServiceRESTUtil.java Lines 274 (patched) <https://reviews.apache.org/r/74229/#comment313782> Shouldn't appliedPolicyItem be added to itemsToAdd only if none of the entries in existingPolicyItems match appliedPolicyItem? i.e. replace #273 - #277 with the following: if (!existingPolicyItems.contains(appliedPolicyItem)) { itemsToAdd.add(appliedPolicyItem); } security-admin/src/main/java/org/apache/ranger/rest/ServiceRESTUtil.java Line 608 (original), 628 (patched) <https://reviews.apache.org/r/74229/#comment313783> addition to deny-exception can/must be skipped in following cases: 1. policy has no deny policy items 2. policy has denyAllElse=true Case #1 is covered by #629. To cover #1, how about updating processApplyPolicyForItemType() as below: List<RangerPolicy.RangerPolicyItem> appliedPolicyItems = null; if (policyItemType == ALLOW) { appliedPolicyItems = appliedPolicy.getPolicyItems(); } else if (existingPolicy.getIsDenyAllElse() || appliedPolicy.getIsDenyAllElse()) { LOG.warn("processApplyPolicyForItemType(): isDenyAllElse=true, ignoring invalid policyItemType=" + policyItemType); } else { if (policyItemType == DENY) { appliedPolicyItems = appliedPolicy.getDenyPolicyItems(); } else if (policyItemType == ALLOW_EXCEPTIONS) { appliedPolicyItems = appliedPolicy.getAllowExceptions(); } else if (policyItemType == DENY_EXCEPTIONS) { appliedPolicyItems = appliedPolicy.getDenyExceptions(); } else { LOG.warn("processApplyPolicyForItemType(): ignoring invalid policyItemType=" + policyItemType); } } - Madhan Neethiraj On Dec. 2, 2022, 6:39 p.m., Abhay Kulkarni wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/74229/ > ----------------------------------------------------------- > > (Updated Dec. 2, 2022, 6:39 p.m.) > > > Review request for ranger, madhan, Madhan Neethiraj, Pradeep Agrawal, Ramesh > Mani, and Velmurugan Periasamy. > > > Bugs: RANGER-3995 > https://issues.apache.org/jira/browse/RANGER-3995 > > > Repository: ranger > > > Description > ------- > > Steps to reproduce :- > > 1. Make a POST request to the below mentioned API endpoint, using a policy > json where isDenyAllElse flag is set true > > /service/public/v2/api/policy/apply > 2. Fetch the policy using the newly created policy id, and try to make a POST > request to "/policy/apply" using the policy json obtained from the GET > request. The request results in an error > > > Diffs > ----- > > security-admin/src/main/java/org/apache/ranger/rest/ServiceRESTUtil.java > b56fd3966 > > > Diff: https://reviews.apache.org/r/74229/diff/1/ > > > Testing > ------- > > Tested per the steps listed above. No error was reported and the policy was > applied correctly > > > Thanks, > > Abhay Kulkarni > >
