> On Feb. 10, 2022, 2 p.m., Abhay Kulkarni wrote:
> > agents-common/src/main/java/org/apache/ranger/plugin/service/RangerBasePlugin.java
> > Lines 103 (patched)
> > <https://reviews.apache.org/r/73833/diff/5/?file=2266218#file2266218line103>
> >
> >     Please consider having default value as false.

Done.


> On Feb. 10, 2022, 2 p.m., Abhay Kulkarni wrote:
> > agents-common/src/main/java/org/apache/ranger/plugin/util/RangerAccessRequestUtil.java
> > Lines 133 (patched)
> > <https://reviews.apache.org/r/73833/diff/5/?file=2266219#file2266219line133>
> >
> >     Why is this explicitly removed from the context?

KEY_CONTEXT_REQUEST in the context is used by script evaluator to obtain 
current request, to populate script-context with request specific details like 
user/groups/resource/access-type. RangerHiveAuthorizer copies request object 
while authorizing access to multiple columns in a table (see link below). If 
KEY_CONTEXT_REQUEST is not cleared, script-context will have incorrect value 
for resource. 

- 
https://github.com/apache/ranger/blob/master/hive-agent/src/main/java/org/apache/ranger/authorization/hive/authorizer/RangerHiveAuthorizer.java#L1035


> On Feb. 10, 2022, 2 p.m., Abhay Kulkarni wrote:
> > agents-common/src/main/java/org/apache/ranger/plugin/util/ServiceDefUtil.java
> > Lines 613 (patched)
> > <https://reviews.apache.org/r/73833/diff/5/?file=2266222#file2266222line613>
> >
> >     If the policyDelta indicates if the policy is deleted, then getPolicy() 
> > will return a dummy place-holder policy with only policy-id properly set. 
> > Any calls on such policy will always return false, and when the only policy 
> > with reference to user/group variable referenece is deleted, the user-store 
> > will continue to be downloaded unnecessarily.
> >     
> >     Please review.

Yes. In case of policy-delta, it will not be enough to look at the deleted 
policy to determine if implicitly added enricher can be removed. It will 
require full scan of all policies (after delta is applied) to determine if the 
enricher can be removed, as another existing policy could have references to 
user/group attributes. With the current approach, if the last policy having 
reference to user/group attributes is deleted, the enricher will be removed 
only on next full-sync of policies.

Given implicit enricher addition is disabled by default (see comment on: 
"enable.implicit.userstore.enricher=false"), I suggest to addressing removal in 
policy-delta via separate patch.


- Madhan


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/73833/#review224028
-----------------------------------------------------------


On Feb. 10, 2022, 6:20 p.m., Madhan Neethiraj wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/73833/
> -----------------------------------------------------------
> 
> (Updated Feb. 10, 2022, 6:20 p.m.)
> 
> 
> Review request for ranger, Ankita Sinha, Kishor Gollapalliwar, Abhay 
> Kulkarni, Mehul Parikh, Ramesh Mani, Sailaja Polavarapu, and Velmurugan 
> Periasamy.
> 
> 
> Bugs: RANGER-3609
>     https://issues.apache.org/jira/browse/RANGER-3609
> 
> 
> Repository: ranger
> 
> 
> Description
> -------
> 
> updated plugin to automatically add userStoreEnricher when any policy has 
> references to user/group attributes
> 
> 
> Diffs
> -----
> 
>   
> agents-common/src/main/java/org/apache/ranger/admin/client/RangerAdminRESTClient.java
>  a787c789f 
>   
> agents-common/src/main/java/org/apache/ranger/plugin/contextenricher/RangerUserStoreEnricher.java
>  315328d65 
>   
> agents-common/src/main/java/org/apache/ranger/plugin/policyengine/RangerRequestScriptEvaluator.java
>  9b72a1b73 
>   
> agents-common/src/main/java/org/apache/ranger/plugin/service/RangerBasePlugin.java
>  827fb1fc6 
>   
> agents-common/src/main/java/org/apache/ranger/plugin/util/RangerAccessRequestUtil.java
>  4415b6c04 
>   
> agents-common/src/main/java/org/apache/ranger/plugin/util/RangerPluginCapability.java
>  b2cecc1f9 
>   
> agents-common/src/main/java/org/apache/ranger/plugin/util/RangerRESTUtils.java
>  e5ab8daaa 
>   
> agents-common/src/main/java/org/apache/ranger/plugin/util/RangerRequestExprResolver.java
>  2e486eda4 
>   
> agents-common/src/main/java/org/apache/ranger/plugin/util/ServiceDefUtil.java 
> b76c96ebc 
>   
> agents-common/src/test/java/org/apache/ranger/plugin/util/ServiceDefUtilTest.java
>  PRE-CREATION 
>   
> knox-agent/src/main/java/org/apache/ranger/admin/client/RangerAdminJersey2RESTClient.java
>  624228e4c 
>   security-admin/src/main/java/org/apache/ranger/rest/XUserREST.java 
> 198b940df 
>   security-admin/src/main/resources/conf.dist/security-applicationContext.xml 
> 3e067a61f 
> 
> 
> Diff: https://reviews.apache.org/r/73833/diff/7/
> 
> 
> Testing
> -------
> 
> - added unit tests
> - verified that existing tests pass successfully
> 
> 
> Thanks,
> 
> Madhan Neethiraj
> 
>

Reply via email to