This is an automated email from the ASF dual-hosted git repository.
morningman pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/doris.git
The following commit(s) were added to refs/heads/master by this push:
new 6a4976921d [fix](auth)Disable column auth temporarily (#23295)
6a4976921d is described below
commit 6a4976921dc43b810d3f891cb6e27ec3f03e20be
Author: zhangdong <[email protected]>
AuthorDate: Thu Aug 24 23:37:06 2023 +0800
[fix](auth)Disable column auth temporarily (#23295)
- add config `enable_col_auth` to temporarily disable column
permissions(because old/new planner has bug when select from view)
- Restore the old optimizer to the previous authentication method
- Support for new optimizer authentication（Legacy issue: When querying the
view, the permissions of the base table will be authenticated. The view's own
permissions should be authenticated and processed after the new optimizer is
improved）
- fix: show grants for non-existent users
- fix: role:`admin` can not grant/revoke to/from user
---
.../main/java/org/apache/doris/common/Config.java | 5 ++
.../java/org/apache/doris/analysis/GrantStmt.java | 6 +-
.../java/org/apache/doris/analysis/RevokeStmt.java | 2 +-
.../java/org/apache/doris/analysis/SelectStmt.java | 9 +++
.../org/apache/doris/analysis/ShowGrantsStmt.java | 3 +
.../apache/doris/datasource/ExternalCatalog.java | 3 +
.../apache/doris/datasource/InternalCatalog.java | 3 +
.../org/apache/doris/mysql/privilege/Auth.java | 2 +-
.../nereids/rules/analysis/UserAuthentication.java | 29 +++++++--
.../org/apache/doris/planner/OriginalPlanner.java | 73 ----------------------
.../apache/doris/datasource/ColumnPrivTest.java | 3 +
.../org/apache/doris/mysql/privilege/AuthTest.java | 2 +
regression-test/data/account_p0/test_account.out | 4 --
.../suites/account_p0/test_account.groovy | 8 ++-
.../account_p0/test_nereids_authentication.groovy | 5 +-
15 files changed, 67 insertions(+), 90 deletions(-)
diff --git a/fe/fe-common/src/main/java/org/apache/doris/common/Config.java
b/fe/fe-common/src/main/java/org/apache/doris/common/Config.java
index af86454985..9da22b20da 100644
--- a/fe/fe-common/src/main/java/org/apache/doris/common/Config.java
+++ b/fe/fe-common/src/main/java/org/apache/doris/common/Config.java
@@ -2112,6 +2112,11 @@ public class Config extends ConfigBase {
"Whether to use mysql's bigint type to return Doris's largeint
type"})
public static boolean use_mysql_bigint_for_largeint = false;
+ @ConfField(description = {
+ "是否开启列权限",
+ "Whether to enable col auth"})
+ public static boolean enable_col_auth = false;
+
@ConfField
public static boolean forbid_running_alter_job = false;
diff --git a/fe/fe-core/src/main/java/org/apache/doris/analysis/GrantStmt.java
b/fe/fe-core/src/main/java/org/apache/doris/analysis/GrantStmt.java
index aa0c21f0f0..53c19add7e 100644
--- a/fe/fe-core/src/main/java/org/apache/doris/analysis/GrantStmt.java
+++ b/fe/fe-core/src/main/java/org/apache/doris/analysis/GrantStmt.java
@@ -21,6 +21,7 @@ import org.apache.doris.catalog.AccessPrivilegeWithCols;
import org.apache.doris.catalog.Env;
import org.apache.doris.cluster.ClusterNamespace;
import org.apache.doris.common.AnalysisException;
+import org.apache.doris.common.Config;
import org.apache.doris.common.ErrorCode;
import org.apache.doris.common.ErrorReport;
import org.apache.doris.common.FeNameFormat;
@@ -149,7 +150,7 @@ public class GrantStmt extends DdlStmt {
} else if (roles != null) {
for (int i = 0; i < roles.size(); i++) {
String originalRoleName = roles.get(i);
- FeNameFormat.checkRoleName(originalRoleName, false /* can not
be admin */, "Can not grant role");
+ FeNameFormat.checkRoleName(originalRoleName, true /* can be
admin */, "Can not grant role");
roles.set(i,
ClusterNamespace.getFullName(analyzer.getClusterName(), originalRoleName));
}
}
@@ -181,7 +182,8 @@ public class GrantStmt extends DdlStmt {
public static void checkAccessPrivileges(
List<AccessPrivilegeWithCols> accessPrivileges) throws
AnalysisException {
for (AccessPrivilegeWithCols access : accessPrivileges) {
- if (!access.getAccessPrivilege().canHasColPriv() &&
!CollectionUtils.isEmpty(access.getCols())) {
+ if ((!access.getAccessPrivilege().canHasColPriv() ||
!Config.enable_col_auth) && !CollectionUtils
+ .isEmpty(access.getCols())) {
throw new AnalysisException(
String.format("%s do not support col auth.",
access.getAccessPrivilege().name()));
}
diff --git a/fe/fe-core/src/main/java/org/apache/doris/analysis/RevokeStmt.java
b/fe/fe-core/src/main/java/org/apache/doris/analysis/RevokeStmt.java
index 74e9a4de91..8e11c3a7d1 100644
--- a/fe/fe-core/src/main/java/org/apache/doris/analysis/RevokeStmt.java
+++ b/fe/fe-core/src/main/java/org/apache/doris/analysis/RevokeStmt.java
@@ -133,7 +133,7 @@ public class RevokeStmt extends DdlStmt {
} else if (roles != null) {
for (int i = 0; i < roles.size(); i++) {
String originalRoleName = roles.get(i);
- FeNameFormat.checkRoleName(originalRoleName, false /* can not
be admin */, "Can not revoke role");
+ FeNameFormat.checkRoleName(originalRoleName, true /* can be
admin */, "Can not revoke role");
roles.set(i,
ClusterNamespace.getFullName(analyzer.getClusterName(), originalRoleName));
}
}
diff --git a/fe/fe-core/src/main/java/org/apache/doris/analysis/SelectStmt.java
b/fe/fe-core/src/main/java/org/apache/doris/analysis/SelectStmt.java
index b69466ab51..0f03afde0a 100644
--- a/fe/fe-core/src/main/java/org/apache/doris/analysis/SelectStmt.java
+++ b/fe/fe-core/src/main/java/org/apache/doris/analysis/SelectStmt.java
@@ -24,6 +24,7 @@ import org.apache.doris.analysis.CompoundPredicate.Operator;
import org.apache.doris.catalog.AggregateFunction;
import org.apache.doris.catalog.Column;
import org.apache.doris.catalog.DatabaseIf;
+import org.apache.doris.catalog.Env;
import org.apache.doris.catalog.FunctionSet;
import org.apache.doris.catalog.OlapTable;
import org.apache.doris.catalog.PrimitiveType;
@@ -44,6 +45,7 @@ import org.apache.doris.common.TreeNode;
import org.apache.doris.common.UserException;
import org.apache.doris.common.util.SqlUtils;
import org.apache.doris.common.util.ToSqlContext;
+import org.apache.doris.mysql.privilege.PrivPredicate;
import org.apache.doris.qe.ConnectContext;
import org.apache.doris.rewrite.ExprRewriter;
import org.apache.doris.rewrite.mvrewrite.MVSelectFailedException;
@@ -392,6 +394,13 @@ public class SelectStmt extends QueryStmt {
View view = (View) table;
view.getQueryStmt().getTables(analyzer, expandView,
tableMap, parentViewNameSet);
} else {
+ // check auth
+ if (!Env.getCurrentEnv().getAccessManager()
+ .checkTblPriv(ConnectContext.get(),
tblRef.getName(), PrivPredicate.SELECT)) {
+
ErrorReport.reportAnalysisException(ErrorCode.ERR_TABLEACCESS_DENIED_ERROR,
"SELECT",
+ ConnectContext.get().getQualifiedUser(),
ConnectContext.get().getRemoteIP(),
+ dbName + "." + tableName);
+ }
tableMap.put(table.getId(), table);
}
}
diff --git
a/fe/fe-core/src/main/java/org/apache/doris/analysis/ShowGrantsStmt.java
b/fe/fe-core/src/main/java/org/apache/doris/analysis/ShowGrantsStmt.java
index a380b27451..e2c41eed0c 100644
--- a/fe/fe-core/src/main/java/org/apache/doris/analysis/ShowGrantsStmt.java
+++ b/fe/fe-core/src/main/java/org/apache/doris/analysis/ShowGrantsStmt.java
@@ -89,6 +89,9 @@ public class ShowGrantsStmt extends ShowStmt {
ErrorReport.reportAnalysisException(ErrorCode.ERR_SPECIFIC_ACCESS_DENIED_ERROR,
"GRANT");
}
}
+ if (userIdent != null &&
!Env.getCurrentEnv().getAccessManager().getAuth().doesUserExist(userIdent)) {
+ throw new AnalysisException(String.format("User: %s does not
exist", userIdent));
+ }
}
@Override
diff --git
a/fe/fe-core/src/main/java/org/apache/doris/datasource/ExternalCatalog.java
b/fe/fe-core/src/main/java/org/apache/doris/datasource/ExternalCatalog.java
index 301e2039ce..9dbc40ff84 100644
--- a/fe/fe-core/src/main/java/org/apache/doris/datasource/ExternalCatalog.java
+++ b/fe/fe-core/src/main/java/org/apache/doris/datasource/ExternalCatalog.java
@@ -350,6 +350,9 @@ public abstract class ExternalCatalog
@Nullable
@Override
public ExternalDatabase<? extends ExternalTable> getDbNullable(String
dbName) {
+ if (StringUtils.isEmpty(dbName)) {
+ return null;
+ }
try {
makeSureInitialized();
} catch (Exception e) {
diff --git
a/fe/fe-core/src/main/java/org/apache/doris/datasource/InternalCatalog.java
b/fe/fe-core/src/main/java/org/apache/doris/datasource/InternalCatalog.java
index a28f276af7..2be8e61fdf 100644
--- a/fe/fe-core/src/main/java/org/apache/doris/datasource/InternalCatalog.java
+++ b/fe/fe-core/src/main/java/org/apache/doris/datasource/InternalCatalog.java
@@ -256,6 +256,9 @@ public class InternalCatalog implements CatalogIf<Database>
{
@Nullable
@Override
public Database getDbNullable(String dbName) {
+ if (StringUtils.isEmpty(dbName)) {
+ return null;
+ }
if (fullNameToDb.containsKey(dbName)) {
return fullNameToDb.get(dbName);
} else {
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 995a016478..636ac50f39 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
@@ -663,7 +663,7 @@ public class Auth implements Writable {
// return true if user ident exist
- private boolean doesUserExist(UserIdentity userIdent) {
+ public boolean doesUserExist(UserIdentity userIdent) {
return userManager.userIdentityExist(userIdent, false);
}
diff --git
a/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/analysis/UserAuthentication.java
b/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/analysis/UserAuthentication.java
index 3c97e90476..86fb64d6e5 100644
---
a/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/analysis/UserAuthentication.java
+++
b/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/analysis/UserAuthentication.java
@@ -17,7 +17,10 @@
package org.apache.doris.nereids.rules.analysis;
+import org.apache.doris.catalog.DatabaseIf;
+import org.apache.doris.catalog.TableIf;
import org.apache.doris.common.ErrorCode;
+import org.apache.doris.datasource.CatalogIf;
import org.apache.doris.mysql.privilege.PrivPredicate;
import org.apache.doris.nereids.exceptions.AnalysisException;
import org.apache.doris.nereids.rules.Rule;
@@ -44,17 +47,31 @@ public class UserAuthentication extends
OneAnalysisRuleFactory {
if (connectContext.getSessionVariable().isPlayNereidsDump()) {
return null;
}
-
- String dbName = relation.getDatabase().getFullName();
- String tableName = relation.getTable().getName();
- if
(!connectContext.getEnv().getAccessManager().checkTblPriv(connectContext,
dbName,
+ TableIf table = relation.getTable();
+ if (table == null) {
+ return null;
+ }
+ String tableName = table.getName();
+ DatabaseIf db = table.getDatabase();
+ // when table inatanceof FunctionGenTable,db will be null
+ if (db == null) {
+ return null;
+ }
+ String dbName = db.getFullName();
+ CatalogIf catalog = db.getCatalog();
+ if (catalog == null) {
+ return null;
+ }
+ String ctlName = catalog.getName();
+ // TODO: 2023/7/19 checkColumnsPriv
+ if
(!connectContext.getEnv().getAccessManager().checkTblPriv(connectContext,
ctlName, dbName,
tableName, PrivPredicate.SELECT)) {
String message =
ErrorCode.ERR_TABLEACCESS_DENIED_ERROR.formatErrorMsg("SELECT",
ConnectContext.get().getQualifiedUser(),
ConnectContext.get().getRemoteIP(),
- dbName + ": " + tableName);
+ ctlName + ": " + dbName + ": " + tableName);
throw new AnalysisException(message);
}
-
return null;
}
+
}
diff --git
a/fe/fe-core/src/main/java/org/apache/doris/planner/OriginalPlanner.java
b/fe/fe-core/src/main/java/org/apache/doris/planner/OriginalPlanner.java
index 0056619376..f77aba25da 100644
--- a/fe/fe-core/src/main/java/org/apache/doris/planner/OriginalPlanner.java
+++ b/fe/fe-core/src/main/java/org/apache/doris/planner/OriginalPlanner.java
@@ -33,23 +33,15 @@ import org.apache.doris.analysis.SlotId;
import org.apache.doris.analysis.SlotRef;
import org.apache.doris.analysis.StatementBase;
import org.apache.doris.analysis.StorageBackend;
-import org.apache.doris.analysis.TableName;
import org.apache.doris.analysis.TupleDescriptor;
import org.apache.doris.catalog.Column;
import org.apache.doris.catalog.Env;
import org.apache.doris.catalog.OlapTable;
import org.apache.doris.catalog.PrimitiveType;
import org.apache.doris.catalog.ScalarType;
-import org.apache.doris.catalog.Table;
-import org.apache.doris.catalog.TableIf;
import org.apache.doris.catalog.Type;
-import org.apache.doris.catalog.external.ExternalTable;
-import org.apache.doris.cluster.ClusterNamespace;
-import org.apache.doris.common.AnalysisException;
import org.apache.doris.common.Config;
import org.apache.doris.common.UserException;
-import org.apache.doris.datasource.InternalCatalog;
-import org.apache.doris.mysql.privilege.PrivPredicate;
import org.apache.doris.qe.CommonResultSet;
import org.apache.doris.qe.ConnectContext;
import org.apache.doris.qe.ResultSet;
@@ -61,10 +53,7 @@ import org.apache.doris.thrift.TQueryOptions;
import org.apache.doris.thrift.TRuntimeFilterMode;
import com.google.common.base.Preconditions;
-import com.google.common.base.Strings;
-import com.google.common.collect.HashMultimap;
import com.google.common.collect.Lists;
-import com.google.common.collect.Maps;
import org.apache.logging.log4j.LogManager;
import org.apache.logging.log4j.Logger;
@@ -179,8 +168,6 @@ public class OriginalPlanner extends Planner {
insertStmt.prepareExpressions();
}
- checkColumnPrivileges(singleNodePlan);
-
// TODO chenhao16 , no used materialization work
// compute referenced slots before calling computeMemLayout()
//analyzer.markRefdSlots(analyzer, singleNodePlan, resultExprs, null);
@@ -319,66 +306,6 @@ public class OriginalPlanner extends Planner {
}
}
- private void checkColumnPrivileges(PlanNode singleNodePlan) throws
UserException {
- if (ConnectContext.get() == null) {
- return;
- }
- // 1. collect all columns from all scan nodes
- List<ScanNode> scanNodes = Lists.newArrayList();
- singleNodePlan.collect((PlanNode planNode) -> planNode instanceof
ScanNode, scanNodes);
- // catalog : <db.table : column>
- Map<String, HashMultimap<TableName, String>> ctlToTableColumnMap =
Maps.newHashMap();
- for (ScanNode scanNode : scanNodes) {
- if (!scanNode.needToCheckColumnPriv()) {
- continue;
- }
- TupleDescriptor tupleDesc = scanNode.getTupleDesc();
- TableIf table = tupleDesc.getTable();
- if (table == null) {
- continue;
- }
- TableName tableName = getFullQualifiedTableNameFromTable(table);
- for (SlotDescriptor slotDesc : tupleDesc.getSlots()) {
- if (!slotDesc.isMaterialized()) {
- continue;
- }
- Column column = slotDesc.getColumn();
- if (column == null) {
- continue;
- }
- HashMultimap<TableName, String> tableColumnMap =
ctlToTableColumnMap.get(tableName.getCtl());
- if (tableColumnMap == null) {
- tableColumnMap = HashMultimap.create();
- ctlToTableColumnMap.put(tableName.getCtl(),
tableColumnMap);
- }
- tableColumnMap.put(tableName, column.getName());
- LOG.debug("collect column {} in {}", column.getName(),
tableName);
- }
- }
- // 2. check privs
- // TODO: only support SELECT_PRIV now
- PrivPredicate wanted = PrivPredicate.SELECT;
- for (Map.Entry<String, HashMultimap<TableName, String>> entry :
ctlToTableColumnMap.entrySet()) {
-
Env.getCurrentEnv().getAccessManager().checkColumnsPriv(ConnectContext.get().getCurrentUserIdentity(),
- entry.getKey(), entry.getValue(), wanted);
- }
- }
-
- private TableName getFullQualifiedTableNameFromTable(TableIf table) throws
AnalysisException {
- if (table instanceof Table) {
- String dbName = ClusterNamespace.getNameFromFullName(((Table)
table).getQualifiedDbName());
- if (Strings.isNullOrEmpty(dbName)) {
- throw new AnalysisException("failed to get db name from table
" + table.getName());
- }
- return new TableName(InternalCatalog.INTERNAL_CATALOG_NAME,
dbName, table.getName());
- } else if (table instanceof ExternalTable) {
- ExternalTable extTable = (ExternalTable) table;
- return new TableName(extTable.getCatalog().getName(),
extTable.getDbName(), extTable.getName());
- } else {
- throw new AnalysisException("table " + table.getName() + " is not
internal or external table instance");
- }
- }
-
/**
* If there are unassigned conjuncts, returns a SelectNode on top of root
that evaluate those conjuncts; otherwise
* returns root unchanged.
diff --git
a/fe/fe-core/src/test/java/org/apache/doris/datasource/ColumnPrivTest.java
b/fe/fe-core/src/test/java/org/apache/doris/datasource/ColumnPrivTest.java
index 151532aee7..b9726cf598 100644
--- a/fe/fe-core/src/test/java/org/apache/doris/datasource/ColumnPrivTest.java
+++ b/fe/fe-core/src/test/java/org/apache/doris/datasource/ColumnPrivTest.java
@@ -48,12 +48,15 @@ import com.google.common.collect.Lists;
import com.google.common.collect.Maps;
import org.junit.Assert;
import org.junit.jupiter.api.Assertions;
+import org.junit.jupiter.api.Disabled;
import org.junit.jupiter.api.Test;
import java.util.List;
import java.util.Map;
import java.util.Set;
+// when `select` suppport `col auth`,will open ColumnPrivTest
+@Disabled
public class ColumnPrivTest extends TestWithFeService {
private static Auth auth;
private static Env env;
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
index 4978304177..b0afc8055d 100644
--- 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
@@ -34,6 +34,7 @@ import org.apache.doris.catalog.AccessPrivilegeWithCols;
import org.apache.doris.catalog.DomainResolver;
import org.apache.doris.catalog.Env;
import org.apache.doris.common.AnalysisException;
+import org.apache.doris.common.Config;
import org.apache.doris.common.DdlException;
import org.apache.doris.common.ExceptionChecker;
import org.apache.doris.common.UserException;
@@ -153,6 +154,7 @@ public class AuthTest {
};
resolver = new MockDomainResolver(auth);
+ Config.enable_col_auth = true;
}
@Test
diff --git a/regression-test/data/account_p0/test_account.out
b/regression-test/data/account_p0/test_account.out
deleted file mode 100644
index ad97022a09..0000000000
--- a/regression-test/data/account_p0/test_account.out
+++ /dev/null
@@ -1,4 +0,0 @@
--- This file is automatically generated. You should know what you did if you
want to edit this
--- !show_grants1 --
-'default_cluster:non_existent_user_1'@'%' \N \N \N \N
\N \N \N \N \N
-
diff --git a/regression-test/suites/account_p0/test_account.groovy
b/regression-test/suites/account_p0/test_account.groovy
index b38d72a80d..1eb07e259c 100644
--- a/regression-test/suites/account_p0/test_account.groovy
+++ b/regression-test/suites/account_p0/test_account.groovy
@@ -18,5 +18,11 @@ suite("test_account") {
// todo: test account management, such as role, user, grant, revoke ...
sql "show roles"
- qt_show_grants1 "show grants for 'non_existent_user_1'"
+ try {
+ sql "show grants for 'non_existent_user_1'"
+ fail()
+ } catch (Exception e) {
+ log.info(e.getMessage())
+ assertTrue(e.getMessage().contains('not exist'))
+ }
}
diff --git
a/regression-test/suites/account_p0/test_nereids_authentication.groovy
b/regression-test/suites/account_p0/test_nereids_authentication.groovy
index 2d5d2cbb13..46d60732d6 100644
--- a/regression-test/suites/account_p0/test_nereids_authentication.groovy
+++ b/regression-test/suites/account_p0/test_nereids_authentication.groovy
@@ -28,6 +28,7 @@ suite("test_nereids_authentication", "query") {
}
sql "set enable_nereids_planner = true"
+ sql "set enable_fallback_to_original_planner = false"
def dbName = "nereids_authentication"
sql "DROP DATABASE IF EXISTS ${dbName}"
@@ -57,7 +58,7 @@ suite("test_nereids_authentication", "query") {
fail()
} catch (Exception e) {
log.info(e.getMessage())
- assertTrue(e.getMessage().contains('Permission denied'))
+ assertTrue(e.getMessage().contains('denied to user'))
}
}
@@ -67,7 +68,7 @@ suite("test_nereids_authentication", "query") {
fail()
} catch (Exception e) {
log.info(e.getMessage())
- assertTrue(e.getMessage().contains('Permission denied'))
+ assertTrue(e.getMessage().contains('denied to user'))
}
}
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
