[
https://issues.apache.org/jira/browse/HIVE-3705?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13505754#comment-13505754
]
Phabricator commented on HIVE-3705:
-----------------------------------
khorgath has commented on the revision "HIVE-3705 [jira] Adding authorization
capability to the metastore".
INLINE COMMENTS
ql/src/java/org/apache/hadoop/hive/ql/security/authorization/StorageBasedAuthorizationProvider.java:49
Good point. Thought about it a bit, and decided that the best place for this
constant was in MetaStoreUtils along with some other similar constants there.
Have refactored to push it there.
ql/src/java/org/apache/hadoop/hive/ql/security/authorization/StorageBasedAuthorizationProvider.java:77
Agreed. Removing. I was mimicing existing code in HCatalog's HDFS Auth
Provider, but you're right, we need to be stricter.
ql/src/java/org/apache/hadoop/hive/ql/security/authorization/StorageBasedAuthorizationProvider.java:89
Agreed, same response.
ql/src/java/org/apache/hadoop/hive/ql/security/authorization/StorageBasedAuthorizationProvider.java:111
Agreed, same response.
ql/src/java/org/apache/hadoop/hive/ql/security/authorization/StorageBasedAuthorizationProvider.java:95
getPath is equivalent to a path constructed on
table.getTTable().getSd().getLocation() only if it is nonEmpty, which it is in
the else segment. Have made it clearer by avoiding referencing .getPath
altogether.
ql/src/java/org/apache/hadoop/hive/ql/security/authorization/StorageBasedAuthorizationProvider.java:117
Done.
ql/src/java/org/apache/hadoop/hive/ql/security/authorization/StorageBasedAuthorizationProvider.java:122
There isn't a case with partition being null, removing that bit as with 77,89
and 111 above. But it is possible for the location to be null as with the case
of creating a partition - when the PreEventListener is triggered, it's possible
for the part location to be null, in which case the correct behaviour is to
check the table's permissions.
ql/src/java/org/apache/hadoop/hive/ql/security/authorization/StorageBasedAuthorizationProvider.java:165
Interesting question - I assumed it was for creating an Index. That said, this
is currently unused in Hive - there's no reference to this that I find in the
codebase.
ql/src/java/org/apache/hadoop/hive/ql/security/authorization/StorageBasedAuthorizationProvider.java:168
Ditto as with 165, but with the exception that HiveOperation does define them
as read privileges for both LOCKTABLE and UNLOCKTABLE. That doesn't sound
terribly right to me, as I don't think read privileges are enough to be able to
perform either of these operations. I'm going to leave this as-is, and ditto
with the INDEX case above unless you think we should change it. At any rate,
these are not privileges currently in use, even if LOCK is partially defined.
ql/src/java/org/apache/hadoop/hive/ql/security/authorization/StorageBasedAuthorizationProvider.java:218
Agreed, changing across all files in this patch.
ql/src/java/org/apache/hadoop/hive/ql/security/authorization/StorageBasedAuthorizationProvider.java:301
I was trying to keep changes minimal and not change HiveMetaStore too much,
but yes, okay, refactoring, moving over to Warehouse
ql/src/java/org/apache/hadoop/hive/ql/security/authorization/StorageBasedAuthorizationProvider.java:308
This does not reimplement Warehouse::getDatabasePath, it extends it(it calls
it) by providing a default path if the location was null.
ql/src/test/org/apache/hadoop/hive/ql/security/TestAuthorizationPreEventListener.java:44
Agreed, done across tests.
ql/src/test/org/apache/hadoop/hive/ql/security/TestDefaultHiveMetastoreAuthorizationProvider.java:137
Added as requested.
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-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
