Repository: incubator-sentry Updated Branches: refs/heads/master 4247d21fa -> 04c4041bf
SENTRY-145: Store needs to handle privilege normalization (Brock Noland via Prasad Mujumdar) Project: http://git-wip-us.apache.org/repos/asf/incubator-sentry/repo Commit: http://git-wip-us.apache.org/repos/asf/incubator-sentry/commit/04c4041b Tree: http://git-wip-us.apache.org/repos/asf/incubator-sentry/tree/04c4041b Diff: http://git-wip-us.apache.org/repos/asf/incubator-sentry/diff/04c4041b Branch: refs/heads/master Commit: 04c4041bf470a8657ce25b8e8759ee1823e25141 Parents: 4247d21 Author: Prasad Mujumdar <[email protected]> Authored: Wed Mar 26 17:21:58 2014 -0700 Committer: Prasad Mujumdar <[email protected]> Committed: Wed Mar 26 17:21:58 2014 -0700 ---------------------------------------------------------------------- .../db/service/persistent/SentryStore.java | 138 ++++++++++++++++++- .../thrift/SentryPolicyStoreProcessor.java | 80 +---------- .../db/service/persistent/TestSentryStore.java | 31 +++-- 3 files changed, 154 insertions(+), 95 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/incubator-sentry/blob/04c4041b/sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java ---------------------------------------------------------------------- diff --git a/sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java b/sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java index 9c678d5..aca1350 100644 --- a/sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java +++ b/sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java @@ -37,9 +37,11 @@ import javax.jdo.Transaction; import org.apache.commons.lang.StringUtils; import org.apache.hadoop.conf.Configuration; +import org.apache.sentry.core.model.db.AccessConstants; import org.apache.sentry.core.model.db.DBModelAuthorizable.AuthorizableType; import org.apache.sentry.provider.common.ProviderConstants; import org.apache.sentry.provider.db.SentryAlreadyExistsException; +import org.apache.sentry.provider.db.SentryInvalidInputException; import org.apache.sentry.provider.db.SentryNoSuchObjectException; import org.apache.sentry.provider.db.service.model.MSentryGroup; import org.apache.sentry.provider.db.service.model.MSentryPrivilege; @@ -49,6 +51,7 @@ import org.apache.sentry.provider.db.service.thrift.TSentryGroup; import org.apache.sentry.provider.db.service.thrift.TSentryPrivilege; import org.apache.sentry.provider.db.service.thrift.TSentryRole; import org.apache.sentry.service.thrift.ServiceConstants.ServerConfig; +import org.apache.sentry.service.thrift.ServiceConstants.PrivilegeScope; import com.google.common.annotations.VisibleForTesting; import com.google.common.base.Preconditions; @@ -199,7 +202,7 @@ public class SentryStore { //TODO: handle case where a) privilege already exists, b) role to privilege mapping already exists public CommitContext alterSentryRoleGrantPrivilege(String roleName, - TSentryPrivilege privilege) throws SentryNoSuchObjectException { + TSentryPrivilege privilege) throws SentryNoSuchObjectException, SentryInvalidInputException { boolean rollbackTransaction = true; PersistenceManager pm = null; roleName = roleName.trim().toLowerCase(); @@ -232,7 +235,7 @@ public class SentryStore { } public CommitContext alterSentryRoleRevokePrivilege(String roleName, - String privilegeName) throws SentryNoSuchObjectException { + TSentryPrivilege tPrivilege) throws SentryNoSuchObjectException, SentryInvalidInputException { boolean rollbackTransaction = true; PersistenceManager pm = null; try { @@ -249,16 +252,21 @@ public class SentryStore { query.setFilter("this.privilegeName == t"); query.declareParameters("java.lang.String t"); query.setUnique(true); + String privilegeName = constructPrivilegeName(tPrivilege); MSentryPrivilege mPrivilege = (MSentryPrivilege) query.execute(privilegeName); if (mPrivilege == null) { - throw new SentryNoSuchObjectException("Privilege: " + privilegeName); + revokePartialPrivilege(pm, mRole, tPrivilege); + CommitContext commit = commitUpdateTransaction(pm); + rollbackTransaction = false; + return commit; } else { - // remove privilege and role objects from each other's set. needed by datanucleus to model - // m:n relationships correctly through a join table. + // remove privilege and role objects from each other's set. needed by + // datanucleus to model m:n relationships correctly through a join table. mRole.removePrivilege(mPrivilege); CommitContext commit = commitUpdateTransaction(pm); rollbackTransaction = false; return commit; + } } } finally { @@ -268,6 +276,120 @@ public class SentryStore { } } + /** + * Our privilege model at present only allows ALL on server and databases. + * However, roles can be granted ALL, SELECT, and INSERT on tables. When + * a role has ALL and SELECT or INSERT are revoked, we need to remove the ALL + * privilege and add SELECT (INSERT was revoked) or INSERT (SELECT was revoked). + */ + private void revokePartialPrivilege(PersistenceManager pm, MSentryRole role, + TSentryPrivilege tPrivilege) + throws SentryNoSuchObjectException, SentryInvalidInputException { + // only perform partial revoke if INSERT/SELECT were the action + // and the privilege being revoked is on a table + String action = tPrivilege.getAction(); + if (AccessConstants.ALL.equalsIgnoreCase(action) || + StringUtils.isEmpty(tPrivilege.getDbName()) || StringUtils.isEmpty(tPrivilege.getTableName())) { + throw new SentryNoSuchObjectException("Unknown privilege: " + tPrivilege); + } + TSentryPrivilege tPrivilegeAll = new TSentryPrivilege(tPrivilege); + tPrivilegeAll.setAction(AccessConstants.ALL); + String allPrivilegeName = constructPrivilegeName(tPrivilegeAll); + Query query = pm.newQuery(MSentryPrivilege.class); + query.setFilter("this.privilegeName == t"); + query.declareParameters("java.lang.String t"); + query.setUnique(true); + MSentryPrivilege allPrivilege = (MSentryPrivilege) query.execute(allPrivilegeName); + if (allPrivilege == null) { + throw new SentryNoSuchObjectException("Unknown privilege: " + tPrivilege); + } + role.removePrivilege(allPrivilege); + if (AccessConstants.SELECT.equalsIgnoreCase(action)) { + tPrivilege.setAction(AccessConstants.INSERT); + } else if (AccessConstants.INSERT.equalsIgnoreCase(action)) { + tPrivilege.setAction(AccessConstants.SELECT); + } else { + throw new IllegalStateException("Unexpected action: " + action); + } + role.appendPrivilege(convertToMSentryPrivilege(tPrivilege)); + } + + //TODO:Validate privilege scope? + @VisibleForTesting + public static String constructPrivilegeName(TSentryPrivilege privilege) throws SentryInvalidInputException { + StringBuilder privilegeName = new StringBuilder(); + String serverName = privilege.getServerName(); + String dbName = privilege.getDbName(); + String tableName = privilege.getTableName(); + String uri = privilege.getURI(); + String action = privilege.getAction(); + PrivilegeScope scope; + + if (serverName == null) { + throw new SentryInvalidInputException("Server name is null"); + } + + if (AccessConstants.SELECT.equalsIgnoreCase(action) || + AccessConstants.INSERT.equalsIgnoreCase(action)) { + if (Strings.nullToEmpty(tableName).trim().isEmpty()) { + throw new SentryInvalidInputException("Table name can't be null for SELECT/INSERT privilege"); + } + } + + // Validate privilege scope + try { + scope = Enum.valueOf(PrivilegeScope.class, privilege.getPrivilegeScope()); + } catch (IllegalArgumentException e) { + throw new SentryInvalidInputException("Invalid Privilege scope: " + + privilege.getPrivilegeScope()); + } + if (PrivilegeScope.SERVER.equals(scope)) { + if (StringUtils.isNotEmpty(dbName) || StringUtils.isNotEmpty(tableName)) { + throw new SentryInvalidInputException("DB and TABLE names should not be " + + "set for SERVER scope"); + } + } else if (PrivilegeScope.DATABASE.equals(scope)) { + if (StringUtils.isEmpty(dbName)) { + throw new SentryInvalidInputException("DB name not set for DB scope"); + } + if (StringUtils.isNotEmpty(tableName)) { + StringUtils.isNotEmpty("TABLE names should not be set for DB scope"); + } + } else if (PrivilegeScope.TABLE.equals(scope)) { + if (StringUtils.isEmpty(dbName) || StringUtils.isEmpty(tableName)) { + throw new SentryInvalidInputException("TABLE or DB name not set for TABLE scope"); + } + } else if (PrivilegeScope.URI.equals(scope)){ + if (StringUtils.isEmpty(uri)) { + throw new SentryInvalidInputException("URI path not set for URI scope"); + } + if (StringUtils.isNotEmpty(tableName)) { + throw new SentryInvalidInputException("TABLE should not be set for URI scope"); + } + } else { + throw new SentryInvalidInputException("Unsupported operation scope: " + scope); + } + + if (uri == null || uri.equals("")) { + privilegeName.append(serverName); + privilegeName.append("+"); + privilegeName.append(dbName); + + if (tableName != null && !tableName.equals("")) { + privilegeName.append("+"); + privilegeName.append(tableName); + } + privilegeName.append("+"); + privilegeName.append(action); + } else { + privilegeName.append(serverName); + privilegeName.append("+"); + privilegeName.append(uri); + } + return privilegeName.toString(); + } + + public CommitContext dropSentryRole(String roleName) throws SentryNoSuchObjectException { boolean rollbackTransaction = true; @@ -534,8 +656,10 @@ public class SentryStore { /** * Converts thrift object to model object. Additionally does normalization * such as trimming whitespace and setting appropriate case. + * @throws SentryInvalidInputException */ - private MSentryPrivilege convertToMSentryPrivilege(TSentryPrivilege privilege) { + private MSentryPrivilege convertToMSentryPrivilege(TSentryPrivilege privilege) + throws SentryInvalidInputException { MSentryPrivilege mSentryPrivilege = new MSentryPrivilege(); mSentryPrivilege.setServerName(safeTrim(privilege.getServerName())); mSentryPrivilege.setDbName(safeTrim(privilege.getDbName())); @@ -545,7 +669,7 @@ public class SentryStore { mSentryPrivilege.setCreateTime(privilege.getCreateTime()); mSentryPrivilege.setGrantorPrincipal(safeTrim(privilege.getGrantorPrincipal())); mSentryPrivilege.setURI(safeTrim(privilege.getURI())); - mSentryPrivilege.setPrivilegeName(safeTrim(privilege.getPrivilegeName())); + mSentryPrivilege.setPrivilegeName(constructPrivilegeName(privilege)); return mSentryPrivilege; } private String safeTrim(String s) { http://git-wip-us.apache.org/repos/asf/incubator-sentry/blob/04c4041b/sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/SentryPolicyStoreProcessor.java ---------------------------------------------------------------------- diff --git a/sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/SentryPolicyStoreProcessor.java b/sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/SentryPolicyStoreProcessor.java index 9793ab4..6c52fa4 100644 --- a/sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/SentryPolicyStoreProcessor.java +++ b/sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/SentryPolicyStoreProcessor.java @@ -32,7 +32,6 @@ import org.apache.sentry.provider.db.SentryNoSuchObjectException; import org.apache.sentry.provider.db.service.persistent.CommitContext; import org.apache.sentry.provider.db.service.persistent.SentryStore; import org.apache.sentry.provider.db.service.thrift.PolicyStoreConstants.PolicyStoreServerConfig; -import org.apache.sentry.service.thrift.ServiceConstants.PrivilegeScope; import org.apache.sentry.service.thrift.Status; import org.apache.sentry.service.thrift.TSentryResponseStatus; import org.apache.sentry.service.thrift.ServiceConstants.ServerConfig; @@ -108,79 +107,6 @@ public class SentryPolicyStoreProcessor implements SentryPolicyService.Iface { return handlers; } - @VisibleForTesting - public static String constructPrivilegeName(TSentryPrivilege privilege) throws SentryInvalidInputException { - StringBuilder privilegeName = new StringBuilder(); - String serverName = privilege.getServerName(); - String dbName = privilege.getDbName(); - String tableName = privilege.getTableName(); - String uri = privilege.getURI(); - String action = privilege.getAction(); - PrivilegeScope scope; - - if (serverName == null) { - throw new SentryInvalidInputException("Server name is null"); - } - - if ("SELECT".equalsIgnoreCase(action) || "INSERT".equalsIgnoreCase(action)) { - if (tableName == null || tableName.equals("")) { - throw new SentryInvalidInputException("Table name can't be null for SELECT/INSERT privilege"); - } - } - - // Validate privilege scope - try { - scope = Enum.valueOf(PrivilegeScope.class, privilege.getPrivilegeScope()); - } catch (IllegalArgumentException e) { - throw new SentryInvalidInputException("Invalid Privilege scope: " + - privilege.getPrivilegeScope()); - } - if (PrivilegeScope.SERVER.equals(scope)) { - if (StringUtils.isNotEmpty(dbName) || StringUtils.isNotEmpty(tableName)) { - throw new SentryInvalidInputException("DB and TABLE names should not be " - + "set for SERVER scope"); - } - } else if (PrivilegeScope.DATABASE.equals(scope)) { - if (StringUtils.isEmpty(dbName)) { - throw new SentryInvalidInputException("DB name not set for DB scope"); - } - if (StringUtils.isNotEmpty(tableName)) { - StringUtils.isNotEmpty("TABLE names should not be set for DB scope"); - } - } else if (PrivilegeScope.TABLE.equals(scope)) { - if (StringUtils.isEmpty(dbName) || StringUtils.isEmpty(tableName)) { - throw new SentryInvalidInputException("TABLE or DB name not set for TABLE scope"); - } - } else if (PrivilegeScope.URI.equals(scope)){ - if (StringUtils.isEmpty(uri)) { - throw new SentryInvalidInputException("URI path not set for URI scope"); - } - if (StringUtils.isNotEmpty(tableName)) { - throw new SentryInvalidInputException("TABLE should not be set for URI scope"); - } - } else { - throw new SentryInvalidInputException("Unsupported operation scope: " + scope); - } - - if (uri == null || uri.equals("")) { - privilegeName.append(serverName); - privilegeName.append("+"); - privilegeName.append(dbName); - - if (tableName != null && !tableName.equals("")) { - privilegeName.append("+"); - privilegeName.append(tableName); - } - privilegeName.append("+"); - privilegeName.append(action); - } else { - privilegeName.append(serverName); - privilegeName.append("+"); - privilegeName.append(uri); - } - return privilegeName.toString(); - } - private static Set<String> toTrimedLower(Set<String> s) { Set<String> result = Sets.newHashSet(); for (String v : s) { @@ -233,8 +159,6 @@ public class SentryPolicyStoreProcessor implements SentryPolicyService.Iface { TAlterSentryRoleGrantPrivilegeResponse response = new TAlterSentryRoleGrantPrivilegeResponse(); try { authorize(request.getRequestorUserName(), request.getRequestorGroupNames()); - String privilegeName = constructPrivilegeName(request.getPrivilege()); - request.getPrivilege().setPrivilegeName(privilegeName); CommitContext commitContext = sentryStore.alterSentryRoleGrantPrivilege(request.getRoleName(), request.getPrivilege()); response.setStatus(Status.OK()); @@ -266,10 +190,8 @@ public class SentryPolicyStoreProcessor implements SentryPolicyService.Iface { TAlterSentryRoleRevokePrivilegeResponse response = new TAlterSentryRoleRevokePrivilegeResponse(); try { authorize(request.getRequestorUserName(), request.getRequestorGroupNames()); - String privilegeName = constructPrivilegeName(request.getPrivilege()); - request.getPrivilege().setPrivilegeName(privilegeName); CommitContext commitContext = sentryStore.alterSentryRoleRevokePrivilege(request.getRoleName(), - request.getPrivilege().getPrivilegeName()); + request.getPrivilege()); response.setStatus(Status.OK()); notificationHandlerInvoker.alter_sentry_role_revoke_privilege(commitContext, request, response); http://git-wip-us.apache.org/repos/asf/incubator-sentry/blob/04c4041b/sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/persistent/TestSentryStore.java ---------------------------------------------------------------------- diff --git a/sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/persistent/TestSentryStore.java b/sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/persistent/TestSentryStore.java index f500c2d..40b9460 100644 --- a/sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/persistent/TestSentryStore.java +++ b/sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/persistent/TestSentryStore.java @@ -28,11 +28,11 @@ import java.util.Set; import org.apache.commons.io.FileUtils; import org.apache.hadoop.conf.Configuration; +import org.apache.sentry.core.model.db.AccessConstants; import org.apache.sentry.provider.db.SentryAlreadyExistsException; import org.apache.sentry.provider.db.SentryNoSuchObjectException; import org.apache.sentry.provider.db.service.model.MSentryPrivilege; import org.apache.sentry.provider.db.service.model.MSentryRole; -import org.apache.sentry.provider.db.service.thrift.SentryPolicyStoreProcessor; import org.apache.sentry.provider.db.service.thrift.TSentryActiveRoleSet; import org.apache.sentry.provider.db.service.thrift.TSentryGroup; import org.apache.sentry.provider.db.service.thrift.TSentryPrivilege; @@ -123,24 +123,37 @@ public class TestSentryStore { public void testGrantRevokePrivilege() throws Exception { String roleName = "test-privilege"; String grantor = "g1"; + String server = "server1"; + String db = "db1"; + String table = "tbl1"; long seqId = sentryStore.createSentryRole(roleName, grantor).getSequenceId(); TSentryPrivilege privilege = new TSentryPrivilege(); privilege.setPrivilegeScope("TABLE"); - privilege.setServerName("server1"); - privilege.setDbName("db1"); - privilege.setTableName("tbl1"); - privilege.setAction("SELECT"); + privilege.setServerName(server); + privilege.setDbName(db); + privilege.setTableName(table); + privilege.setAction(AccessConstants.ALL); privilege.setGrantorPrincipal(grantor); privilege.setCreateTime(System.currentTimeMillis()); - privilege.setPrivilegeName(SentryPolicyStoreProcessor.constructPrivilegeName(privilege)); + privilege.setPrivilegeName(SentryStore.constructPrivilegeName(privilege)); assertEquals(seqId + 1, sentryStore.alterSentryRoleGrantPrivilege(roleName, privilege) .getSequenceId()); MSentryRole role = sentryStore.getMSentryRoleByName(roleName); Set<MSentryPrivilege> privileges = role.getPrivileges(); assertEquals(privileges.toString(), 1, privileges.size()); assertEquals(privilege.getPrivilegeName(), Iterables.get(privileges, 0).getPrivilegeName()); - assertEquals(seqId + 2, sentryStore.alterSentryRoleRevokePrivilege(roleName, privilege.getPrivilegeName()) + privilege.setAction(AccessConstants.SELECT); + assertEquals(seqId + 2, sentryStore.alterSentryRoleRevokePrivilege(roleName, privilege) .getSequenceId()); + // after having ALL and revoking SELECT, we should have INSERT + role = sentryStore.getMSentryRoleByName(roleName); + privileges = role.getPrivileges(); + assertEquals(privileges.toString(), 1, privileges.size()); + MSentryPrivilege mPrivilege = Iterables.get(privileges, 0); + assertEquals(server, mPrivilege.getServerName()); + assertEquals(db, mPrivilege.getDbName()); + assertEquals(table, mPrivilege.getTableName()); + assertEquals(AccessConstants.INSERT, mPrivilege.getAction()); } @Test @@ -158,7 +171,7 @@ public class TestSentryStore { privilege1.setAction("SELECT"); privilege1.setGrantorPrincipal(grantor); privilege1.setCreateTime(System.currentTimeMillis()); - privilege1.setPrivilegeName(SentryPolicyStoreProcessor.constructPrivilegeName(privilege1)); + privilege1.setPrivilegeName(SentryStore.constructPrivilegeName(privilege1)); assertEquals(seqId + 2, sentryStore.alterSentryRoleGrantPrivilege(roleName1, privilege1) .getSequenceId()); assertEquals(seqId + 3, sentryStore.alterSentryRoleGrantPrivilege(roleName2, privilege1) @@ -168,7 +181,7 @@ public class TestSentryStore { privilege2.setServerName("server1"); privilege2.setGrantorPrincipal(grantor); privilege2.setCreateTime(System.currentTimeMillis()); - privilege2.setPrivilegeName(SentryPolicyStoreProcessor.constructPrivilegeName(privilege2)); + privilege2.setPrivilegeName(SentryStore.constructPrivilegeName(privilege2)); assertEquals(seqId + 4, sentryStore.alterSentryRoleGrantPrivilege(roleName2, privilege2) .getSequenceId()); Set<TSentryGroup> groups = Sets.newHashSet();
