[jira] [Commented] (HIVE-3705) Adding authorization capability to the metastore

["Phabricator (JIRA)" <ji...@apache.org>]

    [ https://issues.apache.org/jira/browse/HIVE-3705?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13509042#comment-13509042 ] 

Phabricator commented on HIVE-3705:
-----------------------------------

ashutoshc has requested changes to the revision "HIVE-3705 [jira] Adding authorization capability to the metastore".

  Patch is getting close. I added few more comments, mostly around documenting semantics used in the patch.

INLINE COMMENTS
  ql/src/java/org/apache/hadoop/hive/ql/security/authorization/AuthorizationPreEventListener.java:256 This warrants a java-doc comment that in case of partition for which location is not available yet, table's location is used. This will be the case when some one does alter table add partition.
  Actually, this will be true even in case of create table, where dir will get created only after the authorization has happened, so will it check permissions of parent ?

  I think it will be good to add java-doc of all the scenarios, which dir will be used to base authentication. e.g., in case if user does create table and provides location and dir already exists, then that dir is used, otherwise parent dir.

  Semantics of which dir is used in which use-case need to be called out for explicitly. I assume your semantics model is based on http://wiki.apache.org/pig/Howl/HowlAuthorizationProposal and http://wiki.apache.org/pig/Howl/AuthorizationImplNotes

  If so, please document various pieces of code of this patch with all this information.

  ql/src/java/org/apache/hadoop/hive/ql/security/HadoopDefaultMetastoreAuthenticator.java:27 You don't store the handler, just use its conf. So, I don't see how this interface and its impl is useful as it stands today.
  On the other hand, I agree that, this is an important interface and is good to have this interface here because there could be alternative impl possible here. So, lets have the interface.
  ql/src/java/org/apache/hadoop/hive/ql/security/authorization/StorageBasedAuthorizationProvider.java:122 As I noted above, these kind of semantic decision need to be documented in the code.
  ql/src/java/org/apache/hadoop/hive/ql/security/authorization/StorageBasedAuthorizationProvider.java:168 If they are not used currently than instead of defining semantics now, we should let it fail in default branch and have the exception thrown, such that when someone later does add support for these, we can formalize the semantics.
  ql/src/test/org/apache/hadoop/hive/ql/security/TestAuthorizationPreEventListener.java:44 In the trunk, this is done via  MetaStoreUtils::startMetaStore() . See, https://issues.apache.org/jira/browse/HIVE-3724 for recommended usage pattern for it.

REVISION DETAIL
  https://reviews.facebook.net/D6681

BRANCH
  HIVE-3705

To: JIRA, ashutoshc, khorgath

                
> Adding authorization capability to the metastore
> ------------------------------------------------
>
>                 Key: HIVE-3705
>                 URL: https://issues.apache.org/jira/browse/HIVE-3705
>             Project: Hive
>          Issue Type: New Feature
>          Components: Authorization, Metastore
>            Reporter: Sushanth Sowmyan
>            Assignee: Sushanth Sowmyan
>         Attachments: HIVE-3705.D6681.1.patch, HIVE-3705.D6681.2.patch, HIVE-3705.D6681.3.patch, HIVE-3705.D6681.4.patch, hive-backend-auth.2.git.patch, hive-backend-auth.git.patch, hivesec_investigation.pdf
>
>
> In an environment where multiple clients access a single metastore, and we want to evolve hive security to a point where it's no longer simply preventing users from shooting their own foot, we need to be able to authorize metastore calls as well, instead of simply performing every metastore api call that's made.

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators
For more information on JIRA, see: http://www.atlassian.com/software/jira