You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@hive.apache.org by pengcheng xiong <px...@hortonworks.com> on 2016/04/02 02:55:03 UTC

Review Request 45611: HIVE-13360: Refactoring Hive Authorization

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

Review request for hive and Ashutosh Chauhan.


Repository: hive-git


Description
-------

HIVE-13360: Refactoring Hive Authorization


Diffs
-----

  itests/hive-unit/src/test/java/org/apache/hadoop/hive/ql/security/authorization/plugin/TestHiveAuthorizerCheckInvocation.java 9f47f84 
  itests/hive-unit/src/test/java/org/apache/hadoop/hive/ql/security/authorization/plugin/TestHiveAuthorizerShowFilters.java 5922a8c 
  itests/hive-unit/src/test/java/org/apache/hive/jdbc/authorization/TestHS2AuthzContext.java c43776b 
  itests/hive-unit/src/test/java/org/apache/hive/jdbc/authorization/TestJdbcMetadataApiAuth.java 692bfa0 
  itests/util/src/main/java/org/apache/hadoop/hive/ql/security/DummyAuthenticator.java a296ac5 
  itests/util/src/main/java/org/apache/hadoop/hive/ql/security/InjectableDummyAuthenticator.java 322834e 
  itests/util/src/main/java/org/apache/hadoop/hive/ql/security/authorization/plugin/sqlstd/SQLStdHiveAuthorizationValidatorForTest.java c0387e2 
  ql/src/java/org/apache/hadoop/hive/ql/Driver.java 7276e31 
  ql/src/java/org/apache/hadoop/hive/ql/parse/MaskAndFilterInfo.java PRE-CREATION 
  ql/src/java/org/apache/hadoop/hive/ql/parse/SemanticAnalyzer.java e81d46e 
  ql/src/java/org/apache/hadoop/hive/ql/parse/TableMask.java c47c2bd 
  ql/src/java/org/apache/hadoop/hive/ql/processors/CommandUtil.java d98b30c 
  ql/src/java/org/apache/hadoop/hive/ql/security/HadoopDefaultAuthenticator.java 18e4e00 
  ql/src/java/org/apache/hadoop/hive/ql/security/HiveAuthenticationProvider.java 7befff8 
  ql/src/java/org/apache/hadoop/hive/ql/security/SessionStateConfigUserAuthenticator.java 8c7809e 
  ql/src/java/org/apache/hadoop/hive/ql/security/SessionStateUserAuthenticator.java a77e93f 
  ql/src/java/org/apache/hadoop/hive/ql/security/authorization/plugin/AuthorizationMetaStoreFilterHook.java 6bad99b 
  ql/src/java/org/apache/hadoop/hive/ql/security/authorization/plugin/HiveAuthorizationValidator.java 1b366c2 
  ql/src/java/org/apache/hadoop/hive/ql/security/authorization/plugin/HiveAuthorizer.java 6e2ef8d 
  ql/src/java/org/apache/hadoop/hive/ql/security/authorization/plugin/HiveAuthorizerImpl.java c73d667 
  ql/src/java/org/apache/hadoop/hive/ql/security/authorization/plugin/HiveAuthzContext.java 195e341 
  ql/src/java/org/apache/hadoop/hive/ql/security/authorization/plugin/HivePrivilegeObject.java 0364627 
  ql/src/java/org/apache/hadoop/hive/ql/security/authorization/plugin/HiveV1Authorizer.java c8aa9db 
  ql/src/java/org/apache/hadoop/hive/ql/security/authorization/plugin/QueryContext.java PRE-CREATION 
  ql/src/java/org/apache/hadoop/hive/ql/security/authorization/plugin/sqlstd/DummyHiveAuthorizationValidator.java e4ddc9b 
  ql/src/java/org/apache/hadoop/hive/ql/security/authorization/plugin/sqlstd/SQLStdHiveAuthorizationValidator.java c5d60b3 
  service/src/java/org/apache/hive/service/cli/operation/MetadataOperation.java 285b4f9 

Diff: https://reviews.apache.org/r/45611/diff/


Testing
-------


Thanks,

pengcheng xiong


Re: Review Request 45611: HIVE-13360: Refactoring Hive Authorization

Posted by pengcheng xiong <px...@hortonworks.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/45611/
-----------------------------------------------------------

(Updated April 4, 2016, 10:47 p.m.)


Review request for hive and Ashutosh Chauhan.


Repository: hive-git


Description
-------

HIVE-13360: Refactoring Hive Authorization


