You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@sentry.apache.org by Sergio Pena via Review Board <no...@reviews.apache.org> on 2017/10/12 21:21:33 UTC

Review Request 62951: SENTRY-1231: Sentry doesn't secure index location uri, when do "CREATE INDEX LOCATION ''/uri"

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

Review request for sentry and kalyan kumar kalvagadda.


Bugs: sentry-1231
    https://issues.apache.org/jira/browse/sentry-1231


Repository: sentry


Description
-------

Description is on SENTRY-1231. The patch was not working because Hive 1.1 was not providing the table location, but with Hive 2.0 now the location is provided.


Diffs
-----

  sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/HiveAuthzBindingHook.java 3454910db1950f11e3317011bf4c08041a4ec5ac 
  sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/authz/HiveAuthzBindingHookBase.java b4f220e847031623572df9a8768f701c31edc765 
  sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/authz/HiveAuthzPrivilegesMap.java 2a215c435918a11ad43f748d83894aa62884e25d 
  sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/TestOperationsPart2.java 0e79ecebce19f2cdf2d9edfa262c242fcdb361cb 


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


Testing
-------


Thanks,

Sergio Pena


Re: Review Request 62951: SENTRY-1231: Sentry doesn't secure index location uri, when do "CREATE INDEX LOCATION ''/uri"

Posted by kalyan kumar kalvagadda via Review Board <no...@reviews.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/62951/#review188149
-----------------------------------------------------------




sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/authz/HiveAuthzBindingHookBase.java
Lines 294-311 (patched)
<https://reviews.apache.org/r/62951/#comment265193>

    This method need not have the logic of interating the tree. 
    
    ASTNode provides API's to search for nodes of interest which could be used.
    
    In this case logic of iterating the tree could be avoided if this API is called like
    
    extractTableLocation(ASTNode)ast.getFirstChildWithType(HiveParser.TOK_TABLELOCATION))
    
    It's better to use the exisiting API's to interate the tree as it will make sentry immune to the change that happen to ASTNode implementation.


- kalyan kumar kalvagadda


On Oct. 12, 2017, 9:21 p.m., Sergio Pena wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62951/
> -----------------------------------------------------------
> 
> (Updated Oct. 12, 2017, 9:21 p.m.)
> 
> 
> Review request for sentry and kalyan kumar kalvagadda.
> 
> 
> Bugs: sentry-1231
>     https://issues.apache.org/jira/browse/sentry-1231
> 
> 
> Repository: sentry
> 
> 
> Description
> -------
> 
> Description is on SENTRY-1231. The patch was not working because Hive 1.1 was not providing the table location, but with Hive 2.0 now the location is provided.
> 
> 
> Diffs
> -----
> 
>   sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/HiveAuthzBindingHook.java 3454910db1950f11e3317011bf4c08041a4ec5ac 
>   sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/authz/HiveAuthzBindingHookBase.java b4f220e847031623572df9a8768f701c31edc765 
>   sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/authz/HiveAuthzPrivilegesMap.java 2a215c435918a11ad43f748d83894aa62884e25d 
>   sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/TestOperationsPart2.java 0e79ecebce19f2cdf2d9edfa262c242fcdb361cb 
> 
> 
> Diff: https://reviews.apache.org/r/62951/diff/1/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Sergio Pena
> 
>


Re: Review Request 62951: SENTRY-1231: Sentry doesn't secure index location uri, when do "CREATE INDEX LOCATION ''/uri"

Posted by kalyan kumar kalvagadda via Review Board <no...@reviews.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/62951/#review188198
-----------------------------------------------------------


Ship it!




Ship It!

- kalyan kumar kalvagadda


On Oct. 16, 2017, 6:34 p.m., Sergio Pena wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62951/
> -----------------------------------------------------------
> 
> (Updated Oct. 16, 2017, 6:34 p.m.)
> 
> 
> Review request for sentry and kalyan kumar kalvagadda.
> 
> 
> Bugs: sentry-1231
>     https://issues.apache.org/jira/browse/sentry-1231
> 
> 
> Repository: sentry
> 
> 
> Description
> -------
> 
> Description is on SENTRY-1231. The patch was not working because Hive 1.1 was not providing the table location, but with Hive 2.0 now the location is provided.
> 
> 
> Diffs
> -----
> 
>   sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/HiveAuthzBindingHook.java 3454910db1950f11e3317011bf4c08041a4ec5ac 
>   sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/authz/HiveAuthzBindingHookBase.java b4f220e847031623572df9a8768f701c31edc765 
>   sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/authz/HiveAuthzPrivilegesMap.java 2a215c435918a11ad43f748d83894aa62884e25d 
>   sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/TestOperationsPart2.java 0e79ecebce19f2cdf2d9edfa262c242fcdb361cb 
> 
> 
> Diff: https://reviews.apache.org/r/62951/diff/2/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Sergio Pena
> 
>


Re: Review Request 62951: SENTRY-1231: Sentry doesn't secure index location uri, when do "CREATE INDEX LOCATION ''/uri"

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

(Updated Oct. 16, 2017, 6:34 p.m.)


Review request for sentry and kalyan kumar kalvagadda.


Changes
-------

Addressed Kalyan's feedback.


Bugs: sentry-1231
    https://issues.apache.org/jira/browse/sentry-1231


Repository: sentry


Description
-------

Description is on SENTRY-1231. The patch was not working because Hive 1.1 was not providing the table location, but with Hive 2.0 now the location is provided.


Diffs (updated)
-----

  sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/HiveAuthzBindingHook.java 3454910db1950f11e3317011bf4c08041a4ec5ac 
  sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/authz/HiveAuthzBindingHookBase.java b4f220e847031623572df9a8768f701c31edc765 
  sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/authz/HiveAuthzPrivilegesMap.java 2a215c435918a11ad43f748d83894aa62884e25d 
  sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/TestOperationsPart2.java 0e79ecebce19f2cdf2d9edfa262c242fcdb361cb 


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

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


Testing
-------


Thanks,

Sergio Pena