Repository: sentry Updated Branches: refs/heads/master 36cb81a8e -> 8a99bd22c
SENTRY-1252: grantServerPrivilege and revokeServerPrivilege should treat "*" and "ALL" as synonyms when action is not explicitly specified (Hao Hao, Reviewed by: Anne Yu) Change-Id: I62e54dbb793d32a07684f748be88022f21faccde Project: http://git-wip-us.apache.org/repos/asf/sentry/repo Commit: http://git-wip-us.apache.org/repos/asf/sentry/commit/8a99bd22 Tree: http://git-wip-us.apache.org/repos/asf/sentry/tree/8a99bd22 Diff: http://git-wip-us.apache.org/repos/asf/sentry/diff/8a99bd22 Branch: refs/heads/master Commit: 8a99bd22cb934c2ff9fe85181a9d3fadf844083f Parents: 36cb81a Author: hahao <[email protected]> Authored: Mon May 16 19:03:35 2016 -0700 Committer: hahao <[email protected]> Committed: Mon May 16 19:03:35 2016 -0700 ---------------------------------------------------------------------- .../SentryPolicyServiceClientDefaultImpl.java | 36 ++++++++++++++++++-- .../thrift/TestSentryServiceIntegration.java | 25 ++++++++++++++ 2 files changed, 59 insertions(+), 2 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/sentry/blob/8a99bd22/sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/SentryPolicyServiceClientDefaultImpl.java ---------------------------------------------------------------------- diff --git a/sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/SentryPolicyServiceClientDefaultImpl.java b/sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/SentryPolicyServiceClientDefaultImpl.java index b457387..b43f88b 100644 --- a/sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/SentryPolicyServiceClientDefaultImpl.java +++ b/sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/SentryPolicyServiceClientDefaultImpl.java @@ -338,6 +338,13 @@ public class SentryPolicyServiceClientDefaultImpl implements SentryPolicyService public synchronized void grantServerPrivilege(String requestorUserName, String roleName, String server, String action) throws SentryUserException { + + // "ALL" and "*" should be synonyms for action and need to be unified with grantServerPrivilege without + // action explicitly specified. + if (AccessConstants.ACTION_ALL.equalsIgnoreCase(action) || AccessConstants.ALL.equals(action)) { + action = AccessConstants.ALL; + } + grantPrivilege(requestorUserName, roleName, PrivilegeScope.SERVER, server, null, null, null, null, action); } @@ -356,6 +363,13 @@ public class SentryPolicyServiceClientDefaultImpl implements SentryPolicyService public synchronized TSentryPrivilege grantServerPrivilege(String requestorUserName, String roleName, String server, String action, Boolean grantOption) throws SentryUserException { + + // "ALL" and "*" should be synonyms for action and need to be unified with grantServerPrivilege without + // action explicitly specified. + if (AccessConstants.ACTION_ALL.equalsIgnoreCase(action) || AccessConstants.ALL.equals(action)) { + action = AccessConstants.ALL; + } + return grantPrivilege(requestorUserName, roleName, PrivilegeScope.SERVER, server, null, null, null, null, action, grantOption); } @@ -524,6 +538,13 @@ public class SentryPolicyServiceClientDefaultImpl implements SentryPolicyService public synchronized void revokeServerPrivilege(String requestorUserName, String roleName, String server, String action) throws SentryUserException { + + // "ALL" and "*" should be synonyms for action and need to be unified with revokeServerPrivilege without + // action explicitly specified. + if (AccessConstants.ACTION_ALL.equalsIgnoreCase(action) || AccessConstants.ALL.equals(action)) { + action = AccessConstants.ALL; + } + revokePrivilege(requestorUserName, roleName, PrivilegeScope.SERVER, server, null, null, null, null, action); } @@ -531,10 +552,22 @@ public class SentryPolicyServiceClientDefaultImpl implements SentryPolicyService public synchronized void revokeServerPrivilege(String requestorUserName, String roleName, String server, String action, Boolean grantOption) throws SentryUserException { + + // "ALL" and "*" should be synonyms for action and need to be unified with revokeServerPrivilege without + // action explicitly specified. + if (AccessConstants.ACTION_ALL.equalsIgnoreCase(action) || AccessConstants.ALL.equals(action)) { + action = AccessConstants.ALL; + } + revokePrivilege(requestorUserName, roleName, PrivilegeScope.SERVER, server, null, null, null, null, action, grantOption); } + @Deprecated + /*** + * Should use revokeServerPrivilege(String requestorUserName, + * String roleName, String server, String action, Boolean grantOption) + */ public synchronized void revokeServerPrivilege(String requestorUserName, String roleName, String server, boolean grantOption) throws SentryUserException { @@ -987,8 +1020,7 @@ public class SentryPolicyServiceClientDefaultImpl implements SentryPolicyService Map<String, Map<String, Set<String>>> resultMap = Maps.newHashMap(); resultMap.put(PolicyFileConstants.USER_ROLES, tSentryMappingData.getUserRolesMap()); resultMap.put(PolicyFileConstants.GROUPS, tSentryMappingData.getGroupRolesMap()); - resultMap.put(PolicyFileConstants.ROLES, - convertRolePrivilegesMapForPolicyFile(tSentryMappingData.getRolePrivilegesMap())); + resultMap.put(PolicyFileConstants.ROLES, convertRolePrivilegesMapForPolicyFile(tSentryMappingData.getRolePrivilegesMap())); return resultMap; } catch (TException e) { throw new SentryUserException(THRIFT_EXCEPTION_MESSAGE, e); http://git-wip-us.apache.org/repos/asf/sentry/blob/8a99bd22/sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/thrift/TestSentryServiceIntegration.java ---------------------------------------------------------------------- diff --git a/sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/thrift/TestSentryServiceIntegration.java b/sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/thrift/TestSentryServiceIntegration.java index a076475..6186106 100644 --- a/sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/thrift/TestSentryServiceIntegration.java +++ b/sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/thrift/TestSentryServiceIntegration.java @@ -1074,4 +1074,29 @@ public class TestSentryServiceIntegration extends SentryServiceIntegrationBase { }}); } + + @Test + public void testGranRevokePrivilegeWithoutAction() throws Exception { + runTestAsSubject(new TestOperation(){ + @Override + public void runTestAsSubject() throws Exception { + String requestorUserName = ADMIN_USER; + String roleName1 = "admin_r1"; + Set<String> requestorUserGroupNames = Sets.newHashSet(ADMIN_GROUP); + setLocalGroupMapping(requestorUserName, requestorUserGroupNames); + writePolicyFile(); + + client.dropRoleIfExists(requestorUserName, roleName1); + client.createRole(requestorUserName, roleName1); + client.grantServerPrivilege(requestorUserName, roleName1, "server1", false); + + Set<TSentryPrivilege> listPrivs = client.listAllPrivilegesByRoleName(requestorUserName, roleName1); + assertTrue("Privilege should be all:", listPrivs.iterator().next().getAction().equals("*")); + + client.revokeServerPrivilege(requestorUserName, roleName1, "server1", "ALL", false); + listPrivs = client.listAllPrivilegesByRoleName(requestorUserName, roleName1); + assertTrue("Privilege not correctly revoked !!", listPrivs.size() == 0); + + }}); + } }