Diffs (updated)
-----

  itests/hive-unit/src/test/java/org/apache/hadoop/hive/ql/security/authorization/plugin/TestHiveAuthorizerCheckInvocation.java 9f47f84 
  itests/hive-unit/src/test/java/org/apache/hadoop/hive/ql/security/authorization/plugin/TestHiveAuthorizerShowFilters.java 5922a8c 
  itests/hive-unit/src/test/java/org/apache/hive/jdbc/authorization/TestHS2AuthzContext.java c43776b 
  itests/hive-unit/src/test/java/org/apache/hive/jdbc/authorization/TestJdbcMetadataApiAuth.java 692bfa0 
  itests/util/src/main/java/org/apache/hadoop/hive/ql/security/DummyAuthenticator.java a296ac5 
  itests/util/src/main/java/org/apache/hadoop/hive/ql/security/InjectableDummyAuthenticator.java 322834e 
  itests/util/src/main/java/org/apache/hadoop/hive/ql/security/authorization/plugin/sqlstd/SQLStdHiveAuthorizationValidatorForTest.java c0387e2 
  ql/src/java/org/apache/hadoop/hive/ql/Driver.java 7276e31 
  ql/src/java/org/apache/hadoop/hive/ql/parse/MaskAndFilterInfo.java PRE-CREATION 
  ql/src/java/org/apache/hadoop/hive/ql/parse/SemanticAnalyzer.java e81d46e 
  ql/src/java/org/apache/hadoop/hive/ql/parse/TableMask.java c47c2bd 
  ql/src/java/org/apache/hadoop/hive/ql/processors/CommandUtil.java d98b30c 
  ql/src/java/org/apache/hadoop/hive/ql/security/HadoopDefaultAuthenticator.java 18e4e00 
  ql/src/java/org/apache/hadoop/hive/ql/security/HiveAuthenticationProvider.java 7befff8 
  ql/src/java/org/apache/hadoop/hive/ql/security/SessionStateConfigUserAuthenticator.java 8c7809e 
  ql/src/java/org/apache/hadoop/hive/ql/security/SessionStateUserAuthenticator.java a77e93f 
  ql/src/java/org/apache/hadoop/hive/ql/security/authorization/plugin/AuthorizationMetaStoreFilterHook.java 6bad99b 
  ql/src/java/org/apache/hadoop/hive/ql/security/authorization/plugin/HiveAuthorizationValidator.java 1b366c2 
  ql/src/java/org/apache/hadoop/hive/ql/security/authorization/plugin/HiveAuthorizer.java 6e2ef8d 
  ql/src/java/org/apache/hadoop/hive/ql/security/authorization/plugin/HiveAuthorizerImpl.java c73d667 
  ql/src/java/org/apache/hadoop/hive/ql/security/authorization/plugin/HiveAuthzContext.java 195e341 
  ql/src/java/org/apache/hadoop/hive/ql/security/authorization/plugin/HivePrivilegeObject.java 0364627 
  ql/src/java/org/apache/hadoop/hive/ql/security/authorization/plugin/HiveV1Authorizer.java c8aa9db 
  ql/src/java/org/apache/hadoop/hive/ql/security/authorization/plugin/QueryContext.java PRE-CREATION 
  ql/src/java/org/apache/hadoop/hive/ql/security/authorization/plugin/sqlstd/DummyHiveAuthorizationValidator.java e4ddc9b 
  ql/src/java/org/apache/hadoop/hive/ql/security/authorization/plugin/sqlstd/SQLStdHiveAuthorizationValidator.java c5d60b3 
  ql/src/test/org/apache/hadoop/hive/ql/parse/authorization/TestSessionUserName.java a81ac44 
  service/src/java/org/apache/hive/service/cli/operation/MetadataOperation.java 285b4f9 

Diff: https://reviews.apache.org/r/45611/diff/


Testing
-------


Thanks,

pengcheng xiong


Re: Review Request 45611: HIVE-13360: Refactoring Hive Authorization

Posted by Ashutosh Chauhan <ha...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/45611/#review126670
-----------------------------------------------------------




itests/hive-unit/src/test/java/org/apache/hive/jdbc/authorization/TestHS2AuthzContext.java 
<https://reviews.apache.org/r/45611/#comment189684>

    It will be good to move these tests instead of deleting altogether.



ql/src/java/org/apache/hadoop/hive/ql/parse/SemanticAnalyzer.java (line 10373)
<https://reviews.apache.org/r/45611/#comment189688>

    Can just remove this comment.



ql/src/java/org/apache/hadoop/hive/ql/parse/TableMask.java (line 90)
<https://reviews.apache.org/r/45611/#comment189691>

    We should check size of exprs and passed in list of columns and throw exception if they don't match.



ql/src/java/org/apache/hadoop/hive/ql/parse/TableMask.java (line 92)
<https://reviews.apache.org/r/45611/#comment189692>

    Check for null == expr and use privObj.getCols.get(index) if it is.



ql/src/java/org/apache/hadoop/hive/ql/security/authorization/plugin/HiveAuthorizer.java (line 273)
<https://reviews.apache.org/r/45611/#comment189689>

    No need to provide QueryContext here.



