----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/64317/#review194177 -----------------------------------------------------------
sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/authz/HiveAuthzBindingHookBase.java Line 837 (original), 837 (patched) <https://reviews.apache.org/r/64317/#comment272897> This is not the appropriate error that should be logged. SemanticException is not not right exception, please throw appropriate exception and handle it. sentry-provider/sentry-provider-common/src/main/java/org/apache/sentry/provider/common/ResourceAuthorizationProvider.java Lines 99 (patched) <https://reviews.apache.org/r/64317/#comment272903> You can not silently consume the exception here. It should be thrown in the caller where it can create appripriate error message. If you comsume this exception, AuthorizationException will be throw with message saying user doesn not have privileges to perfrom a operation. Which is not the case. sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/SentryPolicyStoreProcessor.java Lines 752 (patched) <https://reviews.apache.org/r/64317/#comment272896> Whay are you using NoSuchObject here when you are using AccessDenied in rest of places. Any specific reason? I think it should be same as else where. - kalyan kumar kalvagadda On Dec. 5, 2017, 12:55 a.m., Zachary Amsden wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/64317/ > ----------------------------------------------------------- > > (Updated Dec. 5, 2017, 12:55 a.m.) > > > Review request for sentry and Na Li. > > > Repository: sentry > > > Description > ------- > > Instead of leaking new exceptions outside the API, use the > existing authorization exceptions to indicate authorization > failure when a user has no group configured. > > > Diffs > ----- > > > sentry-binding/sentry-binding-hive-common/src/main/java/org/apache/sentry/binding/hive/authz/HiveAuthzBinding.java > 8ce7a02ed4c565e34229a5c80c1b4fd1a84bad19 > > sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/authz/HiveAuthzBindingHookBase.java > 9c60c22aac826affd05cdf28b3816c68c139326d > > sentry-binding/sentry-binding-hive/src/test/java/org/apache/sentry/binding/hive/TestHiveAuthzBindings.java > a41d1bd533157c96430c3bf3569e1612db77c7b2 > > sentry-binding/sentry-binding-solr/src/main/java/org/apache/sentry/binding/solr/authz/SentrySolrPluginImpl.java > 91d08f0bc7f344c87e5bfb1e11b4b68728e676be > > sentry-binding/sentry-binding-solr/src/main/java/org/apache/sentry/binding/solr/authz/SolrAuthzBinding.java > 803e5eabf322cd120456a78c57f127ed4c94f5fc > > sentry-binding/sentry-binding-solr/src/test/java/org/apache/sentry/binding/solr/TestSolrAuthzBinding.java > f060b82da44f642e9a1dbff86e6e834fbc09cb2b > > sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/exception/SentryGroupNotFoundException.java > b978df69df1d777311146406278444ae4e7f83ee > > sentry-provider/sentry-provider-common/src/main/java/org/apache/sentry/provider/common/AuthorizationProvider.java > 2d82bcfcd5343d1b130df2f723d33a106d36ea81 > > sentry-provider/sentry-provider-common/src/main/java/org/apache/sentry/provider/common/GroupMappingService.java > 7e85261070f133a6886434732d23d5a72894f8ef > > sentry-provider/sentry-provider-common/src/main/java/org/apache/sentry/provider/common/HadoopGroupMappingService.java > bde53d5f640c98f41dea54d54dfe708ffee5dcd3 > > sentry-provider/sentry-provider-common/src/main/java/org/apache/sentry/provider/common/ResourceAuthorizationProvider.java > 005724f3e3f8c623c2a266f60825cf77ac1ea777 > > sentry-provider/sentry-provider-common/src/test/java/org/apache/sentry/provider/common/TestNoAuthorizationProvider.java > fe01b062c592e17ffa336552986e83f3f5f294e3 > > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/SentryPolicyStoreProcessor.java > 650880bb682d76c000fa51b497fae484c257b342 > > sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/generic/service/thrift/TestSentryGenericPolicyProcessor.java > 6597a7ca724d1377ad07d8bc18530eb89b659693 > > sentry-provider/sentry-provider-file/src/main/java/org/apache/sentry/provider/file/LocalGroupMappingService.java > 54474203aed4868c3bde8450d4d27427fa1de7f6 > > sentry-provider/sentry-provider-file/src/test/java/org/apache/sentry/provider/file/TestLocalGroupMapping.java > 9864b82bfd9c499ab2b1f8ba9d4664fe19899d4e > > sentry-solr/solr-sentry-handlers/src/main/java/org/apache/solr/handler/component/QueryDocAuthorizationComponent.java > 2338ab8375a6381e8d5fc8b38f766789187f69af > > sentry-tests/sentry-tests-hive-v2/src/test/java/org/apache/sentry/tests/e2e/hive/TestUserManagement.java > 02ac51454a13c0c1c61bb8684872e4815bd88b97 > > sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/TestUserManagement.java > 02ac51454a13c0c1c61bb8684872e4815bd88b97 > > > Diff: https://reviews.apache.org/r/64317/diff/2/ > > > Testing > ------- > > Running JUnit tests with mvn install. > > > Thanks, > > Zachary Amsden > >
