This is an automated email from the ASF dual-hosted git repository.
yao pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/kyuubi.git
The following commit(s) were added to refs/heads/master by this push:
new f75e4acf6 [KYUUBI #5417] Authz should not check dependent subquery
plan privilege
f75e4acf6 is described below
commit f75e4acf6ceee31a74e57b41cd431ee65f184376
Author: Angerszhuuuu <[email protected]>
AuthorDate: Tue Oct 17 11:16:02 2023 +0800
[KYUUBI #5417] Authz should not check dependent subquery plan privilege
### _Why are the changes needed?_
Fix #5417
If there is is a view with subquery, authz will still request this
subquery's interval privilege, it's not we want.
For view
```
CREATE VIEW db.view1
AS
WITH temp AS (
SELECT max(scope) max_scope
FROM db1.table1)
SELECT id as new_id FROM db1.table2
WHERE scope = (SELECT max_scope FROM temp)
```
When we query the view
```
SEELCT * FROM db.view1
```
Before this pr, since spark will first execute subquery, it will first
request `[default/table1/scope]` then request `[default/view1/new_id]`
after this pr, it only request `[default/view1/new_id]`
### _How was this patch tested?_
- [x] Add some test cases that check the changes thoroughly including
negative and positive cases if possible
- [ ] Add screenshots for manual tests if appropriate
- [ ] [Run
test](https://kyuubi.readthedocs.io/en/master/contributing/code/testing.html#running-tests)
locally before make a pull request
### _Was this patch authored or co-authored using generative AI tooling?_
No
Closes #5418 from AngersZhuuuu/KYUUBI-5417.
Closes #5417
e2669faea [Angerszhuuuu] Update tableExtractors.scala
bc72cfc57 [Angerszhuuuu] Update RuleApplyPermanentViewMarker.scala
1731b9317 [Angerszhuuuu] Update RuleEliminateViewMarker.scala
282999ee2 [Angerszhuuuu] follow comment
6b37aaa7f [Angerszhuuuu] Update RuleApplyPermanentViewMarker.scala
d03354d58 [Angerszhuuuu] Revert "update"
7a96627e4 [Angerszhuuuu] update
78a32b3a5 [Angerszhuuuu] follow comment
79e07ab24 [Angerszhuuuu] Update PrivilegesBuilder.scala
518c2b394 [Angerszhuuuu] update
d033624ea [Angerszhuuuu] update
54ff954f0 [Angerszhuuuu] update.
1119f78f6 [Angerszhuuuu] Update RangerSparkExtensionSuite.scala
1918381ac [Angerszhuuuu] Add UT
7723f9002 [Angerszhuuuu] [KYUUBI #5417]Authz will still check source table
when persist view contains a subquery
Authored-by: Angerszhuuuu <[email protected]>
Signed-off-by: Kent Yao <[email protected]>
---
.../ranger/RuleApplyPermanentViewMarker.scala | 10 ++++-
.../plugin/spark/authz/serde/tableExtractors.scala | 12 ++++--
.../spark/authz/util/RuleEliminateViewMarker.scala | 7 +++-
.../authz/ranger/RangerSparkExtensionSuite.scala | 44 ++++++++++++++++++++++
4 files changed, 67 insertions(+), 6 deletions(-)
diff --git
a/extensions/spark/kyuubi-spark-authz/src/main/scala/org/apache/kyuubi/plugin/spark/authz/ranger/RuleApplyPermanentViewMarker.scala
b/extensions/spark/kyuubi-spark-authz/src/main/scala/org/apache/kyuubi/plugin/spark/authz/ranger/RuleApplyPermanentViewMarker.scala
index 424df7e0b..917410807 100644
---
a/extensions/spark/kyuubi-spark-authz/src/main/scala/org/apache/kyuubi/plugin/spark/authz/ranger/RuleApplyPermanentViewMarker.scala
+++
b/extensions/spark/kyuubi-spark-authz/src/main/scala/org/apache/kyuubi/plugin/spark/authz/ranger/RuleApplyPermanentViewMarker.scala
@@ -17,6 +17,7 @@
package org.apache.kyuubi.plugin.spark.authz.ranger
+import org.apache.spark.sql.catalyst.expressions.ScalarSubquery
import org.apache.spark.sql.catalyst.plans.logical.{LogicalPlan, View}
import org.apache.spark.sql.catalyst.rules.Rule
@@ -36,7 +37,14 @@ class RuleApplyPermanentViewMarker extends Rule[LogicalPlan]
{
plan mapChildren {
case p: PermanentViewMarker => p
case permanentView: View if hasResolvedPermanentView(permanentView) =>
- PermanentViewMarker(permanentView, permanentView.desc)
+ val resolvedSubquery = permanentView.transformAllExpressions {
+ case scalarSubquery: ScalarSubquery =>
+ // TODO: Currently, we do not do an auth check in the subquery
+ // as the main query part also secures it. But for performance
consideration,
+ // we also pre-check it in subqueries and fail fast with negative
privileges.
+ scalarSubquery.copy(plan =
PermanentViewMarker(scalarSubquery.plan, null))
+ }
+ PermanentViewMarker(resolvedSubquery, resolvedSubquery.desc)
case other => apply(other)
}
}
diff --git
a/extensions/spark/kyuubi-spark-authz/src/main/scala/org/apache/kyuubi/plugin/spark/authz/serde/tableExtractors.scala
b/extensions/spark/kyuubi-spark-authz/src/main/scala/org/apache/kyuubi/plugin/spark/authz/serde/tableExtractors.scala
index 94641d6d0..57eab9634 100644
---
a/extensions/spark/kyuubi-spark-authz/src/main/scala/org/apache/kyuubi/plugin/spark/authz/serde/tableExtractors.scala
+++
b/extensions/spark/kyuubi-spark-authz/src/main/scala/org/apache/kyuubi/plugin/spark/authz/serde/tableExtractors.scala
@@ -91,10 +91,14 @@ class TableIdentifierTableExtractor extends TableExtractor {
*/
class CatalogTableTableExtractor extends TableExtractor {
override def apply(spark: SparkSession, v1: AnyRef): Option[Table] = {
- val catalogTable = v1.asInstanceOf[CatalogTable]
- val identifier = catalogTable.identifier
- val owner = Option(catalogTable.owner).filter(_.nonEmpty)
- Some(Table(None, identifier.database, identifier.table, owner))
+ if (null == v1) {
+ None
+ } else {
+ val catalogTable = v1.asInstanceOf[CatalogTable]
+ val identifier = catalogTable.identifier
+ val owner = Option(catalogTable.owner).filter(_.nonEmpty)
+ Some(Table(None, identifier.database, identifier.table, owner))
+ }
}
}
diff --git
a/extensions/spark/kyuubi-spark-authz/src/main/scala/org/apache/kyuubi/plugin/spark/authz/util/RuleEliminateViewMarker.scala
b/extensions/spark/kyuubi-spark-authz/src/main/scala/org/apache/kyuubi/plugin/spark/authz/util/RuleEliminateViewMarker.scala
index 9bda84a03..8044f1283 100644
---
a/extensions/spark/kyuubi-spark-authz/src/main/scala/org/apache/kyuubi/plugin/spark/authz/util/RuleEliminateViewMarker.scala
+++
b/extensions/spark/kyuubi-spark-authz/src/main/scala/org/apache/kyuubi/plugin/spark/authz/util/RuleEliminateViewMarker.scala
@@ -17,6 +17,7 @@
package org.apache.kyuubi.plugin.spark.authz.util
+import org.apache.spark.sql.catalyst.expressions.SubqueryExpression
import org.apache.spark.sql.catalyst.plans.logical.LogicalPlan
import org.apache.spark.sql.catalyst.rules.Rule
@@ -25,6 +26,10 @@ import org.apache.spark.sql.catalyst.rules.Rule
*/
class RuleEliminateViewMarker extends Rule[LogicalPlan] {
override def apply(plan: LogicalPlan): LogicalPlan = {
- plan.transformUp { case pvm: PermanentViewMarker => pvm.child }
+ plan.transformUp {
+ case pvm: PermanentViewMarker => pvm.child.transformAllExpressions {
+ case s: SubqueryExpression => s.withNewPlan(apply(s.plan))
+ }
+ }
}
}
diff --git
a/extensions/spark/kyuubi-spark-authz/src/test/scala/org/apache/kyuubi/plugin/spark/authz/ranger/RangerSparkExtensionSuite.scala
b/extensions/spark/kyuubi-spark-authz/src/test/scala/org/apache/kyuubi/plugin/spark/authz/ranger/RangerSparkExtensionSuite.scala
index a4148d9a5..d109a7f2b 100644
---
a/extensions/spark/kyuubi-spark-authz/src/test/scala/org/apache/kyuubi/plugin/spark/authz/ranger/RangerSparkExtensionSuite.scala
+++
b/extensions/spark/kyuubi-spark-authz/src/test/scala/org/apache/kyuubi/plugin/spark/authz/ranger/RangerSparkExtensionSuite.scala
@@ -747,4 +747,48 @@ class HiveCatalogRangerSparkExtensionSuite extends
RangerSparkExtensionSuite {
assert(e.getMessage.contains(s"does not have [select] privilege on
[$db1/$table/id]"))
}
}
+
+ test("[KYUUBI #5417] should not check dependent subquery plan privilege") {
+ val db1 = defaultDb
+ val table1 = "table1"
+ val table2 = "table2"
+ val view1 = "view1"
+ withCleanTmpResources(
+ Seq((s"$db1.$table1", "table"), (s"$db1.$table2", "table"),
(s"$db1.$view1", "view"))) {
+ doAs(admin, sql(s"CREATE TABLE IF NOT EXISTS $db1.$table1 (id int, scope
int)"))
+ doAs(admin, sql(s"CREATE TABLE IF NOT EXISTS $db1.$table2 (id int, scope
int)"))
+
+ val e1 = intercept[AccessControlException] {
+ doAs(
+ someone,
+ sql(
+ s"""
+ |WITH temp AS (
+ | SELECT max(scope) max_scope
+ | FROM $db1.$table1)
+ |SELECT id as new_id FROM $db1.$table2
+ |WHERE scope = (SELECT max_scope FROM temp)
+ |""".stripMargin).show())
+ }
+ // Will first check subquery privilege.
+ assert(e1.getMessage.contains(s"does not have [select] privilege on
[$db1/$table1/scope]"))
+
+ doAs(
+ admin,
+ sql(
+ s"""
+ |CREATE VIEW $db1.$view1
+ |AS
+ |WITH temp AS (
+ | SELECT max(scope) max_scope
+ | FROM $db1.$table1)
+ |SELECT id as new_id FROM $db1.$table2
+ |WHERE scope = (SELECT max_scope FROM temp)
+ |""".stripMargin))
+ // Will just check permanent view privilege.
+ val e2 = intercept[AccessControlException](
+ doAs(someone, sql(s"SELECT * FROM $db1.$view1".stripMargin).show()))
+ assert(e2.getMessage.contains(s"does not have [select] privilege on
[$db1/$view1/new_id]"))
+ }
+ }
}
