This is an automated email from the ASF dual-hosted git repository.

morrysnow pushed a commit to branch branch-3.1
in repository https://gitbox.apache.org/repos/asf/doris.git


The following commit(s) were added to refs/heads/branch-3.1 by this push:
     new 09bf917ff3a branch-3.1: [fix](auth)fix when authentication, the 
permissions of multiple roles should be merged #52349 (#52949)
09bf917ff3a is described below

commit 09bf917ff3a8d4eb11fcd4e97580085833181f11
Author: zhangdong <[email protected]>
AuthorDate: Wed Jul 9 17:32:53 2025 +0800

    branch-3.1: [fix](auth)fix when authentication, the permissions of multiple 
roles should be merged #52349 (#52949)
    
    pick from #52349
---
 .../org/apache/doris/mysql/privilege/Auth.java     | 49 +++++++++-------------
 .../org/apache/doris/mysql/privilege/Role.java     | 46 ++++++--------------
 .../apache/doris/mysql/privilege/RoleManager.java  |  2 +-
 .../suites/account_p0/test_grant_priv.groovy       | 38 +++++++++++++++++
 4 files changed, 70 insertions(+), 65 deletions(-)

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 ab5a9340f7d..1ad3289803c 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
@@ -270,8 +270,9 @@ public class Auth implements Writable {
         readLock();
         try {
             Set<Role> roles = getRolesByUserWithLdap(currentUser);
+            PrivBitSet savedPrivs = PrivBitSet.of();
             for (Role role : roles) {
-                if (role.checkGlobalPriv(wanted)) {
+                if (role.checkGlobalPriv(wanted, savedPrivs)) {
                     return true;
                 }
             }
@@ -293,8 +294,9 @@ public class Auth implements Writable {
         readLock();
         try {
             Set<Role> roles = getRolesByUserWithLdap(currentUser);
+            PrivBitSet savedPrivs = PrivBitSet.of();
             for (Role role : roles) {
-                if (role.checkCtlPriv(ctl, wanted)) {
+                if (role.checkCtlPriv(ctl, wanted, savedPrivs)) {
                     return true;
                 }
             }
@@ -316,8 +318,9 @@ public class Auth implements Writable {
         readLock();
         try {
             Set<Role> roles = getRolesByUserWithLdap(currentUser);
+            PrivBitSet savedPrivs = PrivBitSet.of();
             for (Role role : roles) {
-                if (role.checkDbPriv(ctl, db, wanted)) {
+                if (role.checkDbPriv(ctl, db, wanted, savedPrivs)) {
                     return true;
                 }
             }
@@ -339,8 +342,9 @@ public class Auth implements Writable {
         readLock();
         try {
             Set<Role> roles = getRolesByUserWithLdap(currentUser);
+            PrivBitSet savedPrivs = PrivBitSet.of();
             for (Role role : roles) {
-                if (role.checkTblPriv(ctl, db, tbl, wanted)) {
+                if (role.checkTblPriv(ctl, db, tbl, wanted, savedPrivs)) {
                     return true;
                 }
             }
@@ -367,8 +371,9 @@ public class Auth implements Writable {
 
     private boolean checkColPriv(String ctl, String db, String tbl,
             String col, PrivPredicate wanted, Set<Role> roles) {
+        PrivBitSet savedPrivs = PrivBitSet.of();
         for (Role role : roles) {
-            if (role.checkColPriv(ctl, db, tbl, col, wanted)) {
+            if (role.checkColPriv(ctl, db, tbl, col, wanted, savedPrivs)) {
                 return true;
             }
         }
@@ -380,8 +385,9 @@ public class Auth implements Writable {
         readLock();
         try {
             Set<Role> roles = getRolesByUserWithLdap(currentUser);
+            PrivBitSet savedPrivs = PrivBitSet.of();
             for (Role role : roles) {
-                if (role.checkResourcePriv(resourceName, wanted)) {
+                if (role.checkResourcePriv(resourceName, wanted, savedPrivs)) {
                     return true;
                 }
             }
@@ -396,8 +402,9 @@ public class Auth implements Writable {
         readLock();
         try {
             Set<Role> roles = getRolesByUserWithLdap(currentUser);
+            PrivBitSet savedPrivs = PrivBitSet.of();
             for (Role role : roles) {
-                if (role.checkStorageVaultPriv(storageVaultName, wanted)) {
+                if (role.checkStorageVaultPriv(storageVaultName, wanted, 
savedPrivs)) {
                     return true;
                 }
             }
@@ -418,8 +425,9 @@ public class Auth implements Writable {
             }
 
             Set<Role> roles = getRolesByUserWithLdap(currentUser);
+            PrivBitSet savedPrivs = PrivBitSet.of();
             for (Role role : roles) {
-                if (role.checkWorkloadGroupPriv(workloadGroupName, wanted)) {
+                if (role.checkWorkloadGroupPriv(workloadGroupName, wanted, 
savedPrivs)) {
                     return true;
                 }
             }
@@ -440,29 +448,10 @@ public class Auth implements Writable {
                     return true;
                 }
             }
-            Set<String> roles = userRoleManager.getRolesByUser(currentUser);
-            for (String roleName : roles) {
-                if (roleManager.getRole(roleName).checkCloudPriv(cloudName, 
wanted, type)) {
-                    return true;
-                }
-            }
-            return false;
-        } finally {
-            readUnlock();
-        }
-    }
-
-    // ==== Other ====
-    /*
-     * Check if current user has certain privilege.
-     * This method will check the given privilege levels
-     */
-    public boolean checkHasPriv(ConnectContext ctx, PrivPredicate priv, 
PrivLevel... levels) {
-        readLock();
-        try {
-            Set<Role> roles = 
getRolesByUserWithLdap(ctx.getCurrentUserIdentity());
+            Set<Role> roles = getRolesByUserWithLdap(currentUser);
+            PrivBitSet savedPrivs = PrivBitSet.of();
             for (Role role : roles) {
-                if (role.checkHasPriv(priv, levels)) {
+                if (role.checkCloudPriv(cloudName, wanted, type, savedPrivs)) {
                     return true;
                 }
             }
diff --git 
a/fe/fe-core/src/main/java/org/apache/doris/mysql/privilege/Role.java 
b/fe/fe-core/src/main/java/org/apache/doris/mysql/privilege/Role.java
index 4506afb3399..3d6f496d54b 100644
--- a/fe/fe-core/src/main/java/org/apache/doris/mysql/privilege/Role.java
+++ b/fe/fe-core/src/main/java/org/apache/doris/mysql/privilege/Role.java
@@ -313,13 +313,11 @@ public class Role implements Writable, 
GsonPostProcessable {
         mergeNotCheck(other);
     }
 
-    public boolean checkGlobalPriv(PrivPredicate wanted) {
-        PrivBitSet savedPrivs = PrivBitSet.of();
+    public boolean checkGlobalPriv(PrivPredicate wanted, PrivBitSet 
savedPrivs) {
         return checkGlobalInternal(wanted, savedPrivs);
     }
 
-    public boolean checkCtlPriv(String ctl, PrivPredicate wanted) {
-        PrivBitSet savedPrivs = PrivBitSet.of();
+    public boolean checkCtlPriv(String ctl, PrivPredicate wanted, PrivBitSet 
savedPrivs) {
         if (checkGlobalInternal(wanted, savedPrivs)
                 || checkCatalogInternal(ctl, wanted, savedPrivs)) {
             return true;
@@ -363,8 +361,7 @@ public class Role implements Writable, GsonPostProcessable {
         return false;
     }
 
-    public boolean checkDbPriv(String ctl, String db, PrivPredicate wanted) {
-        PrivBitSet savedPrivs = PrivBitSet.of();
+    public boolean checkDbPriv(String ctl, String db, PrivPredicate wanted, 
PrivBitSet savedPrivs) {
         if (checkGlobalInternal(wanted, savedPrivs)
                 || checkCatalogInternal(ctl, wanted, savedPrivs)
                 || checkDbInternal(ctl, db, wanted, savedPrivs)) {
@@ -435,8 +432,7 @@ public class Role implements Writable, GsonPostProcessable {
         return false;
     }
 
-    public boolean checkTblPriv(String ctl, String db, String tbl, 
PrivPredicate wanted) {
-        PrivBitSet savedPrivs = PrivBitSet.of();
+    public boolean checkTblPriv(String ctl, String db, String tbl, 
PrivPredicate wanted, PrivBitSet savedPrivs) {
         if (checkGlobalInternal(wanted, savedPrivs)
                 || checkCatalogInternal(ctl, wanted, savedPrivs)
                 || checkDbInternal(ctl, db, wanted, savedPrivs)
@@ -457,14 +453,13 @@ public class Role implements Writable, 
GsonPostProcessable {
     }
 
     public boolean checkCloudPriv(String cloudName,
-            PrivPredicate wanted, ResourceTypeEnum type) {
+            PrivPredicate wanted, ResourceTypeEnum type, PrivBitSet 
savedPrivs) {
         ResourcePrivTable cloudPrivTable = getCloudPrivTable(type);
         if (cloudPrivTable == null) {
             LOG.warn("cloud resource type err: {}", type);
             return false;
         }
 
-        PrivBitSet savedPrivs = PrivBitSet.of();
         if (checkGlobalInternal(wanted, savedPrivs)
                 || checkCloudInternal(cloudName, wanted, savedPrivs, 
cloudPrivTable, type)
                 || checkCloudVirtualComputeGroup(cloudName, wanted, 
savedPrivs, cloudPrivTable)) {
@@ -475,12 +470,14 @@ public class Role implements Writable, 
GsonPostProcessable {
         return false;
     }
 
-    public boolean checkColPriv(String ctl, String db, String tbl, String col, 
PrivPredicate wanted) {
+    public boolean checkColPriv(String ctl, String db, String tbl, String col, 
PrivPredicate wanted,
+            PrivBitSet savedPrivs) {
         Optional<Privilege> colPrivilege = wanted.getColPrivilege();
         if (!colPrivilege.isPresent()) {
             throw new IllegalStateException("this privPredicate should not use 
checkColPriv:" + wanted);
         }
-        return checkTblPriv(ctl, db, tbl, wanted) || onlyCheckColPriv(ctl, db, 
tbl, col, colPrivilege.get());
+        return checkTblPriv(ctl, db, tbl, wanted, savedPrivs) || 
onlyCheckColPriv(ctl, db, tbl, col,
+                colPrivilege.get());
     }
 
     private boolean onlyCheckColPriv(String ctl, String db, String tbl, String 
col,
@@ -497,8 +494,7 @@ public class Role implements Writable, GsonPostProcessable {
         return Privilege.satisfy(savedPrivs, wanted);
     }
 
-    public boolean checkResourcePriv(String resourceName, PrivPredicate 
wanted) {
-        PrivBitSet savedPrivs = PrivBitSet.of();
+    public boolean checkResourcePriv(String resourceName, PrivPredicate 
wanted, PrivBitSet savedPrivs) {
         if (checkGlobalInternal(wanted, savedPrivs)
                 || checkResourceInternal(resourceName, wanted, savedPrivs)) {
             return true;
@@ -515,8 +511,7 @@ public class Role implements Writable, GsonPostProcessable {
         return Privilege.satisfy(savedPrivs, wanted);
     }
 
-    public boolean checkStorageVaultPriv(String storageVaultName, 
PrivPredicate wanted) {
-        PrivBitSet savedPrivs = PrivBitSet.of();
+    public boolean checkStorageVaultPriv(String storageVaultName, 
PrivPredicate wanted, PrivBitSet savedPrivs) {
         if (checkGlobalInternal(wanted, savedPrivs)
                 || checkStorageVaultInternal(storageVaultName, wanted, 
savedPrivs)) {
             return true;
@@ -550,12 +545,11 @@ public class Role implements Writable, 
GsonPostProcessable {
         return Privilege.satisfy(savedPrivs, wanted);
     }
 
-    public boolean checkWorkloadGroupPriv(String workloadGroupName, 
PrivPredicate wanted) {
+    public boolean checkWorkloadGroupPriv(String workloadGroupName, 
PrivPredicate wanted, PrivBitSet savedPrivs) {
         // For compatibility with older versions, it is not needed to check 
the privileges of the default group.
         if (WorkloadGroupMgr.DEFAULT_GROUP_NAME.equals(workloadGroupName)) {
             return true;
         }
-        PrivBitSet savedPrivs = PrivBitSet.of();
         // usage priv not in global, but grant_priv may in global
         if (checkGlobalInternal(wanted, savedPrivs)
                 || checkWorkloadGroupInternal(workloadGroupName, wanted, 
savedPrivs)) {
@@ -646,22 +640,6 @@ public class Role implements Writable, GsonPostProcessable 
{
         this.comment = comment;
     }
 
-    public boolean checkCanEnterCluster(String clusterName) {
-        if (checkGlobalPriv(PrivPredicate.ALL)) {
-            return true;
-        }
-
-        if (dbPrivTable.hasClusterPriv(clusterName)) {
-            return true;
-        }
-
-        if (tablePrivTable.hasClusterPriv(clusterName)) {
-            return true;
-        }
-        return false;
-    }
-
-
     private void grantPrivs(ResourcePattern resourcePattern, PrivBitSet privs) 
throws DdlException {
         if (privs.isEmpty()) {
             return;
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 46949ef4739..eba297bfd37 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
@@ -224,7 +224,7 @@ public class RoleManager implements Writable, 
GsonPostProcessable {
             if (limitedRole != null && 
!limitedRole.contains(role.getRoleName())) {
                 continue;
             }
-            String isGrantable = role.checkGlobalPriv(PrivPredicate.ADMIN) ? 
"YES" : "NO";
+            String isGrantable = role.checkGlobalPriv(PrivPredicate.ADMIN, 
PrivBitSet.of()) ? "YES" : "NO";
 
             for (Map.Entry<WorkloadGroupPattern, PrivBitSet> entry : 
role.getWorkloadGroupPatternToPrivs().entrySet()) {
                 List<String> row = Lists.newArrayList();
diff --git a/regression-test/suites/account_p0/test_grant_priv.groovy 
b/regression-test/suites/account_p0/test_grant_priv.groovy
index d2e04945905..ef0dda3a07a 100644
--- a/regression-test/suites/account_p0/test_grant_priv.groovy
+++ b/regression-test/suites/account_p0/test_grant_priv.groovy
@@ -21,6 +21,7 @@ suite("test_grant_priv") {
     def user1 = 'test_grant_priv_user1'
     def user2 = 'test_grant_priv_user2'
     def role1 = 'test_grant_priv_role1'
+    def role2 = 'test_grant_priv_role2'
     def pwd = '123456'
     def dbName = 'test_grant_priv_db'
     def tokens = context.config.jdbcUrl.split('/')
@@ -29,6 +30,7 @@ suite("test_grant_priv") {
     sql """drop user if exists ${user1}"""
     sql """drop user if exists ${user2}"""
     sql """drop role if exists ${role1}"""
+    sql """drop role if exists ${role2}"""
     sql """DROP DATABASE IF EXISTS ${dbName}"""
 
     sql """CREATE DATABASE ${dbName}"""
@@ -82,5 +84,41 @@ suite("test_grant_priv") {
     sql """drop user if exists ${user1}"""
     sql """drop user if exists ${user2}"""
     sql """drop role if exists ${role1}"""
+    sql """drop role if exists ${role2}"""
+
+    sql """CREATE ROLE ${role1}"""
+    sql """CREATE ROLE ${role2}"""
+    sql """CREATE USER '${user1}' IDENTIFIED BY '${pwd}'"""
+     // for login
+    sql """grant select_priv on ${dbName}.* to ${user1}"""
+    sql """CREATE USER '${user2}' IDENTIFIED BY '${pwd}'"""
+    sql """grant grant_priv on *.*.* to role '${role1}'"""
+    sql """grant select_priv on *.*.* to role '${role2}'"""
+    sql """grant '${role1}' to ${user1}"""
+    // test only have role1 can not grant
+    connect(user1, "${pwd}", url) {
+        test {
+                 sql """grant select_priv on *.*.* to ${user2}"""
+                  exception "denied"
+              }
+    }
+    sql """revoke '${role1}' from ${user1}"""
+    sql """grant '${role2}' to ${user1}"""
+    // test only have role2 can not grant
+    connect(user1, "${pwd}", url) {
+        test {
+                  sql """grant select_priv on *.*.* to ${user2}"""
+                  exception "denied"
+              }
+    }
+    // test both have role1 and role2 can grant to other
+    sql """grant '${role1}' to ${user1}"""
+    connect(user1, "${pwd}", url) {
+            sql """grant select_priv on *.*.* to ${user2}"""
+        }
+    sql """drop user if exists ${user1}"""
+    sql """drop user if exists ${user2}"""
+    sql """drop role if exists ${role1}"""
+    sql """drop role if exists ${role2}"""
     sql """DROP DATABASE IF EXISTS ${dbName}"""
 }


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to