Re: Review Request 15855: Check DATABASES/TABLE privilege for metastore authorizer
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
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
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/ --- 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
