This is an automated email from the ASF dual-hosted git repository.
kxiao pushed a commit to branch branch-2.0
in repository https://gitbox.apache.org/repos/asf/doris.git
commit 945a8eced941f1557af939718798eb3cd6732783
Author: zhangdong <493738...@qq.com>
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 +
.../org/apache/doris/mysql/privilege/Auth.java | 2 +-
.../nereids/rules/analysis/UserAuthentication.java | 1 +
.../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 +-
13 files changed, 39 insertions(+), 84 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 0f6671b0a0..35cad6afe2 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
@@ -2093,6 +2093,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/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 f82619fb73..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
@@ -73,4 +73,5 @@ public class UserAuthentication extends
OneAnalysisRuleFactory {
}
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 08af81afbb..7b0fa4512c 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
@@ -31,23 +31,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.ConnectContext;
import org.apache.doris.rewrite.mvrewrite.MVSelectFailedException;
import org.apache.doris.statistics.query.StatsDelta;
@@ -56,10 +48,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;
@@ -173,8 +162,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);
@@ -313,66 +300,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: commits-unsubscr...@doris.apache.org
For additional commands, e-mail: commits-h...@doris.apache.org
