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 9e2fcc3f1 [KYUUBI #5475][FOLLOWUP] Authz check permanent view's
subquery should check view's correct privilege
9e2fcc3f1 is described below
commit 9e2fcc3f1fa1a0ff9060726d95c76c3ba93e7595
Author: Angerszhuuuu <[email protected]>
AuthorDate: Mon Oct 23 21:53:17 2023 +0800
[KYUUBI #5475][FOLLOWUP] Authz check permanent view's subquery should check
view's correct privilege
### _Why are the changes needed?_
To fix #5475
In issue #5417 we fixed the problem that AUTHZ will still check
scalar-subquery/in-subquery in permanent will.
But we just ignore the check, the subquery still will run, in this PR, we
record the permanent view's visited column to check the permanent view's
privilege to avoid extra execution effort.
For the test `[KYUUBI #5417] should not check scalar-subquery in permanent
view` I print all the plan that pass to privilege builder as below
<img width="1398" alt="截屏2023-10-19 下午4 05 46"
src="https://github.com/apache/kyuubi/assets/46485123/b136bb47-816c-4066-aba7-a74cbe323f7d";>
before this pr
<img width="1310" alt="截屏2023-10-19 下午4 15 29"
src="https://github.com/apache/kyuubi/assets/46485123/aa2e3cfe-bca7-493d-a364-b2c196c76c3a";>
This two graph shows this pr deny the execution of subquery when we don't
have the veiw's privilege
### _How was this patch tested?_
- [ ] 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?_
Closes #5476 from AngersZhuuuu/KYUUBI-5475.
Closes #5475
e1f7920f3 [Angerszhuuuu] Merge branch 'master' into KYUUBI-5475
3bfd9e677 [Angerszhuuuu] update
6b8c0e6e5 [Angerszhuuuu] Merge branch 'master' into KYUUBI-5475
f7585a451 [Angerszhuuuu] Update PrivilegesBuilder.scala
faea9c699 [Angerszhuuuu] [KYUUBI #5475] Authz check permanent view's
subquery should check view's correct privilege
Authored-by: Angerszhuuuu <[email protected]>
Signed-off-by: Kent Yao <[email protected]>
---
.../plugin/spark/authz/PrivilegesBuilder.scala | 6 +++
.../ranger/RuleApplyPermanentViewMarker.scala | 14 ++++---
.../spark/authz/util/PermanentViewMarker.scala | 5 ++-
.../authz/ranger/RangerSparkExtensionSuite.scala | 45 ++++++++++++++++++++++
4 files changed, 64 insertions(+), 6 deletions(-)
diff --git
a/extensions/spark/kyuubi-spark-authz/src/main/scala/org/apache/kyuubi/plugin/spark/authz/PrivilegesBuilder.scala
b/extensions/spark/kyuubi-spark-authz/src/main/scala/org/apache/kyuubi/plugin/spark/authz/PrivilegesBuilder.scala
index 5c496b874..a0ed5fb6a 100644
---
a/extensions/spark/kyuubi-spark-authz/src/main/scala/org/apache/kyuubi/plugin/spark/authz/PrivilegesBuilder.scala
+++
b/extensions/spark/kyuubi-spark-authz/src/main/scala/org/apache/kyuubi/plugin/spark/authz/PrivilegesBuilder.scala
@@ -28,6 +28,7 @@ import
org.apache.kyuubi.plugin.spark.authz.OperationType.OperationType
import org.apache.kyuubi.plugin.spark.authz.PrivilegeObjectActionType._
import org.apache.kyuubi.plugin.spark.authz.serde._
import org.apache.kyuubi.plugin.spark.authz.util.AuthZUtils._
+import org.apache.kyuubi.plugin.spark.authz.util.PermanentViewMarker
import org.apache.kyuubi.util.reflect.ReflectUtils._
object PrivilegesBuilder {
@@ -102,6 +103,11 @@ object PrivilegesBuilder {
val cols = conditionList ++ aggCols
buildQuery(a.child, privilegeObjects, projectionList, cols, spark)
+ case pvm: PermanentViewMarker =>
+ getScanSpec(pvm).tables(pvm, spark).foreach { table =>
+ privilegeObjects += PrivilegeObject(table, pvm.visitColNames)
+ }
+
case scan if isKnownScan(scan) && scan.resolved =>
getScanSpec(scan).tables(scan, spark).foreach(mergeProjection(_, scan))
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 679b5d65d..909cd9e93 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
@@ -39,12 +39,16 @@ class RuleApplyPermanentViewMarker extends
Rule[LogicalPlan] {
case permanentView: View if hasResolvedPermanentView(permanentView) =>
val resolvedSubquery = permanentView.transformAllExpressions {
case subquery: SubqueryExpression =>
- // 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.
- subquery.withNewPlan(plan = PermanentViewMarker(subquery.plan,
null))
+ subquery.withNewPlan(plan =
+ PermanentViewMarker(
+ subquery.plan,
+ permanentView.desc,
+ permanentView.output.map(_.name)))
}
- PermanentViewMarker(resolvedSubquery, resolvedSubquery.desc)
+ PermanentViewMarker(
+ resolvedSubquery,
+ resolvedSubquery.desc,
+ resolvedSubquery.output.map(_.name))
case other => apply(other)
}
}
diff --git
a/extensions/spark/kyuubi-spark-authz/src/main/scala/org/apache/kyuubi/plugin/spark/authz/util/PermanentViewMarker.scala
b/extensions/spark/kyuubi-spark-authz/src/main/scala/org/apache/kyuubi/plugin/spark/authz/util/PermanentViewMarker.scala
index 69b55e0fc..d19f7a923 100644
---
a/extensions/spark/kyuubi-spark-authz/src/main/scala/org/apache/kyuubi/plugin/spark/authz/util/PermanentViewMarker.scala
+++
b/extensions/spark/kyuubi-spark-authz/src/main/scala/org/apache/kyuubi/plugin/spark/authz/util/PermanentViewMarker.scala
@@ -21,7 +21,10 @@ import org.apache.spark.sql.catalyst.catalog.CatalogTable
import org.apache.spark.sql.catalyst.expressions.Attribute
import org.apache.spark.sql.catalyst.plans.logical.{LogicalPlan, UnaryNode}
-case class PermanentViewMarker(child: LogicalPlan, catalogTable: CatalogTable)
extends UnaryNode
+case class PermanentViewMarker(
+ child: LogicalPlan,
+ catalogTable: CatalogTable,
+ visitColNames: Seq[String]) extends UnaryNode
with WithInternalChild {
override def output: Seq[Attribute] = child.output
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 e4e3014f5..532a11436 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
@@ -837,6 +837,51 @@ class HiveCatalogRangerSparkExtensionSuite extends
RangerSparkExtensionSuite {
}
}
+ test("[KYUUBI #5475] Check permanent view's subquery should check view's
correct privilege") {
+ val db1 = defaultDb
+ val table1 = "table1"
+ val table2 = "table2"
+ val view1 = "view1"
+ withSingleCallEnabled {
+ 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,
+ | name string,
+ | age int,
+ | scope int)
+ | """.stripMargin))
+ doAs(
+ admin,
+ sql(
+ s"""
+ |CREATE VIEW $db1.$view1
+ |AS
+ |WITH temp AS (
+ | SELECT max(scope) max_scope
+ | FROM $db1.$table1)
+ |SELECT id, name, max(scope) as max_scope, sum(age) sum_age
+ |FROM $db1.$table2
+ |WHERE scope in (SELECT max_scope FROM temp)
+ |GROUP BY id, name
+ |""".stripMargin))
+ // Will just check permanent view privilege.
+ val e2 = intercept[AccessControlException](
+ doAs(
+ someone,
+ sql(s"SELECT id as new_id, name, max_scope FROM
$db1.$view1".stripMargin).show()))
+ assert(e2.getMessage.contains(
+ s"does not have [select] privilege on " +
+
s"[$db1/$view1/id,$db1/$view1/name,$db1/$view1/max_scope,$db1/$view1/sum_age]"))
+ }
+ }
+ }
+
test("[KYUUBI #5492] saveAsTable create DataSource table miss db info") {
val table1 = "table1"
withSingleCallEnabled {
