> On Oct. 7, 2015, 11:24 p.m., Abhay Kulkarni wrote: > > agents-common/src/main/java/org/apache/ranger/plugin/policyengine/RangerPolicyEngineImpl.java, > > line 333 > > <https://reviews.apache.org/r/39103/diff/1/?file=1092530#file1092530line333> > > > > If isAuditedDetermined is true at this point, then the auditCache is > > bypassed, i.e. cacheing of the result of audit will not be done. Will we > > ever need to cache this as an optimization?
I thought about this and decided not to cache the isAudited derived from tag into resource cache. Doing this could cause incorrect auditing if tag was removed from the resource. > On Oct. 7, 2015, 11:24 p.m., Abhay Kulkarni wrote: > > agents-common/src/main/java/org/apache/ranger/plugin/policyengine/RangerPolicyEngineImpl.java, > > line 433 > > <https://reviews.apache.org/r/39103/diff/1/?file=1092530#file1092530line433> > > > > getIsAudited() and setIsAudited() is generally not used anywhere in the > > code. Any specific reason for using those APIs here? To handle the following cases: - if audit was not enabled for the current tag, but it was enabled for earlier tag, we should not overwrite with false - if audit was enabled for the current tag, but not yet for earlier tags, this would set audit to true Using this condition make it simpler to handle both these cases. > On Oct. 7, 2015, 11:24 p.m., Abhay Kulkarni wrote: > > agents-common/src/test/java/org/apache/ranger/plugin/policyengine/TestPolicyEngine.java, > > line 26 > > <https://reviews.apache.org/r/39103/diff/1/?file=1092533#file1092533line26> > > > > Can these headers to kept around? I have some commented out code in > > this file which I sometimes use to set up configuration properties. That > > code requires these headers. I was trying to keep the source clean, such that Eclipse wouldn't show yellow icon for the source files. When you uncomment this block, the IDE should automatically add necessary imports (Eclipse does..). - Madhan ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/39103/#review101827 ----------------------------------------------------------- On Oct. 7, 2015, 9:05 p.m., Madhan Neethiraj wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/39103/ > ----------------------------------------------------------- > > (Updated Oct. 7, 2015, 9:05 p.m.) > > > Review request for ranger, Don Bosco Durai, Gautam Borad, Abhay Kulkarni, > Selvamohan Neethiraj, and Velmurugan Periasamy. > > > Bugs: RANGER-683 > https://issues.apache.org/jira/browse/RANGER-683 > > > Repository: ranger > > > Description > ------- > > Fixed the earlier implementation that allowed access if the user has > permission for the tag even though a resource-based policy denied the access > > > Diffs > ----- > > > agents-common/src/main/java/org/apache/ranger/plugin/policyengine/RangerPolicyEngineImpl.java > 5d1140b > > agents-common/src/main/java/org/apache/ranger/plugin/policyevaluator/RangerDefaultPolicyEvaluator.java > 1764b60 > > agents-common/src/main/java/org/apache/ranger/plugin/policyevaluator/RangerOptimizedPolicyEvaluator.java > a118466 > > agents-common/src/test/java/org/apache/ranger/plugin/policyengine/TestPolicyEngine.java > d7801b9 > > agents-common/src/test/resources/policyengine/test_policyengine_tag_hdfs.json > ed42d5c > > Diff: https://reviews.apache.org/r/39103/diff/ > > > Testing > ------- > > - added unit tests to cover the following combinations: > > ------------------------------------------- > | Resource-policy | Tag-policy | Result | > |-----------------|------------|-----------| > | Allowed | Allowed | Allowed | > |-----------------|------------|-----------| > | Allowed | Denied | Denied | > |-----------------|------------|-----------| > | Allowed | No policy | Allowed | > |-----------------|------------|-----------| > | Denied | Allowed | Denied | > |-----------------|------------|-----------| > | Denied | Denied | Denied | > |-----------------|------------|-----------| > | Denied | No policy | Denied | > |-----------------|------------|-----------| > | No policy | Allowed | Allowed | > |-----------------|------------|-----------| > | No policy | Denied | Denied | > |-----------------|------------|-----------| > | No policy | No policy | No result | > |-----------------|------------|-----------| > > > Thanks, > > Madhan Neethiraj > >
