You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@doris.apache.org by yi...@apache.org on 2024/02/23 11:05:49 UTC
(doris) 09/13: [fix](Nereids) support check authorization for view but skip check in the view (#31289)
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
commit b5ec1e7b7da968c3896d2a15aa384aaf2fe5b36f
Author: 924060929 <92...@qq.com>
AuthorDate: Fri Feb 23 13:05:39 2024 +0800
[fix](Nereids) support check authorization for view but skip check in the view (#31289)
move UserAuthentication in BindRelation, support check authorization view but skip check in the view
relate pr: #23295
---
.../org/apache/doris/nereids/CascadesContext.java | 30 +++++---
.../doris/nereids/jobs/executor/Analyzer.java | 7 +-
.../doris/nereids/rules/analysis/BindRelation.java | 18 +++--
.../nereids/rules/analysis/UserAuthentication.java | 33 +++------
.../apache/doris/external/hms/HmsCatalogTest.java | 20 ++++++
.../org/apache/doris/qe/HmsQueryCacheTest.java | 16 +++++
.../account_p0/test_nereids_row_policy.groovy | 9 ++-
.../authorization/view_authorization.groovy | 82 ++++++++++++++++++++++
8 files changed, 171 insertions(+), 44 deletions(-)
diff --git a/fe/fe-core/src/main/java/org/apache/doris/nereids/CascadesContext.java b/fe/fe-core/src/main/java/org/apache/doris/nereids/CascadesContext.java
index fecdbf650c3..7cd486b47f7 100644
--- a/fe/fe-core/src/main/java/org/apache/doris/nereids/CascadesContext.java
+++ b/fe/fe-core/src/main/java/org/apache/doris/nereids/CascadesContext.java
@@ -119,6 +119,7 @@ public class CascadesContext implements ScheduleContext {
private boolean isLeadingJoin = false;
private final Map<String, Hint> hintMap = Maps.newLinkedHashMap();
+ private final boolean shouldCheckRelationAuthentication;
/**
* Constructor of OptimizerContext.
@@ -128,7 +129,7 @@ public class CascadesContext implements ScheduleContext {
*/
private CascadesContext(Optional<CascadesContext> parent, Optional<CTEId> currentTree,
StatementContext statementContext, Plan plan, Memo memo,
- CTEContext cteContext, PhysicalProperties requireProperties) {
+ CTEContext cteContext, PhysicalProperties requireProperties, boolean shouldCheckRelationAuthentication) {
this.parent = Objects.requireNonNull(parent, "parent should not null");
this.currentTree = Objects.requireNonNull(currentTree, "currentTree should not null");
this.statementContext = Objects.requireNonNull(statementContext, "statementContext should not null");
@@ -142,6 +143,7 @@ public class CascadesContext implements ScheduleContext {
this.subqueryExprIsAnalyzed = new HashMap<>();
this.runtimeFilterContext = new RuntimeFilterContext(getConnectContext().getSessionVariable());
this.materializationContexts = new ArrayList<>();
+ this.shouldCheckRelationAuthentication = shouldCheckRelationAuthentication;
}
/**
@@ -150,7 +152,13 @@ public class CascadesContext implements ScheduleContext {
public static CascadesContext initContext(StatementContext statementContext,
Plan initPlan, PhysicalProperties requireProperties) {
return newContext(Optional.empty(), Optional.empty(), statementContext,
- initPlan, new CTEContext(), requireProperties);
+ initPlan, new CTEContext(), requireProperties, true);
+ }
+
+ public static CascadesContext initViewContext(StatementContext statementContext,
+ Plan initPlan, PhysicalProperties requireProperties) {
+ return newContext(Optional.empty(), Optional.empty(), statementContext,
+ initPlan, new CTEContext(), requireProperties, false);
}
/**
@@ -159,13 +167,14 @@ public class CascadesContext implements ScheduleContext {
public static CascadesContext newContextWithCteContext(CascadesContext cascadesContext,
Plan initPlan, CTEContext cteContext) {
return newContext(Optional.of(cascadesContext), Optional.empty(),
- cascadesContext.getStatementContext(), initPlan, cteContext, PhysicalProperties.ANY);
+ cascadesContext.getStatementContext(), initPlan, cteContext, PhysicalProperties.ANY,
+ cascadesContext.shouldCheckRelationAuthentication);
}
public static CascadesContext newCurrentTreeContext(CascadesContext context) {
return CascadesContext.newContext(context.getParent(), context.getCurrentTree(), context.getStatementContext(),
context.getRewritePlan(), context.getCteContext(),
- context.getCurrentJobContext().getRequiredProperties());
+ context.getCurrentJobContext().getRequiredProperties(), context.shouldCheckRelationAuthentication);
}
/**
@@ -174,13 +183,14 @@ public class CascadesContext implements ScheduleContext {
public static CascadesContext newSubtreeContext(Optional<CTEId> subtree, CascadesContext context,
Plan plan, PhysicalProperties requireProperties) {
return CascadesContext.newContext(Optional.of(context), subtree, context.getStatementContext(),
- plan, context.getCteContext(), requireProperties);
+ plan, context.getCteContext(), requireProperties, context.shouldCheckRelationAuthentication);
}
private static CascadesContext newContext(Optional<CascadesContext> parent, Optional<CTEId> subtree,
- StatementContext statementContext, Plan initPlan,
- CTEContext cteContext, PhysicalProperties requireProperties) {
- return new CascadesContext(parent, subtree, statementContext, initPlan, null, cteContext, requireProperties);
+ StatementContext statementContext, Plan initPlan, CTEContext cteContext,
+ PhysicalProperties requireProperties, boolean shouldCheckRelationAuthentication) {
+ return new CascadesContext(parent, subtree, statementContext, initPlan, null,
+ cteContext, requireProperties, shouldCheckRelationAuthentication);
}
public CascadesContext getRoot() {
@@ -636,6 +646,10 @@ public class CascadesContext implements ScheduleContext {
isLeadingJoin = leadingJoin;
}
+ public boolean shouldCheckRelationAuthentication() {
+ return shouldCheckRelationAuthentication;
+ }
+
public Map<String, Hint> getHintMap() {
return hintMap;
}
diff --git a/fe/fe-core/src/main/java/org/apache/doris/nereids/jobs/executor/Analyzer.java b/fe/fe-core/src/main/java/org/apache/doris/nereids/jobs/executor/Analyzer.java
index 2f3e0ccfe2f..d1e38d014e6 100644
--- a/fe/fe-core/src/main/java/org/apache/doris/nereids/jobs/executor/Analyzer.java
+++ b/fe/fe-core/src/main/java/org/apache/doris/nereids/jobs/executor/Analyzer.java
@@ -44,7 +44,6 @@ import org.apache.doris.nereids.rules.analysis.ProjectWithDistinctToAggregate;
import org.apache.doris.nereids.rules.analysis.ReplaceExpressionByChildOutput;
import org.apache.doris.nereids.rules.analysis.ResolveOrdinalInOrderByAndGroupBy;
import org.apache.doris.nereids.rules.analysis.SubqueryToApply;
-import org.apache.doris.nereids.rules.analysis.UserAuthentication;
import org.apache.doris.nereids.rules.rewrite.MergeProjects;
import org.apache.doris.nereids.rules.rewrite.SemiJoinCommute;
@@ -119,8 +118,7 @@ public class Analyzer extends AbstractBatchJobExecutor {
topDown(new EliminateLogicalSelectHint()),
bottomUp(
new BindRelation(customTableResolver),
- new CheckPolicy(),
- new UserAuthentication()
+ new CheckPolicy()
)
);
}
@@ -132,8 +130,7 @@ public class Analyzer extends AbstractBatchJobExecutor {
topDown(new EliminateLogicalSelectHint()),
bottomUp(
new BindRelation(customTableResolver),
- new CheckPolicy(),
- new UserAuthentication()
+ new CheckPolicy()
),
bottomUp(new BindExpression()),
bottomUp(new BindSlotWithPaths()),
diff --git a/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/analysis/BindRelation.java b/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/analysis/BindRelation.java
index 9d9591e34b1..54fa965e0eb 100644
--- a/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/analysis/BindRelation.java
+++ b/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/analysis/BindRelation.java
@@ -157,7 +157,7 @@ public class BindRelation extends OneAnalysisRuleFactory {
}
// TODO: should generate different Scan sub class according to table's type
- LogicalPlan scan = getLogicalPlan(table, unboundRelation, tableQualifier, cascadesContext);
+ LogicalPlan scan = getAndCheckLogicalPlan(table, unboundRelation, tableQualifier, cascadesContext);
if (cascadesContext.isLeadingJoin()) {
LeadingHint leading = (LeadingHint) cascadesContext.getHintMap().get("Leading");
leading.putRelationIdAndTableName(Pair.of(unboundRelation.getRelationId(), tableName));
@@ -178,7 +178,7 @@ public class BindRelation extends OneAnalysisRuleFactory {
if (table == null) {
table = RelationUtil.getTable(tableQualifier, cascadesContext.getConnectContext().getEnv());
}
- return getLogicalPlan(table, unboundRelation, tableQualifier, cascadesContext);
+ return getAndCheckLogicalPlan(table, unboundRelation, tableQualifier, cascadesContext);
}
private LogicalPlan makeOlapScan(TableIf table, UnboundRelation unboundRelation, List<String> tableQualifier) {
@@ -234,7 +234,17 @@ public class BindRelation extends OneAnalysisRuleFactory {
return scan;
}
- private LogicalPlan getLogicalPlan(TableIf table, UnboundRelation unboundRelation, List<String> tableQualifier,
+ private LogicalPlan getAndCheckLogicalPlan(TableIf table, UnboundRelation unboundRelation,
+ List<String> tableQualifier, CascadesContext cascadesContext) {
+ // if current context is in the view, we can skip check authentication because
+ // the view already checked authentication
+ if (cascadesContext.shouldCheckRelationAuthentication()) {
+ UserAuthentication.checkPermission(table, cascadesContext.getConnectContext());
+ }
+ return doGetLogicalPlan(table, unboundRelation, tableQualifier, cascadesContext);
+ }
+
+ private LogicalPlan doGetLogicalPlan(TableIf table, UnboundRelation unboundRelation, List<String> tableQualifier,
CascadesContext cascadesContext) {
switch (table.getType()) {
case OLAP:
@@ -289,7 +299,7 @@ public class BindRelation extends OneAnalysisRuleFactory {
if (parsedViewPlan instanceof UnboundResultSink) {
parsedViewPlan = (LogicalPlan) ((UnboundResultSink<?>) parsedViewPlan).child();
}
- CascadesContext viewContext = CascadesContext.initContext(
+ CascadesContext viewContext = CascadesContext.initViewContext(
parentContext.getStatementContext(), parsedViewPlan, PhysicalProperties.ANY);
viewContext.newAnalyzer(true, customTableResolver).analyze();
// we should remove all group expression of the plan which in other memo, so the groupId would not conflict
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 86fb64d6e56..ffed8d4ea93 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
@@ -23,44 +23,31 @@ 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;
-import org.apache.doris.nereids.rules.RuleType;
-import org.apache.doris.nereids.trees.plans.Plan;
-import org.apache.doris.nereids.trees.plans.algebra.CatalogRelation;
import org.apache.doris.qe.ConnectContext;
/**
* Check whether a user is permitted to scan specific tables.
*/
-public class UserAuthentication extends OneAnalysisRuleFactory {
-
- @Override
- public Rule build() {
- return logicalRelation()
- .when(CatalogRelation.class::isInstance)
- .thenApply(ctx -> checkPermission((CatalogRelation) ctx.root, ctx.connectContext))
- .toRule(RuleType.RELATION_AUTHENTICATION);
- }
-
- private Plan checkPermission(CatalogRelation relation, ConnectContext connectContext) {
+public class UserAuthentication {
+ /** checkPermission. */
+ public static void checkPermission(TableIf table, ConnectContext connectContext) {
+ if (table == null) {
+ return;
+ }
// do not check priv when replaying dump file
if (connectContext.getSessionVariable().isPlayNereidsDump()) {
- return null;
- }
- TableIf table = relation.getTable();
- if (table == null) {
- return null;
+ return;
}
String tableName = table.getName();
DatabaseIf db = table.getDatabase();
// when table inatanceof FunctionGenTable,db will be null
if (db == null) {
- return null;
+ return;
}
String dbName = db.getFullName();
CatalogIf catalog = db.getCatalog();
if (catalog == null) {
- return null;
+ return;
}
String ctlName = catalog.getName();
// TODO: 2023/7/19 checkColumnsPriv
@@ -71,7 +58,5 @@ public class UserAuthentication extends OneAnalysisRuleFactory {
ctlName + ": " + dbName + ": " + tableName);
throw new AnalysisException(message);
}
- return null;
}
-
}
diff --git a/fe/fe-core/src/test/java/org/apache/doris/external/hms/HmsCatalogTest.java b/fe/fe-core/src/test/java/org/apache/doris/external/hms/HmsCatalogTest.java
index 220813a305c..e90e1f8889f 100644
--- a/fe/fe-core/src/test/java/org/apache/doris/external/hms/HmsCatalogTest.java
+++ b/fe/fe-core/src/test/java/org/apache/doris/external/hms/HmsCatalogTest.java
@@ -127,6 +127,10 @@ public class HmsCatalogTest extends AnalyzeCheckTestBase {
tbl.getType();
minTimes = 0;
result = TableIf.TableType.HMS_EXTERNAL_TABLE;
+
+ tbl.getDatabase();
+ minTimes = 0;
+ result = db;
}
};
@@ -169,6 +173,10 @@ public class HmsCatalogTest extends AnalyzeCheckTestBase {
view1.isSupportedHmsTable();
minTimes = 0;
result = true;
+
+ view1.getDatabase();
+ minTimes = 0;
+ result = db;
}
};
@@ -211,6 +219,10 @@ public class HmsCatalogTest extends AnalyzeCheckTestBase {
view2.isSupportedHmsTable();
minTimes = 0;
result = true;
+
+ view2.getDatabase();
+ minTimes = 0;
+ result = db;
}
};
@@ -253,6 +265,10 @@ public class HmsCatalogTest extends AnalyzeCheckTestBase {
view3.isSupportedHmsTable();
minTimes = 0;
result = true;
+
+ view3.getDatabase();
+ minTimes = 0;
+ result = db;
}
};
@@ -295,6 +311,10 @@ public class HmsCatalogTest extends AnalyzeCheckTestBase {
view4.isSupportedHmsTable();
minTimes = 0;
result = true;
+
+ view4.getDatabase();
+ minTimes = 0;
+ result = db;
}
};
diff --git a/fe/fe-core/src/test/java/org/apache/doris/qe/HmsQueryCacheTest.java b/fe/fe-core/src/test/java/org/apache/doris/qe/HmsQueryCacheTest.java
index 1c066706947..14ba32d61be 100644
--- a/fe/fe-core/src/test/java/org/apache/doris/qe/HmsQueryCacheTest.java
+++ b/fe/fe-core/src/test/java/org/apache/doris/qe/HmsQueryCacheTest.java
@@ -159,6 +159,10 @@ public class HmsQueryCacheTest extends AnalyzeCheckTestBase {
// mock initSchemaAndUpdateTime and do nothing
tbl.initSchemaAndUpdateTime();
minTimes = 0;
+
+ tbl.getDatabase();
+ minTimes = 0;
+ result = db;
}
};
@@ -203,6 +207,10 @@ public class HmsQueryCacheTest extends AnalyzeCheckTestBase {
// mock initSchemaAndUpdateTime and do nothing
tbl2.initSchemaAndUpdateTime();
minTimes = 0;
+
+ tbl2.getDatabase();
+ minTimes = 0;
+ result = db;
}
};
@@ -254,6 +262,10 @@ public class HmsQueryCacheTest extends AnalyzeCheckTestBase {
view1.getUpdateTime();
minTimes = 0;
result = NOW;
+
+ view1.getDatabase();
+ minTimes = 0;
+ result = db;
}
};
@@ -304,6 +316,10 @@ public class HmsQueryCacheTest extends AnalyzeCheckTestBase {
view2.getUpdateTime();
minTimes = 0;
result = NOW;
+
+ view2.getDatabase();
+ minTimes = 0;
+ result = db;
}
};
diff --git a/regression-test/suites/account_p0/test_nereids_row_policy.groovy b/regression-test/suites/account_p0/test_nereids_row_policy.groovy
index d12b11261d8..a803e4bcda4 100644
--- a/regression-test/suites/account_p0/test_nereids_row_policy.groovy
+++ b/regression-test/suites/account_p0/test_nereids_row_policy.groovy
@@ -32,14 +32,16 @@ suite("test_nereids_row_policy") {
sql "set enable_fallback_to_original_planner = false"
sql "SELECT * FROM ${tableName}"
}
- def result3 = connect(user=user, password='123abc!@#', url=url) {
+ connect(user=user, password='123abc!@#', url=url) {
sql "set enable_nereids_planner = true"
sql "set enable_fallback_to_original_planner = false"
- sql "SELECT * FROM ${viewName}"
+ test {
+ sql "SELECT * FROM ${viewName}"
+ exception "SELECT command denied to user"
+ }
}
assertEquals(size, result1.size())
assertEquals(size, result2.size())
- assertEquals(size, result3.size())
}
def createPolicy = { name, predicate, type ->
@@ -79,6 +81,7 @@ suite("test_nereids_row_policy") {
sql "CREATE USER ${user} IDENTIFIED BY '123abc!@#'"
sql "GRANT SELECT_PRIV ON internal.${dbName}.${tableName} TO ${user}"
+ sql 'sync'
// no policy
assertQueryResult 3
diff --git a/regression-test/suites/nereids_p0/authorization/view_authorization.groovy b/regression-test/suites/nereids_p0/authorization/view_authorization.groovy
new file mode 100644
index 00000000000..672cd680ec8
--- /dev/null
+++ b/regression-test/suites/nereids_p0/authorization/view_authorization.groovy
@@ -0,0 +1,82 @@
+// 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.
+
+suite("view_authorization") {
+ def db = context.config.getDbNameByFile(context.file)
+ def user1 = "test_view_auth_user1"
+ def baseTable = "test_view_auth_base_table"
+ def view1 = "test_view_auth_view1"
+ def view2 = "test_view_auth_view2"
+ def view3 = "test_view_auth_view3"
+
+
+ sql "drop table if exists ${baseTable}"
+ sql "drop view if exists ${view1}"
+ sql "drop view if exists ${view2}"
+ sql "drop view if exists ${view3}"
+ sql "drop user if exists ${user1}"
+
+ sql """
+ CREATE TABLE ${baseTable} (id INT, name TEXT)
+ DISTRIBUTED BY HASH(`id`)
+ PROPERTIES (
+ "replication_allocation" = "tag.location.default: 1"
+ );
+ """
+
+ sql "insert into ${baseTable} values(1, 'hello'), (2, 'world'), (3, 'doris');"
+ sql "create view ${view1} as select *, concat(name, '_', id) from ${db}.${baseTable} where id=1;"
+ sql "create view ${view2} as select *, concat(name, '_', id) as xxx from ${db}.${baseTable} where id != 1;"
+ sql "create view ${view3} as select xxx, 100 from ${db}.${view2} where id=3"
+
+ sql "create user ${user1}"
+ sql "grant SELECT_PRIV on ${db}.${view1} to '${user1}'@'%';"
+ sql "grant SELECT_PRIV on ${db}.${view3} to '${user1}'@'%';"
+
+ sql 'sync'
+
+ def defaultDbUrl = context.config.jdbcUrl.substring(0, context.config.jdbcUrl.lastIndexOf("/"))
+ logger.info("connect to ${defaultDbUrl}".toString())
+ connect(user = user1, password = null, url = defaultDbUrl) {
+ sql "set enable_fallback_to_original_planner=false"
+
+ // no privilege to base table
+ test {
+ sql "select * from ${db}.${baseTable}"
+ exception "SELECT command denied to user"
+ }
+
+ // has privilege to view1
+ test {
+ sql "select * from ${db}.${view1}"
+ result([[1, 'hello', 'hello_1']])
+ }
+
+ // no privilege to view2
+ test {
+ sql "select * from ${db}.${view2}"
+ exception "SELECT command denied to user"
+ }
+
+ // nested view
+ // has privilege to view3
+ test {
+ sql "select * from ${db}.${view3}"
+ result([['doris_3', 100]])
+ }
+ }
+}
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org