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 325ee0ba9a..c39863cb23 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 e5e8a9d71a..fa546cccec 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 ecf3cd600e..4c9d31b514 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 fd3f51a408..c026cfd91e 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_nonexist_table.groovy b/regression-test/suites/auth_p0/test_grant_nonexist_table.groovy new file mode 100644 index 0000000000..36e75707be --- /dev/null +++ b/regression-test/suites/auth_p0/test_grant_nonexist_table.groovy @@ -0,0 +1,45 @@ +// 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. + +import org.junit.Assert; + +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}'""" + + 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 62ba97551f..13fe4eeec3 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 b837f799e5..1e3fed6892 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)