This is an automated email from the ASF dual-hosted git repository. stigahuang pushed a commit to branch master in repository https://gitbox.apache.org/repos/asf/impala.git
commit 8e3c99b801454433bbda98943d5ea6f4a3eb5d08 Author: Fang-Yu Rao <[email protected]> AuthorDate: Wed Dec 24 16:17:50 2025 -0800 IMPALA-14085: Implement GRANT/REVOKE ROLE TO/FROM a user As a follow-up to IMPALA-10211, this patch adds the support for the following role-related statements. 1. GRANT ROLE <role_name> TO USER <user_name>. 2. REVOKE ROLE <role_name> FROM USER <user_name>. 3. SHOW ROLE GRANT USER <user_name>. Testing: - Extended the test file grant_revoke.test to additionally cover the scenario in which the grantee/revokee is a user. - Added an end-to-end test to briefly verify the statements SHOW ROLE GRANT USER <user_name> and SHOW CURRENT ROLES are working as expected. Change-Id: Ie5a16aeb4bbf8637ad326a1ec3d5fce1b196d73f Reviewed-on: http://gerrit.cloudera.org:8080/23815 Tested-by: Impala Public Jenkins <[email protected]> Reviewed-by: Fang-Yu Rao <[email protected]> --- common/thrift/Frontend.thrift | 9 +- common/thrift/JniCatalog.thrift | 7 + fe/src/main/cup/sql-parser.cup | 27 ++- .../impala/analysis/GrantRevokeRoleStmt.java | 36 +++- .../org/apache/impala/analysis/ShowRolesStmt.java | 23 ++- .../impala/authorization/AuthorizationManager.java | 4 +- .../authorization/NoopAuthorizationFactory.java | 6 +- .../ranger/RangerCatalogdAuthorizationManager.java | 61 ++++--- .../ranger/RangerImpaladAuthorizationManager.java | 36 ++-- .../apache/impala/service/CatalogOpExecutor.java | 24 ++- .../java/org/apache/impala/util/CatalogOpUtil.java | 9 +- .../org/apache/impala/util/CatalogOpUtilTest.java | 20 +- .../queries/QueryTest/grant_revoke.test | 117 ++++++------ tests/authorization/test_ranger.py | 203 ++++++++++++++------- 14 files changed, 386 insertions(+), 196 deletions(-) diff --git a/common/thrift/Frontend.thrift b/common/thrift/Frontend.thrift index affb317ae..9632c0b7e 100644 --- a/common/thrift/Frontend.thrift +++ b/common/thrift/Frontend.thrift @@ -307,7 +307,8 @@ struct TShowFilesParams { 3: optional list<string> selected_files } -// Parameters for SHOW [CURRENT] ROLES and SHOW ROLE GRANT GROUP <groupName> commands +// Parameters for SHOW [CURRENT] ROLES, SHOW ROLE GRANT GROUP <groupName> and +// SHOW ROLE GRANT USER <userName> commands struct TShowRolesParams { // The effective user who submitted this request. 1: optional string requesting_user @@ -320,9 +321,11 @@ struct TShowRolesParams { // True if the statement is "SHOW CURRENT ROLES". 3: required bool is_show_current_roles - // Filters roles to the specified grant group. If null or not set, show roles for all - // groups. + // Filters roles to the specified grant group. 4: optional string grant_group + + // Filters roles to the specified grant user. + 5: optional string grant_user } // Result of a SHOW ROLES command diff --git a/common/thrift/JniCatalog.thrift b/common/thrift/JniCatalog.thrift index 53a55bcd2..b3c871fa7 100644 --- a/common/thrift/JniCatalog.thrift +++ b/common/thrift/JniCatalog.thrift @@ -736,10 +736,17 @@ struct TGrantRevokeRoleParams { 1: required list<string> role_names // The group names that are being granted/revoked. + // When 'user_names' is empty, the grantee/revokee would be a group, and thus + // 'group_names' would consist of exactly one non-empty name of the grantee/revokee. 2: required list<string> group_names // True if this is a GRANT statement false if this is a REVOKE statement. 3: required bool is_grant + + // The user names that are being granted/revoked. + // When 'group_names' is empty, the grantee/revokee would be a user, and thus + // 'user_names' would consist of exactly one non-empty name of the grantee/revokee. + 4: optional list<string> user_names } // Parameters for GRANT/REVOKE privilege TO/FROM user/role. diff --git a/fe/src/main/cup/sql-parser.cup b/fe/src/main/cup/sql-parser.cup index 14a10cd3d..cb4449230 100755 --- a/fe/src/main/cup/sql-parser.cup +++ b/fe/src/main/cup/sql-parser.cup @@ -1170,11 +1170,16 @@ opt_kw_table ::= show_roles_stmt ::= KW_SHOW KW_ROLES - {: RESULT = new ShowRolesStmt(false, null); :} + {: RESULT = new ShowRolesStmt(false, null, null); :} | KW_SHOW KW_ROLE KW_GRANT KW_GROUP ident_or_unreserved:group - {: RESULT = new ShowRolesStmt(false, group); :} + {: RESULT = new ShowRolesStmt(false, group, null); :} + | KW_SHOW KW_ROLE KW_GRANT IDENT:user_id ident_or_unreserved:user + {: + parser.checkIdentKeyword("USER", user_id); + RESULT = new ShowRolesStmt(false, null, user); + :} | KW_SHOW KW_CURRENT KW_ROLES - {: RESULT = new ShowRolesStmt(true, null); :} + {: RESULT = new ShowRolesStmt(true, null, null); :} ; show_grant_principal_stmt ::= @@ -1236,12 +1241,24 @@ create_drop_role_stmt ::= grant_role_stmt ::= KW_GRANT KW_ROLE ident_or_unreserved:role KW_TO KW_GROUP ident_or_unreserved:group - {: RESULT = new GrantRevokeRoleStmt(role, group, true); :} + {: RESULT = new GrantRevokeRoleStmt(role, group, null, true); :} + | KW_GRANT KW_ROLE ident_or_unreserved:role KW_TO IDENT:user_id + ident_or_unreserved:user + {: + parser.checkIdentKeyword("USER", user_id); + RESULT = new GrantRevokeRoleStmt(role, null, user, true); + :} ; revoke_role_stmt ::= KW_REVOKE KW_ROLE ident_or_unreserved:role KW_FROM KW_GROUP ident_or_unreserved:group - {: RESULT = new GrantRevokeRoleStmt(role, group, false); :} + {: RESULT = new GrantRevokeRoleStmt(role, group, null, false); :} + | KW_REVOKE KW_ROLE ident_or_unreserved:role KW_FROM IDENT:user_id + ident_or_unreserved:user + {: + parser.checkIdentKeyword("USER", user_id); + RESULT = new GrantRevokeRoleStmt(role, null, user, false); + :} ; // For backwards compatibility, a grant without the principal type will default to diff --git a/fe/src/main/java/org/apache/impala/analysis/GrantRevokeRoleStmt.java b/fe/src/main/java/org/apache/impala/analysis/GrantRevokeRoleStmt.java index 170a24d7e..28581e3ec 100644 --- a/fe/src/main/java/org/apache/impala/analysis/GrantRevokeRoleStmt.java +++ b/fe/src/main/java/org/apache/impala/analysis/GrantRevokeRoleStmt.java @@ -29,27 +29,42 @@ import com.google.common.collect.Lists; public class GrantRevokeRoleStmt extends AuthorizationStmt { private final String roleName_; private final String groupName_; + private final String userName_; private final boolean isGrantStmt_; + private final boolean principalIsGroup_; - public GrantRevokeRoleStmt(String roleName, String groupName, boolean isGrantStmt) { + public GrantRevokeRoleStmt(String roleName, String groupName, + String userName, boolean isGrantStmt) { Preconditions.checkNotNull(roleName); - Preconditions.checkNotNull(groupName); + Preconditions.checkArgument((groupName != null && userName == null) || + (groupName == null && userName != null)); roleName_ = roleName; groupName_ = groupName; + userName_ = userName; isGrantStmt_ = isGrantStmt; + // If 'userName_' is null, then the grantee/revokee is a group. + principalIsGroup_ = (userName_ == null); } @Override public String toSql(ToSqlOptions options) { - return String.format("%s ROLE %s %s GROUP %s", + return String.format("%s ROLE %s %s %s %s", isGrantStmt_ ? "GRANT" : "REVOKE", roleName_, - isGrantStmt_ ? "TO" : "FROM", groupName_); + isGrantStmt_ ? "TO" : "FROM", + principalIsGroup_ ? "GROUP" : "USER", + principalIsGroup_ ? groupName_ : userName_); } public TGrantRevokeRoleParams toThrift() { TGrantRevokeRoleParams params = new TGrantRevokeRoleParams(); params.setRole_names(Lists.newArrayList(roleName_)); - params.setGroup_names(Lists.newArrayList(groupName_)); + if (principalIsGroup_) { + params.setGroup_names(Lists.newArrayList(groupName_)); + params.setUser_names(Lists.newArrayList()); + } else { + params.setGroup_names(Lists.newArrayList()); + params.setUser_names(Lists.newArrayList(userName_)); + } params.setIs_grant(isGrantStmt_); return params; } @@ -60,8 +75,15 @@ public class GrantRevokeRoleStmt extends AuthorizationStmt { if (Strings.isNullOrEmpty(roleName_)) { throw new AnalysisException("Role name in GRANT/REVOKE ROLE cannot be empty."); } - if (Strings.isNullOrEmpty(groupName_)) { - throw new AnalysisException("Group name in GRANT/REVOKE ROLE cannot be empty."); + if (principalIsGroup_ && Strings.isNullOrEmpty(groupName_)) { + throw new AnalysisException(String.format("Group name cannot be empty in " + + "%s ROLE %s GROUP.", isGrantStmt_ ? "GRANT" : "REVOKE", + isGrantStmt_ ? "TO" : "FROM")); + } + if (!principalIsGroup_ && Strings.isNullOrEmpty(userName_)) { + throw new AnalysisException(String.format("User name cannot be empty in " + + "%s ROLE %s USER.", isGrantStmt_ ? "GRANT" : "REVOKE", + isGrantStmt_ ? "TO" : "FROM")); } } diff --git a/fe/src/main/java/org/apache/impala/analysis/ShowRolesStmt.java b/fe/src/main/java/org/apache/impala/analysis/ShowRolesStmt.java index 9c84d1366..ce66a2309 100644 --- a/fe/src/main/java/org/apache/impala/analysis/ShowRolesStmt.java +++ b/fe/src/main/java/org/apache/impala/analysis/ShowRolesStmt.java @@ -31,26 +31,36 @@ public class ShowRolesStmt extends AuthorizationStmt { // If null, all roles will be shown. Otherwise only roles granted to this // group will be shown. private final String groupName_; + private final String userName_; private final boolean isShowCurrentRoles_; // Set during analysis. private User requestingUser_; - public ShowRolesStmt(boolean isShowCurrentRoles, String groupName) { - // An empty group name should never be possible since group name is an identifier - // and Impala does not allow empty identifiers. + public ShowRolesStmt(boolean isShowCurrentRoles, String groupName, String userName) { + // For SHOW CURRENT ROLES, 'groupName' and 'userName' should be null. Preconditions.checkState(!isShowCurrentRoles || - (groupName == null || !groupName.isEmpty())); + (groupName == null && userName == null)); + // 'groupName' and 'userName' should not both be set whether 'isShowCurrentRoles' is + // true. + Preconditions.checkState(groupName == null || userName == null); + // An empty 'groupName' or 'userName' should never be possible since 'groupName' and + // 'userName' should be an identifier and Impala does not allow empty identifiers. + Preconditions.checkState(groupName == null || !groupName.isEmpty()); + Preconditions.checkState(userName == null || !userName.isEmpty()); groupName_ = groupName; + userName_ = userName; isShowCurrentRoles_ = isShowCurrentRoles; } @Override public String toSql(ToSqlOptions options) { - if (groupName_ == null) { + if (groupName_ == null && userName_ == null) { return isShowCurrentRoles_ ? "SHOW CURRENT ROLES" : "SHOW ROLES"; } else { - return "SHOW ROLE GRANT GROUP " + groupName_; + String granteeType = groupName_ != null ? "GROUP" : "USER"; + String granteeName = groupName_ != null ? groupName_ : userName_; + return "SHOW ROLE GRANT " + granteeType + " " + granteeName; } } @@ -59,6 +69,7 @@ public class ShowRolesStmt extends AuthorizationStmt { params.setRequesting_user(requestingUser_.getShortName()); params.setIs_show_current_roles(isShowCurrentRoles_); if (groupName_ != null) params.setGrant_group(groupName_); + if (userName_ != null) params.setGrant_user(userName_); // Users should always be able to execute SHOW CURRENT ROLES. return params; } diff --git a/fe/src/main/java/org/apache/impala/authorization/AuthorizationManager.java b/fe/src/main/java/org/apache/impala/authorization/AuthorizationManager.java index d8d7b0f5b..b8103b365 100644 --- a/fe/src/main/java/org/apache/impala/authorization/AuthorizationManager.java +++ b/fe/src/main/java/org/apache/impala/authorization/AuthorizationManager.java @@ -56,13 +56,13 @@ public interface AuthorizationManager { /** * Grants a role to a group. */ - void grantRoleToGroup(User requestingUser, TGrantRevokeRoleParams params, + void grantRoleToGroupOrUser(User requestingUser, TGrantRevokeRoleParams params, TDdlExecResponse response) throws ImpalaException; /** * Revokes a role from a group. */ - void revokeRoleFromGroup(User requestingUser, TGrantRevokeRoleParams params, + void revokeRoleFromGroupOrUser(User requestingUser, TGrantRevokeRoleParams params, TDdlExecResponse response) throws ImpalaException; /** diff --git a/fe/src/main/java/org/apache/impala/authorization/NoopAuthorizationFactory.java b/fe/src/main/java/org/apache/impala/authorization/NoopAuthorizationFactory.java index 8714b3294..51ffcb27d 100644 --- a/fe/src/main/java/org/apache/impala/authorization/NoopAuthorizationFactory.java +++ b/fe/src/main/java/org/apache/impala/authorization/NoopAuthorizationFactory.java @@ -97,15 +97,15 @@ public class NoopAuthorizationFactory implements AuthorizationFactory { } @Override - public void grantRoleToGroup(User requestingUser, TGrantRevokeRoleParams params, + public void grantRoleToGroupOrUser(User requestingUser, TGrantRevokeRoleParams params, TDdlExecResponse response) throws ImpalaException { throw new UnsupportedOperationException(String.format("%s is not supported", ClassUtil.getMethodName())); } @Override - public void revokeRoleFromGroup(User requestingUser, TGrantRevokeRoleParams params, - TDdlExecResponse response) throws ImpalaException { + public void revokeRoleFromGroupOrUser(User requestingUser, + TGrantRevokeRoleParams params, TDdlExecResponse response) throws ImpalaException { throw new UnsupportedOperationException(String.format("%s is not supported", ClassUtil.getMethodName())); } diff --git a/fe/src/main/java/org/apache/impala/authorization/ranger/RangerCatalogdAuthorizationManager.java b/fe/src/main/java/org/apache/impala/authorization/ranger/RangerCatalogdAuthorizationManager.java index 344aab08d..9233b8b54 100644 --- a/fe/src/main/java/org/apache/impala/authorization/ranger/RangerCatalogdAuthorizationManager.java +++ b/fe/src/main/java/org/apache/impala/authorization/ranger/RangerCatalogdAuthorizationManager.java @@ -18,6 +18,7 @@ package org.apache.impala.authorization.ranger; import com.google.common.annotations.VisibleForTesting; +import com.google.common.base.Preconditions; import org.apache.hadoop.hive.metastore.api.PrincipalType; import org.apache.impala.authorization.AuthorizationDelta; import org.apache.impala.authorization.AuthorizationManager; @@ -138,12 +139,18 @@ public class RangerCatalogdAuthorizationManager implements AuthorizationManager } @Override - public void grantRoleToGroup(User requestingUser, TGrantRevokeRoleParams params, + public void grantRoleToGroupOrUser(User requestingUser, TGrantRevokeRoleParams params, TDdlExecResponse response) throws ImpalaException { + Preconditions.checkState( + (params.getGroup_names().size() == 1 && params.getUser_names().size() == 0) || + (params.getGroup_names().size() == 0 && params.getUser_names().size() == 1)); GrantRevokeRoleRequest request = createGrantRevokeRoleRequest( requestingUser.getShortName(), new HashSet<>(params.getRole_names()), - new HashSet<>(params.getGroup_names())); - + params.getGroup_names(), params.getUser_names()); + boolean isGranteeGroup = params.getUser_names().isEmpty(); + String granteeType = isGranteeGroup ? "group" : "user"; + String granteeName = isGranteeGroup ? params.getGroup_names().get(0) : + params.getUser_names().get(0); try { // We found that granting a role to a group that is already assigned the role would // actually revoke the role from the group. This should be considered a bug of @@ -171,25 +178,27 @@ public class RangerCatalogdAuthorizationManager implements AuthorizationManager // We note that we will also get this message when a Ranger administrator is // trying to grant a non-existing role to a group whether or not this group // exists. - LOG.error("Error granting role {} to group {} by user {} in Ranger. " + + LOG.error("Error granting role {} to {} {} by user {} in Ranger. " + "Ranger error message: HTTP 400 Error: User doesn't have permissions to " + "grant role " + params.getRole_names().get(0), params.getRole_names().get(0), - params.getGroup_names().get(0), requestingUser.getShortName()); + granteeType, granteeName, + requestingUser.getShortName()); throw new InternalException("Error granting role " + - params.getRole_names().get(0) + " to group " + - params.getGroup_names().get(0) + " by user " + - requestingUser.getShortName() + " in Ranger. " + + params.getRole_names().get(0) + " to " + + granteeType + " " + granteeName + + " by user " + requestingUser.getShortName() + " in Ranger. " + "Ranger error message: HTTP 400 Error: User doesn't have permissions to " + "grant role " + params.getRole_names().get(0)); } else { // When a Ranger administrator tries to grant an existing role to a non-existing // group, we will get this error. - LOG.error("Error granting role {} to group {} by user {} in Ranger. " + + LOG.error("Error granting role {} to {} {} by user {} in Ranger. " + "Ranger error message: " + e.getMessage(), params.getRole_names().get(0), - params.getGroup_names().get(0), requestingUser.getShortName()); + granteeType, granteeName, + requestingUser.getShortName()); throw new InternalException("Error granting role " + - params.getRole_names().get(0) + " to group " + - params.getGroup_names().get(0) + " by user " + + params.getRole_names().get(0) + " to " + + granteeType + " " + granteeName + " by user " + requestingUser.getShortName() + " in Ranger. " + "Ranger error message: " + e.getMessage()); } @@ -200,21 +209,27 @@ public class RangerCatalogdAuthorizationManager implements AuthorizationManager } @Override - public void revokeRoleFromGroup(User requestingUser, TGrantRevokeRoleParams params, - TDdlExecResponse response) throws ImpalaException { + public void revokeRoleFromGroupOrUser(User requestingUser, + TGrantRevokeRoleParams params, TDdlExecResponse response) throws ImpalaException { + Preconditions.checkState( + (params.getGroup_names().size() == 1 && params.getUser_names().size() == 0) || + (params.getGroup_names().size() == 0 && params.getUser_names().size() == 1)); GrantRevokeRoleRequest request = createGrantRevokeRoleRequest( requestingUser.getShortName(), new HashSet<>(params.getRole_names()), - new HashSet<>(params.getGroup_names())); - + params.getGroup_names(), params.getUser_names()); + boolean isRevokeeGroup = params.getUser_names().isEmpty(); + String revokeeType = isRevokeeGroup ? "group" : "user"; + String revokeeName = isRevokeeGroup ? params.getGroup_names().get(0) : + params.getUser_names().get(0); try { plugin_.get().revokeRole(request, /*resultProcessor*/ null); } catch (Exception e) { - LOG.error("Error revoking role {} from group {} by user {} in Ranger. " + + LOG.error("Error revoking role {} from {} {} by user {} in Ranger. " + "Ranger error message: " + e.getMessage(), params.getRole_names().get(0), - params.getGroup_names().get(0), requestingUser.getShortName()); + revokeeType, revokeeName, requestingUser.getShortName()); throw new InternalException("Error revoking role " + - params.getRole_names().get(0) + " from group " + - params.getGroup_names().get(0) + " by user " + + params.getRole_names().get(0) + " from " + + revokeeType + " " + revokeeName + " by user " + requestingUser.getShortName() + " in Ranger. " + "Ranger error message: " + e.getMessage()); } @@ -544,11 +559,13 @@ public class RangerCatalogdAuthorizationManager implements AuthorizationManager * corresponding to 'targetRoleNames' to/from groups associated with 'groupNames'. */ private static GrantRevokeRoleRequest createGrantRevokeRoleRequest( - String grantor, Set<String> targetRoleNames, Set<String> groupNames) { + String grantor, Set<String> targetRoleNames, List<String> groupNames, + List<String> userNames) { GrantRevokeRoleRequest request = new GrantRevokeRoleRequest(); request.setGrantor(grantor); request.setTargetRoles(targetRoleNames); - request.setGroups(groupNames); + if (groupNames != null) { request.setGroups(new HashSet<>(groupNames)); } + if (userNames != null) { request.setUsers(new HashSet<>(userNames)); } // We do not set the field of 'grantOption' since WITH GRANT OPTION is not supported // when granting/revoking roles. By default, 'grantOption' is set to Boolean.FALSE so // that a user in a group assigned a role is not able to grant/revoke the role to/from diff --git a/fe/src/main/java/org/apache/impala/authorization/ranger/RangerImpaladAuthorizationManager.java b/fe/src/main/java/org/apache/impala/authorization/ranger/RangerImpaladAuthorizationManager.java index 3747c5d74..e542aaa07 100644 --- a/fe/src/main/java/org/apache/impala/authorization/ranger/RangerImpaladAuthorizationManager.java +++ b/fe/src/main/java/org/apache/impala/authorization/ranger/RangerImpaladAuthorizationManager.java @@ -103,7 +103,8 @@ public class RangerImpaladAuthorizationManager implements AuthorizationManager { /** * This method will be called for the statements of 1) SHOW ROLES, - * 2) SHOW CURRENT ROLES, or 3) SHOW ROLE GRANT GROUP <group_name>. + * 2) SHOW CURRENT ROLES, 3) SHOW ROLE GRANT GROUP <group_name>, + * or, 4) SHOW ROLE GRANT USER <user_name>. */ @Override public TShowRolesResult getRoles(TShowRolesParams params) throws ImpalaException { @@ -112,23 +113,31 @@ public class RangerImpaladAuthorizationManager implements AuthorizationManager { Set<String> groups = RangerUtil.getGroups(params.getRequesting_user()); boolean adminOp = - !(groups.contains(params.getGrant_group()) || params.is_show_current_roles); + !(groups.contains(params.getGrant_group()) + || params.getRequesting_user().equals(params.getGrant_user()) + || params.is_show_current_roles); if (adminOp) { RangerUtil.validateRangerAdmin(plugin_.get(), params.getRequesting_user()); } - // The branch for SHOW CURRENT ROLES and SHOW ROLE GRANT GROUP. + // The branch for SHOW CURRENT ROLES, SHOW ROLE GRANT GROUP/USER. Set<String> roleNames; - if (params.isIs_show_current_roles() || params.isSetGrant_group()) { + if (params.isIs_show_current_roles() || params.isSetGrant_group() || + params.isSetGrant_user()) { Set<String> groupNames; if (params.isIs_show_current_roles()) { - groupNames = groups; - } else { - Preconditions.checkState(params.isSetGrant_group()); + roleNames = plugin_.get().getRolesFromUserAndGroups( + params.getRequesting_user(), groups); + } else if (params.isSetGrant_group()) { groupNames = Sets.newHashSet(params.getGrant_group()); + roleNames = plugin_.get().getRolesFromUserAndGroups(/* user */ null, + groupNames); + } else { + Preconditions.checkState(params.isSetGrant_user()); + roleNames = plugin_.get().getRolesFromUserAndGroups(params.getGrant_user(), + /* group */ null); } - roleNames = plugin_.get().getRolesFromUserAndGroups(null, groupNames); } else { // The branch for SHOW ROLES. Preconditions.checkState(!params.isIs_show_current_roles()); @@ -154,6 +163,11 @@ public class RangerImpaladAuthorizationManager implements AuthorizationManager { "."); throw new InternalException("Error executing SHOW ROLE GRANT GROUP " + params.getGrant_group() + ". Ranger error message: " + e.getMessage()); + } else if (params.isSetGrant_user()) { + LOG.error("Error executing SHOW ROLE GRANT USER " + params.getGrant_user() + + "."); + throw new InternalException("Error executing SHOW ROLE GRANT USER " + + params.getGrant_user() + ". Ranger error message: " + e.getMessage()); } else { LOG.error("Error executing SHOW ROLES."); throw new InternalException("Error executing SHOW ROLES." @@ -163,15 +177,15 @@ public class RangerImpaladAuthorizationManager implements AuthorizationManager { } @Override - public void grantRoleToGroup(User requestingUser, TGrantRevokeRoleParams params, + public void grantRoleToGroupOrUser(User requestingUser, TGrantRevokeRoleParams params, TDdlExecResponse response) throws ImpalaException { throw new UnsupportedOperationException(String.format( "%s is not supported in Impalad", ClassUtil.getMethodName())); } @Override - public void revokeRoleFromGroup(User requestingUser, TGrantRevokeRoleParams params, - TDdlExecResponse response) throws ImpalaException { + public void revokeRoleFromGroupOrUser(User requestingUser, + TGrantRevokeRoleParams params, TDdlExecResponse response) throws ImpalaException { throw new UnsupportedOperationException(String.format( "%s is not supported in Impalad", ClassUtil.getMethodName())); } diff --git a/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java b/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java index a8984499d..7c72531a7 100644 --- a/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java +++ b/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java @@ -611,17 +611,25 @@ public class CatalogOpExecutor { TGrantRevokeRoleParams grant_role_params = ddlRequest.getGrant_revoke_role_params(); tTableName = Optional.of(new TTableName( - StringUtils.join(",", grant_role_params.group_names), "")); + StringUtils.join(",", + grant_role_params.getUser_names().isEmpty() ? + grant_role_params.getGroup_names() : + grant_role_params.getUser_names()), + /* table_name */ "")); catalogOpTracker_.increment(ddlRequest, tTableName); - grantRoleToGroup(requestingUser, grant_role_params, response); + grantRoleToGroupOrUser(requestingUser, grant_role_params, response); break; case REVOKE_ROLE: TGrantRevokeRoleParams revoke_role_params = ddlRequest.getGrant_revoke_role_params(); tTableName = Optional.of(new TTableName( - StringUtils.join(",", revoke_role_params.group_names), "")); + StringUtils.join(",", + revoke_role_params.getUser_names().isEmpty() ? + revoke_role_params.getGroup_names() : + revoke_role_params.getUser_names()), + /* table_name */ "")); catalogOpTracker_.increment(ddlRequest, tTableName); - revokeRoleFromGroup( + revokeRoleFromGroupOrUser( requestingUser, revoke_role_params, response); break; case GRANT_PRIVILEGE: @@ -7081,28 +7089,28 @@ public class CatalogOpExecutor { /** * Grants a role to the given group on behalf of the requestingUser. */ - private void grantRoleToGroup(User requestingUser, + private void grantRoleToGroupOrUser(User requestingUser, TGrantRevokeRoleParams grantRevokeRoleParams, TDdlExecResponse resp) throws ImpalaException { Preconditions.checkNotNull(requestingUser); Preconditions.checkNotNull(grantRevokeRoleParams); Preconditions.checkNotNull(resp); Preconditions.checkArgument(grantRevokeRoleParams.isIs_grant()); - authzManager_.grantRoleToGroup(requestingUser, grantRevokeRoleParams, resp); + authzManager_.grantRoleToGroupOrUser(requestingUser, grantRevokeRoleParams, resp); addSummary(resp, "Role has been granted."); } /** * Revokes a role from the given group on behalf of the requestingUser. */ - private void revokeRoleFromGroup(User requestingUser, + private void revokeRoleFromGroupOrUser(User requestingUser, TGrantRevokeRoleParams grantRevokeRoleParams, TDdlExecResponse resp) throws ImpalaException { Preconditions.checkNotNull(requestingUser); Preconditions.checkNotNull(grantRevokeRoleParams); Preconditions.checkNotNull(resp); Preconditions.checkArgument(!grantRevokeRoleParams.isIs_grant()); - authzManager_.revokeRoleFromGroup(requestingUser, grantRevokeRoleParams, resp); + authzManager_.revokeRoleFromGroupOrUser(requestingUser, grantRevokeRoleParams, resp); addSummary(resp, "Role has been revoked."); } diff --git a/fe/src/main/java/org/apache/impala/util/CatalogOpUtil.java b/fe/src/main/java/org/apache/impala/util/CatalogOpUtil.java index c62005d45..fa3e13980 100644 --- a/fe/src/main/java/org/apache/impala/util/CatalogOpUtil.java +++ b/fe/src/main/java/org/apache/impala/util/CatalogOpUtil.java @@ -23,6 +23,7 @@ import org.apache.impala.analysis.TableName; import org.apache.impala.thrift.TAlterTableParams; import org.apache.impala.thrift.TCommentOnParams; import org.apache.impala.thrift.TDdlExecRequest; +import org.apache.impala.thrift.TGrantRevokeRoleParams; import org.apache.impala.thrift.TResetMetadataRequest; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -103,8 +104,12 @@ public class CatalogOpUtil { break; case GRANT_ROLE: case REVOKE_ROLE: - target = req.getGrant_revoke_role_params().getRole_names() + " GROUP " + - req.getGrant_revoke_role_params().getGroup_names(); + boolean principalIsGroup = + req.getGrant_revoke_role_params().getUser_names().isEmpty(); + TGrantRevokeRoleParams params = req.getGrant_revoke_role_params(); + target = req.getGrant_revoke_role_params().getRole_names() + + (principalIsGroup ? " GROUP " : " USER ") + + (principalIsGroup ? params.getGroup_names() : params.getUser_names()); break; case GRANT_PRIVILEGE: target = "TO " + req.getGrant_revoke_priv_params().getPrincipal_name(); diff --git a/fe/src/test/java/org/apache/impala/util/CatalogOpUtilTest.java b/fe/src/test/java/org/apache/impala/util/CatalogOpUtilTest.java index 3849201d1..4555a331b 100644 --- a/fe/src/test/java/org/apache/impala/util/CatalogOpUtilTest.java +++ b/fe/src/test/java/org/apache/impala/util/CatalogOpUtilTest.java @@ -214,15 +214,29 @@ public class CatalogOpUtilTest { CatalogOpUtil.getShortDescForExecDdl(req)); req.setDdl_type(TDdlType.GRANT_ROLE); - TGrantRevokeRoleParams grantRevokeRoleParams; - grantRevokeRoleParams = new GrantRevokeRoleStmt("my_role", "my_group", true) + TGrantRevokeRoleParams grantRevokeRoleGroupParams; + TGrantRevokeRoleParams grantRevokeRoleUserParams; + grantRevokeRoleGroupParams = new GrantRevokeRoleStmt("my_role", "my_group", + /* userName */ null, /* isGrantStmt */ true) .toThrift(); - req.setGrant_revoke_role_params(grantRevokeRoleParams); + grantRevokeRoleUserParams = new GrantRevokeRoleStmt("my_role", /* groupName */ null, + "my_user", /* isGrantStmt */ true) + .toThrift(); + + req.setGrant_revoke_role_params(grantRevokeRoleGroupParams); assertEquals("GRANT_ROLE [my_role] GROUP [my_group] issued by unknown user", CatalogOpUtil.getShortDescForExecDdl(req)); + req.setGrant_revoke_role_params(grantRevokeRoleUserParams); + assertEquals("GRANT_ROLE [my_role] USER [my_user] issued by unknown user", + CatalogOpUtil.getShortDescForExecDdl(req)); + req.setDdl_type(TDdlType.REVOKE_ROLE); + req.setGrant_revoke_role_params(grantRevokeRoleGroupParams); assertEquals("REVOKE_ROLE [my_role] GROUP [my_group] issued by unknown user", CatalogOpUtil.getShortDescForExecDdl(req)); + req.setGrant_revoke_role_params(grantRevokeRoleUserParams); + assertEquals("REVOKE_ROLE [my_role] USER [my_user] issued by unknown user", + CatalogOpUtil.getShortDescForExecDdl(req)); req.setDdl_type(TDdlType.GRANT_PRIVILEGE); TGrantRevokePrivParams grantRevokePrivParams = new GrantRevokePrivStmt("my_role", diff --git a/testdata/workloads/functional-query/queries/QueryTest/grant_revoke.test b/testdata/workloads/functional-query/queries/QueryTest/grant_revoke.test index 47903d071..747891692 100644 --- a/testdata/workloads/functional-query/queries/QueryTest/grant_revoke.test +++ b/testdata/workloads/functional-query/queries/QueryTest/grant_revoke.test @@ -51,12 +51,13 @@ grant all on server to grant_revoke_test_ALL_SERVER ---- USER admin ---- QUERY -# Grant the privilege to the group `$GROUP_NAME`, to which the user 'getuser()' belongs -# so that the user 'getuser()' could perform the some of the following operations on the -# database 'grant_rev_db'. -# Group name will be replaced with the group of 'getuser()' in the test. -# framework. -grant role grant_revoke_test_ALL_SERVER to group `$GROUP_NAME` +# Grant the privilege to the group `$GRANTEE_NAME`, to which the user 'getuser()' +# belongs, or to the user 'getuser()' itself so that the user 'getuser()' could perform +# some of the following operations on the database 'grant_rev_db'. +# $GRANTEE_TYPE will be replaced with 'GROUP' or 'USER' depending on the argument +# 'grantee_type' when we run TestRanger._test_grant_revoke_with_role(). +# $GRANTEE_NAME name will be replaced with 'getuser()' in the test framework. +grant role grant_revoke_test_ALL_SERVER to $GRANTEE_TYPE `$GRANTEE_NAME` ==== ---- USER admin @@ -109,9 +110,12 @@ User does_not_exist does not have permission for this operation ---- USER does_not_exist ---- QUERY -# A user that is not a Ranger administrator should not have privilege to execute -# SHOW ROLE GRANT GROUP for a group the user does not belong to. -show role grant group non_owner +# The following should be true for a user that is not a Ranger administrator. +# When $GRANTEE_TYPE is GROUP, the user should not have the privilege to execute +# SHOW ROLE GRANT for a group the user does not belong to. +# When $GRANTEE_TYPE is USER, the user should not have the privilege to execute +# SHOW ROLE GRANT if the user is executing the query against another user. +show role grant $GRANTEE_TYPE non_owner ---- RESULTS: VERIFY_IS_SUBSET ---- TYPES STRING @@ -121,13 +125,15 @@ User does_not_exist does not have permission for this operation ---- USER non_owner ---- QUERY -# The 'non_owner' user doesn't have any roles granted to them, but since it is part of the -# 'non_owner' group, they should have privileges to execute this statement. +# The 'non_owner' user doesn't have any roles granted to it, but since it is part of +# the 'non_owner' group, it should have privileges to execute this statement +# when $GRANTEE_TYPE is GROUP. When $GRANTEE_TYPE is USER, the 'non_owner' user should +# have the privilege to execute this statement to get its associated roles. # Note that this test case requires that impalad was started with the argument # '--use_customized_user_groups_mapper_for_ranger' so that a customized user-to-groups # mapper will be used to retrieve the groups the user 'non_owner' belongs to, which # consists of exactly one group, the group 'non_owner'. -show role grant group non_owner +show role grant $GRANTEE_TYPE non_owner ---- RESULTS: VERIFY_IS_SUBSET ---- TYPES STRING @@ -179,11 +185,11 @@ STRING ---- USER admin ---- QUERY -# To prevent the user 'getuser()' (which belongs to the group '$GROUP_NAME') from -# performing any operation on the database 'grant_rev_db', we also need to alter -# the owner of 'grant_rev_db' since 'getuser()' is the owner of this database, which by -# default is allowed by Ranger to perform any operation on the database. -revoke role grant_revoke_test_ALL_SERVER from group `$GROUP_NAME`; +# To prevent the user 'getuser()' from performing any operation on the database +# 'grant_rev_db', we also need to alter the owner of 'grant_rev_db' since 'getuser()' is +# the owner of this database, which by default is allowed by Ranger to perform any +# operation on the database. +revoke role grant_revoke_test_ALL_SERVER from $GRANTEE_TYPE `$GRANTEE_NAME`; alter database grant_rev_db set owner user admin; refresh authorization; ==== @@ -220,7 +226,7 @@ STRING ---- USER admin ---- QUERY -grant role grant_revoke_test_ALL_TEST_DB to group `$GROUP_NAME` +grant role grant_revoke_test_ALL_TEST_DB to $GRANTEE_TYPE `$GRANTEE_NAME` ==== ---- USER admin @@ -245,7 +251,7 @@ does not have privileges to access: $NAMENODE/test-warehouse/grant_rev_test_tbl2 ---- USER admin ---- QUERY -grant role grant_revoke_test_ALL_URI to group `$GROUP_NAME` +grant role grant_revoke_test_ALL_URI to $GRANTEE_TYPE `$GRANTEE_NAME` ==== ---- USER admin @@ -345,9 +351,9 @@ principal_type, principal_name, database, table, column, uri, storage_type, stor STRING, STRING, STRING, STRING, STRING, STRING, STRING, STRING, STRING, STRING, BOOLEAN, STRING ==== ---- QUERY -# Does not result in an AuthorizationException since '$GROUP_NAME' is still assigned the -# role 'grant_revoke_test_ALL_TEST_DB', which is granted the 'ALL' privilege on the -# database 'grant_rev_db' +# Does not result in an AuthorizationException since '$GRANTEE_NAME' is still assigned +# the role 'grant_revoke_test_ALL_TEST_DB', which is granted the 'ALL' privilege on the +# database 'grant_rev_db'. # TODO(IMPALA-10401): Investigate whether the privilege on the provided uri should be # verified. create database grant_rev_db location '$FILESYSTEM_PREFIX/test-warehouse/grant_rev_db.db' @@ -409,7 +415,7 @@ does not have privileges to execute 'SELECT' on: grant_rev_db.test_tbl1 ---- USER admin ---- QUERY -grant role grant_revoke_test_SELECT_INSERT_TEST_TBL to group `$GROUP_NAME` +grant role grant_revoke_test_SELECT_INSERT_TEST_TBL to $GRANTEE_TYPE `$GRANTEE_NAME` ==== ---- USER admin @@ -490,14 +496,14 @@ User test_user does not have permission for this operation ---- USER test_user ---- QUERY -grant role grant_revoke_test_ALL_SERVER to group `$GROUP_NAME` +grant role grant_revoke_test_ALL_SERVER to $GRANTEE_TYPE `$GRANTEE_NAME` ---- CATCH User doesn't have permissions to grant role grant_revoke_test_ALL_SERVER ==== ---- USER test_user ---- QUERY -revoke role grant_revoke_test_ALL_SERVER from group `$GROUP_NAME` +revoke role grant_revoke_test_ALL_SERVER from $GRANTEE_TYPE `$GRANTEE_NAME` ---- CATCH User doesn't have permissions to revoke role grant_revoke_test_ALL_SERVER ==== @@ -528,7 +534,7 @@ admin # user 'non_owner' belongs according to the user-to-groups mapper provided for impalad # and catalogd via the argument of "--use_customized_user_groups_mapper_for_ranger". create role grant_revoke_test_NON_OWNER; -grant role grant_revoke_test_NON_OWNER to group non_owner; +grant role grant_revoke_test_NON_OWNER to $GRANTEE_TYPE non_owner; grant all on database functional to grant_revoke_test_NON_OWNER WITH GRANT OPTION; ==== ---- USER @@ -612,7 +618,7 @@ User non_owner does not have permission for this operation non_owner ---- QUERY # Also cannot create/drop/grant roles -grant role grant_revoke_test_NON_OWNER to group non_owner +grant role grant_revoke_test_NON_OWNER to $GRANTEE_TYPE non_owner ---- CATCH User doesn't have permissions to grant role grant_revoke_test_NON_OWNER ==== @@ -659,17 +665,17 @@ STRING, STRING, STRING, STRING, STRING, STRING, STRING, STRING, STRING, STRING, ---- USER admin ---- QUERY -# After the following two statements, there is no role assigned to the group -# '$GROUP_NAME' to which the user 'getuser()' belongs. -REVOKE ROLE grant_revoke_test_ALL_URI FROM GROUP `$GROUP_NAME`; -REVOKE ROLE grant_revoke_test_SELECT_INSERT_TEST_TBL FROM GROUP `$GROUP_NAME`; +# After the following two statements, there is no role assigned to the group (or user) +# 'GRANTEE_NAME'. +REVOKE ROLE grant_revoke_test_ALL_URI FROM $GRANTEE_TYPE `$GRANTEE_NAME`; +REVOKE ROLE grant_revoke_test_SELECT_INSERT_TEST_TBL FROM $GRANTEE_TYPE `$GRANTEE_NAME`; ---- RESULTS 'Role has been revoked.' ==== ---- USER admin ---- QUERY -GRANT ROLE grant_revoke_test_ALL_SERVER TO GROUP `$GROUP_NAME` +GRANT ROLE grant_revoke_test_ALL_SERVER TO $GRANTEE_TYPE `$GRANTEE_NAME` ---- RESULTS 'Role has been granted.' ==== @@ -915,7 +921,7 @@ STRING, STRING, STRING, STRING, STRING, STRING, STRING, STRING, STRING, STRING, admin ---- QUERY # Grant SELECT on table to 'non_owner' without 'WITH GRANT' option. -GRANT ROLE grant_revoke_test_NON_OWNER TO GROUP non_owner; +GRANT ROLE grant_revoke_test_NON_OWNER TO $GRANTEE_TYPE non_owner; GRANT SELECT ON TABLE grant_rev_db.test_tbl3 TO grant_revoke_test_NON_OWNER; REVOKE ALL ON DATABASE functional FROM grant_revoke_test_NON_OWNER; ---- RESULTS @@ -1063,9 +1069,9 @@ STRING, STRING, STRING, STRING, STRING, STRING, STRING, STRING, STRING, STRING, ---- USER admin ---- QUERY -# At this point, the group '$GROUP_NAME' is still assigned the role of +# At this point, '$GRANTEE_NAME' is still assigned the role of # 'grant_revoke_test_ALL_SERVER'. -revoke role grant_revoke_test_ALL_SERVER from group `$GROUP_NAME` +revoke role grant_revoke_test_ALL_SERVER from $GRANTEE_TYPE `$GRANTEE_NAME` ==== ---- USER admin @@ -1083,8 +1089,9 @@ grant all on server server1 to grant_revoke_test_ALL_SERVER1 ---- USER admin ---- QUERY -# Grant role to the group '$GROUP_NAME' to which the user 'getuser()' belongs. -grant role grant_revoke_test_ALL_SERVER1 to group `$GROUP_NAME` +# Grant role to the group '$GRANTEE_NAME' to which the user 'getuser()' belongs or to the +# user 'getuser()' itself. +grant role grant_revoke_test_ALL_SERVER1 to $GRANTEE_TYPE `$GRANTEE_NAME` ==== ---- USER admin @@ -1112,11 +1119,12 @@ create database grant_rev_db location '$FILESYSTEM_PREFIX/test-warehouse/grant_r ---- USER admin ---- QUERY -# To prevent the user 'getuser()' (which belongs to the group '$GROUP_NAME') from -# performing any operation on the database 'grant_rev_db', we also need to alter -# the owner of 'grant_rev_db' since 'getuser()' is the owner of this database, which by -# default is allowed by Ranger to perform any operation on the database. -revoke role grant_revoke_test_ALL_SERVER1 from group `$GROUP_NAME`; +# To prevent the user 'getuser()' (which belongs to the group '$GRANTEE_NAME' when +# $GRANTEE_TYPE is 'GROUP') from performing any operation on the database 'grant_rev_db', +# we also need to alter the owner of 'grant_rev_db' since 'getuser()' is the owner of +# this database, which by default is allowed by Ranger to perform any operation on the +# database. +revoke role grant_revoke_test_ALL_SERVER1 from $GRANTEE_TYPE `$GRANTEE_NAME`; alter database grant_rev_db set owner user admin; ==== ---- QUERY @@ -1159,7 +1167,7 @@ admin ---- QUERY # IMPALA-4951: make sure database is visible to a user having only column level access # to a table in the database -grant role grant_revoke_test_ALL_SERVER to group non_owner +grant role grant_revoke_test_ALL_SERVER to $GRANTEE_TYPE non_owner ---- RESULTS 'Role has been granted.' ==== @@ -1171,7 +1179,7 @@ create role grant_revoke_test_COLUMN_PRIV ---- USER admin ---- QUERY -grant role grant_revoke_test_COLUMN_PRIV to group non_owner +grant role grant_revoke_test_COLUMN_PRIV to $GRANTEE_TYPE non_owner ==== ---- USER non_owner @@ -1186,7 +1194,7 @@ create table grant_rev_db.test_tbl4 (col1 int, col2 int) ---- USER admin ---- QUERY -revoke role grant_revoke_test_ALL_SERVER from group non_owner +revoke role grant_revoke_test_ALL_SERVER from $GRANTEE_TYPE non_owner ==== ---- USER non_owner @@ -1241,12 +1249,12 @@ STRING,STRING admin ---- QUERY # We will be testing whether the user 'getuser()' is able to drop a database if the group -# 'getuser()' belongs to is assigned the role 'grant_revoke_test_ALL_SERVER', which is -# granted the ALL privilege on the server. Before dropping a database, the user also has -# to revoke every privilege granted on the resources of the database and thus we grant -# the ALL privilege with the grant option. +# 'getuser()' belongs to, or the user 'getuser()' itself is assigned the role +# 'grant_revoke_test_ALL_SERVER', which is granted the ALL privilege on the server. +# Before dropping a database, the user also has to revoke every privilege granted on the +# resources of the database and thus we grant the ALL privilege with the grant option. grant all on server to role grant_revoke_test_ALL_SERVER with grant option; -grant role grant_revoke_test_ALL_SERVER to group `$GROUP_NAME`; +grant role grant_revoke_test_ALL_SERVER to $GRANTEE_TYPE `$GRANTEE_NAME`; ---- RESULTS 'Role has been granted.' ==== @@ -1260,18 +1268,19 @@ drop database if exists grant_rev_db cascade; ---- USER admin ---- QUERY -revoke role grant_revoke_test_ALL_SERVER from group `$GROUP_NAME` +revoke role grant_revoke_test_ALL_SERVER from $GRANTEE_TYPE `$GRANTEE_NAME` ---- RESULTS 'Role has been revoked.' ==== ---- USER admin ---- QUERY -revoke role grant_revoke_test_COLUMN_PRIV from group `$GROUP_NAME` +revoke role grant_revoke_test_COLUMN_PRIV from $GRANTEE_TYPE `$GRANTEE_NAME` ==== ---- QUERY -# Verify that the user 'getuser()', which belongs to the group '$GROUP_NAME' is not -# assigned any role afterwards. +# Verify that the user 'getuser()', which belongs to the group '$GRANTEE_NAME' when +# $GRANTEE_TYPE is 'GROUP', or that the user 'getuser()' itself is not assigned any role +# afterwards. show current roles ---- RESULTS ==== \ No newline at end of file diff --git a/tests/authorization/test_ranger.py b/tests/authorization/test_ranger.py index 46fbd89ce..87bac2d4c 100644 --- a/tests/authorization/test_ranger.py +++ b/tests/authorization/test_ranger.py @@ -796,9 +796,13 @@ class TestRanger(CustomClusterTestSuite): @staticmethod def _check_privileges(result, expected): + TestRanger._check_rows(result, expected, True) + + @staticmethod + def _check_rows(result, expected, omit_last_column=False): def columns(row): cols = row.split("\t") - return cols[0:len(cols) - 1] + return cols[:-1] if omit_last_column else cols assert list(map(columns, result.data)) == expected @@ -1679,6 +1683,80 @@ class TestRanger(CustomClusterTestSuite): for i in range(policy_cnt): TestRanger._remove_policy(unique_name + str(i)) + def _test_grant_revoke_with_role(self, grantee_type, vector, admin_client): + try: + self.run_test_case('QueryTest/grant_revoke', vector, use_db="default", + test_file_vars={ + "$GRANTEE_TYPE": grantee_type, + "$GRANTEE_NAME": getuser() + }) + finally: + # Below are the statements that need to be executed in order to clean up the + # privileges granted to the test roles as well as the test roles themselves. + # Note that we need to revoke those previously granted privileges so that each role + # is not referenced by any policy before we delete those roles. + # Moreover, we need to revoke the privilege on the database 'grant_rev_db' before + # dropping 'grant_rev_db'. Otherwise, the revocation would fail due to an + # AnalysisException thrown because 'grant_rev_db' does not exist. + cleanup_statements = [ + "revoke all on database grant_rev_db from grant_revoke_test_ALL_TEST_DB", + "revoke all on server from grant_revoke_test_ALL_SERVER", + "revoke all on table functional.alltypes from grant_revoke_test_NON_OWNER", + "revoke grant option for all on database functional " + "from grant_revoke_test_NON_OWNER", + "REVOKE SELECT (a, b, c, d, e, x, y) ON TABLE grant_rev_db.test_tbl3 " + "FROM grant_revoke_test_ALL_SERVER", + "REVOKE ALL ON DATABASE functional FROM grant_revoke_test_NON_OWNER", + "REVOKE SELECT ON TABLE grant_rev_db.test_tbl3 FROM grant_revoke_test_NON_OWNER", + "REVOKE GRANT OPTION FOR SELECT (a, c) ON TABLE grant_rev_db.test_tbl3 " + "FROM grant_revoke_test_ALL_SERVER", + "REVOKE SELECT ON TABLE grant_rev_db.test_tbl1 " + "FROM grant_revoke_test_SELECT_INSERT_TEST_TBL", + "REVOKE INSERT ON TABLE grant_rev_db.test_tbl1 " + "FROM grant_revoke_test_SELECT_INSERT_TEST_TBL", + "REVOKE SELECT ON TABLE grant_rev_db.test_tbl3 " + "FROM grant_revoke_test_NON_OWNER", + "REVOKE SELECT (a) ON TABLE grant_rev_db.test_tbl3 " + "FROM grant_revoke_test_NON_OWNER", + "REVOKE SELECT (a, c, e) ON TABLE grant_rev_db.test_tbl3 " + "FROM grant_revoke_test_ALL_SERVER", + "revoke all on server server1 from grant_revoke_test_ALL_SERVER1", + "revoke select(col1) on table grant_rev_db.test_tbl4 " + "from role grant_revoke_test_COLUMN_PRIV", + "{0}{1}{2}".format("revoke all on uri '", + os.getenv("FILESYSTEM_PREFIX"), + "/test-warehouse/grant_rev_test_tbl2'" + "from grant_revoke_test_ALL_URI"), + "{0}{1}{2}".format("revoke all on uri '", + os.getenv("FILESYSTEM_PREFIX"), + "/test-warehouse/GRANT_REV_TEST_TBL3'" + "from grant_revoke_test_ALL_URI"), + "{0}{1}{2}".format("revoke all on uri '", + os.getenv("FILESYSTEM_PREFIX"), + "/test-warehouse/grant_rev_test_prt'" + "from grant_revoke_test_ALL_URI"), + "drop role grant_revoke_test_ALL_TEST_DB", + "drop role grant_revoke_test_ALL_SERVER", + "drop role grant_revoke_test_SELECT_INSERT_TEST_TBL", + "drop role grant_revoke_test_ALL_URI", + "drop role grant_revoke_test_NON_OWNER", + "drop role grant_revoke_test_ALL_SERVER1", + "drop role grant_revoke_test_COLUMN_PRIV", + "drop database grant_rev_db cascade" + ] + + for statement in cleanup_statements: + try: + admin_client.execute(statement) + except Exception: + # There could be an exception thrown due to the non-existence of the role or + # resource involved in a statement that aims to revoke the privilege on a + # resource from a role, but we do not have to handle such an exception. We only + # need to make sure in the case when the role and the corresponding resource + # exist, the granted privilege is revoked. The same applies to the case when we + # drop a role. + pass + class TestRangerIndependent(TestRanger): """ @@ -1965,76 +2043,61 @@ class TestRangerIndependent(TestRanger): catalogd_args="{0} {1}".format(CATALOGD_ARGS, "--use_customized_user_groups_mapper_for_ranger")) def test_grant_revoke_with_role(self, vector): - """Test grant/revoke with role.""" - admin_client = self.create_impala_client(user=ADMIN) - try: - self.run_test_case('QueryTest/grant_revoke', vector, use_db="default") - finally: - # Below are the statements that need to be executed in order to clean up the - # privileges granted to the test roles as well as the test roles themselves. - # Note that we need to revoke those previously granted privileges so that each role - # is not referenced by any policy before we delete those roles. - # Moreover, we need to revoke the privilege on the database 'grant_rev_db' before - # dropping 'grant_rev_db'. Otherwise, the revocation would fail due to an - # AnalysisException thrown because 'grant_rev_db' does not exist. - cleanup_statements = [ - "revoke all on database grant_rev_db from grant_revoke_test_ALL_TEST_DB", - "revoke all on server from grant_revoke_test_ALL_SERVER", - "revoke all on table functional.alltypes from grant_revoke_test_NON_OWNER", - "revoke grant option for all on database functional " - "from grant_revoke_test_NON_OWNER", - "REVOKE SELECT (a, b, c, d, e, x, y) ON TABLE grant_rev_db.test_tbl3 " - "FROM grant_revoke_test_ALL_SERVER", - "REVOKE ALL ON DATABASE functional FROM grant_revoke_test_NON_OWNER", - "REVOKE SELECT ON TABLE grant_rev_db.test_tbl3 FROM grant_revoke_test_NON_OWNER", - "REVOKE GRANT OPTION FOR SELECT (a, c) ON TABLE grant_rev_db.test_tbl3 " - "FROM grant_revoke_test_ALL_SERVER", - "REVOKE SELECT ON TABLE grant_rev_db.test_tbl1 " - "FROM grant_revoke_test_SELECT_INSERT_TEST_TBL", - "REVOKE INSERT ON TABLE grant_rev_db.test_tbl1 " - "FROM grant_revoke_test_SELECT_INSERT_TEST_TBL", - "REVOKE SELECT ON TABLE grant_rev_db.test_tbl3 " - "FROM grant_revoke_test_NON_OWNER", - "REVOKE SELECT (a) ON TABLE grant_rev_db.test_tbl3 " - "FROM grant_revoke_test_NON_OWNER", - "REVOKE SELECT (a, c, e) ON TABLE grant_rev_db.test_tbl3 " - "FROM grant_revoke_test_ALL_SERVER", - "revoke all on server server1 from grant_revoke_test_ALL_SERVER1", - "revoke select(col1) on table grant_rev_db.test_tbl4 " - "from role grant_revoke_test_COLUMN_PRIV", - "{0}{1}{2}".format("revoke all on uri '", - os.getenv("FILESYSTEM_PREFIX"), - "/test-warehouse/grant_rev_test_tbl2'" - "from grant_revoke_test_ALL_URI"), - "{0}{1}{2}".format("revoke all on uri '", - os.getenv("FILESYSTEM_PREFIX"), - "/test-warehouse/GRANT_REV_TEST_TBL3'" - "from grant_revoke_test_ALL_URI"), - "{0}{1}{2}".format("revoke all on uri '", - os.getenv("FILESYSTEM_PREFIX"), - "/test-warehouse/grant_rev_test_prt'" - "from grant_revoke_test_ALL_URI"), - "drop role grant_revoke_test_ALL_TEST_DB", - "drop role grant_revoke_test_ALL_SERVER", - "drop role grant_revoke_test_SELECT_INSERT_TEST_TBL", - "drop role grant_revoke_test_ALL_URI", - "drop role grant_revoke_test_NON_OWNER", - "drop role grant_revoke_test_ALL_SERVER1", - "drop role grant_revoke_test_COLUMN_PRIV", - "drop database grant_rev_db cascade" - ] + """Test grant/revoke with roles.""" + with self.create_impala_client(user=ADMIN) as admin_client: + TestRanger._test_grant_revoke_with_role(self, 'GROUP', vector, admin_client) + TestRanger._test_grant_revoke_with_role(self, 'USER', vector, admin_client) - for statement in cleanup_statements: - try: - admin_client.execute(statement) - except Exception: - # There could be an exception thrown due to the non-existence of the role or - # resource involved in a statement that aims to revoke the privilege on a - # resource from a role, but we do not have to handle such an exception. We only - # need to make sure in the case when the role and the corresponding resource - # exist, the granted privilege is revoked. The same applies to the case when we - # drop a role. - pass + @CustomClusterTestSuite.with_args( + impalad_args=IMPALAD_ARGS, catalogd_args=CATALOGD_ARGS) + def test_show_roles(self): + """Test SHOW ROLE GRANT GROUP/USER and SHOW CURRENT ROLES.""" + user = getuser() + group = getuser() + with self.create_impala_client(user=ADMIN) as admin_client, \ + self.create_impala_client(user=user) as user_client: + try: + admin_client.execute("create role r_1") + admin_client.execute("create role r_2") + admin_client.execute("grant role r_1 to group {}".format(group)) + admin_client.execute("grant role r_2 to user {}".format(user)) + + # Verify that the group with the name 'getuser()' is associated with the role + # 'r_1'. + result_1 = admin_client.execute("show role grant group {}".format(group)) + TestRanger._check_rows(result_1, [['r_1']]) + + # Verify that the user 'getuser()' is associated with the role 'r_2'. + result_2 = admin_client.execute("show role grant user {}".format(user)) + TestRanger._check_rows(result_2, [['r_2']]) + + # Verify as the user 'getuser()' that its current roles are 'r_1' and 'r_2'. + result_3 = user_client.execute("show current roles") + TestRanger._check_rows(result_3, [['r_1'], ['r_2']]) + + # Try to revoke role 'r_1' from the user 'getuser()'. + admin_client.execute("revoke role r_1 from user {}".format(user)) + # Verify as the user 'getuser()' that its current roles are still 'r_1' and + # 'r_2', i.e., the role 'r_1' is still associated with the user 'getuser()' + # because the user 'getuser()' belongs to the group 'getuser()', and 'r_1' is + # still associated with the group 'getuser()'. + result_4 = user_client.execute("show current roles") + TestRanger._check_rows(result_4, [['r_1'], ['r_2']]) + + # Revoke the role 'r_2' from the group 'getuser()'. + admin_client.execute("revoke role r_1 from group {}".format(user)) + # Verify as the user 'getuser()' its current roles do not include 'r_1'. + result_5 = user_client.execute("show current roles") + TestRanger._check_rows(result_5, [['r_2']]) + finally: + admin_client.execute("revoke role r_1 from group {}".format(group)) + admin_client.execute("revoke role r_2 from user {}".format(user)) + result_6 = admin_client.execute("show role grant user {}".format(user)) + TestRanger._check_rows(result_6, []) + result_7 = admin_client.execute("show role grant group {}".format(group)) + TestRanger._check_rows(result_7, []) + admin_client.execute("drop role r_1") + admin_client.execute("drop role r_2") @pytest.mark.execute_serially @CustomClusterTestSuite.with_args(
