You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@ranger.apache.org by Yan Zhou via Review Board <no...@reviews.apache.org> on 2017/03/10 20:27:09 UTC

Review Request 57519: Ranger-1446: Ranger Solr Plugin does not work when the collection list in the request is empty

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

Review request for ranger and Colm O hEigeartaigh.


Repository: ranger


Description
-------

The fix of Ranger-1095 set the initial value of "denied" to "true" from the previous "false". One impact of this change is that, when context.getCollectionRequests() is empty which could be the case in many invocations Solr makes to Ranger on authorization per client request, the permission is plainly denied without going to Ranger policy engine. So the fix changed the default behavior related to "denied".
A proper fix of Ranger-1095 IMO should be just to set the "denied" to "true" in the catch block without changing the initial value of the variable.


Diffs
-----

  plugin-solr/src/main/java/org/apache/ranger/authorization/solr/authorizer/RangerSolrAuthorizer.java 6160438 


Diff: https://reviews.apache.org/r/57519/diff/1/


Testing
-------

manual unit


Thanks,

Yan Zhou


Re: Review Request 57519: Ranger-1446: Ranger Solr Plugin does not work when the collection list in the request is empty

Posted by Colm O hEigeartaigh <co...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/57519/#review168774
-----------------------------------------------------------


Ship it!




Ship It!

- Colm O hEigeartaigh


On March 13, 2017, 3:26 p.m., Yan Zhou wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/57519/
> -----------------------------------------------------------
> 
> (Updated March 13, 2017, 3:26 p.m.)
> 
> 
> Review request for ranger and Colm O hEigeartaigh.
> 
> 
> Repository: ranger
> 
> 
> Description
> -------
> 
> The fix of Ranger-1095 set the initial value of "denied" to "true" from the previous "false". One impact of this change is that, when context.getCollectionRequests() is empty which could be the case in many invocations Solr makes to Ranger on authorization per client request, the permission is plainly denied without going to Ranger policy engine. So the fix changed the default behavior related to "denied".
> A proper fix of Ranger-1095 IMO should be just to set the "denied" to "true" in the catch block without changing the initial value of the variable.
> 
> 
> Diffs
> -----
> 
>   plugin-solr/src/main/java/org/apache/ranger/authorization/solr/authorizer/RangerSolrAuthorizer.java 6160438 
> 
> 
> Diff: https://reviews.apache.org/r/57519/diff/2/
> 
> 
> Testing
> -------
> 
> manual unit
> 
> 
> Thanks,
> 
> Yan Zhou
> 
>


Re: Review Request 57519: Ranger-1446: Ranger Solr Plugin does not work when the collection list in the request is empty

Posted by Yan Zhou via Review Board <no...@reviews.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/57519/
-----------------------------------------------------------

(Updated March 13, 2017, 3:26 p.m.)


Review request for ranger and Colm O hEigeartaigh.


Repository: ranger


Description
-------

The fix of Ranger-1095 set the initial value of "denied" to "true" from the previous "false". One impact of this change is that, when context.getCollectionRequests() is empty which could be the case in many invocations Solr makes to Ranger on authorization per client request, the permission is plainly denied without going to Ranger policy engine. So the fix changed the default behavior related to "denied".
A proper fix of Ranger-1095 IMO should be just to set the "denied" to "true" in the catch block without changing the initial value of the variable.


Diffs (updated)
-----

  plugin-solr/src/main/java/org/apache/ranger/authorization/solr/authorizer/RangerSolrAuthorizer.java 6160438 


Diff: https://reviews.apache.org/r/57519/diff/2/

Changes: https://reviews.apache.org/r/57519/diff/1-2/


Testing
-------

manual unit


Thanks,

Yan Zhou


Re: Review Request 57519: Ranger-1446: Ranger Solr Plugin does not work when the collection list in the request is empty

Posted by Yan Zhou via Review Board <no...@reviews.apache.org>.

> On March 13, 2017, 10:50 a.m., Colm O hEigeartaigh wrote:
> > Looks good, but please submit the patch using "git commit" and then "git format-patch -n HEAD~"

Done, thanks.


- Yan


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


On March 13, 2017, 3:26 p.m., Yan Zhou wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/57519/
> -----------------------------------------------------------
> 
> (Updated March 13, 2017, 3:26 p.m.)
> 
> 
> Review request for ranger and Colm O hEigeartaigh.
> 
> 
> Repository: ranger
> 
> 
> Description
> -------
> 
> The fix of Ranger-1095 set the initial value of "denied" to "true" from the previous "false". One impact of this change is that, when context.getCollectionRequests() is empty which could be the case in many invocations Solr makes to Ranger on authorization per client request, the permission is plainly denied without going to Ranger policy engine. So the fix changed the default behavior related to "denied".
> A proper fix of Ranger-1095 IMO should be just to set the "denied" to "true" in the catch block without changing the initial value of the variable.
> 
> 
> Diffs
> -----
> 
>   plugin-solr/src/main/java/org/apache/ranger/authorization/solr/authorizer/RangerSolrAuthorizer.java 6160438 
> 
> 
> Diff: https://reviews.apache.org/r/57519/diff/2/
> 
> 
> Testing
> -------
> 
> manual unit
> 
> 
> Thanks,
> 
> Yan Zhou
> 
>


Re: Review Request 57519: Ranger-1446: Ranger Solr Plugin does not work when the collection list in the request is empty

Posted by Colm O hEigeartaigh <co...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/57519/#review168749
-----------------------------------------------------------



Looks good, but please submit the patch using "git commit" and then "git format-patch -n HEAD~"

- Colm O hEigeartaigh


On March 10, 2017, 8:27 p.m., Yan Zhou wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/57519/
> -----------------------------------------------------------
> 
> (Updated March 10, 2017, 8:27 p.m.)
> 
> 
> Review request for ranger and Colm O hEigeartaigh.
> 
> 
> Repository: ranger
> 
> 
> Description
> -------
> 
> The fix of Ranger-1095 set the initial value of "denied" to "true" from the previous "false". One impact of this change is that, when context.getCollectionRequests() is empty which could be the case in many invocations Solr makes to Ranger on authorization per client request, the permission is plainly denied without going to Ranger policy engine. So the fix changed the default behavior related to "denied".
> A proper fix of Ranger-1095 IMO should be just to set the "denied" to "true" in the catch block without changing the initial value of the variable.
> 
> 
> Diffs
> -----
> 
>   plugin-solr/src/main/java/org/apache/ranger/authorization/solr/authorizer/RangerSolrAuthorizer.java 6160438 
> 
> 
> Diff: https://reviews.apache.org/r/57519/diff/1/
> 
> 
> Testing
> -------
> 
> manual unit
> 
> 
> Thanks,
> 
> Yan Zhou
> 
>