> On Dec. 5, 2017, 8:28 p.m., Na Li wrote: > > sentry-provider/sentry-provider-common/src/main/java/org/apache/sentry/provider/common/ResourceAuthorizationProvider.java > > Line 101 (original), 108 (patched) > > <https://reviews.apache.org/r/64317/diff/2/?file=1908508#file1908508line108> > > > > We are using both group and user to get privilege. So we should catch > > group not found exception, and user based privilege can be found. Need to > > add test cases that there is user-based privilege, but the user does not > > belong to a group. And the privilege can be found > > Zachary Amsden wrote: > I don't think this is possible to test. The only way to generate > SentryGroupNotFoundException with the test suite is using > LocalGroupMappingService. But this requires initialization and setup of a > policy engine, and there is no policy engine which supports adding > user-specific privileges. I'd have to either add support for that to the > policy engine or the local group mapping service, and these both seem like > rather large changes. > > A simple test of adding privileges to a group named "user1" did not > succeed in adding privileges to a user named "user1". Note that this did not > throw an exception, so at least SentryGroupNotFoundException is not getting > raised, it is just the local policy engine has no idea how to deal with or > add user-specific privileges: > > ```java > @Test > public void testUserPrivilegeWithoutGroups() throws Exception { > Subject user1 = new Subject("user1"); > Server server1 = new Server("server1"); > AccessURI uri = new AccessURI("file:///path/to/"); > Set<? extends Action> actions = EnumSet.of(DBModelAction.ALL, > DBModelAction.SELECT, DBModelAction.INSERT); > policyFile.addRolesToGroup("user1", true, "role1", "role1") > .addPermissionsToRole("role1", true, "server=" + server1.getName() > + "->uri=" + uri.getName(), > "server=" + server1.getName() + "->uri=" + uri.getName()); > policyFile.write(iniFile); > PolicyEngine policy = > DBPolicyTestUtil.createPolicyEngineForTest(server1.getName(), initResource); > authzProvider = new > LocalGroupResourceAuthorizationProvider(initResource, policy, > HivePrivilegeModel.getInstance()); > List<? extends Authorizable> authorizableHierarchy = > ImmutableList.of(server1, uri); > Assert.assertTrue(authorizableHierarchy.toString(), > authzProvider.hasAccess(user1, authorizableHierarchy, actions, > ActiveRoleSet.ALL)); > } > ``` > > Na Li wrote: > You don't need to add test for this. I will. > > Since "authProvider.getGroupMapping().getGroups(userName)" could throw > exception, and causes getPrivileges() now executed with user privilege, I > want to have the following code change > > Before: > > Set<String> userPrivileges = > authProvider.getPolicyEngine().getPrivileges( > authProvider.getGroupMapping().getGroups(userName), > Sets.newHashSet(userName), > hiveAuthzBinding.getActiveRoleSet(), > hiveAuthzBinding.getAuthServer()); > > After: > > try { > Set<String> groups = null; > try { > groups = authProvider.getGroupMapping().getGroups(userName) > } catch (SentryGroupNotFoundException ex) { > log.debug(...); > groups = new HashSet<String>(); > } > > Set<String> userPrivileges = > authProvider.getPolicyEngine().getPrivileges( > groups, Sets.newHashSet(userName), > hiveAuthzBinding.getActiveRoleSet(), > hiveAuthzBinding.getAuthServer()); > > ... > } > > In this way, when user is not in a group, it can still get access if the > user has user-based privilege.
I have created SENTRY-2091, which will fix this issue for user-based privilege and add testing cases. So you don't need to do it for this jira - Na ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/64317/#review192925 ----------------------------------------------------------- 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 > >
