----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/73981/#review224432 -----------------------------------------------------------
agents-common/src/main/java/org/apache/ranger/plugin/model/validation/RangerPolicyValidator.java Line 966 (original), 966 (patched) <https://reviews.apache.org/r/73981/#comment313233> Please consider renaming newAccessTypes as uniqueAccesses. Also, it can be written as Set<String> uniqueAccesses = new HashSet<>(); agents-common/src/main/java/org/apache/ranger/plugin/model/validation/RangerPolicyValidator.java Lines 967 (patched) <https://reviews.apache.org/r/73981/#comment313235> Consider naming itrRangerPoicyAccesses simply as accessTypeIterator. agents-common/src/main/java/org/apache/ranger/plugin/model/validation/RangerPolicyValidator.java Lines 982 (patched) <https://reviews.apache.org/r/73981/#comment313234> The following rewrite will avoid checking validity of a duplicate access-type repeatedly. if (uniqueAccesses.contains(access.getType()) { itrRangerPolicyAcccess.remove(); } else { valid = isValidPolicyItemAccess(access, failures, accessTypes) && valid; uniqueAccesses.add(access.getType()); } - Abhay Kulkarni On May 10, 2022, 7:43 a.m., Pradeep Agrawal wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/73981/ > ----------------------------------------------------------- > > (Updated May 10, 2022, 7:43 a.m.) > > > Review request for ranger, bhavik patel, Dhaval Shah, Abhay Kulkarni, Madhan > Neethiraj, Ramesh Mani, Sailaja Polavarapu, and Velmurugan Periasamy. > > > Bugs: RANGER-3752 > https://issues.apache.org/jira/browse/RANGER-3752 > > > Repository: ranger > > > Description > ------- > > **Problem Statement:** While making a REST request to create Ranger policy, > it is possible that user can put same access type more than one. Since there > is no validation or restriction on duplicate entry of access type in the same > policy resource-policy items, policy get created successfully and policy text > json contains duplicate entries. > When user makes a GET request then duplicate entries are also shown. To > display the policy content, policy is read from policy text column of > x_policy table, since json entry also contains duplicate entry user will get > duplicate entry of access permission as response. > > This is not an issue if user uses create/update policy rest from Ranger UI as > restriction is placed from UI itself. > > **Steps to reproduce:** > 1. Make the following request to create ranger policy in the "dev_hive" > service (if needed, please change the request data as per you env) > > curl -ivk --header text/json -H 'Content-Type: text/json' -u admin:admin -X > POST http://localhost:6080/service/public/v2/api/policy -d > '{"service":"dev_hive","name":"URL policy: > /dev/db/table/resource","policyType":0,"policyPriority":0,"isAuditEnabled":true,"resources":{"url":{"values":["hdfs://localhost/dev/db/table/resource"],"isExcludes":false,"isRecursive":true}},"policyItems":[{"accesses":[{"type":"alter","isAllowed":true},{"type":"drop","isAllowed":true},{"type":"select","isAllowed":true},{"type":"create","isAllowed":true},{"type":"update","isAllowed":true},{"type":"lock","isAllowed":true},{"type":"all","isAllowed":true},{"type":"alter","isAllowed":true},{"type":"drop","isAllowed":true},{"type":"select","isAllowed":true},{"type":"create","isAllowed":true},{"type":"update","isAllowed":true},{"type":"lock","isAllowed":true},{"type":"all","isAllowed":true}],"users":[],"groups":["public"],"roles":[],"conditions":[],"delegateAdmin":true}],"denyPolicyItems":[],"allowExceptions":[], "denyExceptions":[{"accesses":[{"type":"alter","isAllowed":true},{"type":"drop","isAllowed":true},{"type":"select","isAllowed":true},{"type":"create","isAllowed":true},{"type":"update","isAllowed":true},{"type":"lock","isAllowed":true},{"type":"all","isAllowed":true},{"type":"alter","isAllowed":true},{"type":"drop","isAllowed":true},{"type":"select","isAllowed":true},{"type":"create","isAllowed":true},{"type":"update","isAllowed":true},{"type":"lock","isAllowed":true},{"type":"all","isAllowed":true}],"users":[],"groups":["public"],"roles":[],"conditions":[],"delegateAdmin":true}],"dataMaskPolicyItems":[],"rowFilterPolicyItems":[],"serviceType":"hive","options":{},"validitySchedules":[],"policyLabels":[],"zoneName":"","isDenyAllElse":false}' > > > 2. make a curl request to get the policy and compare the json. json content > will be having the duplicate entries of access permissions as provided in the > create policy request. > > **Proposed solution:** > Option-1: Since policy validation is done before policy creation, hence > during validation phase we can filter out duplicate access permissions. > Option-2: Add a validation to detect duplicate entries of access-permissions > and if there are any duplicate entries then fail the policy request. > > I have provided the patch with option-1 mentioned above. > > > Diffs > ----- > > > agents-common/src/main/java/org/apache/ranger/plugin/model/validation/RangerPolicyValidator.java > fb6556b59 > > > Diff: https://reviews.apache.org/r/73981/diff/1/ > > > Testing > ------- > > With patch tested the create policy request with duplicate access-permissions > entries, policy was created successfully and get request is not having > duplicate access-permissions entries. > With patch tested the update policy request with duplicate access-permissions > entries, policy was updated successfully and get request is not having > duplicate access-permissions entries. > > > Thanks, > > Pradeep Agrawal > >
