SENTRY-2372: SentryStore should not implement grantOptionCheck (Sergio Pena, reviewed by Na Li)
Project: http://git-wip-us.apache.org/repos/asf/sentry/repo Commit: http://git-wip-us.apache.org/repos/asf/sentry/commit/8e050570 Tree: http://git-wip-us.apache.org/repos/asf/sentry/tree/8e050570 Diff: http://git-wip-us.apache.org/repos/asf/sentry/diff/8e050570 Branch: refs/heads/master Commit: 8e0505703eaf7c75c268593f5f31a3e66bb04a56 Parents: 6734686 Author: Sergio Pena <[email protected]> Authored: Sat Oct 27 10:00:59 2018 -0500 Committer: Sergio Pena <[email protected]> Committed: Sat Oct 27 10:00:59 2018 -0500 ---------------------------------------------------------------------- .../sentry/core/common/utils/SentryUtils.java | 12 + .../org/apache/sentry/hdfs/SentryPlugin.java | 6 +- .../thrift/SentryPolicyStoreProcessor.java | 77 +- .../service/thrift/SentryPolicyStoreUtils.java | 93 +++ .../db/service/model/MSentryPrivilege.java | 3 +- .../db/service/persistent/SentryStore.java | 288 +------ .../persistent/SentryStoreInterface.java | 18 +- .../SentryPolicyStoreProcessorTestUtils.java | 238 ++++++ .../thrift/TestSentryPolicyStoreProcessor.java | 159 ++++ .../TestHMSFollowerSentryStoreIntegration.java | 18 +- .../db/service/persistent/TestSentryStore.java | 741 ++++--------------- 11 files changed, 763 insertions(+), 890 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/sentry/blob/8e050570/sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/utils/SentryUtils.java ---------------------------------------------------------------------- diff --git a/sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/utils/SentryUtils.java b/sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/utils/SentryUtils.java index b225af0..f3394b8 100644 --- a/sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/utils/SentryUtils.java +++ b/sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/utils/SentryUtils.java @@ -18,6 +18,9 @@ package org.apache.sentry.core.common.utils; +import static org.apache.sentry.core.common.utils.SentryConstants.NULL_COL; + +import com.google.common.base.Strings; import java.util.ArrayList; import java.util.List; @@ -74,4 +77,13 @@ public final class SentryUtils { return collapsedStrings.toString(); } + + /** + * Function to check if a string is null, empty or @NULL_COLL specifier + * @param s string input, and can be null. + * @return True if the input string represents a NULL string - when it is null, empty or equals @NULL_COL + */ + public static boolean isNULL(String s) { + return Strings.isNullOrEmpty(s) || s.equals(NULL_COL); + } } http://git-wip-us.apache.org/repos/asf/sentry/blob/8e050570/sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/SentryPlugin.java ---------------------------------------------------------------------- diff --git a/sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/SentryPlugin.java b/sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/SentryPlugin.java index b8f5ce7..0f3c162 100644 --- a/sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/SentryPlugin.java +++ b/sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/SentryPlugin.java @@ -27,6 +27,7 @@ import org.apache.hadoop.conf.Configuration; import org.apache.sentry.api.common.ApiConstants.PrivilegeScope; import org.apache.sentry.core.common.exception.SentryInvalidInputException; import org.apache.sentry.core.common.utils.PubSub; +import org.apache.sentry.core.common.utils.SentryUtils; import org.apache.sentry.core.common.utils.SigUtils; import org.apache.sentry.hdfs.ServiceConstants.ServerConfig; import org.apache.sentry.hdfs.service.thrift.TPrivilegeChanges; @@ -34,7 +35,6 @@ import org.apache.sentry.hdfs.service.thrift.TPrivilegePrincipal; import org.apache.sentry.hdfs.service.thrift.TPrivilegePrincipalType; import org.apache.sentry.hdfs.service.thrift.TRoleChanges; import org.apache.sentry.provider.db.SentryPolicyStorePlugin; -import org.apache.sentry.provider.db.service.persistent.SentryStore; import org.apache.sentry.provider.db.service.persistent.SentryStoreInterface; import org.apache.sentry.api.common.SentryServiceUtil; import org.apache.sentry.api.service.thrift.TAlterSentryRoleAddGroupsRequest; @@ -489,10 +489,10 @@ public class SentryPlugin implements SentryPolicyStorePlugin, SigUtils.SigListen private String getAuthzObj(TSentryPrivilege privilege) { String authzObj = null; - if (!SentryStore.isNULL(privilege.getDbName())) { + if (!SentryUtils.isNULL(privilege.getDbName())) { String dbName = privilege.getDbName(); String tblName = privilege.getTableName(); - if (SentryStore.isNULL(tblName)) { + if (SentryUtils.isNULL(tblName)) { authzObj = dbName; } else { authzObj = dbName + "." + tblName; http://git-wip-us.apache.org/repos/asf/sentry/blob/8e050570/sentry-service/sentry-service-server/src/main/java/org/apache/sentry/api/service/thrift/SentryPolicyStoreProcessor.java ---------------------------------------------------------------------- diff --git a/sentry-service/sentry-service-server/src/main/java/org/apache/sentry/api/service/thrift/SentryPolicyStoreProcessor.java b/sentry-service/sentry-service-server/src/main/java/org/apache/sentry/api/service/thrift/SentryPolicyStoreProcessor.java index 709434c..b9e3bf2 100644 --- a/sentry-service/sentry-service-server/src/main/java/org/apache/sentry/api/service/thrift/SentryPolicyStoreProcessor.java +++ b/sentry-service/sentry-service-server/src/main/java/org/apache/sentry/api/service/thrift/SentryPolicyStoreProcessor.java @@ -37,6 +37,7 @@ import org.apache.hadoop.conf.Configuration; import org.apache.hadoop.hive.metastore.messaging.EventMessage.EventType; import org.apache.sentry.SentryOwnerInfo; import org.apache.sentry.api.common.ThriftConstants; +import org.apache.sentry.core.common.exception.SentryGrantDeniedException; import org.apache.sentry.core.common.exception.SentryUserException; import org.apache.sentry.core.common.exception.SentrySiteConfigurationException; import org.apache.sentry.core.common.utils.SentryConstants; @@ -245,6 +246,60 @@ public class SentryPolicyStoreProcessor implements SentryPolicyService.Iface { return response; } + /** + * Throws an exception if one of the set of privileges passed as a parameter cannot be granted by + * the grantor user. + * + * <p/> The check is done by looking at the grant option flag each user or user/group role have + * stored on the DB, and compare it with set of privileges that the user is attempting to grant. + * If one of the privileges has the grant option disabled, then this method throws an exception + * to let the caller know it cannot continue with the grant of the privilege. + * + * @param grantorUser The user who is attempting to grant the set or privileges. + * @param checkPrivileges The set of privileges to check. + * @throws Exception If the user does not have grant privileges. + */ + private void checkGrantOptionPrivileges(String grantorUser, Set<TSentryPrivilege> checkPrivileges) + throws Exception { + Preconditions.checkNotNull(checkPrivileges, "Privileges to check for grant option must not be null."); + + Set<String> groups = getGroupsFromUserName(conf, grantorUser); + if (groups != null && inAdminGroups(groups)) { + // grantorUser is part of one of the admin groups, so we permit the grant action + return; + } + + // Get all the privileges a user has (either directly granted to the user or through a role + // which the user belongs too) + Set<TSentryPrivilege> userPrivileges = sentryStore.listSentryPrivilegesByUsersAndGroups( + groups, Collections.singleton(grantorUser), new TSentryActiveRoleSet(true, null), null + ); + + if (userPrivileges == null || userPrivileges.isEmpty()) { + throw new SentryGrantDeniedException( + String.format("User %s does not have privileges to grant.", grantorUser)); + } + + // Check if each privilege grant will be permitted. Throws an exception in the first privilege + // that is not permitted. + for (TSentryPrivilege checkPrivilege : checkPrivileges) { + boolean hasGrant = false; + for (TSentryPrivilege p : userPrivileges) { + if (p.getGrantOption() == TSentryGrantOption.TRUE + && SentryPolicyStoreUtils.privilegeImplies(p, checkPrivilege)) { + hasGrant = true; + break; + } + } + + if (!hasGrant) { + throw new SentryGrantDeniedException( + String.format("User %s does not have privileges to grant %s.", grantorUser, + checkPrivilege.getAction().toUpperCase())); + } + } + } + @Override public TAlterSentryRoleGrantPrivilegeResponse alter_sentry_role_grant_privilege (TAlterSentryRoleGrantPrivilegeRequest request) throws TException { @@ -264,6 +319,9 @@ public class SentryPolicyStoreProcessor implements SentryPolicyService.Iface { // Throw an exception if one of the grants is not permitted. SentryServiceUtil.checkDbExplicitGrantsPermitted(conf, request.getPrivileges()); + // Throw an exception if the user has not rights to grant one of the grants requested + checkGrantOptionPrivileges(request.getRequestorUserName(), request.getPrivileges()); + // TODO: now only has SentryPlugin. Once add more SentryPolicyStorePlugins, // TODO: need to differentiate the updates for different Plugins. Preconditions.checkState(sentryPlugins.size() <= 1); @@ -273,11 +331,11 @@ public class SentryPolicyStoreProcessor implements SentryPolicyService.Iface { } if (!privilegesUpdateMap.isEmpty()) { - sentryStore.alterSentryRoleGrantPrivileges(request.getRequestorUserName(), - request.getRoleName(), request.getPrivileges(), privilegesUpdateMap); + sentryStore.alterSentryRoleGrantPrivileges(request.getRoleName(), + request.getPrivileges(), privilegesUpdateMap); } else { - sentryStore.alterSentryRoleGrantPrivileges(request.getRequestorUserName(), - request.getRoleName(), request.getPrivileges()); + sentryStore.alterSentryRoleGrantPrivileges(request.getRoleName(), + request.getPrivileges()); } GrantPrivilegeRequestValidator.validate(request); response.setStatus(Status.OK()); @@ -332,6 +390,9 @@ public class SentryPolicyStoreProcessor implements SentryPolicyService.Iface { request.setPrivileges(Sets.newHashSet(request.getPrivilege())); } + // Throw an exception if the user has not rights to revoke one of the revokes requested + checkGrantOptionPrivileges(request.getRequestorUserName(), request.getPrivileges()); + // TODO: now only has SentryPlugin. Once add more SentryPolicyStorePlugins, // TODO: need to differentiate the updates for different Plugins. Preconditions.checkState(sentryPlugins.size() <= 1); @@ -341,11 +402,11 @@ public class SentryPolicyStoreProcessor implements SentryPolicyService.Iface { } if (!privilegesUpdateMap.isEmpty()) { - sentryStore.alterSentryRoleRevokePrivileges(request.getRequestorUserName(), - request.getRoleName(), request.getPrivileges(), privilegesUpdateMap); + sentryStore.alterSentryRoleRevokePrivileges(request.getRoleName(), + request.getPrivileges(), privilegesUpdateMap); } else { - sentryStore.alterSentryRoleRevokePrivileges(request.getRequestorUserName(), - request.getRoleName(), request.getPrivileges()); + sentryStore.alterSentryRoleRevokePrivileges(request.getRoleName(), + request.getPrivileges()); } RevokePrivilegeRequestValidator.validate(request); response.setStatus(Status.OK()); http://git-wip-us.apache.org/repos/asf/sentry/blob/8e050570/sentry-service/sentry-service-server/src/main/java/org/apache/sentry/api/service/thrift/SentryPolicyStoreUtils.java ---------------------------------------------------------------------- diff --git a/sentry-service/sentry-service-server/src/main/java/org/apache/sentry/api/service/thrift/SentryPolicyStoreUtils.java b/sentry-service/sentry-service-server/src/main/java/org/apache/sentry/api/service/thrift/SentryPolicyStoreUtils.java new file mode 100644 index 0000000..8d8b407 --- /dev/null +++ b/sentry-service/sentry-service-server/src/main/java/org/apache/sentry/api/service/thrift/SentryPolicyStoreUtils.java @@ -0,0 +1,93 @@ +/* + Licensed to the Apache Software Foundation (ASF) under one + or more contributor license agreements. See the NOTICE file + distributed with this work for additional information + regarding copyright ownership. The ASF licenses this file + to you under the Apache License, Version 2.0 (the + "License"); you may not use this file except in compliance + with the License. You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + + Unless required by applicable law or agreed to in writing, software + distributed under the License is distributed on an "AS IS" BASIS, + WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + See the License for the specific language governing permissions and + limitations under the License. + */ +package org.apache.sentry.api.service.thrift; + +import static org.apache.sentry.core.common.utils.SentryUtils.isNULL; + +import org.apache.sentry.core.common.utils.PathUtils; +import org.apache.sentry.core.model.db.AccessConstants; + +/** + * Utilities for Sentry policy store functions. + */ +public class SentryPolicyStoreUtils { + // Private to avoid instantiate this class + private SentryPolicyStoreUtils() { + } + + /** + * Return true if this privilege implies other privilege (copied from MSentryPrivilege.implies). + * Otherwise, return false + * + * @param privilege, privilege to compare against the other privilege + * @param other, the other privilege + * @return True if both privilege are the same; False otherwise. + */ + static boolean privilegeImplies(TSentryPrivilege privilege, TSentryPrivilege other) { + // serverName never be null + if (isNULL(privilege.getServerName()) || isNULL(other.getServerName())) { + return false; + } else if (!privilege.getServerName().equals(other.getServerName())) { + return false; + } + + // check URI implies + if (!isNULL(privilege.getURI()) && !isNULL(other.getURI())) { + if (!PathUtils.impliesURI(privilege.getURI(), other.getURI())) { + return false; + } + // if URI is NULL, check dbName and tableName + } else if (isNULL(privilege.getURI()) && isNULL(other.getURI())) { + if (!isNULL(privilege.getDbName())) { + if (isNULL(other.getDbName())) { + return false; + } else if (!privilege.getDbName().equals(other.getDbName())) { + return false; + } + } + if (!isNULL(privilege.getTableName())) { + if (isNULL(other.getTableName())) { + return false; + } else if (!privilege.getTableName().equals(other.getTableName())) { + return false; + } + } + if (!isNULL(privilege.getColumnName())) { + if (isNULL(other.getColumnName())) { + return false; + } else if (!privilege.getColumnName().equals(other.getColumnName())) { + return false; + } + } + // if URI is not NULL, but other's URI is NULL, return false + } else if (!isNULL(privilege.getURI()) && isNULL(other.getURI())){ + return false; + } + + // check action implies + String action = privilege.getAction(); + if (!action.equalsIgnoreCase(AccessConstants.ALL) + && !action.equalsIgnoreCase(AccessConstants.OWNER) + && !action.equalsIgnoreCase(other.getAction()) + && !action.equalsIgnoreCase(AccessConstants.ACTION_ALL)) { + return false; + } + + return true; + } +} http://git-wip-us.apache.org/repos/asf/sentry/blob/8e050570/sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/service/model/MSentryPrivilege.java ---------------------------------------------------------------------- diff --git a/sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/service/model/MSentryPrivilege.java b/sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/service/model/MSentryPrivilege.java index eba40d2..009b326 100644 --- a/sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/service/model/MSentryPrivilege.java +++ b/sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/service/model/MSentryPrivilege.java @@ -24,6 +24,7 @@ import java.util.Set; import javax.jdo.annotations.PersistenceCapable; import org.apache.sentry.core.common.utils.PathUtils; +import org.apache.sentry.core.common.utils.SentryUtils; import org.apache.sentry.core.model.db.AccessConstants; import org.apache.sentry.provider.db.service.persistent.SentryStore; import org.apache.sentry.provider.db.service.persistent.PrivilegePrincipal; @@ -351,7 +352,7 @@ public class MSentryPrivilege { } private boolean isNULL(String s) { - return SentryStore.isNULL(s); + return SentryUtils.isNULL(s); } public boolean isActionALL() { http://git-wip-us.apache.org/repos/asf/sentry/blob/8e050570/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 869605e..01b3634 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 @@ -54,11 +54,9 @@ import org.apache.hadoop.conf.Configuration; import org.apache.sentry.SentryOwnerInfo; import org.apache.sentry.core.common.exception.SentryAccessDeniedException; import org.apache.sentry.core.common.exception.SentryAlreadyExistsException; -import org.apache.sentry.core.common.exception.SentryGrantDeniedException; import org.apache.sentry.core.common.exception.SentryInvalidInputException; import org.apache.sentry.core.common.exception.SentryNoSuchObjectException; import org.apache.sentry.core.common.exception.SentrySiteConfigurationException; -import org.apache.sentry.core.common.exception.SentryUserException; import org.apache.sentry.core.common.utils.PathUtils; import org.apache.sentry.core.common.utils.SentryConstants; import org.apache.sentry.core.model.db.AccessConstants; @@ -83,7 +81,6 @@ import org.apache.sentry.provider.db.service.model.MSentryUtil; import org.apache.sentry.provider.db.service.model.MPath; import org.apache.sentry.hdfs.service.thrift.TPrivilegePrincipal; import org.apache.sentry.api.common.ApiConstants.PrivilegeScope; -import org.apache.sentry.api.service.thrift.SentryPolicyStoreProcessor; import org.apache.sentry.api.service.thrift.TSentryActiveRoleSet; import org.apache.sentry.api.service.thrift.TSentryAuthorizable; import org.apache.sentry.api.service.thrift.TSentryGrantOption; @@ -114,6 +111,7 @@ import static org.apache.sentry.core.common.utils.SentryConstants.NULL_COL; import static org.apache.sentry.core.common.utils.SentryConstants.SERVER_NAME; import static org.apache.sentry.core.common.utils.SentryConstants.TABLE_NAME; import static org.apache.sentry.core.common.utils.SentryConstants.URI; +import static org.apache.sentry.core.common.utils.SentryUtils.isNULL; import static org.apache.sentry.hdfs.Updateable.Update; /** @@ -725,47 +723,25 @@ public class SentryStore implements SentryStoreInterface { } } - /** - * Alter a given sentry role to grant a set of privileges. - * Internally calls alterSentryGrantPrivilege. - * - * @param grantorPrincipal User name - * @param roleName Role name - * @param privileges Set of privileges - * @throws Exception - */ - public void alterSentryRoleGrantPrivileges(final String grantorPrincipal, - final String roleName, final Set<TSentryPrivilege> privileges) throws Exception { + @Override + public void alterSentryRoleGrantPrivileges(final String roleName, + final Set<TSentryPrivilege> privileges) throws Exception { for (TSentryPrivilege privilege : privileges) { - alterSentryGrantPrivilege(grantorPrincipal, SentryPrincipalType.ROLE, roleName, privilege, null); + alterSentryGrantPrivilege(SentryPrincipalType.ROLE, roleName, privilege, null); } } - /** - * Alter a given sentry role/user to grant a privilege, as well as persist the corresponding - * permission change to MSentryPermChange table in a single transaction. - * - * @param grantorPrincipal User name - * @param type Type of principal to which privilege is granted. - * @param name the name of the principal to which privilege is granted. - * @param privilege the given privilege - * @param update the corresponding permission delta update if any. - * @throws Exception - * - */ - synchronized void alterSentryGrantPrivilege(final String grantorPrincipal, SentryPrincipalType type, - final String name, final TSentryPrivilege privilege, - final Update update) throws Exception { + synchronized void alterSentryGrantPrivilege(SentryPrincipalType type, final String name, + final TSentryPrivilege privilege, + final Update update) throws Exception { execute(update, pm -> { pm.setDetachAllOnCommit(false); // No need to detach objects - - // first do grant check - grantOptionCheck(pm, grantorPrincipal, privilege); + String trimmedEntityName = trimAndLower(name); // Alter sentry Role and grant Privilege. MSentryPrivilege mPrivilege = alterSentryGrantPrivilegeCore(pm, type, - name, privilege); + trimmedEntityName, privilege); if (mPrivilege != null) { // update the privilege to be the one actually updated. @@ -775,27 +751,15 @@ public class SentryStore implements SentryStoreInterface { }); } - /** - * Alter a given sentry role to grant a set of privileges, as well as persist the - * corresponding permission change to MSentryPermChange table in a single transaction. - * Internally calls alterSentryGrantPrivilege. - * - * @param grantorPrincipal User name - * @param roleName the given role name - * @param privileges a Set of privileges - * @param privilegesUpdateMap the corresponding <privilege, DeltaTransactionBlock> map - * @throws Exception - * - */ - public void alterSentryRoleGrantPrivileges(final String grantorPrincipal, - final String roleName, final Set<TSentryPrivilege> privileges, - final Map<TSentryPrivilege, Update> privilegesUpdateMap) throws Exception { + @Override + public void alterSentryRoleGrantPrivileges(final String roleName, + final Set<TSentryPrivilege> privileges, + final Map<TSentryPrivilege, Update> privilegesUpdateMap) throws Exception { Preconditions.checkNotNull(privilegesUpdateMap); for (TSentryPrivilege privilege : privileges) { Update update = privilegesUpdateMap.get(privilege); - alterSentryGrantPrivilege(grantorPrincipal, SentryPrincipalType.ROLE, roleName, privilege, - update); + alterSentryGrantPrivilege(SentryPrincipalType.ROLE, roleName, privilege, update); } } @@ -923,13 +887,12 @@ public class SentryStore implements SentryStoreInterface { * Alter a given sentry user to grant a set of privileges. * Internally calls alterSentryGrantPrivilege. * - * @param grantorPrincipal User name * @param userName User name * @param privileges Set of privileges * @throws Exception */ - public void alterSentryUserGrantPrivileges(final String grantorPrincipal, - final String userName, final Set<TSentryPrivilege> privileges) throws Exception { + public void alterSentryUserGrantPrivileges(final String userName, + final Set<TSentryPrivilege> privileges) throws Exception { try { MSentryUser userEntry = getMSentryUserByName(userName, false); @@ -941,7 +904,7 @@ public class SentryStore implements SentryStoreInterface { } for (TSentryPrivilege privilege : privileges) { - alterSentryGrantPrivilege(grantorPrincipal, SentryPrincipalType.USER, userName, privilege, null); + alterSentryGrantPrivilege(SentryPrincipalType.USER, userName, privilege, null); } } @@ -975,39 +938,6 @@ public class SentryStore implements SentryStoreInterface { } /** - * Alter a given sentry user to grant a set of privileges, as well as persist the - * corresponding permission change to MSentryPermChange table in a single transaction. - * Internally calls alterSentryGrantPrivilege. - * - * @param grantorPrincipal User name - * @param userName the given user name - * @param privileges a Set of privileges - * @param privilegesUpdateMap the corresponding <privilege, DeltaTransactionBlock> map - * @throws Exception - * - */ - public void alterSentryUserGrantPrivileges(final String grantorPrincipal, - final String userName, final Set<TSentryPrivilege> privileges, - final Map<TSentryPrivilege, Update> privilegesUpdateMap) throws Exception { - - try { - MSentryUser userEntry = getMSentryUserByName(userName, false); - if (userEntry == null) { - createSentryUser(userName); - } - } catch (SentryAlreadyExistsException e) { - // the user may be created by other thread, so swallow the exception and proeed - } - - Preconditions.checkNotNull(privilegesUpdateMap); - for (TSentryPrivilege privilege : privileges) { - Update update = privilegesUpdateMap.get(privilege); - alterSentryGrantPrivilege(grantorPrincipal, SentryPrincipalType.USER, userName, privilege, - update); - } - } - - /** * Get the user entry by user name * @param userName the name of the user * @return the user entry @@ -1048,109 +978,48 @@ public class SentryStore implements SentryStoreInterface { * Alter a given sentry user to revoke a set of privileges. * Internally calls alterSentryRevokePrivilege. * - * @param grantorPrincipal User name - * @param userName the given user name - * @param tPrivileges a Set of privileges - * @throws Exception - * - */ - public void alterSentryUserRevokePrivileges(final String grantorPrincipal, - final String userName, final Set<TSentryPrivilege> tPrivileges) throws Exception { - for (TSentryPrivilege tPrivilege : tPrivileges) { - alterSentryRevokePrivilege(grantorPrincipal, SentryPrincipalType.USER, userName, tPrivilege, null); - } - } - - /** - * Alter a given sentry user to revoke a set of privileges, as well as persist the - * corresponding permission change to MSentryPermChange table in a single transaction. - * Internally calls alterSentryRevokePrivilege. - * - * @param grantorPrincipal User name * @param userName the given user name * @param tPrivileges a Set of privileges - * @param privilegesUpdateMap the corresponding <privilege, Update> map * @throws Exception * */ - public void alterSentryUserRevokePrivileges(final String grantorPrincipal, - final String userName, final Set<TSentryPrivilege> tPrivileges, - final Map<TSentryPrivilege, Update> privilegesUpdateMap) - throws Exception { - - Preconditions.checkNotNull(privilegesUpdateMap); + public void alterSentryUserRevokePrivileges(final String userName, + final Set<TSentryPrivilege> tPrivileges) throws Exception { for (TSentryPrivilege tPrivilege : tPrivileges) { - Update update = privilegesUpdateMap.get(tPrivilege); - alterSentryRevokePrivilege(grantorPrincipal, SentryPrincipalType.USER, userName, - tPrivilege, update); + alterSentryRevokePrivilege(SentryPrincipalType.USER, userName, tPrivilege, null); } } - /** - * Alter a given sentry role to revoke a set of privileges. - * Internally calls alterSentryRevokePrivilege. - * - * @param grantorPrincipal User name - * @param roleName the given role name - * @param tPrivileges a Set of privileges - * @throws Exception - * - */ - public void alterSentryRoleRevokePrivileges(final String grantorPrincipal, - final String roleName, final Set<TSentryPrivilege> tPrivileges) throws Exception { + @Override + public void alterSentryRoleRevokePrivileges(final String roleName, + final Set<TSentryPrivilege> tPrivileges) + throws Exception { for (TSentryPrivilege tPrivilege : tPrivileges) { - alterSentryRevokePrivilege(grantorPrincipal, SentryPrincipalType.ROLE, roleName, tPrivilege, null); + alterSentryRevokePrivilege(SentryPrincipalType.ROLE, roleName, tPrivilege, null); } } - /** - * Alter a given sentry role to revoke a privilege, as well as persist the corresponding - * permission change to MSentryPermChange table in a single transaction. - * - * @param grantorPrincipal User name - * @param type Type of principal to which privilege is granted. - * @param principalName the name of the principal from which privilege is revoked. - * @param tPrivilege the given privilege - * @param update the corresponding permission delta update transaction block - * @throws Exception - * - */ - synchronized void alterSentryRevokePrivilege(final String grantorPrincipal, SentryPrincipalType type, - final String principalName, final TSentryPrivilege tPrivilege, - final Update update) throws Exception { + synchronized void alterSentryRevokePrivilege(SentryPrincipalType type, final String principalName, + final TSentryPrivilege tPrivilege, + final Update update) throws Exception { execute(update, pm -> { pm.setDetachAllOnCommit(false); // No need to detach objects + String trimmedEntityName = safeTrimLower(principalName); - // first do revoke check - grantOptionCheck(pm, grantorPrincipal, tPrivilege); - - alterSentryRevokePrivilegeCore(pm, type, principalName, tPrivilege); + alterSentryRevokePrivilegeCore(pm, type, trimmedEntityName, tPrivilege); return null; }); } - /** - * Alter a given sentry role to revoke a set of privileges, as well as persist the - * corresponding permission change to MSentryPermChange table in a single transaction. - * Internally calls alterSentryRevokePrivilege. - * - * @param grantorPrincipal User name - * @param roleName the given role name - * @param tPrivileges a Set of privileges - * @param privilegesUpdateMap the corresponding <privilege, Update> map - * @throws Exception - * - */ - public void alterSentryRoleRevokePrivileges(final String grantorPrincipal, - final String roleName, final Set<TSentryPrivilege> tPrivileges, - final Map<TSentryPrivilege, Update> privilegesUpdateMap) - throws Exception { + @Override + public void alterSentryRoleRevokePrivileges(final String roleName, final Set<TSentryPrivilege> tPrivileges, + final Map<TSentryPrivilege, Update> privilegesUpdateMap) + throws Exception { Preconditions.checkNotNull(privilegesUpdateMap); for (TSentryPrivilege tPrivilege : tPrivileges) { Update update = privilegesUpdateMap.get(tPrivilege); - alterSentryRevokePrivilege(grantorPrincipal, SentryPrincipalType.ROLE, roleName, - tPrivilege, update); + alterSentryRevokePrivilege(SentryPrincipalType.ROLE, roleName, tPrivilege, update); } } @@ -2144,12 +2013,6 @@ public class SentryStore implements SentryStoreInterface { return mSentryUser.getPrivileges(); } - private Set<MSentryPrivilege> getMSentryPrivilegesByUserNameIfExists(String userName) - throws Exception { - MSentryUser mSentryUser = getMSentryUserByName(userName, false); - return mSentryUser != null ? mSentryUser.getPrivileges() : Collections.emptySet(); - } - /** * Gets sentry privilege objects for a given userName from the persistence layer * @param userName : userName to look up @@ -2478,6 +2341,7 @@ public class SentryStore implements SentryStoreInterface { return result; } + @Override public Set<TSentryPrivilege> listSentryPrivilegesByUsersAndGroups( Set<String> groups, Set<String> users, TSentryActiveRoleSet roleSet, TSentryAuthorizable authHierarchy) throws Exception { @@ -3147,84 +3011,6 @@ public class SentryStore implements SentryStoreInterface { } /** - * Function to check if a string is null, empty or @NULL_COLL specifier - * @param s string input, and can be null. - * @return True if the input string represents a NULL string - when it is null, empty or equals @NULL_COL - */ - public static boolean isNULL(String s) { - return Strings.isNullOrEmpty(s) || s.equals(NULL_COL); - } - - /** - * Grant option check - * @param pm Persistence manager instance - * @param grantorPrincipal User name - * @param privilege Privilege to check - * @throws SentryUserException - */ - private void grantOptionCheck(PersistenceManager pm, String grantorPrincipal, - TSentryPrivilege privilege) - throws Exception { - MSentryPrivilege mPrivilege = convertToMSentryPrivilege(privilege); - if (grantorPrincipal == null) { - throw new SentryInvalidInputException("grantorPrincipal should not be null"); - } - - Set<String> groups = SentryPolicyStoreProcessor.getGroupsFromUserName(conf, grantorPrincipal); - - // if grantor is in adminGroup, don't need to do check - Set<String> admins = getAdminGroups(); - boolean isAdminGroup = false; - if (groups != null && !admins.isEmpty()) { - for (String g : groups) { - if (admins.contains(g)) { - isAdminGroup = true; - break; - } - } - } - - if (!isAdminGroup) { - boolean hasGrant = false; - Set<MSentryPrivilege> privilegeSet = new HashSet<>(); - Set<MSentryPrivilege> enityPrivilegeSet = null; - // Collect the privileges granted to all roles to that user. - Set<MSentryRole> roles = getRolesForGroups(pm, groups); - roles.addAll(getRolesForUsers(pm, Sets.newHashSet(grantorPrincipal))); - for (MSentryRole role : roles) { - enityPrivilegeSet = role.getPrivileges(); - if(enityPrivilegeSet != null && !enityPrivilegeSet.isEmpty()) { - privilegeSet.addAll(enityPrivilegeSet); - } - } - // Collect the privileges granted to user - enityPrivilegeSet = getMSentryPrivilegesByUserNameIfExists(grantorPrincipal.trim()); - if(enityPrivilegeSet != null && !enityPrivilegeSet.isEmpty()) { - privilegeSet.addAll(enityPrivilegeSet); - - } - // Compare the privileges that user has with the privilege he/she is trying to grant. - for (MSentryPrivilege p : privilegeSet) { - if (p.getGrantOption() && p.implies(mPrivilege)) { - hasGrant = true; - break; - } - } - - if (!hasGrant) { - throw new SentryGrantDeniedException(grantorPrincipal - + " has no grant!"); - } - } - } - - // get adminGroups from conf - private Set<String> getAdminGroups() { - return Sets.newHashSet(conf.getStrings( - ServerConfig.ADMIN_GROUPS, new String[]{})); - } - - /** * Retrieves an up-to-date sentry permission snapshot. * <p> * It reads hiveObj to < role, privileges > mapping from {@link MSentryPrivilege} http://git-wip-us.apache.org/repos/asf/sentry/blob/8e050570/sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStoreInterface.java ---------------------------------------------------------------------- diff --git a/sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStoreInterface.java b/sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStoreInterface.java index 6217719..97407ff 100644 --- a/sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStoreInterface.java +++ b/sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStoreInterface.java @@ -96,31 +96,28 @@ public interface SentryStoreInterface { */ void alterSentryRoleDeleteUsers(final String roleName, final Set<String> userNames) throws Exception; + /** * Alter a given sentry role to grant a set of privileges. * Internally calls alterSentryRoleGrantPrivilege. * - * @param grantorPrincipal User name * @param roleName Role name * @param privileges Set of privileges * @throws Exception */ - void alterSentryRoleGrantPrivileges(final String grantorPrincipal, - final String roleName, + void alterSentryRoleGrantPrivileges(final String roleName, final Set<TSentryPrivilege> privileges) throws Exception; /** * Alter a given sentry role to revoke a set of privileges. * Internally calls alterSentryRoleRevokePrivilege. * - * @param grantorPrincipal User name * @param roleName the given role name * @param tPrivileges a Set of privileges * @throws Exception * */ - void alterSentryRoleRevokePrivileges(final String grantorPrincipal, - final String roleName, + void alterSentryRoleRevokePrivileges(final String roleName, final Set<TSentryPrivilege> tPrivileges) throws Exception; @@ -515,33 +512,30 @@ public interface SentryStoreInterface { * corresponding permission change to MSentryPermChange table in a single transaction. * Internally calls alterSentryRoleGrantPrivilege. * - * @param grantorPrincipal User name * @param roleName the given role name * @param privileges a Set of privileges * @param privilegesUpdateMap the corresponding <privilege, DeltaTransactionBlock> map * @throws Exception * */ - void alterSentryRoleGrantPrivileges(final String grantorPrincipal, - final String roleName, + void alterSentryRoleGrantPrivileges(final String roleName, final Set<TSentryPrivilege> privileges, final Map<TSentryPrivilege, Update> privilegesUpdateMap) throws Exception; + /** * Alter a given sentry role to revoke a set of privileges, as well as persist the * corresponding permission change to MSentryPermChange table in a single transaction. * Internally calls alterSentryRoleRevokePrivilege. * - * @param grantorPrincipal User name * @param roleName the given role name * @param tPrivileges a Set of privileges * @param privilegesUpdateMap the corresponding <privilege, Update> map * @throws Exception * */ - void alterSentryRoleRevokePrivileges(final String grantorPrincipal, - final String roleName, final Set<TSentryPrivilege> tPrivileges, + void alterSentryRoleRevokePrivileges(final String roleName, final Set<TSentryPrivilege> tPrivileges, final Map<TSentryPrivilege, Update> privilegesUpdateMap) throws Exception; http://git-wip-us.apache.org/repos/asf/sentry/blob/8e050570/sentry-service/sentry-service-server/src/test/java/org/apache/sentry/api/service/thrift/SentryPolicyStoreProcessorTestUtils.java ---------------------------------------------------------------------- diff --git a/sentry-service/sentry-service-server/src/test/java/org/apache/sentry/api/service/thrift/SentryPolicyStoreProcessorTestUtils.java b/sentry-service/sentry-service-server/src/test/java/org/apache/sentry/api/service/thrift/SentryPolicyStoreProcessorTestUtils.java new file mode 100644 index 0000000..7a48cca --- /dev/null +++ b/sentry-service/sentry-service-server/src/test/java/org/apache/sentry/api/service/thrift/SentryPolicyStoreProcessorTestUtils.java @@ -0,0 +1,238 @@ +/** + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.sentry.api.service.thrift; + +import static org.junit.Assert.assertEquals; + +import java.util.Collections; +import java.util.Set; +import org.apache.hadoop.conf.Configuration; +import org.apache.sentry.api.common.ApiConstants; +import org.apache.sentry.api.common.ApiConstants.PrivilegeScope; +import org.apache.sentry.api.common.Status; +import org.apache.sentry.api.common.ThriftConstants; +import org.apache.sentry.provider.db.service.persistent.SentryStoreInterface; +import org.mockito.Mockito; + +// Test utils for the TestSentryPolicyStoreProcessor class +class SentryPolicyStoreProcessorTestUtils { + // Constants variables + private static final int NO_TIME = 0; + private final Configuration conf; + private final SentryPolicyStoreProcessor processor; + private final SentryStoreInterface sentryStore; + + SentryPolicyStoreProcessorTestUtils(Configuration conf, SentryStoreInterface sentryStore) throws Exception { + this.conf = conf; + this.sentryStore = sentryStore; + this.processor = new SentryPolicyStoreProcessor( + ApiConstants.SentryPolicyServiceConstants.SENTRY_POLICY_SERVICE_NAME, conf, sentryStore); + } + + PolicyStoreProcessorTestVerifier givenUser(String userName) { + return new PolicyStoreProcessorTestVerifier(userName); + } + + static TSentryPrivilege newPrivilegeOnDatabase(String action, String serverName, String dbName) { + TSentryPrivilege privilege = new TSentryPrivilege(){{ + setPrivilegeScope(PrivilegeScope.DATABASE.toString()); + setServerName(serverName); + setDbName(dbName); + setCreateTime(NO_TIME); + setAction(action); + }}; + + return privilege; + } + + static TSentryPrivilege newPrivilegeOnDatabaseWithGrant(String action, String serverName, String dbName) { + TSentryPrivilege privilege = new TSentryPrivilege(){{ + setPrivilegeScope(PrivilegeScope.DATABASE.toString()); + setServerName(serverName); + setDbName(dbName); + setCreateTime(NO_TIME); + setAction(action); + setGrantOption(TSentryGrantOption.TRUE); + }}; + + return privilege; + } + + static TSentryPrivilege newPrivilegeOnTable(String action, String serverName, String dbName, String tableName) { + TSentryPrivilege privilege = new TSentryPrivilege(){{ + setPrivilegeScope(PrivilegeScope.TABLE.toString()); + setServerName(serverName); + setDbName(dbName); + setTableName(tableName); + setCreateTime(NO_TIME); + setAction(action); + }}; + + return privilege; + } + + static TSentryPrivilege newPrivilegeOnTableWithGrant(String action, String serverName, String dbName, String tableName) { + TSentryPrivilege privilege = new TSentryPrivilege(){{ + setPrivilegeScope(PrivilegeScope.TABLE.toString()); + setServerName(serverName); + setDbName(dbName); + setTableName(tableName); + setCreateTime(NO_TIME); + setAction(action); + setGrantOption(TSentryGrantOption.TRUE); + }}; + + return privilege; + } + + static TSentryPrivilege newPrivilegeOnColumn(String action, String serverName, String dbName, String tableName, String column) { + TSentryPrivilege privilege = new TSentryPrivilege(){{ + setPrivilegeScope(PrivilegeScope.COLUMN.toString()); + setServerName(serverName); + setDbName(dbName); + setTableName(tableName); + setColumnName(column); + setCreateTime(NO_TIME); + setAction(action); + setGrantOption(TSentryGrantOption.FALSE); + }}; + + return privilege; + } + + class PolicyStoreProcessorTestVerifier { + // Constant variables + private final String REQUESTOR_USER; + + PolicyStoreProcessorTestVerifier(String requestorUser) { + this.REQUESTOR_USER = requestorUser; + } + + PolicyGrantPrivilegeVerifier grantPrivilegeToRole(TSentryPrivilege privilege, String roleName) { + return new PolicyGrantPrivilegeVerifier(REQUESTOR_USER, privilege, roleName); + } + + PolicyRevokePrivilegeVerifier revokePrivilegeFromRole(TSentryPrivilege privilege, String roleName) { + return new PolicyRevokePrivilegeVerifier(REQUESTOR_USER, privilege, roleName); + } + } + + class PolicyRevokePrivilegeVerifier { + // Constant variables + private final String REQUESTOR_USER; + private final TSentryPrivilege privilege; + + // Class variables + private String roleName; + + PolicyRevokePrivilegeVerifier(String requestorUser, TSentryPrivilege privilege, String roleName) { + this.REQUESTOR_USER = requestorUser; + this.privilege = privilege; + this.roleName = roleName; + } + + PolicyRevokePrivilegeVerifier whenRequestStorePrivilegesReturn(Set<TSentryPrivilege> privileges) + throws Exception { + + Set<String> groups = SentryPolicyStoreProcessor.getGroupsFromUserName(conf, REQUESTOR_USER); + Mockito.when(sentryStore.listSentryPrivilegesByUsersAndGroups( + Mockito.eq(groups), + Mockito.eq(Collections.singleton(REQUESTOR_USER)), + Mockito.eq(Mockito.any()), + null) + ).thenReturn(privileges); + + return this; + } + + void verify(Status status) throws Exception { + TAlterSentryRoleRevokePrivilegeRequest revokeRequest = new TAlterSentryRoleRevokePrivilegeRequest(); + revokeRequest.setProtocol_version(ThriftConstants.TSENTRY_SERVICE_VERSION_CURRENT); + revokeRequest.setRequestorUserName(REQUESTOR_USER); + revokeRequest.setRoleName(roleName); + revokeRequest.setPrivilege(privilege); + + TAlterSentryRoleRevokePrivilegeResponse response = + processor.alter_sentry_role_revoke_privilege(revokeRequest); + if (response.getStatus().getValue() == Status.OK.getCode()) { + Mockito.verify(sentryStore).alterSentryRoleRevokePrivileges(revokeRequest.getRoleName(), + revokeRequest.getPrivileges()); + } else { + Mockito.verify(sentryStore, Mockito.times(0)) + .alterSentryRoleRevokePrivileges(Mockito.anyString(), Mockito.anySet()); + } + + assertEquals("Revoke " + privilege.getAction() + " response is not valid", + status.getCode(), response.getStatus().getValue()); + + Mockito.reset(sentryStore); + } + } + + class PolicyGrantPrivilegeVerifier { + // Constant variables + private final String REQUESTOR_USER; + private final TSentryPrivilege privilege; + + // Class variables + private String roleName; + + PolicyGrantPrivilegeVerifier(String requestorUser, TSentryPrivilege privilege, String roleName) { + this.REQUESTOR_USER = requestorUser; + this.privilege = privilege; + this.roleName = roleName; + } + + PolicyGrantPrivilegeVerifier whenRequestStorePrivilegesReturn(Set<TSentryPrivilege> privileges) + throws Exception { + + Set<String> groups = SentryPolicyStoreProcessor.getGroupsFromUserName(conf, REQUESTOR_USER); + Mockito.when(sentryStore.listSentryPrivilegesByUsersAndGroups( + Mockito.eq(groups), + Mockito.eq(Collections.singleton(REQUESTOR_USER)), + Mockito.eq(Mockito.any()), + null) + ).thenReturn(privileges); + + return this; + } + + void verify(Status status) throws Exception { + TAlterSentryRoleGrantPrivilegeRequest grantRequest = new TAlterSentryRoleGrantPrivilegeRequest(); + grantRequest.setProtocol_version(ThriftConstants.TSENTRY_SERVICE_VERSION_CURRENT); + grantRequest.setRequestorUserName(REQUESTOR_USER); + grantRequest.setRoleName(roleName); + grantRequest.setPrivilege(privilege); + + TAlterSentryRoleGrantPrivilegeResponse response = + processor.alter_sentry_role_grant_privilege(grantRequest); + if (response.getStatus().getValue() == Status.OK.getCode()) { + Mockito.verify(sentryStore).alterSentryRoleGrantPrivileges(grantRequest.getRoleName(), + grantRequest.getPrivileges()); + } else { + Mockito.verify(sentryStore, Mockito.times(0)) + .alterSentryRoleGrantPrivileges(Mockito.anyString(), Mockito.anySet()); + } + + assertEquals("Grant " + privilege.getAction() + " response is not valid", + status.getCode(), response.getStatus().getValue()); + + Mockito.reset(sentryStore); + } + } +} http://git-wip-us.apache.org/repos/asf/sentry/blob/8e050570/sentry-service/sentry-service-server/src/test/java/org/apache/sentry/api/service/thrift/TestSentryPolicyStoreProcessor.java ---------------------------------------------------------------------- diff --git a/sentry-service/sentry-service-server/src/test/java/org/apache/sentry/api/service/thrift/TestSentryPolicyStoreProcessor.java b/sentry-service/sentry-service-server/src/test/java/org/apache/sentry/api/service/thrift/TestSentryPolicyStoreProcessor.java index 16ff79e..8cea339 100644 --- a/sentry-service/sentry-service-server/src/test/java/org/apache/sentry/api/service/thrift/TestSentryPolicyStoreProcessor.java +++ b/sentry-service/sentry-service-server/src/test/java/org/apache/sentry/api/service/thrift/TestSentryPolicyStoreProcessor.java @@ -17,6 +17,8 @@ */ package org.apache.sentry.api.service.thrift; +import static org.apache.sentry.core.model.db.AccessConstants.ALL; +import static org.apache.sentry.core.model.db.AccessConstants.SELECT; import static org.apache.sentry.service.common.ServiceConstants.ServerConfig.SENTRY_DB_POLICY_STORE_OWNER_AS_PRIVILEGE; import static org.junit.Assert.assertNull; import static org.junit.Assert.assertTrue; @@ -672,4 +674,161 @@ public class TestSentryPolicyStoreProcessor { Assert.assertFalse("SELECT privileges should be permitted", response.getStatus().getMessage().contains("SELECT")); } + + @Test + public void testGrantToRoleWithGrantCheck() throws Exception { + final String DB = "db"; + final String ROLE = "role"; + final String USER = "user"; + + SentryPolicyStoreProcessorTestUtils test = new SentryPolicyStoreProcessorTestUtils(conf, sentryStore); + + final TSentryPrivilege ALL_ON_DB = test.newPrivilegeOnDatabase(ALL, SERVERNAME, DB); + final TSentryPrivilege ALL_ON_DB_WGRANT = test.newPrivilegeOnDatabaseWithGrant(ALL, SERVERNAME, DB); + final TSentryPrivilege SELECT_ON_DB = test.newPrivilegeOnDatabase(SELECT, SERVERNAME, DB); + final TSentryPrivilege SELECT_ON_DB_WGRANT = test.newPrivilegeOnDatabaseWithGrant(SELECT, SERVERNAME, DB); + + // Admin user can grant privileges + test.givenUser(ADMIN_USER).grantPrivilegeToRole(ALL_ON_DB, ROLE) + .verify(Status.OK); + test.givenUser(ADMIN_USER).grantPrivilegeToRole(ALL_ON_DB_WGRANT, ROLE) + .verify(Status.OK); + test.givenUser(ADMIN_USER).grantPrivilegeToRole(SELECT_ON_DB, ROLE) + .verify(Status.OK); + test.givenUser(ADMIN_USER).grantPrivilegeToRole(SELECT_ON_DB_WGRANT, ROLE) + .verify(Status.OK); + + // User without grant option cannot grant privileges + test.givenUser(USER).grantPrivilegeToRole(ALL_ON_DB, ROLE) + .whenRequestStorePrivilegesReturn(Collections.emptySet()) + .verify(Status.ACCESS_DENIED); + test.givenUser(USER).grantPrivilegeToRole(ALL_ON_DB_WGRANT, ROLE) + .whenRequestStorePrivilegesReturn(Collections.emptySet()) + .verify(Status.ACCESS_DENIED); + test.givenUser(USER).grantPrivilegeToRole(SELECT_ON_DB, ROLE) + .whenRequestStorePrivilegesReturn(Collections.emptySet()) + .verify(Status.ACCESS_DENIED); + test.givenUser(USER).grantPrivilegeToRole(SELECT_ON_DB_WGRANT, ROLE) + .whenRequestStorePrivilegesReturn(Collections.emptySet()) + .verify(Status.ACCESS_DENIED); + test.givenUser(USER).grantPrivilegeToRole(ALL_ON_DB, ROLE) + .whenRequestStorePrivilegesReturn(Collections.singleton(SELECT_ON_DB_WGRANT)) + .verify(Status.ACCESS_DENIED); + + // User with grant option can grant privileges + test.givenUser(USER).grantPrivilegeToRole(ALL_ON_DB, ROLE) + .whenRequestStorePrivilegesReturn(Collections.singleton(ALL_ON_DB_WGRANT)) + .verify(Status.OK); + test.givenUser(USER).grantPrivilegeToRole(SELECT_ON_DB, ROLE) + .whenRequestStorePrivilegesReturn(Collections.singleton(ALL_ON_DB_WGRANT)) + .verify(Status.OK); + test.givenUser(USER).grantPrivilegeToRole(SELECT_ON_DB, ROLE) + .whenRequestStorePrivilegesReturn(Collections.singleton(SELECT_ON_DB_WGRANT)) + .verify(Status.OK); + + // User with grant option can grant privileges with grant option + test.givenUser(USER).grantPrivilegeToRole(ALL_ON_DB_WGRANT, ROLE) + .whenRequestStorePrivilegesReturn(Collections.singleton(ALL_ON_DB_WGRANT)) + .verify(Status.OK); + test.givenUser(USER).grantPrivilegeToRole(SELECT_ON_DB_WGRANT, ROLE) + .whenRequestStorePrivilegesReturn(Collections.singleton(ALL_ON_DB_WGRANT)) + .verify(Status.OK); + test.givenUser(USER).grantPrivilegeToRole(SELECT_ON_DB_WGRANT, ROLE) + .whenRequestStorePrivilegesReturn(Collections.singleton(SELECT_ON_DB_WGRANT)) + .verify(Status.OK); + } + + @Test + public void testRevokeFromRoleWithGrantCheck() throws Exception { + final String DB = "db"; + final String ROLE = "role"; + final String USER = "user"; + + SentryPolicyStoreProcessorTestUtils test = new SentryPolicyStoreProcessorTestUtils(conf, sentryStore); + + final TSentryPrivilege ALL_ON_DB = test.newPrivilegeOnDatabase(ALL, SERVERNAME, DB); + final TSentryPrivilege ALL_ON_DB_WGRANT = test.newPrivilegeOnDatabaseWithGrant(ALL, SERVERNAME, DB); + final TSentryPrivilege SELECT_ON_DB = test.newPrivilegeOnDatabase(SELECT, SERVERNAME, DB); + final TSentryPrivilege SELECT_ON_DB_WGRANT = test.newPrivilegeOnDatabaseWithGrant(SELECT, SERVERNAME, DB); + + // Admin user can revoke privileges + test.givenUser(ADMIN_USER).revokePrivilegeFromRole(ALL_ON_DB, ROLE) + .verify(Status.OK); + test.givenUser(ADMIN_USER).revokePrivilegeFromRole(ALL_ON_DB_WGRANT, ROLE) + .verify(Status.OK); + test.givenUser(ADMIN_USER).revokePrivilegeFromRole(SELECT_ON_DB, ROLE) + .verify(Status.OK); + test.givenUser(ADMIN_USER).revokePrivilegeFromRole(SELECT_ON_DB_WGRANT, ROLE) + .verify(Status.OK); + + // User without grant option cannot revoke privileges + test.givenUser(USER).revokePrivilegeFromRole(ALL_ON_DB, ROLE) + .whenRequestStorePrivilegesReturn(Collections.emptySet()) + .verify(Status.ACCESS_DENIED); + test.givenUser(USER).revokePrivilegeFromRole(ALL_ON_DB_WGRANT, ROLE) + .whenRequestStorePrivilegesReturn(Collections.emptySet()) + .verify(Status.ACCESS_DENIED); + test.givenUser(USER).revokePrivilegeFromRole(SELECT_ON_DB, ROLE) + .whenRequestStorePrivilegesReturn(Collections.emptySet()) + .verify(Status.ACCESS_DENIED); + test.givenUser(USER).revokePrivilegeFromRole(SELECT_ON_DB_WGRANT, ROLE) + .whenRequestStorePrivilegesReturn(Collections.emptySet()) + .verify(Status.ACCESS_DENIED); + test.givenUser(USER).revokePrivilegeFromRole(ALL_ON_DB, ROLE) + .whenRequestStorePrivilegesReturn(Collections.singleton(SELECT_ON_DB_WGRANT)) + .verify(Status.ACCESS_DENIED); + + // User with grant option can revoke privileges + test.givenUser(USER).revokePrivilegeFromRole(ALL_ON_DB, ROLE) + .whenRequestStorePrivilegesReturn(Collections.singleton(ALL_ON_DB_WGRANT)) + .verify(Status.OK); + test.givenUser(USER).revokePrivilegeFromRole(SELECT_ON_DB, ROLE) + .whenRequestStorePrivilegesReturn(Collections.singleton(ALL_ON_DB_WGRANT)) + .verify(Status.OK); + test.givenUser(USER).revokePrivilegeFromRole(SELECT_ON_DB, ROLE) + .whenRequestStorePrivilegesReturn(Collections.singleton(SELECT_ON_DB_WGRANT)) + .verify(Status.OK); + + // User with grant option can revoke privileges with grant option + test.givenUser(USER).revokePrivilegeFromRole(ALL_ON_DB_WGRANT, ROLE) + .whenRequestStorePrivilegesReturn(Collections.singleton(ALL_ON_DB_WGRANT)) + .verify(Status.OK); + test.givenUser(USER).revokePrivilegeFromRole(SELECT_ON_DB_WGRANT, ROLE) + .whenRequestStorePrivilegesReturn(Collections.singleton(ALL_ON_DB_WGRANT)) + .verify(Status.OK); + test.givenUser(USER).revokePrivilegeFromRole(SELECT_ON_DB_WGRANT, ROLE) + .whenRequestStorePrivilegesReturn(Collections.singleton(SELECT_ON_DB_WGRANT)) + .verify(Status.OK); + } + + @Test + public void testGrantCheckWithColumn() throws Exception { + final String DB = "db"; + final String TABLE = "table"; + final String COLUMN = "column"; + final String ROLE = "role"; + final String USER = "user"; + + SentryPolicyStoreProcessorTestUtils test = new SentryPolicyStoreProcessorTestUtils(conf, sentryStore); + + final TSentryPrivilege SELECT_ON_TABLE = test.newPrivilegeOnTable(SELECT, SERVERNAME, DB, TABLE); + final TSentryPrivilege SELECT_ON_TABLE_WGRANT = test.newPrivilegeOnTableWithGrant(SELECT, SERVERNAME, DB, TABLE); + final TSentryPrivilege SELECT_ON_COLUMN = test.newPrivilegeOnColumn(SELECT, SERVERNAME, DB, TABLE, COLUMN); + + // Admin user can revoke privileges + test.givenUser(ADMIN_USER).grantPrivilegeToRole(SELECT_ON_TABLE_WGRANT, ROLE) + .verify(Status.OK); + test.givenUser(ADMIN_USER).grantPrivilegeToRole(SELECT_ON_COLUMN, ROLE) + .verify(Status.OK); + + // User with grant option on table can grant select on a column + test.givenUser(USER).grantPrivilegeToRole(SELECT_ON_COLUMN, ROLE) + .whenRequestStorePrivilegesReturn(Collections.singleton(SELECT_ON_TABLE_WGRANT)) + .verify(Status.OK); + + // User without grant option on table cannot grant select on a column + test.givenUser(USER).grantPrivilegeToRole(SELECT_ON_COLUMN, ROLE) + .whenRequestStorePrivilegesReturn(Collections.singleton(SELECT_ON_TABLE)) + .verify(Status.ACCESS_DENIED); + } } http://git-wip-us.apache.org/repos/asf/sentry/blob/8e050570/sentry-service/sentry-service-server/src/test/java/org/apache/sentry/provider/db/service/persistent/TestHMSFollowerSentryStoreIntegration.java ---------------------------------------------------------------------- diff --git a/sentry-service/sentry-service-server/src/test/java/org/apache/sentry/provider/db/service/persistent/TestHMSFollowerSentryStoreIntegration.java b/sentry-service/sentry-service-server/src/test/java/org/apache/sentry/provider/db/service/persistent/TestHMSFollowerSentryStoreIntegration.java index a886836..1d87b0b 100644 --- a/sentry-service/sentry-service-server/src/test/java/org/apache/sentry/provider/db/service/persistent/TestHMSFollowerSentryStoreIntegration.java +++ b/sentry-service/sentry-service-server/src/test/java/org/apache/sentry/provider/db/service/persistent/TestHMSFollowerSentryStoreIntegration.java @@ -154,7 +154,6 @@ public class TestHMSFollowerSentryStoreIntegration { // configure permission of the table String roleName1 = "list-privs-r1"; - String grantor = "g1"; sentryStore.createSentryRole(roleName1); TSentryPrivilege privilege_tbl1 = new TSentryPrivilege(); @@ -177,11 +176,11 @@ public class TestHMSFollowerSentryStoreIntegration { privilege_server.setServerName(serverName1); privilege_server.setCreateTime(System.currentTimeMillis()); - sentryStore.alterSentryGrantPrivilege(grantor, ServiceConstants.SentryPrincipalType.ROLE, roleName1, privilege1, null); + sentryStore.alterSentryGrantPrivilege(ServiceConstants.SentryPrincipalType.ROLE, roleName1, privilege1, null); - sentryStore.alterSentryGrantPrivilege(grantor, ServiceConstants.SentryPrincipalType.ROLE, roleName1, privilege1_2, null); - sentryStore.alterSentryGrantPrivilege(grantor, ServiceConstants.SentryPrincipalType.ROLE, roleName1, privilege_server, null); - sentryStore.alterSentryGrantPrivilege(grantor, ServiceConstants.SentryPrincipalType.ROLE, roleName1, privilege1_3, null); + sentryStore.alterSentryGrantPrivilege(ServiceConstants.SentryPrincipalType.ROLE, roleName1, privilege1_2, null); + sentryStore.alterSentryGrantPrivilege(ServiceConstants.SentryPrincipalType.ROLE, roleName1, privilege_server, null); + sentryStore.alterSentryGrantPrivilege(ServiceConstants.SentryPrincipalType.ROLE, roleName1, privilege1_3, null); // Create notification events to drop the table StorageDescriptor sd = new StorageDescriptor(); @@ -214,7 +213,6 @@ public class TestHMSFollowerSentryStoreIntegration { // configure permission of the database String roleName1 = "list-privs-r1"; - String grantor = "g1"; sentryStore.createSentryRole(roleName1); TSentryPrivilege privilege_tbl1 = new TSentryPrivilege(); @@ -237,11 +235,11 @@ public class TestHMSFollowerSentryStoreIntegration { privilege_server.setServerName(serverName1); privilege_server.setCreateTime(System.currentTimeMillis()); - sentryStore.alterSentryGrantPrivilege(grantor, ServiceConstants.SentryPrincipalType.ROLE, roleName1, privilege1, null); + sentryStore.alterSentryGrantPrivilege(ServiceConstants.SentryPrincipalType.ROLE, roleName1, privilege1, null); - sentryStore.alterSentryGrantPrivilege(grantor, ServiceConstants.SentryPrincipalType.ROLE, roleName1, privilege1_2, null); - sentryStore.alterSentryGrantPrivilege(grantor, ServiceConstants.SentryPrincipalType.ROLE, roleName1, privilege_server, null); - sentryStore.alterSentryGrantPrivilege(grantor, ServiceConstants.SentryPrincipalType.ROLE, roleName1, privilege1_3, null); + sentryStore.alterSentryGrantPrivilege(ServiceConstants.SentryPrincipalType.ROLE, roleName1, privilege1_2, null); + sentryStore.alterSentryGrantPrivilege(ServiceConstants.SentryPrincipalType.ROLE, roleName1, privilege_server, null); + sentryStore.alterSentryGrantPrivilege(ServiceConstants.SentryPrincipalType.ROLE, roleName1, privilege1_3, null); // Create notification events to drop the database NotificationEvent notificationEvent = new NotificationEvent(1, 0, EventType.DROP_DATABASE.toString(),
