Repository: sentry Updated Branches: refs/heads/master fb7bb7bc6 -> c30d56111
SENTRY-2255: alter table set owner command can be executed only by user with proper privilege (Na Li, reviewed by Sergio Pena, Kalyan Kumar Kalvagadda) Project: http://git-wip-us.apache.org/repos/asf/sentry/repo Commit: http://git-wip-us.apache.org/repos/asf/sentry/commit/c30d5611 Tree: http://git-wip-us.apache.org/repos/asf/sentry/tree/c30d5611 Diff: http://git-wip-us.apache.org/repos/asf/sentry/diff/c30d5611 Branch: refs/heads/master Commit: c30d5611194d852df76ea34d3b6c3dd3676ccf34 Parents: fb7bb7b Author: lina.li <[email protected]> Authored: Wed Aug 8 15:27:42 2018 -0500 Committer: lina.li <[email protected]> Committed: Wed Aug 8 15:27:42 2018 -0500 ---------------------------------------------------------------------- .../binding/hive/authz/HiveAuthzBinding.java | 6 +- .../binding/hive/authz/HiveAuthzPrivileges.java | 24 +- .../hive/authz/HiveAuthzPrivilegesMap.java | 12 +- .../sentry/policy/common/CommonPrivilege.java | 31 +- .../provider/common/AuthorizationProvider.java | 16 ++ .../common/NoAuthorizationProvider.java | 6 + .../common/ResourceAuthorizationProvider.java | 26 +- .../db/service/persistent/SentryStore.java | 8 + .../e2e/dbprovider/TestOwnerPrivileges.java | 282 +++++++++++++++++-- .../tests/e2e/hdfs/TestHDFSIntegrationBase.java | 1 + 10 files changed, 380 insertions(+), 32 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/sentry/blob/c30d5611/sentry-binding/sentry-binding-hive-common/src/main/java/org/apache/sentry/binding/hive/authz/HiveAuthzBinding.java ---------------------------------------------------------------------- diff --git a/sentry-binding/sentry-binding-hive-common/src/main/java/org/apache/sentry/binding/hive/authz/HiveAuthzBinding.java b/sentry-binding/sentry-binding-hive-common/src/main/java/org/apache/sentry/binding/hive/authz/HiveAuthzBinding.java index f1cbbb6..6a1556f 100644 --- a/sentry-binding/sentry-binding-hive-common/src/main/java/org/apache/sentry/binding/hive/authz/HiveAuthzBinding.java +++ b/sentry-binding/sentry-binding-hive-common/src/main/java/org/apache/sentry/binding/hive/authz/HiveAuthzBinding.java @@ -338,7 +338,8 @@ public class HiveAuthzBinding { for (List<DBModelAuthorizable> inputHierarchy : inputHierarchyList) { if (getAuthzType(inputHierarchy).equals(key)) { found = true; - if (!authProvider.hasAccess(subject, inputHierarchy, entry.getValue(), activeRoleSet)) { + if (!authProvider.hasAccess(subject, inputHierarchy, entry.getValue(), + stmtAuthPrivileges.getGrantOption(), activeRoleSet)) { throw new AuthorizationException("User " + subject.getName() + " does not have privileges for " + hiveOp.name()); } @@ -362,7 +363,8 @@ public class HiveAuthzBinding { for (List<DBModelAuthorizable> outputHierarchy : outputHierarchyList) { if (getAuthzType(outputHierarchy).equals(key)) { found = true; - if (!authProvider.hasAccess(subject, outputHierarchy, entry.getValue(), activeRoleSet)) { + if (!authProvider.hasAccess(subject, outputHierarchy, entry.getValue(), + stmtAuthPrivileges.getGrantOption(), activeRoleSet)) { throw new AuthorizationException("User " + subject.getName() + " does not have privileges for " + hiveOp.name()); } http://git-wip-us.apache.org/repos/asf/sentry/blob/c30d5611/sentry-binding/sentry-binding-hive-common/src/main/java/org/apache/sentry/binding/hive/authz/HiveAuthzPrivileges.java ---------------------------------------------------------------------- diff --git a/sentry-binding/sentry-binding-hive-common/src/main/java/org/apache/sentry/binding/hive/authz/HiveAuthzPrivileges.java b/sentry-binding/sentry-binding-hive-common/src/main/java/org/apache/sentry/binding/hive/authz/HiveAuthzPrivileges.java index f164b30..c37ce64 100644 --- a/sentry-binding/sentry-binding-hive-common/src/main/java/org/apache/sentry/binding/hive/authz/HiveAuthzPrivileges.java +++ b/sentry-binding/sentry-binding-hive-common/src/main/java/org/apache/sentry/binding/hive/authz/HiveAuthzPrivileges.java @@ -68,6 +68,7 @@ public class HiveAuthzPrivileges { new HashMap<AuthorizableType,EnumSet<DBModelAction>>(); private HiveOperationType operationType; private HiveOperationScope operationScope; + private boolean grantOption = false; public AuthzPrivilegeBuilder addInputObjectPriviledge(AuthorizableType inputObjectType, EnumSet<DBModelAction> inputPrivilege) { inputPrivileges.put(inputObjectType, inputPrivilege); @@ -94,6 +95,11 @@ public class HiveAuthzPrivileges { return this; } + public AuthzPrivilegeBuilder setGrantOption(boolean requireGrantOption) { + this.grantOption = requireGrantOption; + return this; + } + public HiveAuthzPrivileges build() { if (operationScope.equals(HiveOperationScope.UNKNOWN)) { throw new UnsupportedOperationException("Operation scope is not set"); @@ -103,7 +109,8 @@ public class HiveAuthzPrivileges { throw new UnsupportedOperationException("Operation scope is not set"); } - return new HiveAuthzPrivileges(inputPrivileges, outputPrivileges, operationType, operationScope); + return new HiveAuthzPrivileges(inputPrivileges, outputPrivileges, operationType, + operationScope, grantOption); } } @@ -113,14 +120,22 @@ public class HiveAuthzPrivileges { new HashMap<AuthorizableType,EnumSet<DBModelAction>>(); private final HiveOperationType operationType; private final HiveOperationScope operationScope; + private final boolean grantOption; protected HiveAuthzPrivileges(Map<AuthorizableType,EnumSet<DBModelAction>> inputPrivileges, Map<AuthorizableType,EnumSet<DBModelAction>> outputPrivileges, HiveOperationType operationType, HiveOperationScope operationScope) { + this(inputPrivileges, outputPrivileges, operationType, operationScope, false); + } + + protected HiveAuthzPrivileges(Map<AuthorizableType,EnumSet<DBModelAction>> inputPrivileges, + Map<AuthorizableType,EnumSet<DBModelAction>> outputPrivileges, HiveOperationType operationType, + HiveOperationScope operationScope, boolean requireGrantOption) { this.inputPrivileges.putAll(inputPrivileges); this.outputPrivileges.putAll(outputPrivileges); this.operationScope = operationScope; this.operationType = operationType; + this.grantOption = requireGrantOption; } /** @@ -138,6 +153,13 @@ public class HiveAuthzPrivileges { } /** + * @return the grantOption + */ + public boolean getGrantOption() { + return grantOption; + } + + /** * @return the operationType */ public HiveOperationType getOperationType() { http://git-wip-us.apache.org/repos/asf/sentry/blob/c30d5611/sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/authz/HiveAuthzPrivilegesMap.java ---------------------------------------------------------------------- diff --git a/sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/authz/HiveAuthzPrivilegesMap.java b/sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/authz/HiveAuthzPrivilegesMap.java index 9350af0..d976512 100644 --- a/sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/authz/HiveAuthzPrivilegesMap.java +++ b/sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/authz/HiveAuthzPrivilegesMap.java @@ -112,6 +112,15 @@ public class HiveAuthzPrivilegesMap { setOperationType(HiveOperationType.DDL). build(); + /* TODO: once HIVE-18762 is in the hiver version integrated with Sentry, uncomment this block + HiveAuthzPrivileges alterTableSetOwnerPrivilege = new HiveAuthzPrivileges.AuthzPrivilegeBuilder(). + addInputObjectPriviledge(AuthorizableType.Table, EnumSet.of(DBModelAction.ALL)). + setOperationScope(HiveOperationScope.TABLE). + setOperationType(HiveOperationType.DDL). + setGrantOption(true). + build(); + */ + // input required privilege from Hive: SELECT on column level and DELETE on table level // output required privilege from Hive: INSERT on table level // Sentry makes it more restrictive, and requires ALL at input, INSERT and ALTER at output @@ -270,7 +279,8 @@ public class HiveAuthzPrivilegesMap { hiveAuthzStmtPrivMap.put(HiveOperation.ALTERVIEW_PROPERTIES, alterTablePrivilege); hiveAuthzStmtPrivMap.put(HiveOperation.ALTERVIEW_AS, createViewPrivilege); hiveAuthzStmtPrivMap.put(HiveOperation.ALTERVIEW_RENAME, alterTableRenamePrivilege); - + // TODO: once HIVE-18762 is in the hiver version integrated with Sentry, uncomment next line + // hiveAuthzStmtPrivMap.put(HiveOperation.ALTERTABLE_OWNER, alterTableSetOwnerPrivilege); hiveAuthzStmtPrivMap.put(HiveOperation.ALTERTABLE_DROPPARTS, dropPartitionPrivilege); hiveAuthzStmtPrivMap.put(HiveOperation.ALTERTABLE_ADDPARTS, addPartitionPrivilege); http://git-wip-us.apache.org/repos/asf/sentry/blob/c30d5611/sentry-policy/sentry-policy-common/src/main/java/org/apache/sentry/policy/common/CommonPrivilege.java ---------------------------------------------------------------------- diff --git a/sentry-policy/sentry-policy-common/src/main/java/org/apache/sentry/policy/common/CommonPrivilege.java b/sentry-policy/sentry-policy-common/src/main/java/org/apache/sentry/policy/common/CommonPrivilege.java index ab55609..61d442a 100644 --- a/sentry-policy/sentry-policy-common/src/main/java/org/apache/sentry/policy/common/CommonPrivilege.java +++ b/sentry-policy/sentry-policy-common/src/main/java/org/apache/sentry/policy/common/CommonPrivilege.java @@ -35,6 +35,7 @@ import java.util.List; public class CommonPrivilege implements Privilege { private ImmutableList<KeyValue> parts; + private boolean grantOption = false; public CommonPrivilege(String privilegeStr) { privilegeStr = Strings.nullToEmpty(privilegeStr).trim(); @@ -52,6 +53,14 @@ public class CommonPrivilege implements Privilege { if (parts.isEmpty()) { throw new AssertionError("Should never occur: " + privilegeStr); } + + // check if grant option is present + KeyValue lastPart = parts.get(parts.size() - 1); + if (lastPart.getKey().equalsIgnoreCase(SentryConstants.GRANT_OPTION)) { + grantOption = lastPart.getValue().equalsIgnoreCase("true"); + parts.remove(parts.size() - 1); + } + this.parts = ImmutableList.copyOf(parts); } @@ -62,7 +71,13 @@ public class CommonPrivilege implements Privilege { return false; } - List<KeyValue> otherParts = ((CommonPrivilege) privilege).getParts(); + CommonPrivilege requiredPrivilege = (CommonPrivilege) privilege; + if ((requiredPrivilege.grantOption == true) && (this.grantOption == false)) { + // the required privilege wp needs grant option, but this privilege does not have grant option + return false; + } + + List<KeyValue> otherParts = requiredPrivilege.getParts(); if(parts.equals(otherParts)) { return true; } @@ -106,7 +121,7 @@ public class CommonPrivilege implements Privilege { // all of the other parts are wildcards for (; index < parts.size(); index++) { KeyValue part = parts.get(index); - if (!SentryConstants.PRIVILEGE_WILDCARD_VALUE.equals(part.getValue())) { + if (!isPrivilegeActionAll(part, model.getBitFieldActionFactory())) { return false; } } @@ -114,6 +129,18 @@ public class CommonPrivilege implements Privilege { return true; } + /** + * Check if the action part in a privilege is ALL. Owner privilege is + * treated as ALL for authorization + * @param actionPart it must be the action of a privilege + * @return true if the action is ALL; false otherwise + */ + private boolean isPrivilegeActionAll(KeyValue actionPart, + BitFieldActionFactory bitFieldActionFactory) { + return impliesAction(actionPart.getValue(), SentryConstants.PRIVILEGE_WILDCARD_VALUE, + bitFieldActionFactory); + } + @Override public List<KeyValue> getAuthorizable() { List<KeyValue> authorizable = new ArrayList<>(); http://git-wip-us.apache.org/repos/asf/sentry/blob/c30d5611/sentry-provider/sentry-provider-common/src/main/java/org/apache/sentry/provider/common/AuthorizationProvider.java ---------------------------------------------------------------------- diff --git a/sentry-provider/sentry-provider-common/src/main/java/org/apache/sentry/provider/common/AuthorizationProvider.java b/sentry-provider/sentry-provider-common/src/main/java/org/apache/sentry/provider/common/AuthorizationProvider.java index 73fcda8..aecfe5b 100644 --- a/sentry-provider/sentry-provider-common/src/main/java/org/apache/sentry/provider/common/AuthorizationProvider.java +++ b/sentry-provider/sentry-provider-common/src/main/java/org/apache/sentry/provider/common/AuthorizationProvider.java @@ -53,6 +53,22 @@ public interface AuthorizationProvider { Set<? extends Action> actions, ActiveRoleSet roleSet); /*** + * Returns validate subject privileges on given Authorizable object + * + * @param subject: UserID to validate privileges + * @param authorizableHierarchy : List of object according to namespace hierarchy. + * eg. Server->Db->Table or Server->Function + * The privileges will be validated from the higher to lower scope + * @param actions : Privileges to validate + * @param requireGrantOption: true: require grant option of matching privilege; false: otherwise + * @param roleSet : Roles which should be used when obtaining privileges + * @return + * True if the subject is authorized to perform requested action on the given object + */ + boolean hasAccess(Subject subject, List<? extends Authorizable> authorizableHierarchy, + Set<? extends Action> actions, boolean requireGrantOption, ActiveRoleSet roleSet); + + /*** * Get the GroupMappingService used by the AuthorizationProvider * * @return GroupMappingService used by the AuthorizationProvider http://git-wip-us.apache.org/repos/asf/sentry/blob/c30d5611/sentry-provider/sentry-provider-common/src/main/java/org/apache/sentry/provider/common/NoAuthorizationProvider.java ---------------------------------------------------------------------- diff --git a/sentry-provider/sentry-provider-common/src/main/java/org/apache/sentry/provider/common/NoAuthorizationProvider.java b/sentry-provider/sentry-provider-common/src/main/java/org/apache/sentry/provider/common/NoAuthorizationProvider.java index be0830d..61400ca 100644 --- a/sentry-provider/sentry-provider-common/src/main/java/org/apache/sentry/provider/common/NoAuthorizationProvider.java +++ b/sentry-provider/sentry-provider-common/src/main/java/org/apache/sentry/provider/common/NoAuthorizationProvider.java @@ -38,6 +38,12 @@ public class NoAuthorizationProvider implements AuthorizationProvider { } @Override + public boolean hasAccess(Subject subject, List<? extends Authorizable> authorizableHierarchy, + Set<? extends Action> actions, boolean grantOption, ActiveRoleSet roleSet) { + return false; + } + + @Override public GroupMappingService getGroupMapping() { return noGroupMappingService; } http://git-wip-us.apache.org/repos/asf/sentry/blob/c30d5611/sentry-provider/sentry-provider-common/src/main/java/org/apache/sentry/provider/common/ResourceAuthorizationProvider.java ---------------------------------------------------------------------- diff --git a/sentry-provider/sentry-provider-common/src/main/java/org/apache/sentry/provider/common/ResourceAuthorizationProvider.java b/sentry-provider/sentry-provider-common/src/main/java/org/apache/sentry/provider/common/ResourceAuthorizationProvider.java index a9b98f3..1e1aa63 100644 --- a/sentry-provider/sentry-provider-common/src/main/java/org/apache/sentry/provider/common/ResourceAuthorizationProvider.java +++ b/sentry-provider/sentry-provider-common/src/main/java/org/apache/sentry/provider/common/ResourceAuthorizationProvider.java @@ -18,6 +18,7 @@ package org.apache.sentry.provider.common; import static org.apache.sentry.core.common.utils.SentryConstants.AUTHORIZABLE_JOINER; import static org.apache.sentry.core.common.utils.SentryConstants.AUTHORIZABLE_SPLITTER; +import static org.apache.sentry.core.common.utils.SentryConstants.GRANT_OPTION; import static org.apache.sentry.core.common.utils.SentryConstants.KV_JOINER; import static org.apache.sentry.core.common.utils.SentryConstants.PRIVILEGE_NAME; @@ -83,6 +84,21 @@ public abstract class ResourceAuthorizationProvider implements AuthorizationProv @Override public boolean hasAccess(Subject subject, List<? extends Authorizable> authorizableHierarchy, Set<? extends Action> actions, ActiveRoleSet roleSet) { + return hasAccess(subject, authorizableHierarchy, actions, false, roleSet); + } + + /*** + * @param subject: UserID to validate privileges + * @param authorizableHierarchy : List of object according to namespace hierarchy. + * eg. Server->Db->Table or Server->Function + * The privileges will be validated from the higher to lower scope + * @param actions : Privileges to validate + * @return + * True if the subject is authorized to perform requested action on the given object + */ + @Override + public boolean hasAccess(Subject subject, List<? extends Authorizable> authorizableHierarchy, + Set<? extends Action> actions, boolean requireGrantOption, ActiveRoleSet roleSet) { if(LOGGER.isDebugEnabled()) { LOGGER.debug("Authorization Request for " + subject + " " + authorizableHierarchy + " and " + actions); @@ -94,13 +110,13 @@ public abstract class ResourceAuthorizationProvider implements AuthorizationProv Preconditions.checkArgument(!actions.isEmpty(), "Actions cannot be empty"); Preconditions.checkNotNull(roleSet, "ActiveRoleSet cannot be null"); boolean hasAccess = false; - hasAccess = doHasAccess(subject, authorizableHierarchy, actions, roleSet); + hasAccess = doHasAccess(subject, authorizableHierarchy, actions, requireGrantOption, roleSet); return hasAccess; } private boolean doHasAccess(Subject subject, List<? extends Authorizable> authorizables, Set<? extends Action> actions, - ActiveRoleSet roleSet) { + boolean requireGrantOption, ActiveRoleSet roleSet) { Set<String> groups; try { groups = getGroups(subject); @@ -113,7 +129,7 @@ public abstract class ResourceAuthorizationProvider implements AuthorizationProv for (Authorizable authorizable : authorizables) { hierarchy.add(KV_JOINER.join(authorizable.getTypeName(), authorizable.getName())); } - List<String> requestPrivileges = buildPermissions(authorizables, actions); + List<String> requestPrivileges = buildPermissions(authorizables, actions, requireGrantOption); Iterable<Privilege> privileges = getPrivileges(groups, users, roleSet, authorizables.toArray(new Authorizable[0])); lastFailedPrivileges.get().clear(); @@ -213,7 +229,7 @@ public abstract class ResourceAuthorizationProvider implements AuthorizationProv } private List<String> buildPermissions(List<? extends Authorizable> authorizables, - Set<? extends Action> actions) { + Set<? extends Action> actions, boolean requireGrantOption) { List<String> hierarchy = new ArrayList<String>(); List<String> requestedPermissions = new ArrayList<String>(); @@ -225,6 +241,8 @@ public abstract class ResourceAuthorizationProvider implements AuthorizationProv String requestPermission = AUTHORIZABLE_JOINER.join(hierarchy); requestPermission = AUTHORIZABLE_JOINER.join(requestPermission, KV_JOINER.join(PRIVILEGE_NAME, action.getValue())); + requestPermission = AUTHORIZABLE_JOINER.join(requestPermission, + KV_JOINER.join(GRANT_OPTION, requireGrantOption)); requestedPermissions.add(requestPermission); } return requestedPermissions; http://git-wip-us.apache.org/repos/asf/sentry/blob/c30d5611/sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java ---------------------------------------------------------------------- diff --git a/sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java b/sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java index 621ce89..5644181 100644 --- a/sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java +++ b/sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java @@ -2419,6 +2419,14 @@ public class SentryStore implements SentryStoreInterface { .add(KV_JOINER.join(SentryConstants.PRIVILEGE_NAME.toLowerCase(), privilege.getAction())); } + + if (privilege.getGrantOption()) { + // include grant option field when it is true + authorizable + .add(KV_JOINER.join(SentryConstants.GRANT_OPTION.toLowerCase(), + privilege.getGrantOption())); + } + return AUTHORIZABLE_JOINER.join(authorizable); } http://git-wip-us.apache.org/repos/asf/sentry/blob/c30d5611/sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestOwnerPrivileges.java ---------------------------------------------------------------------- diff --git a/sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestOwnerPrivileges.java b/sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestOwnerPrivileges.java index 8a10214..2ecf8fe 100644 --- a/sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestOwnerPrivileges.java +++ b/sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestOwnerPrivileges.java @@ -31,9 +31,11 @@ import java.util.Set; import org.apache.sentry.tests.e2e.hdfs.TestHDFSIntegrationBase; import org.apache.sentry.tests.e2e.hive.StaticUserGroup; +import org.apache.sentry.service.common.ServiceConstants.SentryPrincipalType; import org.junit.Assert; import org.junit.Before; import org.junit.BeforeClass; +import org.junit.Ignore; import org.junit.Test; import com.google.common.collect.Lists; @@ -52,6 +54,8 @@ public class TestOwnerPrivileges extends TestHDFSIntegrationBase { USER1_1 = StaticUserGroup.USER1_1, USER1_2 = StaticUserGroup.USER1_2, USERGROUP1 = StaticUserGroup.USERGROUP1, + USERGROUP2 = StaticUserGroup.USERGROUP2, + USER2_1 = StaticUserGroup.USER2_1, DB1 = "db_1"; private final static String renameTag = "_new"; @@ -100,7 +104,7 @@ public class TestOwnerPrivileges extends TestHDFSIntegrationBase { statementUSER1_1.execute("CREATE DATABASE " + DB1); // verify privileges created for new database - verifyTablePrivilegeExistForUser(statementUSER1_1, Lists.newArrayList(USER1_1), + verifyTableOwnerPrivilegeExistForEntity(statementUSER1_1, SentryPrincipalType.USER, Lists.newArrayList(USER1_1), DB1, null, 1); // verify that user has all privilege on this database, i.e., "OWNER" means "ALL" @@ -146,7 +150,7 @@ public class TestOwnerPrivileges extends TestHDFSIntegrationBase { // verify user user1_2 has no privileges created for new database Connection connectionUSER1_2 = hiveServer2.createConnection(USER1_2, USER1_2); Statement statementUSER1_2 = connectionUSER1_2.createStatement(); - verifyTablePrivilegeExistForUser(statementUSER1_2, Lists.newArrayList(USER1_2), + verifyTableOwnerPrivilegeExistForEntity(statementUSER1_2, SentryPrincipalType.USER, Lists.newArrayList(USER1_2), DB1, null, 0); // verify that user user1_2 does not have any privilege on this database except create @@ -187,7 +191,7 @@ public class TestOwnerPrivileges extends TestHDFSIntegrationBase { statement.execute("CREATE DATABASE " + DB1); // verify no privileges created for new database - verifyTablePrivilegeExistForUser(statement, Lists.newArrayList(admin), + verifyTableOwnerPrivilegeExistForEntity(statement, SentryPrincipalType.USER, Lists.newArrayList(admin), DB1, null, 0); statement.close(); @@ -220,7 +224,7 @@ public class TestOwnerPrivileges extends TestHDFSIntegrationBase { statementUSER1_1.execute("DROP DATABASE " + DB1 + " CASCADE"); // verify owner privileges created for new database no longer exists - verifyTablePrivilegeExistForUser(statementUSER1_1, Lists.newArrayList(USER1_1), + verifyTableOwnerPrivilegeExistForEntity(statementUSER1_1, SentryPrincipalType.USER, Lists.newArrayList(USER1_1), DB1, null, 0); statement.close(); @@ -260,7 +264,7 @@ public class TestOwnerPrivileges extends TestHDFSIntegrationBase { // verify privileges created for new table - verifyTablePrivilegeExistForUser(statementUSER1_1, Lists.newArrayList(USER1_1), + verifyTableOwnerPrivilegeExistForEntity(statementUSER1_1, SentryPrincipalType.USER, Lists.newArrayList(USER1_1), DB1, tableName1, 1); // verify that user has all privilege on this table, i.e., "OWNER" means "ALL" @@ -312,7 +316,7 @@ public class TestOwnerPrivileges extends TestHDFSIntegrationBase { // verify user1_2 does not have privileges on table created by user1_1 Connection connectionUSER1_2 = hiveServer2.createConnection(USER1_2, USER1_2); Statement statementUSER1_2 = connectionUSER1_2.createStatement(); - verifyTablePrivilegeExistForUser(statementUSER1_2, Lists.newArrayList(USER1_2), + verifyTableOwnerPrivilegeExistForEntity(statementUSER1_2, SentryPrincipalType.USER, Lists.newArrayList(USER1_2), DB1, tableName1, 0); // verify that user user1_2 does not have any privilege on this table @@ -369,7 +373,7 @@ public class TestOwnerPrivileges extends TestHDFSIntegrationBase { + " (under_col int comment 'the under column')"); // verify no owner privileges created for new table - verifyTablePrivilegeExistForUser(statement, Lists.newArrayList(admin), + verifyTableOwnerPrivilegeExistForEntity(statement, SentryPrincipalType.USER, Lists.newArrayList(admin), DB1, tableName1, 0); statement.close(); @@ -404,17 +408,234 @@ public class TestOwnerPrivileges extends TestHDFSIntegrationBase { statementUSER1_1.execute("DROP TABLE " + DB1 + "." + tableName1); // verify privileges created for new table - verifyTablePrivilegeExistForUser(statementUSER1_1, Lists.newArrayList(USER1_1), + verifyTableOwnerPrivilegeExistForEntity(statementUSER1_1, SentryPrincipalType.USER, Lists.newArrayList(USER1_1), DB1, tableName1, 0); statement.close(); connection.close(); } - + /** + * Verify that the owner privilege is updated when the ownership is changed + * + * @throws Exception + */ + @Ignore("Enable the test once HIVE-18762 is in the hiver version integrated with Sentry") + @Test + public void testAlterTable() throws Exception { + dbNames = new String[]{DB1}; + roles = new String[]{"admin_role", "create_db1", "owner_role"}; + + // create required roles + setupUserRoles(roles, statement); + + // create test DB + statement.execute("DROP DATABASE IF EXISTS " + DB1 + " CASCADE"); + statement.execute("CREATE DATABASE " + DB1); + + // setup privileges for USER1 + statement.execute("GRANT CREATE ON DATABASE " + DB1 + " TO ROLE create_db1"); + statement.execute("USE " + DB1); + + // USER1 create table + Connection connectionUSER1_1 = hiveServer2.createConnection(USER1_1, USER1_1); + Statement statementUSER1_1 = connectionUSER1_1.createStatement(); + statementUSER1_1.execute("CREATE TABLE " + DB1 + "." + tableName1 + + " (under_col int comment 'the under column')"); + + + // verify privileges created for new table + verifyTableOwnerPrivilegeExistForEntity(statementUSER1_1, SentryPrincipalType.USER, Lists.newArrayList(USER1_1), + DB1, tableName1, 1); + + // verify that user has all privilege on this table, i.e., "OWNER" means "ALL" + // for authorization + statementUSER1_1.execute("INSERT INTO TABLE " + DB1 + "." + tableName1 + " VALUES (35)"); + + // Changing the owner to a role + statementUSER1_1.execute("ALTER TABLE " + DB1 + "." + tableName1 + " SET OWNER ROLE " + + "owner_role"); - // TODO: once hive supports alter table set owner, need to add testing cases for owner - // privilege associated with role + // alter table rename is not blocked for notification processing in upstream due to + // hive bug HIVE-18783, which is fixed in Hive 2.4.0 and 3.0 + Thread.sleep(WAIT_BEFORE_TESTVERIFY); + + // Verify that old owner does not have owner privilege + verifyTableOwnerPrivilegeExistForEntity(statementUSER1_1, SentryPrincipalType.USER, Lists.newArrayList(USER1_1), + DB1, tableName1, 0); + // Verify that new owner has owner privilege + + verifyTableOwnerPrivilegeExistForEntity(statementUSER1_1, SentryPrincipalType.ROLE, Lists.newArrayList("owner_role"), + DB1, tableName1, 1); + + + // Changing the owner to a user + statementUSER1_1.execute("ALTER TABLE " + DB1 + "." + tableName1 + " SET OWNER USER " + + USER1_1); + + // Verify that old owner does not have owner privilege + verifyTableOwnerPrivilegeExistForEntity(statementUSER1_1, SentryPrincipalType.ROLE, Lists.newArrayList("owner_role"), + DB1, tableName1, 0); + + // Verify that new owner has owner privilege + verifyTableOwnerPrivilegeExistForEntity(statementUSER1_1, SentryPrincipalType.USER, Lists.newArrayList(USER1_1), + DB1, tableName1, 1); + + statement.close(); + connection.close(); + + statementUSER1_1.close(); + connectionUSER1_1.close(); + } + + /** + * Verify that the user who can call alter table set owner on this table + * + * @throws Exception + */ + @Ignore("Enable the test once HIVE-18762 is in the hiver version integrated with Sentry") + @Test + public void testAuthorizeAlterTableSetOwner() throws Exception { + String ownerRole = "owner_role"; + String allWithGrantRole = "allWithGrant_role"; + dbNames = new String[]{DB1}; + roles = new String[]{"admin_role", "create_db1", ownerRole}; + + // create required roles, and assign them to USERGROUP1 + setupUserRoles(roles, statement); + + // create test DB + statement.execute("DROP DATABASE IF EXISTS " + DB1 + " CASCADE"); + statement.execute("CREATE DATABASE " + DB1); + + // setup privileges for USER1 + statement.execute("GRANT CREATE ON DATABASE " + DB1 + " TO ROLE create_db1"); + statement.execute("USE " + DB1); + + // USER1_1 create table + Connection connectionUSER1_1 = hiveServer2.createConnection(USER1_1, USER1_1); + Statement statementUSER1_1 = connectionUSER1_1.createStatement(); + statementUSER1_1.execute("CREATE TABLE " + DB1 + "." + tableName1 + + " (under_col int comment 'the under column')"); + + // owner issues alter table set owner + if (!ownerPrivilegeGrantEnabled) { + try { + statementUSER1_1 + .execute("ALTER TABLE " + DB1 + "." + tableName1 + " SET OWNER ROLE " + ownerRole); + Assert.fail("Expect altering table set owner to fail for owner without grant option"); + } catch (Exception ex) { + // owner without grant option cannot issue this command + } + } + + // admin issues alter table set owner + try { + statement.execute("ALTER TABLE " + DB1 + "." + tableName1 + " SET OWNER ROLE " + ownerRole); + Assert.fail("Expect altering table set owner to fail for admin"); + } catch (Exception ex) { + // admin does not have grant option, so cannot issue this command + } + + Connection connectionUSER2_1 = hiveServer2.createConnection(USER2_1, USER2_1); + Statement statementUSER2_1 = connectionUSER2_1.createStatement(); + + try { + // create role that has all with grant on the table + statement.execute("create role " + allWithGrantRole); + statement.execute("grant role " + allWithGrantRole + " to group " + USERGROUP2); + statement.execute("grant all on table " + DB1 + "." + tableName1 + " to role " + + allWithGrantRole + " with grant option"); + + // cannot issue command on a different table + try { + statementUSER2_1.execute("ALTER TABLE " + DB1 + ".non_exit_table" + " SET OWNER ROLE " + ownerRole); + Assert.fail("Expect altering table set owner to fail on non-exist table"); + } catch (Exception ex) { + // admin does not have grant option, so cannot issue this command + } + + // user2_1 having all with grant on this table and can issue command: alter table set owner + // alter table set owner to a role + statementUSER2_1 + .execute("ALTER TABLE " + DB1 + "." + tableName1 + " SET OWNER ROLE " + ownerRole); + + // verify privileges is transferred to role owner_role, which is associated with USERGROUP1, + // therefore to USER1_1 + verifyTableOwnerPrivilegeExistForEntity(statement, SentryPrincipalType.ROLE, + Lists.newArrayList(ownerRole), + DB1, tableName1, 1); + + // alter table set owner to user USER1_1 and verify privileges is transferred to USER USER1_1 + statementUSER2_1 + .execute("ALTER TABLE " + DB1 + "." + tableName1 + " SET OWNER USER " + USER1_1); + verifyTableOwnerPrivilegeExistForEntity(statement, SentryPrincipalType.USER, + Lists.newArrayList(USER1_1), DB1, tableName1, 1); + + // alter table set owner to user USER2_1, who already has explicit all with grant + statementUSER2_1 + .execute("ALTER TABLE " + DB1 + "." + tableName1 + " SET OWNER USER " + USER2_1); + verifyTableOwnerPrivilegeExistForEntity(statement, SentryPrincipalType.USER, + Lists.newArrayList(USER2_1), + DB1, tableName1, 1); + + } finally { + statement.execute("drop role " + allWithGrantRole); + + statement.close(); + connection.close(); + + statementUSER1_1.close(); + connectionUSER1_1.close(); + + statementUSER2_1.close(); + connectionUSER2_1.close(); + } + } + + /** + * Verify that no owner privilege is granted when the ownership is changed to sentry admin user + * @throws Exception + */ + @Ignore("Enable the test once HIVE-18762 is in the hiver version integrated with Sentry") + @Test + public void testAlterTableAdmin() throws Exception { + dbNames = new String[]{DB1}; + roles = new String[]{"admin_role", "create_db1"}; + + // create required roles + setupUserRoles(roles, statement); + + // create test DB + statement.execute("DROP DATABASE IF EXISTS " + DB1 + " CASCADE"); + statement.execute("CREATE DATABASE " + DB1); + + // setup privileges for USER1 + statement.execute("GRANT CREATE ON DATABASE " + DB1 + " TO ROLE create_db1"); + statement.execute("USE " + DB1); + + // USER1 create table + Connection connectionUSER1_1 = hiveServer2.createConnection(USER1_1, USER1_1); + Statement statementUSER1_1 = connectionUSER1_1.createStatement(); + statementUSER1_1.execute("CREATE TABLE " + DB1 + "." + tableName1 + + " (under_col int comment 'the under column')"); + + // verify owner privileges created for new table + verifyTableOwnerPrivilegeExistForEntity(statement, SentryPrincipalType.USER, Lists.newArrayList(USER1_1), + DB1, tableName1, 1); + + // Changing the owner to an admin user + statementUSER1_1.execute("ALTER TABLE " + DB1 + "." + tableName1 + " SET OWNER USER " + + admin); + + // verify no owner privileges to the new owner as the owner is admin user + + verifyTableOwnerPrivilegeExistForEntity(statement, SentryPrincipalType.USER, Lists.newArrayList(admin), + DB1, tableName1, 0); + + statement.close(); + connection.close(); + } // Create test roles private void setupUserRoles(String[] roles, Statement statement) throws Exception { @@ -428,36 +649,54 @@ public class TestOwnerPrivileges extends TestHDFSIntegrationBase { } // verify given table is part of every user in the list - private void verifyTablePrivilegeExistForUser(Statement statement, - List<String> users, String dbName, String tableName, int expectedResultCount) throws Exception { + // verify that each entity in the list has owner privilege on the given database or table + protected void verifyTableOwnerPrivilegeExistForEntity(Statement statement, SentryPrincipalType entityType, + List<String> entities, String dbName, String tableName, int expectedResultCount) throws Exception { - for (String userName : users) { + for (String entity : entities) { String command; if (tableName == null) { - command = "SHOW GRANT USER " + userName + " ON DATABASE " + dbName; + command = "SHOW GRANT " + entityType.toString() + " " + entity + " ON DATABASE " + dbName; } else { - command = "SHOW GRANT USER " + userName + " ON TABLE " + dbName + "." + tableName; + command = "SHOW GRANT " + entityType.toString() + " " + entity + " ON TABLE " + dbName + "." + tableName; } ResultSet resultSet = statement.executeQuery(command); int resultSize = 0; while(resultSet.next()) { - resultSize ++; + + String actionValue = resultSet.getString(7); + if (!actionValue.equalsIgnoreCase("owner")) { + // only check owner privilege, and skip other privileges + continue; + } assertThat(resultSet.getString(1), equalToIgnoringCase(dbName)); // db name + String tableNameValue = resultSet.getString(2); if (tableName != null) { - assertThat(resultSet.getString(2), equalToIgnoringCase(tableName)); // table name + if (!tableNameValue.equalsIgnoreCase(tableName)) { + // it is possible the entity has owner privilege on both DB and table + // only check the owner privilege on table + continue; + } + } else { + if (!tableNameValue.equalsIgnoreCase("")) { + // it is possible the entity has owner privilege on both DB and table + // only check the owner privilege on db + continue; + } } assertThat(resultSet.getString(3), equalToIgnoringCase(""));//partition assertThat(resultSet.getString(4), equalToIgnoringCase(""));//column - assertThat(resultSet.getString(5), equalToIgnoringCase(userName));//principalName - assertThat(resultSet.getString(6), equalToIgnoringCase("user"));//principalType - assertThat(resultSet.getString(7), equalToIgnoringCase("owner")); + assertThat(resultSet.getString(5), equalToIgnoringCase(entity));//principalName + assertThat(resultSet.getString(6), equalToIgnoringCase(entityType.toString()));//principalType assertThat(resultSet.getBoolean(8), is(ownerPrivilegeGrantEnabled));//grantOption + + resultSize ++; } assertEquals(expectedResultCount, resultSize); @@ -465,5 +704,4 @@ public class TestOwnerPrivileges extends TestHDFSIntegrationBase { resultSet.close(); } } - } http://git-wip-us.apache.org/repos/asf/sentry/blob/c30d5611/sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hdfs/TestHDFSIntegrationBase.java ---------------------------------------------------------------------- diff --git a/sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hdfs/TestHDFSIntegrationBase.java b/sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hdfs/TestHDFSIntegrationBase.java index 4588963..f5e4db8 100644 --- a/sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hdfs/TestHDFSIntegrationBase.java +++ b/sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hdfs/TestHDFSIntegrationBase.java @@ -622,6 +622,7 @@ public abstract class TestHDFSIntegrationBase { hiveConf.set("hive.metastore.uris", "thrift://localhost:" + hmsPort); hiveConf.set("hive.metastore.pre.event.listeners", "org.apache.sentry.binding.metastore.MetastoreAuthzBinding"); hiveConf.set("hive.metastore.transactional.event.listeners", "org.apache.hive.hcatalog.listener.DbNotificationListener"); + hiveConf.set("hive.metastore.event.listeners", "org.apache.sentry.binding.metastore.SentrySyncHMSNotificationsPostEventListener"); hiveConf.set("hive.metastore.event.message.factory", "org.apache.sentry.binding.metastore.messaging.json.SentryJSONMessageFactory"); hiveConf.set("hive.security.authorization.task.factory", "org.apache.sentry.binding.hive.SentryHiveAuthorizationTaskFactoryImpl"); hiveConf.set("hive.server2.session.hook", "org.apache.sentry.binding.hive.HiveAuthzBindingSessionHook");
