> On June 23, 2014, 3:11 a.m., Prasad Mujumdar wrote: > > sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/HiveAuthzBindingHook.java, > > lines 313-314 > > <https://reviews.apache.org/r/22847/diff/5/?file=614860#file614860line313> > > > > I think this is problematic. We are looking string 'insert' in the > > statement which could match with any token that contains this string. For > > example, > > "SELECT insert_code from table1" > > "INSERT OVERWRITE TABLE directory_table SELECT .." > > > >
Right, we need a more robust way of handling this. For now, reverting the privilege map for QUERY and put a condition to not have strict check for all required privileges when hiveOp == QUERY > On June 23, 2014, 3:11 a.m., Prasad Mujumdar wrote: > > sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/HiveAuthzBindingPreExecHook.java, > > line 60 > > <https://reviews.apache.org/r/22847/diff/5/?file=614861#file614861line60> > > > > Does this need to be input hierarchy ? There was some inconsistency, some operations were setting it to output hierarchy and some to input, which were surfaced by making the check for required privileges stricter. Made it consistent now. > On June 23, 2014, 3:11 a.m., Prasad Mujumdar wrote: > > sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/authz/HiveAuthzBinding.java, > > line 361 > > <https://reviews.apache.org/r/22847/diff/5/?file=614862#file614862line361> > > > > Is this method used anymore ? Removed it. > On June 23, 2014, 3:11 a.m., Prasad Mujumdar wrote: > > sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/authz/HiveAuthzPrivilegesMap.java, > > line 123 > > <https://reviews.apache.org/r/22847/diff/5/?file=614863#file614863line123> > > > > I can't think of a reason to keep URI privilege in there. But just to > > be on safer side, can we leave it in there? Given that URI is optional, it > > shouldn't cause a problem. If it does require any additional code changes, > > then it's okay. Added it back for now with a comment. - Sravya ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/22847/#review46377 ----------------------------------------------------------- On June 23, 2014, 1:54 a.m., Sravya Tirukkovalur wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/22847/ > ----------------------------------------------------------- > > (Updated June 23, 2014, 1:54 a.m.) > > > Review request for sentry, Arun Suresh, Jarek Cecho, Lenni Kuff, and Prasad > Mujumdar. > > > Bugs: SENTRY-310 > https://issues.apache.org/jira/browse/SENTRY-310 > > > Repository: sentry > > > Description > ------- > > Required privileges for a given hive operation is too restrictive in some > cases. This patch cleans that up. The new model is documented as a pdf > attached to the ticket. > > In short: > - All DDL statements on an object require ALL on that object, except the > create database/table/view/partition which requires all on the parent, as we > should not allow granting privileges on non existing objects. > - Cleaned up some unwanted uri privileges, now we only support all on URI. > - Fixed some more non intuitive mappings > > > Diffs > ----- > > > sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/HiveAuthzBindingHook.java > 6c507b83419ab5e5e2797c62dc71bfa0fdf36776 > > sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/HiveAuthzBindingPreExecHook.java > 7859521b2c56372280d73934293d9cd419119be4 > > sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/authz/HiveAuthzBinding.java > cedf368825a153be13d3a05d1519a581bc30082f > > sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/authz/HiveAuthzPrivilegesMap.java > 7d241d0ea7957e6b6c334c78c6bcf0934f1a36ab > > sentry-binding/sentry-binding-hive/src/test/java/org/apache/sentry/binding/hive/TestHiveAuthzBindings.java > 1f9d1eccceb45a8f4d600a36e72e3a2ad4dbc5fa > > sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/AbstractTestWithStaticConfiguration.java > fd969a6cb221656d2dee65a068cdce77e1efc5cd > > sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/Context.java > e725eb06fc9915b0bcc2609e428a62feea80ec43 > > sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/TestCrossDbOps.java > 8552cc062fc7ebf6f093ef044321b13b860aaebc > > sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/TestOperations.java > PRE-CREATION > > sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/TestPrivilegeAtTransform.java > 1267e6bfc035371fb48346cbcd00c15c327a2c42 > > sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/TestPrivilegesAtDatabaseScope.java > c9658abafc7ad77ed18ce5bb9b33397dccab625c > > sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/TestSandboxOps.java > 0d6e0b656ea0af48869c28d7d4938586f34084e7 > > sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/TestUriPermissions.java > 5a620ba23a74e4ae85d019681d595172b3a86540 > > Diff: https://reviews.apache.org/r/22847/diff/ > > > Testing > ------- > > Captured most of the Hive operations in TestOperations test class. All of > them pass. > > Added todos for the operations which need test cases. Now running the entire > suite. > > > Thanks, > > Sravya Tirukkovalur > >
