You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@hive.apache.org by Ashutosh Chauhan <ha...@apache.org> on 2014/05/24 03:17:03 UTC
Re: Review Request 15855: Check DATABASES/TABLE privilege for metastore
authorizer
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/15855/#review43888
-----------------------------------------------------------
metastore/src/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java
<https://reviews.apache.org/r/15855/#comment78174>
Technically, its a post event since event = ObjectStore call already happened on previous line.
metastore/src/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java
<https://reviews.apache.org/r/15855/#comment78172>
Technically, its a post event since event = ObjectStore call already happened on previous line.
metastore/src/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java
<https://reviews.apache.org/r/15855/#comment78173>
Same comment as previous.
ql/src/java/org/apache/hadoop/hive/ql/security/authorization/AuthorizationPreEventListener.java
<https://reviews.apache.org/r/15855/#comment78175>
This is expensive. We already made one call on mysql to retrieve all names, now we are gonna make n calls again to get constructed Database object. Better way is to retrieve all constructed Database Objects in one mysql call.
Though, this is performance optimization, so I am ok to do it later.
ql/src/java/org/apache/hadoop/hive/ql/security/authorization/AuthorizationPreEventListener.java
<https://reviews.apache.org/r/15855/#comment78170>
If user is not authorized for show databases, than authorization exception should be thrown instead of not showing that database.
ql/src/java/org/apache/hadoop/hive/ql/security/authorization/AuthorizationPreEventListener.java
<https://reviews.apache.org/r/15855/#comment78176>
Same comment as previous.
ql/src/java/org/apache/hadoop/hive/ql/security/authorization/AuthorizationPreEventListener.java
<https://reviews.apache.org/r/15855/#comment78171>
Same comment as previous.
- Ashutosh Chauhan
On Nov. 26, 2013, 8:49 a.m., Navis Ryu wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/15855/
> -----------------------------------------------------------
>
> (Updated Nov. 26, 2013, 8:49 a.m.)
>
>
> Review request for hive.
>
>
> Bugs: HIVE-5890
> https://issues.apache.org/jira/browse/HIVE-5890
>
>
> Repository: hive-git
>
>
> Description
> -------
>
> Commands like "show database" are not checked in metastore event listener.
>
>
> Diffs
> -----
>
> metastore/src/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java 01c2626
> metastore/src/java/org/apache/hadoop/hive/metastore/events/PreEventContext.java 5021a73
> metastore/src/java/org/apache/hadoop/hive/metastore/events/PreGetDatabasesEvent.java PRE-CREATION
> metastore/src/java/org/apache/hadoop/hive/metastore/events/PreGetTablesEvent.java PRE-CREATION
> ql/src/java/org/apache/hadoop/hive/ql/security/authorization/AuthorizationPreEventListener.java 9a90549
>
> Diff: https://reviews.apache.org/r/15855/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Navis Ryu
>
>
Re: Review Request 15855: Check DATABASES/TABLE privilege for
metastore authorizer
Posted by Navis Ryu <na...@nexr.com>.
> On May 24, 2014, 1:17 a.m., Ashutosh Chauhan wrote:
> > metastore/src/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java, line 830
> > <https://reviews.apache.org/r/15855/diff/1/?file=391130#file391130line830>
> >
> > Technically, its a post event since event = ObjectStore call already happened on previous line.
Yes, technically it is post event. But if the result is not delivered to client, can we regard that as pre-event? It whould be meaningless to authorize without databases/tables to be used. Should we introduce post-event listeners?
> On May 24, 2014, 1:17 a.m., Ashutosh Chauhan wrote:
> > ql/src/java/org/apache/hadoop/hive/ql/security/authorization/AuthorizationPreEventListener.java, line 246
> > <https://reviews.apache.org/r/15855/diff/1/?file=391134#file391134line246>
> >
> > If user is not authorized for show databases, than authorization exception should be thrown instead of not showing that database.
If it's some real access(read or write) to the database or table, it's definitely right to throw exception (in that action). But this is just for showing/listing legitimated databases/tables for the user. Isn't it?
- Navis
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/15855/#review43888
-----------------------------------------------------------
On Nov. 26, 2013, 8:49 a.m., Navis Ryu wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/15855/
> -----------------------------------------------------------
>
> (Updated Nov. 26, 2013, 8:49 a.m.)
>
>
> Review request for hive.
>
>
> Bugs: HIVE-5890
> https://issues.apache.org/jira/browse/HIVE-5890
>
>
> Repository: hive-git
>
>
> Description
> -------
>
> Commands like "show database" are not checked in metastore event listener.
>
>
> Diffs
> -----
>
> metastore/src/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java 01c2626
> metastore/src/java/org/apache/hadoop/hive/metastore/events/PreEventContext.java 5021a73
> metastore/src/java/org/apache/hadoop/hive/metastore/events/PreGetDatabasesEvent.java PRE-CREATION
> metastore/src/java/org/apache/hadoop/hive/metastore/events/PreGetTablesEvent.java PRE-CREATION
> ql/src/java/org/apache/hadoop/hive/ql/security/authorization/AuthorizationPreEventListener.java 9a90549
>
> Diff: https://reviews.apache.org/r/15855/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Navis Ryu
>
>