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 bf5aec69dce54d..5e5670e10bdb9e 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 @@ -274,8 +274,9 @@ protected boolean checkGlobalPriv(UserIdentity currentUser, PrivPredicate wanted readLock(); try { Set roles = getRolesByUserWithLdap(currentUser); + PrivBitSet savedPrivs = PrivBitSet.of(); for (Role role : roles) { - if (role.checkGlobalPriv(wanted)) { + if (role.checkGlobalPriv(wanted, savedPrivs)) { return true; } } @@ -297,8 +298,9 @@ protected boolean checkCtlPriv(UserIdentity currentUser, String ctl, PrivPredica readLock(); try { Set roles = getRolesByUserWithLdap(currentUser); + PrivBitSet savedPrivs = PrivBitSet.of(); for (Role role : roles) { - if (role.checkCtlPriv(ctl, wanted)) { + if (role.checkCtlPriv(ctl, wanted, savedPrivs)) { return true; } } @@ -320,8 +322,9 @@ protected boolean checkDbPriv(UserIdentity currentUser, String ctl, String db, P readLock(); try { Set 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; } } @@ -343,8 +346,9 @@ protected boolean checkTblPriv(UserIdentity currentUser, String ctl, String db, readLock(); try { Set 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; } } @@ -371,8 +375,9 @@ protected void checkColsPriv(UserIdentity currentUser, String ctl, String db, St private boolean checkColPriv(String ctl, String db, String tbl, String col, PrivPredicate wanted, Set 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; } } @@ -384,8 +389,9 @@ protected boolean checkResourcePriv(UserIdentity currentUser, String resourceNam readLock(); try { Set roles = getRolesByUserWithLdap(currentUser); + PrivBitSet savedPrivs = PrivBitSet.of(); for (Role role : roles) { - if (role.checkResourcePriv(resourceName, wanted)) { + if (role.checkResourcePriv(resourceName, wanted, savedPrivs)) { return true; } } @@ -400,8 +406,9 @@ protected boolean checkStorageVaultPriv(UserIdentity currentUser, String storage readLock(); try { Set roles = getRolesByUserWithLdap(currentUser); + PrivBitSet savedPrivs = PrivBitSet.of(); for (Role role : roles) { - if (role.checkStorageVaultPriv(storageVaultName, wanted)) { + if (role.checkStorageVaultPriv(storageVaultName, wanted, savedPrivs)) { return true; } } @@ -422,8 +429,9 @@ protected boolean checkWorkloadGroupPriv(UserIdentity currentUser, String worklo } Set roles = getRolesByUserWithLdap(currentUser); + PrivBitSet savedPrivs = PrivBitSet.of(); for (Role role : roles) { - if (role.checkWorkloadGroupPriv(workloadGroupName, wanted)) { + if (role.checkWorkloadGroupPriv(workloadGroupName, wanted, savedPrivs)) { return true; } } @@ -444,29 +452,10 @@ protected boolean checkCloudPriv(UserIdentity currentUser, String cloudName, return true; } } - Set 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 roles = getRolesByUserWithLdap(ctx.getCurrentUserIdentity()); + Set 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 5bf39ad04a62d5..36f86d3255e85c 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 @@ -301,13 +301,11 @@ public void merge(Role other) throws DdlException { 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; @@ -351,8 +349,7 @@ private boolean checkCatalogInternal(String ctl, PrivPredicate wanted, PrivBitSe 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)) { @@ -423,8 +420,7 @@ private boolean checkDbInternal(String ctl, String db, PrivPredicate wanted, 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) @@ -445,14 +441,13 @@ public boolean checkTblPriv(String ctl, String db, String tbl, PrivPredicate wan } 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)) { return true; @@ -462,12 +457,14 @@ public boolean checkCloudPriv(String cloudName, 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 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, @@ -484,8 +481,7 @@ private boolean checkTblInternal(String ctl, String db, String tbl, PrivPredicat 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; @@ -502,8 +498,7 @@ private boolean checkResourceInternal(String resourceName, PrivPredicate wanted, 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; @@ -526,12 +521,11 @@ private boolean checkCloudInternal(String cloudName, PrivPredicate wanted, 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)) { @@ -622,22 +616,6 @@ public void setComment(String comment) { 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 4e43f4fd267a30..2da93c298e30b7 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 @@ -223,7 +223,7 @@ public void getRoleWorkloadGroupPrivs(List> result, Set lim 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 entry : role.getWorkloadGroupPatternToPrivs().entrySet()) { List row = Lists.newArrayList(); 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 new file mode 100644 index 00000000000000..3498cd5365076f --- /dev/null +++ b/fe/fe-core/src/test/java/org/apache/doris/mysql/privilege/AuthTest.java @@ -0,0 +1,48 @@ +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. + +package org.apache.doris.mysql.privilege; + +import org.apache.doris.analysis.CompoundPredicate.Operator; +import org.apache.doris.analysis.UserIdentity; +import org.apache.doris.catalog.Env; +import org.apache.doris.datasource.InternalCatalog; +import org.apache.doris.utframe.TestWithFeService; + +import org.junit.jupiter.api.Test; + +public class AuthTest extends TestWithFeService { + @Override + protected void runBeforeAll() throws Exception { + createDatabase("test"); + } + + @Test + public void testMergeRolePriv() throws Exception { + addUser("u1", true); + createRole("role1"); + createRole("role2"); + grantPriv("GRANT LOAD_PRIV ON internal.test.* TO ROLE 'role1';"); + grantPriv("GRANT GRANT_PRIV ON internal.test.* TO ROLE 'role2';"); + + grantRole("GRANT 'role1','role2' TO 'u1'@'%'"); + Env.getCurrentEnv().getAuth().checkDbPriv(UserIdentity.createAnalyzedUserIdentWithIp("u1", "%"), + InternalCatalog.INTERNAL_CATALOG_NAME, "test", + PrivPredicate.of(PrivBitSet.of(Privilege.GRANT_PRIV, Privilege.LOAD_PRIV), Operator.AND)); + } + +} diff --git a/fe/fe-core/src/test/java/org/apache/doris/utframe/TestWithFeService.java b/fe/fe-core/src/test/java/org/apache/doris/utframe/TestWithFeService.java index 3ce3fcf8d7b337..7d51a1e2a8565e 100644 --- a/fe/fe-core/src/test/java/org/apache/doris/utframe/TestWithFeService.java +++ b/fe/fe-core/src/test/java/org/apache/doris/utframe/TestWithFeService.java @@ -62,12 +62,15 @@ import org.apache.doris.nereids.trees.plans.commands.AlterMTMVCommand; import org.apache.doris.nereids.trees.plans.commands.CreateMTMVCommand; import org.apache.doris.nereids.trees.plans.commands.CreatePolicyCommand; +import org.apache.doris.nereids.trees.plans.commands.CreateRoleCommand; import org.apache.doris.nereids.trees.plans.commands.CreateTableCommand; import org.apache.doris.nereids.trees.plans.commands.CreateUserCommand; import org.apache.doris.nereids.trees.plans.commands.CreateViewCommand; import org.apache.doris.nereids.trees.plans.commands.DropConstraintCommand; import org.apache.doris.nereids.trees.plans.commands.DropMTMVCommand; import org.apache.doris.nereids.trees.plans.commands.DropRowPolicyCommand; +import org.apache.doris.nereids.trees.plans.commands.GrantRoleCommand; +import org.apache.doris.nereids.trees.plans.commands.GrantTablePrivilegeCommand; import org.apache.doris.nereids.trees.plans.commands.info.TableNameInfo; import org.apache.doris.nereids.trees.plans.logical.LogicalPlan; import org.apache.doris.nereids.util.MemoTestUtils; @@ -821,6 +824,34 @@ protected void addUser(String userName, boolean ifNotExists) throws Exception { } } + protected void createRole(String roleName) throws Exception { + NereidsParser nereidsParser = new NereidsParser(); + String sql = "CREATE ROLE " + roleName; + LogicalPlan parsed = nereidsParser.parseSingle(sql); + StmtExecutor stmtExecutor = new StmtExecutor(connectContext, sql); + if (parsed instanceof CreateRoleCommand) { + ((CreateRoleCommand) parsed).run(connectContext, stmtExecutor); + } + } + + protected void grantRole(String sql) throws Exception { + NereidsParser nereidsParser = new NereidsParser(); + LogicalPlan parsed = nereidsParser.parseSingle(sql); + StmtExecutor stmtExecutor = new StmtExecutor(connectContext, sql); + if (parsed instanceof GrantRoleCommand) { + ((GrantRoleCommand) parsed).run(connectContext, stmtExecutor); + } + } + + protected void grantPriv(String sql) throws Exception { + NereidsParser nereidsParser = new NereidsParser(); + LogicalPlan parsed = nereidsParser.parseSingle(sql); + StmtExecutor stmtExecutor = new StmtExecutor(connectContext, sql); + if (parsed instanceof GrantTablePrivilegeCommand) { + ((GrantTablePrivilegeCommand) parsed).run(connectContext, stmtExecutor); + } + } + protected void addRollup(String sql) throws Exception { AlterTableStmt alterTableStmt = (AlterTableStmt) UtFrameUtils.parseAndAnalyzeStmt(sql, connectContext); Env.getCurrentEnv().alterTable(alterTableStmt); diff --git a/regression-test/suites/account_p0/test_grant_priv.groovy b/regression-test/suites/account_p0/test_grant_priv.groovy index d2e04945905bee..21d26dd30d8330 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}""" }