morningman commented on code in PR #17236:
URL: https://github.com/apache/doris/pull/17236#discussion_r1125591052
##########
fe/fe-core/src/main/java/org/apache/doris/mysql/privilege/UserRoleManager.java:
##########
@@ -58,6 +61,35 @@ public void addUserRole(UserIdentity userIdentity, String
roleName) {
roleToUsers.put(roleName, userIdentities);
}
+ public void addUserRole(UserIdentity userIdentity, List<String> roles) {
+ for (String roleName : roles) {
+ addUserRole(userIdentity, roleName);
+ }
+ }
+
+ public void reduceUserRole(UserIdentity userIdentity, List<String> roles) {
Review Comment:
```suggestion
public void reduceUserRoles(UserIdentity userIdentity, List<String>
roles) {
```
##########
fe/fe-core/src/main/java/org/apache/doris/common/proc/AuthProcDir.java:
##########
@@ -31,7 +31,7 @@
*/
public class AuthProcDir implements ProcDirInterface {
public static final ImmutableList<String> TITLE_NAMES = new
ImmutableList.Builder<String>()
-
.add("UserIdentity").add("Password").add("GlobalPrivs").add("CatalogPrivs")
+
.add("UserIdentity").add("Password").add("Roles").add("GlobalPrivs").add("CatalogPrivs")
Review Comment:
Is it suitable to add a new column here?
how about move it to `show roles`?
##########
fe/fe-core/src/main/java/org/apache/doris/mysql/privilege/Auth.java:
##########
@@ -668,6 +701,30 @@ private void revokeInternal(UserIdentity userIdent, String
role, ResourcePattern
}
}
+ // revoke for roles
+ private void revokeInternal(UserIdentity userIdent, List<String> 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.reduceUserRole(userIdent, roles);
Review Comment:
```suggestion
userRoleManager.removeUserRole(userIdent, roles);
```
add <-> remove
##########
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
Review Comment:
Need to add case for new feature
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
