> 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));
> }
> ```
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.
- 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
>
>