ql/src/java/org/apache/hadoop/hive/ql/security/authorization/plugin/HiveAuthorizer.java (line 288)
<https://reviews.apache.org/r/45611/#comment189693>

    I dont think there is need for this call anymore, since we are collecting all tables and columns and making one call for it. We can just examine List<HivePrivilegeObject> returned from applyRowFilterAndColumnMasking() to determine which tables need transforms.



ql/src/java/org/apache/hadoop/hive/ql/security/authorization/plugin/HivePrivilegeObject.java (lines 110 - 111)
<https://reviews.apache.org/r/45611/#comment189690>

    it will be good to add comments for these.


- Ashutosh Chauhan


On April 2, 2016, 12:54 a.m., pengcheng xiong wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45611/
> -----------------------------------------------------------
> 
> (Updated April 2, 2016, 12:54 a.m.)
> 
> 
> Review request for hive and Ashutosh Chauhan.
> 
> 
> Repository: hive-git
> 
> 
> Description
> -------
> 
> HIVE-13360: Refactoring Hive Authorization
> 
> 
> Diffs
> -----
> 
>   itests/hive-unit/src/test/java/org/apache/hadoop/hive/ql/security/authorization/plugin/TestHiveAuthorizerCheckInvocation.java 9f47f84 
>   itests/hive-unit/src/test/java/org/apache/hadoop/hive/ql/security/authorization/plugin/TestHiveAuthorizerShowFilters.java 5922a8c 
>   itests/hive-unit/src/test/java/org/apache/hive/jdbc/authorization/TestHS2AuthzContext.java c43776b 
>   itests/hive-unit/src/test/java/org/apache/hive/jdbc/authorization/TestJdbcMetadataApiAuth.java 692bfa0 
>   itests/util/src/main/java/org/apache/hadoop/hive/ql/security/DummyAuthenticator.java a296ac5 
>   itests/util/src/main/java/org/apache/hadoop/hive/ql/security/InjectableDummyAuthenticator.java 322834e 
>   itests/util/src/main/java/org/apache/hadoop/hive/ql/security/authorization/plugin/sqlstd/SQLStdHiveAuthorizationValidatorForTest.java c0387e2 
>   ql/src/java/org/apache/hadoop/hive/ql/Driver.java 7276e31 
>   ql/src/java/org/apache/hadoop/hive/ql/parse/MaskAndFilterInfo.java PRE-CREATION 
>   ql/src/java/org/apache/hadoop/hive/ql/parse/SemanticAnalyzer.java e81d46e 
>   ql/src/java/org/apache/hadoop/hive/ql/parse/TableMask.java c47c2bd 
>   ql/src/java/org/apache/hadoop/hive/ql/processors/CommandUtil.java d98b30c 
>   ql/src/java/org/apache/hadoop/hive/ql/security/HadoopDefaultAuthenticator.java 18e4e00 
>   ql/src/java/org/apache/hadoop/hive/ql/security/HiveAuthenticationProvider.java 7befff8 
>   ql/src/java/org/apache/hadoop/hive/ql/security/SessionStateConfigUserAuthenticator.java 8c7809e 
>   ql/src/java/org/apache/hadoop/hive/ql/security/SessionStateUserAuthenticator.java a77e93f 
>   ql/src/java/org/apache/hadoop/hive/ql/security/authorization/plugin/AuthorizationMetaStoreFilterHook.java 6bad99b 
>   ql/src/java/org/apache/hadoop/hive/ql/security/authorization/plugin/HiveAuthorizationValidator.java 1b366c2 
>   ql/src/java/org/apache/hadoop/hive/ql/security/authorization/plugin/HiveAuthorizer.java 6e2ef8d 
>   ql/src/java/org/apache/hadoop/hive/ql/security/authorization/plugin/HiveAuthorizerImpl.java c73d667 
>   ql/src/java/org/apache/hadoop/hive/ql/security/authorization/plugin/HiveAuthzContext.java 195e341 
>   ql/src/java/org/apache/hadoop/hive/ql/security/authorization/plugin/HivePrivilegeObject.java 0364627 
>   ql/src/java/org/apache/hadoop/hive/ql/security/authorization/plugin/HiveV1Authorizer.java c8aa9db 
>   ql/src/java/org/apache/hadoop/hive/ql/security/authorization/plugin/QueryContext.java PRE-CREATION 
>   ql/src/java/org/apache/hadoop/hive/ql/security/authorization/plugin/sqlstd/DummyHiveAuthorizationValidator.java e4ddc9b 
>   ql/src/java/org/apache/hadoop/hive/ql/security/authorization/plugin/sqlstd/SQLStdHiveAuthorizationValidator.java c5d60b3 
>   service/src/java/org/apache/hive/service/cli/operation/MetadataOperation.java 285b4f9 
> 
> Diff: https://reviews.apache.org/r/45611/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> pengcheng xiong
> 
>