Skip to content

Commit

Permalink
[opt](privilege) Grant check name (apache#39597)
Browse files Browse the repository at this point in the history
grant stmt check ctl/db/table if exist
  • Loading branch information
zddr committed Aug 23, 2024
1 parent e716658 commit d0d390b
Show file tree
Hide file tree
Showing 7 changed files with 78 additions and 11 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -35,8 +35,10 @@
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;
Expand All @@ -51,6 +53,7 @@
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;
Expand Down Expand Up @@ -83,6 +86,7 @@
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;
Expand Down Expand Up @@ -593,6 +597,7 @@ private void grantInternal(UserIdentity userIdent, String role, TablePattern tbl
throws DdlException {
writeLock();
try {
checkTablePatternExist(tblPattern);
if (role == null) {
if (!doesUserExist(userIdent)) {
throw new DdlException("user " + userIdent + " does not exist");
Expand All @@ -611,6 +616,32 @@ private void grantInternal(UserIdentity userIdent, String role, TablePattern tbl
}
}

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 {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -105,13 +105,6 @@ protected void runBeforeAll() throws Exception {
// 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", "%");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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};"
Expand Down
2 changes: 1 addition & 1 deletion regression-test/suites/auth_p0/test_grant_auth.groovy
Original file line number Diff line number Diff line change
Expand Up @@ -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}")
}
45 changes: 45 additions & 0 deletions regression-test/suites/auth_p0/test_grant_nonexist_table.groovy
Original file line number Diff line number Diff line change
@@ -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}")
}
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down

0 comments on commit d0d390b

Please sign in to comment.