This is an automated email from the ASF dual-hosted git repository. yiguolei pushed a commit to branch branch-2.1 in repository https://gitbox.apache.org/repos/asf/doris.git
The following commit(s) were added to refs/heads/branch-2.1 by this push: new d87a220d2b2 [opt](privilege) Grant check name (#39597) (#39856) d87a220d2b2 is described below commit d87a220d2b2bde53da6493f19e09e0b3735846d5 Author: zhangdong <493738...@qq.com> AuthorDate: Mon Aug 26 09:53:45 2024 +0800 [opt](privilege) Grant check name (#39597) (#39856) pick https://github.com/apache/doris/pull/39597 --- .../org/apache/doris/mysql/privilege/Auth.java | 31 ++++++++++++++++++++++ .../apache/doris/datasource/CatalogMgrTest.java | 7 ----- .../suites/auth_p0/test_create_view_auth.groovy | 2 +- .../suites/auth_p0/test_grant_auth.groovy | 2 +- ...uth.groovy => test_grant_nonexist_table.groovy} | 23 +++++++++++++--- .../ccr_mow_syncer_p0/test_get_binlog.groovy | 1 - .../inverted_index/test_get_binlog.groovy | 1 - 7 files changed, 53 insertions(+), 14 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 325ee0ba9a0..c39863cb23d 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 @@ -35,8 +35,10 @@ import org.apache.doris.analysis.SetUserPropertyStmt; import org.apache.doris.analysis.TablePattern; import org.apache.doris.analysis.UserIdentity; import org.apache.doris.analysis.WorkloadGroupPattern; +import org.apache.doris.catalog.DatabaseIf; import org.apache.doris.catalog.Env; import org.apache.doris.catalog.InfoSchemaDb; +import org.apache.doris.catalog.TableIf; import org.apache.doris.cluster.ClusterNamespace; import org.apache.doris.common.AnalysisException; import org.apache.doris.common.AuthenticationException; @@ -51,6 +53,7 @@ import org.apache.doris.common.Pair; import org.apache.doris.common.PatternMatcherException; import org.apache.doris.common.UserException; import org.apache.doris.common.io.Writable; +import org.apache.doris.datasource.CatalogIf; import org.apache.doris.datasource.InternalCatalog; import org.apache.doris.mysql.MysqlPassword; import org.apache.doris.mysql.authenticate.AuthenticateType; @@ -83,6 +86,7 @@ import java.util.HashSet; import java.util.List; import java.util.Map; import java.util.Map.Entry; +import java.util.Objects; import java.util.Set; import java.util.concurrent.locks.ReentrantReadWriteLock; import java.util.stream.Collectors; @@ -593,6 +597,7 @@ public class Auth implements Writable { throws DdlException { writeLock(); try { + checkTablePatternExist(tblPattern); if (role == null) { if (!doesUserExist(userIdent)) { throw new DdlException("user " + userIdent + " does not exist"); @@ -611,6 +616,32 @@ public class Auth implements Writable { } } + private void checkTablePatternExist(TablePattern tablePattern) throws DdlException { + Objects.requireNonNull(tablePattern, "tablePattern can not be null"); + PrivLevel privLevel = tablePattern.getPrivLevel(); + if (privLevel == PrivLevel.GLOBAL) { + return; + } + CatalogIf catalog = Env.getCurrentEnv().getCatalogMgr().getCatalog(tablePattern.getQualifiedCtl()); + if (catalog == null) { + throw new DdlException("catalog:" + tablePattern.getQualifiedCtl() + " does not exist"); + } + if (privLevel == PrivLevel.CATALOG) { + return; + } + DatabaseIf db = catalog.getDbNullable(tablePattern.getQualifiedDb()); + if (db == null) { + throw new DdlException("database:" + tablePattern.getQualifiedDb() + " does not exist"); + } + if (privLevel == PrivLevel.DATABASE) { + return; + } + TableIf table = db.getTableNullable(tablePattern.getTbl()); + if (table == null) { + throw new DdlException("table:" + tablePattern.getTbl() + " does not exist"); + } + } + // grant for ResourcePattern private void grantInternal(UserIdentity userIdent, String role, ResourcePattern resourcePattern, PrivBitSet privs, boolean errOnNonExist, boolean isReplay) throws DdlException { diff --git a/fe/fe-core/src/test/java/org/apache/doris/datasource/CatalogMgrTest.java b/fe/fe-core/src/test/java/org/apache/doris/datasource/CatalogMgrTest.java index e5e8a9d71a5..fa546cccec9 100644 --- a/fe/fe-core/src/test/java/org/apache/doris/datasource/CatalogMgrTest.java +++ b/fe/fe-core/src/test/java/org/apache/doris/datasource/CatalogMgrTest.java @@ -105,13 +105,6 @@ public class CatalogMgrTest extends TestWithFeService { // grant with no catalog is switched, internal catalog works. CreateRoleStmt createRole1 = (CreateRoleStmt) parseAndAnalyzeStmt("create role role1;", rootCtx); auth.createRole(createRole1); - GrantStmt grantRole1 = (GrantStmt) parseAndAnalyzeStmt("grant grant_priv on tpch.* to role 'role1';", rootCtx); - auth.grant(grantRole1); - // grant with ctl.db.tbl. grant can succeed even if the catalog does not exist - GrantStmt grantRole1WithCtl = (GrantStmt) parseAndAnalyzeStmt( - "grant select_priv on testc.testdb.* to role 'role1';", rootCtx); - auth.grant(grantRole1WithCtl); - // user1 can't switch to hive auth.createUser((CreateUserStmt) parseAndAnalyzeStmt( "create user 'user1'@'%' identified by 'pwd1' default role 'role1';", rootCtx)); user1 = new UserIdentity("user1", "%"); diff --git a/regression-test/suites/auth_p0/test_create_view_auth.groovy b/regression-test/suites/auth_p0/test_create_view_auth.groovy index ecf3cd600e7..4c9d31b5145 100644 --- a/regression-test/suites/auth_p0/test_create_view_auth.groovy +++ b/regression-test/suites/auth_p0/test_create_view_auth.groovy @@ -47,7 +47,7 @@ suite("test_create_view_auth","p0,auth") { assertTrue(e.getMessage().contains("Admin_priv,Create_priv")) } } - sql """grant create_priv on ${dbName}.v1 to ${user}""" + sql """grant create_priv on ${dbName}.* to ${user}""" connect(user=user, password="${pwd}", url=context.config.jdbcUrl) { try { sql "create view ${dbName}.v1 as select * from ${dbName}.${tableName};" diff --git a/regression-test/suites/auth_p0/test_grant_auth.groovy b/regression-test/suites/auth_p0/test_grant_auth.groovy index fd3f51a4081..c026cfd91ed 100644 --- a/regression-test/suites/auth_p0/test_grant_auth.groovy +++ b/regression-test/suites/auth_p0/test_grant_auth.groovy @@ -22,7 +22,7 @@ suite("test_grant_auth","p0,auth") { String pwd = 'C123_567p' try_sql("DROP USER ${user}") sql """CREATE USER '${user}' IDENTIFIED BY '${pwd}'""" - sql """grant select_priv on `_internal_schema`.* to ${user}""" + sql """grant select_priv on `__internal_schema`.* to ${user}""" try_sql("DROP USER ${user}") } diff --git a/regression-test/suites/auth_p0/test_grant_auth.groovy b/regression-test/suites/auth_p0/test_grant_nonexist_table.groovy similarity index 61% copy from regression-test/suites/auth_p0/test_grant_auth.groovy copy to regression-test/suites/auth_p0/test_grant_nonexist_table.groovy index fd3f51a4081..36e75707be7 100644 --- a/regression-test/suites/auth_p0/test_grant_auth.groovy +++ b/regression-test/suites/auth_p0/test_grant_nonexist_table.groovy @@ -17,12 +17,29 @@ import org.junit.Assert; -suite("test_grant_auth","p0,auth") { - String user = 'test_grant_auth_user' +suite("test_grant_nonexist_table","p0,auth") { + String suiteName = "test_grant_nonexist_table" + String dbName = context.config.getDbNameByFile(context.file) + String user = "${suiteName}_user" String pwd = 'C123_567p' try_sql("DROP USER ${user}") sql """CREATE USER '${user}' IDENTIFIED BY '${pwd}'""" - sql """grant select_priv on `_internal_schema`.* to ${user}""" + + test { + sql """grant select_priv on non_exist_catalog.*.* to ${user}""" + exception "catalog" + } + + test { + sql """grant select_priv on internal.non_exist_db.* to ${user}""" + exception "database" + } + + test { + sql """grant select_priv on internal.${dbName}.non_exist_table to ${user}""" + exception "table" + } + try_sql("DROP USER ${user}") } diff --git a/regression-test/suites/ccr_mow_syncer_p0/test_get_binlog.groovy b/regression-test/suites/ccr_mow_syncer_p0/test_get_binlog.groovy index 62ba97551f6..13fe4eeec33 100644 --- a/regression-test/suites/ccr_mow_syncer_p0/test_get_binlog.groovy +++ b/regression-test/suites/ccr_mow_syncer_p0/test_get_binlog.groovy @@ -133,7 +133,6 @@ suite("test_mow_get_binlog_case") { sql """DROP USER IF EXISTS ${noPrivUser}""" sql """CREATE USER ${noPrivUser} IDENTIFIED BY '123456'""" sql """GRANT ALL ON ${context.config.defaultDb}.* TO ${noPrivUser}""" - sql """GRANT ALL ON TEST_${context.dbName}.${emptyTable} TO ${noPrivUser}""" syncer.context.user = "${noPrivUser}" syncer.context.passwd = "123456" assertTrue((syncer.getBinlog("${seqTableName}")) == false) diff --git a/regression-test/suites/ccr_syncer_p0/inverted_index/test_get_binlog.groovy b/regression-test/suites/ccr_syncer_p0/inverted_index/test_get_binlog.groovy index b837f799e58..1e3fed68923 100644 --- a/regression-test/suites/ccr_syncer_p0/inverted_index/test_get_binlog.groovy +++ b/regression-test/suites/ccr_syncer_p0/inverted_index/test_get_binlog.groovy @@ -226,7 +226,6 @@ suite("test_get_binlog_case_index") { """ sql """CREATE USER IF NOT EXISTS ${noPrivUser} IDENTIFIED BY '123456'""" sql """GRANT ALL ON ${context.config.defaultDb}.* TO ${noPrivUser}""" - sql """GRANT ALL ON TEST_${context.dbName}.${emptyTable} TO ${noPrivUser}""" syncer.context.user = "${noPrivUser}" syncer.context.passwd = "123456" assertTrue((syncer.getBinlog("${tableName}")) == false) --------------------------------------------------------------------- To unsubscribe, e-mail: commits-unsubscr...@doris.apache.org For additional commands, e-mail: commits-h...@doris.apache.org