Re: Review Request 15855: Check DATABASES/TABLE privilege for metastore authorizer

[Navis Ryu]

> On May 24, 2014, 1:17 a.m., Ashutosh Chauhan wrote:
> > metastore/src/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java, 
> > line 830
> > 
> >
> > 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
> > 
> >
> > 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
> 
>

Re: Review Request 15855: Check DATABASES/TABLE privilege for metastore authorizer

[Ashutosh Chauhan]

---
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


Technically, its a post event since event = ObjectStore call already 
happened on previous line.



metastore/src/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java


Technically, its a post event since event = ObjectStore call already 
happened on previous line.



metastore/src/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java


Same comment as previous.



ql/src/java/org/apache/hadoop/hive/ql/security/authorization/AuthorizationPreEventListener.java


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


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


Same comment as previous.



ql/src/java/org/apache/hadoop/hive/ql/security/authorization/AuthorizationPreEventListener.java


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
> 
>

Review Request 15855: Check DATABASES/TABLE privilege for metastore authorizer

[Navis Ryu]

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

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