[
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