> On Dec. 20, 2018, 2:46 p.m., Sergio Pena wrote: > > standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java > > Lines 1405 (patched) > > <https://reviews.apache.org/r/69585/diff/3/?file=2115114#file2115114line1405> > > > > I thought we were going to use the PreReadEvent instead. Also, I'm > > concerned the implementation of filterDatabase does not throw > > NoSuchObjectException, and instead returns null. We should check both cases > > to guarantee get_database and others methods will deny the access to this > > database. > > > > I think we should use the PreReadEvent which is meant for authorization > > hooks. > > Na Li wrote: > The PreReadEvent calls sentry MetastoreAuthzBindingBase for > authorization. As you can see in > > https://github.com/apache/sentry/blob/master/sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/metastore/MetastoreAuthzBindingBase.java#L169, > it does not do anything on reading metadata. Therefore, we have to use > filter to prevent user from reading metadata it does not have privileges. > > I can only remove the filter for get_database() or get_table() if we > change sentry's implementation for that file.
for get_table(), get_database(), get_parittion*(), I remove the filter check, and use firePreEvent() to verify that user can access the metadata. When getting list of names, we only call filter hook to remove names that user should not see. Accessing metadata of an object requires more privileges than just getting the names. For example, "CREATE" privilege on server allows listing DB names, so user does not create a DB using a name of an existing DB. "CREATE" privilege on DB allows listing Table names. But "CREATE" privilege does not allow describe a particular table (show meta data of a table user has no privilege) regardless it is for authorization or filtering. So we should only check authorization (firePreEvent()) for single item like get_database() or get_table(). For getting a list of item names, we only call filter hook. - Na ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/69585/#review211465 ----------------------------------------------------------- On Dec. 21, 2018, 9:36 p.m., Na Li wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/69585/ > ----------------------------------------------------------- > > (Updated Dec. 21, 2018, 9:36 p.m.) > > > Review request for hive, Adam Holley, Morio Ramdenbourg, Peter Vary, Sergio > Pena, and Vihang Karajgaonkar. > > > Bugs: HIVE-20776 > https://issues.apache.org/jira/browse/HIVE-20776 > > > Repository: hive-git > > > Description > ------- > > add filtering to read result at HMS server, so user cannot see metadata > he/she has no privileges. Filtering is enabled/disabled based on > configuration. > > > Diffs > ----- > > > standalone-metastore/metastore-common/src/main/java/org/apache/hadoop/hive/metastore/HiveMetaStoreClient.java > 9eb1193a27120b5167f92daf67bf6a1c4e1d9927 > > standalone-metastore/metastore-common/src/main/java/org/apache/hadoop/hive/metastore/conf/MetastoreConf.java > fb0b2fe6fb9fd4b4c92a6a39f06f39a4641aaabd > > standalone-metastore/metastore-common/src/main/java/org/apache/hadoop/hive/metastore/utils/FilterUtils.java > PRE-CREATION > > standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java > 0a1b96dcf62d3536cab2ce074d27a6225b2d3443 > > standalone-metastore/metastore-server/src/test/java/org/apache/hadoop/hive/metastore/TestHiveMetastoreFilterHook.java > PRE-CREATION > > > Diff: https://reviews.apache.org/r/69585/diff/4/ > > > Testing > ------- > > Existing unit tests passed. > add new unit tests for filtering at HMS server and HMS client > add code to enabled/disable filtering at HMS client based on configuration > > > Thanks, > > Na Li > >