From 7e96b06e6cb45110775dec632ada510a977086f1 Mon Sep 17 00:00:00 2001 From: zhangdong <493738387@qq.com> Date: Tue, 7 Mar 2023 10:28:56 +0800 Subject: [PATCH] [Enhance](auth)Users support multiple roles (#17236) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Describe your changes. 1.support GRANT role [, role] TO user_identity 2.support REVOKE role [, role] FROM user_identity 3.’Show grants‘ Add a column to display the roles owned by users 4.‘alter user’ prohibit deleting user's role 5.Repair Logic of roleName cannot start with RoleManager.DEFAULT_ ROLE --- fe/fe-core/src/main/cup/sql_parser.cup | 11 ++- .../org/apache/doris/analysis/GrantStmt.java | 52 +++++++++-- .../org/apache/doris/analysis/RevokeStmt.java | 46 ++++++++-- .../org/apache/doris/common/FeNameFormat.java | 3 +- .../apache/doris/common/proc/AuthProcDir.java | 2 +- .../apache/doris/mysql/privilege/Auth.java | 80 ++++++++++++++--- .../doris/mysql/privilege/RoleManager.java | 3 +- .../mysql/privilege/UserRoleManager.java | 43 +++++++++ .../org/apache/doris/persist/PrivInfo.java | 14 +++ .../doris/mysql/privilege/AuthTest.java | 88 +++++++++++++++++++ .../data/account_p0/test_alter_user.out | 10 --- .../suites/account_p0/test_alter_user.groovy | 23 ----- 12 files changed, 308 insertions(+), 67 deletions(-) delete mode 100644 regression-test/data/account_p0/test_alter_user.out diff --git a/fe/fe-core/src/main/cup/sql_parser.cup b/fe/fe-core/src/main/cup/sql_parser.cup index 939729e55d..ca10463d88 100644 --- a/fe/fe-core/src/main/cup/sql_parser.cup +++ b/fe/fe-core/src/main/cup/sql_parser.cup @@ -1332,10 +1332,9 @@ alter_stmt ::= | KW_ALTER KW_USER opt_if_exists:ifExists grant_user:user - opt_user_role:userRole opt_password_option:passwdOptions {: - RESULT = new AlterUserStmt(ifExists, user, userRole, passwdOptions); + RESULT = new AlterUserStmt(ifExists, user, null, passwdOptions); :} ; @@ -2797,6 +2796,10 @@ grant_stmt ::= {: RESULT = new GrantStmt(null, role, resourcePattern, privs); :} + | KW_GRANT string_list:roles KW_TO user_identity:userId + {: + RESULT = new GrantStmt(roles, userId); + :} ; tbl_pattern ::= @@ -2854,6 +2857,10 @@ revoke_stmt ::= {: RESULT = new RevokeStmt(null, role, resourcePattern, privs); :} + | KW_REVOKE string_list:roles KW_FROM user_identity:userId + {: + RESULT = new RevokeStmt(roles, userId); + :} ; // Drop statement diff --git a/fe/fe-core/src/main/java/org/apache/doris/analysis/GrantStmt.java b/fe/fe-core/src/main/java/org/apache/doris/analysis/GrantStmt.java index bb56d68056..328f8b5279 100644 --- a/fe/fe-core/src/main/java/org/apache/doris/analysis/GrantStmt.java +++ b/fe/fe-core/src/main/java/org/apache/doris/analysis/GrantStmt.java @@ -33,18 +33,23 @@ import org.apache.doris.qe.ConnectContext; import com.google.common.base.Joiner; import com.google.common.base.Strings; +import org.apache.commons.collections.CollectionUtils; import java.util.List; // GRANT STMT -// GRANT privilege [, privilege] ON db.tbl TO user [ROLE 'role']; -// GRANT privilege [, privilege] ON RESOURCE 'resource' TO user [ROLE 'role']; +// GRANT privilege [, privilege] ON db.tbl TO user_identity [ROLE 'role']; +// GRANT privilege [, privilege] ON RESOURCE 'resource' TO user_identity [ROLE 'role']; +// GRANT role [, role] TO user_identity public class GrantStmt extends DdlStmt { private UserIdentity userIdent; + // Indicates which permissions are granted to this role private String role; private TablePattern tblPattern; private ResourcePattern resourcePattern; private List privileges; + // Indicates that these roles are granted to a user + private List roles; public GrantStmt(UserIdentity userIdent, String role, TablePattern tblPattern, List privileges) { this.userIdent = userIdent; @@ -71,6 +76,11 @@ public class GrantStmt extends DdlStmt { this.privileges = privs.toPrivilegeList(); } + public GrantStmt(List roles, UserIdentity userIdent) { + this.userIdent = userIdent; + this.roles = roles; + } + public UserIdentity getUserIdent() { return userIdent; } @@ -95,6 +105,10 @@ public class GrantStmt extends DdlStmt { return privileges; } + public List getRoles() { + return roles; + } + @Override public void analyze(Analyzer analyzer) throws AnalysisException, UserException { super.analyze(analyzer); @@ -107,18 +121,26 @@ public class GrantStmt extends DdlStmt { if (tblPattern != null) { tblPattern.analyze(analyzer); - } else { + } else if (resourcePattern != null) { resourcePattern.analyze(); + } else if (roles != null) { + for (int i = 0; i < roles.size(); i++) { + String originalRoleName = roles.get(i); + FeNameFormat.checkRoleName(originalRoleName, false /* can not be admin */, "Can not grant role"); + roles.set(i, ClusterNamespace.getFullName(analyzer.getClusterName(), originalRoleName)); + } } - if (privileges == null || privileges.isEmpty()) { - throw new AnalysisException("No privileges in grant statement."); + if (CollectionUtils.isEmpty(privileges) && CollectionUtils.isEmpty(roles)) { + throw new AnalysisException("No privileges or roles in grant statement."); } if (tblPattern != null) { checkTablePrivileges(privileges, role, tblPattern); - } else { + } else if (resourcePattern != null) { checkResourcePrivileges(privileges, role, resourcePattern); + } else if (roles != null) { + checkRolePrivileges(); } } @@ -223,14 +245,28 @@ public class GrantStmt extends DdlStmt { } } + public static void checkRolePrivileges() throws AnalysisException { + if (!Env.getCurrentEnv().getAccessManager().checkGlobalPriv(ConnectContext.get(), PrivPredicate.GRANT)) { + ErrorReport.reportAnalysisException(ErrorCode.ERR_SPECIFIC_ACCESS_DENIED_ERROR, "GRANT/ROVOKE"); + } + } + @Override public String toSql() { StringBuilder sb = new StringBuilder(); - sb.append("GRANT ").append(Joiner.on(", ").join(privileges)); + sb.append("GRANT "); + if (privileges != null) { + sb.append(Joiner.on(", ").join(privileges)); + } else { + sb.append(Joiner.on(", ").join(roles)); + } + if (tblPattern != null) { sb.append(" ON ").append(tblPattern).append(" TO "); - } else { + } else if (resourcePattern != null) { sb.append(" ON RESOURCE '").append(resourcePattern).append("' TO "); + } else { + sb.append(" TO "); } if (!Strings.isNullOrEmpty(role)) { sb.append(" ROLE '").append(role).append("'"); diff --git a/fe/fe-core/src/main/java/org/apache/doris/analysis/RevokeStmt.java b/fe/fe-core/src/main/java/org/apache/doris/analysis/RevokeStmt.java index 4054741ef6..8dd7a1bafe 100644 --- a/fe/fe-core/src/main/java/org/apache/doris/analysis/RevokeStmt.java +++ b/fe/fe-core/src/main/java/org/apache/doris/analysis/RevokeStmt.java @@ -26,20 +26,25 @@ import org.apache.doris.mysql.privilege.Privilege; import com.google.common.base.Joiner; import com.google.common.base.Strings; +import org.apache.commons.collections.CollectionUtils; import java.util.List; // REVOKE STMT // revoke privilege from some user, this is an administrator operation. // -// REVOKE privilege [, privilege] ON db.tbl FROM user [ROLE 'role']; -// REVOKE privilege [, privilege] ON resource 'resource' FROM user [ROLE 'role']; +// REVOKE privilege [, privilege] ON db.tbl FROM user_identity [ROLE 'role']; +// REVOKE privilege [, privilege] ON resource 'resource' FROM user_identity [ROLE 'role']; +// REVOKE role [, role] FROM user_identity public class RevokeStmt extends DdlStmt { private UserIdentity userIdent; + // Indicates which permissions are revoked from this role private String role; private TablePattern tblPattern; private ResourcePattern resourcePattern; private List privileges; + // Indicates that these roles are revoked from a user + private List roles; public RevokeStmt(UserIdentity userIdent, String role, TablePattern tblPattern, List privileges) { this.userIdent = userIdent; @@ -66,6 +71,11 @@ public class RevokeStmt extends DdlStmt { this.privileges = privs.toPrivilegeList(); } + public RevokeStmt(List roles, UserIdentity userIdent) { + this.roles = roles; + this.userIdent = userIdent; + } + public UserIdentity getUserIdent() { return userIdent; } @@ -86,6 +96,10 @@ public class RevokeStmt extends DdlStmt { return privileges; } + public List getRoles() { + return roles; + } + @Override public void analyze(Analyzer analyzer) throws AnalysisException { if (userIdent != null) { @@ -97,30 +111,46 @@ public class RevokeStmt extends DdlStmt { if (tblPattern != null) { tblPattern.analyze(analyzer); - } else { + } else if (resourcePattern != null) { resourcePattern.analyze(); + } else if (roles != null) { + for (int i = 0; i < roles.size(); i++) { + String originalRoleName = roles.get(i); + FeNameFormat.checkRoleName(originalRoleName, false /* can not be admin */, "Can not revoke role"); + roles.set(i, ClusterNamespace.getFullName(analyzer.getClusterName(), originalRoleName)); + } } - if (privileges == null || privileges.isEmpty()) { - throw new AnalysisException("No privileges in revoke statement."); + if (CollectionUtils.isEmpty(privileges) && CollectionUtils.isEmpty(roles)) { + throw new AnalysisException("No privileges or roles in revoke statement."); } // Revoke operation obey the same rule as Grant operation. reuse the same method if (tblPattern != null) { GrantStmt.checkTablePrivileges(privileges, role, tblPattern); - } else { + } else if (resourcePattern != null) { GrantStmt.checkResourcePrivileges(privileges, role, resourcePattern); + } else if (roles != null) { + GrantStmt.checkRolePrivileges(); } } @Override public String toSql() { StringBuilder sb = new StringBuilder(); - sb.append("REVOKE ").append(Joiner.on(", ").join(privileges)); + sb.append("REVOKE "); + if (privileges != null) { + sb.append(Joiner.on(", ").join(privileges)); + } else { + sb.append(Joiner.on(", ").join(roles)); + } + if (tblPattern != null) { sb.append(" ON ").append(tblPattern).append(" FROM "); - } else { + } else if (resourcePattern != null) { sb.append(" ON RESOURCE '").append(resourcePattern).append("' FROM "); + } else { + sb.append(" FROM "); } if (!Strings.isNullOrEmpty(role)) { sb.append(" ROLE '").append(role).append("'"); diff --git a/fe/fe-core/src/main/java/org/apache/doris/common/FeNameFormat.java b/fe/fe-core/src/main/java/org/apache/doris/common/FeNameFormat.java index 428f793ce7..1e6eb0f195 100644 --- a/fe/fe-core/src/main/java/org/apache/doris/common/FeNameFormat.java +++ b/fe/fe-core/src/main/java/org/apache/doris/common/FeNameFormat.java @@ -21,6 +21,7 @@ import org.apache.doris.alter.SchemaChangeHandler; import org.apache.doris.analysis.CreateMaterializedViewStmt; import org.apache.doris.datasource.InternalCatalog; import org.apache.doris.mysql.privilege.Role; +import org.apache.doris.mysql.privilege.RoleManager; import org.apache.doris.qe.ConnectContext; import org.apache.doris.qe.VariableMgr; @@ -119,7 +120,7 @@ public class FeNameFormat { || (!canBeAdmin && role.equalsIgnoreCase(Role.ADMIN_ROLE)); } - if (res) { + if (res || role.startsWith(RoleManager.DEFAULT_ROLE_PREFIX)) { throw new AnalysisException(errMsg + ": " + role); } } diff --git a/fe/fe-core/src/main/java/org/apache/doris/common/proc/AuthProcDir.java b/fe/fe-core/src/main/java/org/apache/doris/common/proc/AuthProcDir.java index 5b4e78408c..dc33ae3f35 100644 --- a/fe/fe-core/src/main/java/org/apache/doris/common/proc/AuthProcDir.java +++ b/fe/fe-core/src/main/java/org/apache/doris/common/proc/AuthProcDir.java @@ -31,7 +31,7 @@ import com.google.common.collect.ImmutableList; */ public class AuthProcDir implements ProcDirInterface { public static final ImmutableList TITLE_NAMES = new ImmutableList.Builder() - .add("UserIdentity").add("Password").add("GlobalPrivs").add("CatalogPrivs") + .add("UserIdentity").add("Password").add("Roles").add("GlobalPrivs").add("CatalogPrivs") .add("DatabasePrivs").add("TablePrivs").add("ResourcePrivs").build(); private Auth auth; diff --git a/fe/fe-core/src/main/java/org/apache/doris/mysql/privilege/Auth.java b/fe/fe-core/src/main/java/org/apache/doris/mysql/privilege/Auth.java index 14af84b91e..22ed226254 100644 --- a/fe/fe-core/src/main/java/org/apache/doris/mysql/privilege/Auth.java +++ b/fe/fe-core/src/main/java/org/apache/doris/mysql/privilege/Auth.java @@ -508,13 +508,16 @@ public class Auth implements Writable { // grant public void grant(GrantStmt stmt) throws DdlException { - PrivBitSet privs = PrivBitSet.of(stmt.getPrivileges()); if (stmt.getTblPattern() != null) { + PrivBitSet privs = PrivBitSet.of(stmt.getPrivileges()); grantInternal(stmt.getUserIdent(), stmt.getQualifiedRole(), stmt.getTblPattern(), privs, true /* err on non exist */, false /* not replay */); - } else { + } else if (stmt.getResourcePattern() != null) { + PrivBitSet privs = PrivBitSet.of(stmt.getPrivileges()); grantInternal(stmt.getUserIdent(), stmt.getQualifiedRole(), stmt.getResourcePattern(), privs, true /* err on non exist */, false /* not replay */); + } else { + grantInternal(stmt.getUserIdent(), stmt.getRoles(), false); } } @@ -524,10 +527,12 @@ public class Auth implements Writable { grantInternal(privInfo.getUserIdent(), privInfo.getRole(), privInfo.getTblPattern(), privInfo.getPrivs(), true /* err on non exist */, true /* is replay */); - } else { + } else if (privInfo.getResourcePattern() != null) { grantInternal(privInfo.getUserIdent(), privInfo.getRole(), privInfo.getResourcePattern(), privInfo.getPrivs(), true /* err on non exist */, true /* is replay */); + } else { + grantInternal(privInfo.getUserIdent(), privInfo.getRoles(), true); } } catch (DdlException e) { LOG.error("should not happen", e); @@ -568,7 +573,6 @@ public class Auth implements Writable { role = roleManager.getUserDefaultRoleName(userIdent); } - // grant privs to role, role must exist Role newRole = new Role(role, resourcePattern, privs); roleManager.addOrMergeRole(newRole, false /* err on exist */); @@ -582,6 +586,30 @@ public class Auth implements Writable { } } + // grant for roles + private void grantInternal(UserIdentity userIdent, List roles, boolean isReplay) throws DdlException { + writeLock(); + try { + if (userManager.getUserByUserIdentity(userIdent) == null) { + throw new DdlException("user: " + userIdent + " does not exist"); + } + //roles must exist + for (String roleName : roles) { + if (roleManager.getRole(roleName) == null) { + throw new DdlException("role:" + roleName + " does not exist"); + } + } + userRoleManager.addUserRoles(userIdent, roles); + if (!isReplay) { + PrivInfo info = new PrivInfo(userIdent, roles); + Env.getCurrentEnv().getEditLog().logGrantPriv(info); + } + LOG.info("finished to grant role privilege. is replay: {}", isReplay); + } finally { + writeUnlock(); + } + } + // return true if user ident exist private boolean doesUserExist(UserIdentity userIdent) { @@ -603,13 +631,16 @@ public class Auth implements Writable { // revoke public void revoke(RevokeStmt stmt) throws DdlException { - PrivBitSet privs = PrivBitSet.of(stmt.getPrivileges()); if (stmt.getTblPattern() != null) { + PrivBitSet privs = PrivBitSet.of(stmt.getPrivileges()); revokeInternal(stmt.getUserIdent(), stmt.getQualifiedRole(), stmt.getTblPattern(), privs, true /* err on non exist */, false /* is replay */); - } else { + } else if (stmt.getResourcePattern() != null) { + PrivBitSet privs = PrivBitSet.of(stmt.getPrivileges()); revokeInternal(stmt.getUserIdent(), stmt.getQualifiedRole(), stmt.getResourcePattern(), privs, true /* err on non exist */, false /* is replay */); + } else { + revokeInternal(stmt.getUserIdent(), stmt.getRoles(), false); } } @@ -618,9 +649,11 @@ public class Auth implements Writable { if (info.getTblPattern() != null) { revokeInternal(info.getUserIdent(), info.getRole(), info.getTblPattern(), info.getPrivs(), true /* err on non exist */, true /* is replay */); - } else { + } else if (info.getResourcePattern() != null) { revokeInternal(info.getUserIdent(), info.getRole(), info.getResourcePattern(), info.getPrivs(), true /* err on non exist */, true /* is replay */); + } else { + revokeInternal(info.getUserIdent(), info.getRoles(), false); } } catch (DdlException e) { LOG.error("should not happened", e); @@ -668,6 +701,30 @@ public class Auth implements Writable { } } + // revoke for roles + private void revokeInternal(UserIdentity userIdent, List roles, boolean isReplay) throws DdlException { + writeLock(); + try { + if (userManager.getUserByUserIdentity(userIdent) == null) { + throw new DdlException("user: " + userIdent + " does not exist"); + } + //roles must exist + for (String roleName : roles) { + if (roleManager.getRole(roleName) == null) { + throw new DdlException("role:" + roleName + " does not exist"); + } + } + userRoleManager.removeUserRoles(userIdent, roles); + if (!isReplay) { + PrivInfo info = new PrivInfo(userIdent, roles); + Env.getCurrentEnv().getEditLog().logRevokePriv(info); + } + LOG.info("finished to revoke role privilege. is replay: {}", isReplay); + } finally { + writeUnlock(); + } + } + // set password public void setPassword(SetPassVar stmt) throws DdlException { setPasswordInternal(stmt.getUserIdent(), stmt.getPassword(), null, true /* err on non exist */, @@ -739,9 +796,6 @@ public class Auth implements Writable { Role emptyPrivsRole = new Role(role); writeLock(); try { - if (role.startsWith(RoleManager.DEFAULT_ROLE_PREFIX)) { - throw new DdlException("Can not create role starts with " + RoleManager.DEFAULT_ROLE_PREFIX); - } if (ignoreIfExists && roleManager.getRole(role) != null) { LOG.info("role exists, ignored to create role: {}, is replay: {}", role, isReplay); return; @@ -775,9 +829,6 @@ public class Auth implements Writable { private void dropRoleInternal(String role, boolean ignoreIfNonExists, boolean isReplay) throws DdlException { writeLock(); try { - if (role.startsWith(RoleManager.DEFAULT_ROLE_PREFIX)) { - throw new DdlException("Can not drop role starts with " + RoleManager.DEFAULT_ROLE_PREFIX); - } if (ignoreIfNonExists && roleManager.getRole(role) == null) { LOG.info("role non exists, ignored to drop role: {}, is replay: {}", role, isReplay); return; @@ -949,6 +1000,9 @@ public class Auth implements Writable { userAuthInfo.add(userIdent.toString()); // ============== Password ============== userAuthInfo.add(user.hasPassword() ? "Yes" : "No"); + // ============== Roles ============== + userAuthInfo.add(Joiner.on(",").join(userRoleManager + .getRolesByUser(userIdent, ConnectContext.get().getSessionVariable().showUserDefaultRole))); // ==============GlobalPrivs============== PrivBitSet ldapGlobalPrivs = LdapPrivsChecker.getGlobalPrivFromLdap(userIdent); PrivBitSet globalPrivs = ldapGlobalPrivs.copy(); diff --git a/fe/fe-core/src/main/java/org/apache/doris/mysql/privilege/RoleManager.java b/fe/fe-core/src/main/java/org/apache/doris/mysql/privilege/RoleManager.java index 80fba40814..633b840477 100644 --- a/fe/fe-core/src/main/java/org/apache/doris/mysql/privilege/RoleManager.java +++ b/fe/fe-core/src/main/java/org/apache/doris/mysql/privilege/RoleManager.java @@ -22,6 +22,7 @@ import org.apache.doris.analysis.TablePattern; import org.apache.doris.analysis.UserIdentity; import org.apache.doris.catalog.Env; import org.apache.doris.catalog.InfoSchemaDb; +import org.apache.doris.cluster.ClusterNamespace; import org.apache.doris.common.AnalysisException; import org.apache.doris.common.DdlException; import org.apache.doris.common.FeConstants; @@ -125,7 +126,7 @@ public class RoleManager implements Writable { public void getRoleInfo(List> results) { for (Role role : roles.values()) { - if (role.getRoleName().startsWith(DEFAULT_ROLE_PREFIX)) { + if (ClusterNamespace.getNameFromFullName(role.getRoleName()).startsWith(DEFAULT_ROLE_PREFIX)) { if (ConnectContext.get() == null || !ConnectContext.get().getSessionVariable().showUserDefaultRole) { continue; } diff --git a/fe/fe-core/src/main/java/org/apache/doris/mysql/privilege/UserRoleManager.java b/fe/fe-core/src/main/java/org/apache/doris/mysql/privilege/UserRoleManager.java index 60939a370d..f4c8fb90a9 100644 --- a/fe/fe-core/src/main/java/org/apache/doris/mysql/privilege/UserRoleManager.java +++ b/fe/fe-core/src/main/java/org/apache/doris/mysql/privilege/UserRoleManager.java @@ -18,6 +18,7 @@ package org.apache.doris.mysql.privilege; import org.apache.doris.analysis.UserIdentity; +import org.apache.doris.cluster.ClusterNamespace; import org.apache.doris.common.io.Text; import org.apache.doris.common.io.Writable; import org.apache.doris.persist.gson.GsonPostProcessable; @@ -32,9 +33,11 @@ import java.io.DataInput; import java.io.DataOutput; import java.io.IOException; import java.util.Collections; +import java.util.List; import java.util.Map; import java.util.Map.Entry; import java.util.Set; +import java.util.stream.Collectors; public class UserRoleManager implements Writable, GsonPostProcessable { // Concurrency control is delegated by Auth, so not concurrentMap @@ -58,6 +61,35 @@ public class UserRoleManager implements Writable, GsonPostProcessable { roleToUsers.put(roleName, userIdentities); } + public void addUserRoles(UserIdentity userIdentity, List roles) { + for (String roleName : roles) { + addUserRole(userIdentity, roleName); + } + } + + public void removeUserRoles(UserIdentity userIdentity, List roles) { + for (String roleName : roles) { + removeUserRole(userIdentity, roleName); + } + } + + public void removeUserRole(UserIdentity userIdentity, String roleName) { + Set roles = userToRoles.get(userIdentity); + if (!CollectionUtils.isEmpty(roles)) { + roles.remove(roleName); + } + if (CollectionUtils.isEmpty(roles)) { + userToRoles.remove(userIdentity); + } + Set userIdentities = roleToUsers.get(roleName); + if (!CollectionUtils.isEmpty(userIdentities)) { + userIdentities.remove(userIdentity); + } + if (CollectionUtils.isEmpty(userIdentities)) { + roleToUsers.remove(roleName); + } + } + public void dropUser(UserIdentity userIdentity) { if (!userToRoles.containsKey(userIdentity)) { return; @@ -97,6 +129,17 @@ public class UserRoleManager implements Writable, GsonPostProcessable { return roles == null ? Collections.EMPTY_SET : roles; } + public Set getRolesByUser(UserIdentity user, boolean showUserDefaultRole) { + Set rolesByUser = getRolesByUser(user); + if (showUserDefaultRole) { + return rolesByUser; + } else { + return rolesByUser.stream().filter(role -> + !ClusterNamespace.getNameFromFullName(role).startsWith(RoleManager.DEFAULT_ROLE_PREFIX)).collect( + Collectors.toSet()); + } + } + public Set getUsersByRole(String roleName) { Set userIdentities = roleToUsers.get(roleName); return userIdentities == null ? Collections.EMPTY_SET : userIdentities; diff --git a/fe/fe-core/src/main/java/org/apache/doris/persist/PrivInfo.java b/fe/fe-core/src/main/java/org/apache/doris/persist/PrivInfo.java index f00324e9c4..c3af6940fe 100644 --- a/fe/fe-core/src/main/java/org/apache/doris/persist/PrivInfo.java +++ b/fe/fe-core/src/main/java/org/apache/doris/persist/PrivInfo.java @@ -33,6 +33,7 @@ import com.google.gson.annotations.SerializedName; import java.io.DataInput; import java.io.DataOutput; import java.io.IOException; +import java.util.List; public class PrivInfo implements Writable { @SerializedName(value = "userIdent") @@ -49,6 +50,9 @@ public class PrivInfo implements Writable { private String role; @SerializedName(value = "passwordOptions") private PasswordOptions passwordOptions; + // Indicates that these roles are granted to a user + @SerializedName(value = "roles") + private List roles; private PrivInfo() { @@ -88,6 +92,12 @@ public class PrivInfo implements Writable { this.role = role; } + // For grant/revoke roles to/from userIdent + public PrivInfo(UserIdentity userIdent, List roles) { + this.userIdent = userIdent; + this.roles = roles; + } + public UserIdentity getUserIdent() { return userIdent; } @@ -116,6 +126,10 @@ public class PrivInfo implements Writable { return passwordOptions == null ? PasswordOptions.UNSET_OPTION : passwordOptions; } + public List getRoles() { + return roles; + } + public static PrivInfo read(DataInput in) throws IOException { if (Env.getCurrentEnvJournalVersion() < FeMetaVersion.VERSION_113) { PrivInfo info = new PrivInfo(); diff --git a/fe/fe-core/src/test/java/org/apache/doris/mysql/privilege/AuthTest.java b/fe/fe-core/src/test/java/org/apache/doris/mysql/privilege/AuthTest.java index 55248c8df3..c75fb797e1 100644 --- a/fe/fe-core/src/test/java/org/apache/doris/mysql/privilege/AuthTest.java +++ b/fe/fe-core/src/test/java/org/apache/doris/mysql/privilege/AuthTest.java @@ -857,6 +857,17 @@ public class AuthTest { } Assert.assertTrue(hasException); + // 23. create role start with RoleManager.DEFAULT_ROLE_PREFIX, which is not allowed + roleStmt = new CreateRoleStmt(RoleManager.DEFAULT_ROLE_PREFIX + "role"); + hasException = false; + try { + roleStmt.analyze(analyzer); + } catch (UserException e1) { + e1.printStackTrace(); + hasException = true; + } + Assert.assertTrue(hasException); + // 24. create role roleStmt = new CreateRoleStmt("role1"); try { @@ -1447,6 +1458,83 @@ public class AuthTest { } } + @Test + public void testGrantRole() { + UserIdentity userIdentity = new UserIdentity("testUser", "%"); + UserDesc userDesc = new UserDesc(userIdentity, "12345", true); + String role = "role1"; + // 1. create role + CreateRoleStmt createRoleStmt = new CreateRoleStmt(role); + try { + createRoleStmt.analyze(analyzer); + auth.createRole(createRoleStmt); + } catch (UserException e1) { + e1.printStackTrace(); + Assert.fail(); + } + // grant select_priv on db 'db1' to role 'role1' + GrantStmt grantStmt = new GrantStmt(null, role, new TablePattern("db1", "*"), + Lists.newArrayList(AccessPrivilege.SELECT_PRIV)); + try { + grantStmt.analyze(analyzer); + auth.grant(grantStmt); + } catch (UserException e) { + e.printStackTrace(); + Assert.fail(); + } + + // create user with no role + CreateUserStmt createUserStmt = new CreateUserStmt(false, userDesc, null); + try { + createUserStmt.analyze(analyzer); + auth.createUser(createUserStmt); + } catch (UserException e) { + e.printStackTrace(); + Assert.fail(); + } + Assert.assertFalse(accessManager + .checkDbPriv(userIdentity, SystemInfoService.DEFAULT_CLUSTER + ":db1", PrivPredicate.SELECT)); + // grant 'role1' to testUser + grantStmt = new GrantStmt(Lists.newArrayList(role), userIdentity); + try { + grantStmt.analyze(analyzer); + auth.grant(grantStmt); + } catch (UserException e) { + e.printStackTrace(); + Assert.fail(); + } + Assert.assertTrue(accessManager + .checkDbPriv(userIdentity, SystemInfoService.DEFAULT_CLUSTER + ":db1", PrivPredicate.SELECT)); + // revoke 'role1' from testUser + RevokeStmt revokeStmt = new RevokeStmt(Lists.newArrayList(role), userIdentity); + try { + revokeStmt.analyze(analyzer); + auth.revoke(revokeStmt); + } catch (UserException e) { + e.printStackTrace(); + Assert.fail(); + } + Assert.assertFalse(accessManager + .checkDbPriv(userIdentity, SystemInfoService.DEFAULT_CLUSTER + ":db1", PrivPredicate.SELECT)); + // grant not exist role to testUser + grantStmt = new GrantStmt(Lists.newArrayList("norole"), userIdentity); + try { + grantStmt.analyze(analyzer); + } catch (UserException e) { + e.printStackTrace(); + Assert.fail(); + } + + boolean hasException = false; + try { + auth.grant(grantStmt); + } catch (DdlException e) { + e.printStackTrace(); + hasException = true; + } + Assert.assertTrue(hasException); + } + @Test public void testResource() { UserIdentity userIdentity = new UserIdentity("testUser", "%"); diff --git a/regression-test/data/account_p0/test_alter_user.out b/regression-test/data/account_p0/test_alter_user.out deleted file mode 100644 index 4976e5d715..0000000000 --- a/regression-test/data/account_p0/test_alter_user.out +++ /dev/null @@ -1,10 +0,0 @@ --- This file is automatically generated. You should know what you did if you want to edit this --- !show_grants1 -- -'default_cluster:test_auth_user1'@'%' Yes \N \N internal.default_cluster:db1: Select_priv ; internal.default_cluster:information_schema: Select_priv \N \N - --- !show_grants2 -- -'default_cluster:test_auth_user1'@'%' Yes \N ctl: Drop_priv internal.default_cluster:information_schema: Select_priv \N \N - --- !show_grants3 -- -'default_cluster:test_auth_user1'@'%' Yes \N ctl: Load_priv internal.default_cluster:db1: Select_priv ; internal.default_cluster:information_schema: Select_priv \N \N - diff --git a/regression-test/suites/account_p0/test_alter_user.groovy b/regression-test/suites/account_p0/test_alter_user.groovy index 5e8e6c165f..d97c1243a0 100644 --- a/regression-test/suites/account_p0/test_alter_user.groovy +++ b/regression-test/suites/account_p0/test_alter_user.groovy @@ -17,32 +17,9 @@ suite("test_alter_user", "account") { - sql """drop role if exists test_auth_role1""" - sql """drop role if exists test_auth_role2""" - sql """drop user if exists test_auth_user1""" sql """drop user if exists test_auth_user2""" sql """drop user if exists test_auth_user3""" sql """drop user if exists test_auth_user4""" - - // 1. change user's default role - sql """set global validate_password_policy=NONE""" - sql """create role test_auth_role1""" - sql """grant select_priv on db1.* to role 'test_auth_role1'""" - sql """create role test_auth_role2""" - sql """grant drop_priv on ctl.*.* to role 'test_auth_role2'""" - - sql """create user test_auth_user1 identified by '12345' default role 'test_auth_role1'""" - order_qt_show_grants1 """show grants for 'test_auth_user1'""" - - sql """alter user test_auth_user1 default role 'test_auth_role2'""" - order_qt_show_grants2 """show grants for 'test_auth_user1'""" - - sql """grant load_priv on ctl.*.* to test_auth_user1""" - sql """grant load_priv on ctl.*.* to role 'test_auth_role2'""" - - // change user's role again - sql """alter user test_auth_user1 default role 'test_auth_role1'""" - order_qt_show_grants3 """show grants for 'test_auth_user1'""" // 2. test password history sql """set global password_history=0""" // disabled