This is an automated email from the ASF dual-hosted git repository.
chengpan 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 8da180126 [KYUUBI #5503][AUTHZ] Check plan auth checked should not set
tag to all child nodes
8da180126 is described below
commit 8da1801268c87ecfa39e5730e705b669a118d22b
Author: Angerszhuuuu <[email protected]>
AuthorDate: Sun Oct 29 15:45:03 2023 +0800
[KYUUBI #5503][AUTHZ] Check plan auth checked should not set tag to all
child nodes
### _Why are the changes needed?_
To close #5503
For lateral sql won't check table2's privilege
```
test("xxx") {
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,
age int)"))
doAs(admin, sql(
s"""
|SELECT t1.id, age
|FROM $db1.$table1 t1,
|LATERAL (
| SELECT *
| FROM $db1.$table2 t2
| WHERE t1.id = t2.id
|)
|""".stripMargin).explain(true))
}
}
```
It was caused by https://github.com/apache/kyuubi/pull/4529
But after we fix #5417 and #5415, we didn't need this change. We should
remove it.
### _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 #5540 from AngersZhuuuu/TEST_REVERT_4504.
Closes #5503
63fa03771 [Angerszhuuuu] Update RangerSparkExtensionSuite.scala
f3118d293 [Angerszhuuuu] Update RuleAuthorization.scala
f6b6dcccc [Angerszhuuuu] follow comment
2c7ae5d50 [Angerszhuuuu] Update RuleAuthorization.scala
bddc11733 [Angerszhuuuu] Update RangerSparkExtensionSuite.scala
6bb0c39b4 [Angerszhuuuu] Update RangerSparkExtensionSuite.scala
42c924f45 [Angerszhuuuu] Update RangerSparkExtensionSuite.scala
bbb2c3170 [Angerszhuuuu] [KYUUBI #5503][AUTHZ] Check plan auth checked
should not set tag to all child nodes
Authored-by: Angerszhuuuu <[email protected]>
Signed-off-by: Cheng Pan <[email protected]>
---
.../spark/authz/ranger/RuleAuthorization.scala | 18 ++++++--
.../authz/ranger/RangerSparkExtensionSuite.scala | 54 ++++++++++++++++++++--
2 files changed, 62 insertions(+), 10 deletions(-)
diff --git
a/extensions/spark/kyuubi-spark-authz/src/main/scala/org/apache/kyuubi/plugin/spark/authz/ranger/RuleAuthorization.scala
b/extensions/spark/kyuubi-spark-authz/src/main/scala/org/apache/kyuubi/plugin/spark/authz/ranger/RuleAuthorization.scala
index 3203108df..91221c522 100644
---
a/extensions/spark/kyuubi-spark-authz/src/main/scala/org/apache/kyuubi/plugin/spark/authz/ranger/RuleAuthorization.scala
+++
b/extensions/spark/kyuubi-spark-authz/src/main/scala/org/apache/kyuubi/plugin/spark/authz/ranger/RuleAuthorization.scala
@@ -30,6 +30,8 @@ import org.apache.kyuubi.plugin.spark.authz.ObjectType._
import org.apache.kyuubi.plugin.spark.authz.ranger.RuleAuthorization._
import org.apache.kyuubi.plugin.spark.authz.ranger.SparkRangerAdminPlugin._
import org.apache.kyuubi.plugin.spark.authz.util.AuthZUtils._
+import org.apache.kyuubi.plugin.spark.authz.util.PermanentViewMarker
+
class RuleAuthorization(spark: SparkSession) extends Rule[LogicalPlan] {
override def apply(plan: LogicalPlan): LogicalPlan = {
plan match {
@@ -41,7 +43,7 @@ class RuleAuthorization(spark: SparkSession) extends
Rule[LogicalPlan] {
object RuleAuthorization {
- val KYUUBI_AUTHZ_TAG = TreeNodeTag[Boolean]("__KYUUBI_AUTHZ_TAG")
+ val KYUUBI_AUTHZ_TAG = TreeNodeTag[Unit]("__KYUUBI_AUTHZ_TAG")
private def checkPrivileges(spark: SparkSession, plan: LogicalPlan):
LogicalPlan = {
val auditHandler = new SparkRangerAuditHandler
@@ -97,13 +99,19 @@ object RuleAuthorization {
}
private def markAuthChecked(plan: LogicalPlan): LogicalPlan = {
- plan.transformUp { case p =>
- p.setTagValue(KYUUBI_AUTHZ_TAG, true)
- p
+ plan match {
+ case _: PermanentViewMarker =>
+ plan.transformUp { case p =>
+ p.setTagValue(KYUUBI_AUTHZ_TAG, ())
+ p
+ }
+ case _ =>
+ plan.setTagValue(KYUUBI_AUTHZ_TAG, ())
}
+ plan
}
private def isAuthChecked(plan: LogicalPlan): Boolean = {
- plan.find(_.getTagValue(KYUUBI_AUTHZ_TAG).contains(true)).nonEmpty
+ plan.find(_.getTagValue(KYUUBI_AUTHZ_TAG).nonEmpty).nonEmpty
}
}
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 07c8efeda..8923819c3 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
@@ -113,12 +113,12 @@ abstract class RangerSparkExtensionSuite extends
AnyFunSuite
if (i == 1) {
assert(logicalPlan.getTagValue(KYUUBI_AUTHZ_TAG).isEmpty)
} else {
- assert(logicalPlan.getTagValue(KYUUBI_AUTHZ_TAG).getOrElse(false))
+ assert(logicalPlan.getTagValue(KYUUBI_AUTHZ_TAG).nonEmpty)
}
rule.apply(logicalPlan)
}
- assert(logicalPlan.getTagValue(KYUUBI_AUTHZ_TAG).getOrElse(false))
+ assert(logicalPlan.getTagValue(KYUUBI_AUTHZ_TAG).nonEmpty)
}
test("[KYUUBI #3226]: Another session should also check even if the plan is
cached.") {
@@ -140,7 +140,7 @@ abstract class RangerSparkExtensionSuite extends AnyFunSuite
// session1: first query, should auth once.[LogicalRelation]
val df = sql(select)
val plan1 = df.queryExecution.optimizedPlan
- assert(plan1.getTagValue(KYUUBI_AUTHZ_TAG).getOrElse(false))
+ assert(plan1.getTagValue(KYUUBI_AUTHZ_TAG).nonEmpty)
// cache
df.cache()
@@ -148,7 +148,7 @@ abstract class RangerSparkExtensionSuite extends AnyFunSuite
// session1: second query, should auth once.[InMemoryRelation]
// (don't need to check in again, but it's okay to check in once)
val plan2 = sql(select).queryExecution.optimizedPlan
- assert(plan1 != plan2 &&
plan2.getTagValue(KYUUBI_AUTHZ_TAG).getOrElse(false))
+ assert(plan1 != plan2 &&
plan2.getTagValue(KYUUBI_AUTHZ_TAG).nonEmpty)
// session2: should auth once.
val otherSessionDf = spark.newSession().sql(select)
@@ -159,7 +159,7 @@ abstract class RangerSparkExtensionSuite extends AnyFunSuite
// make sure it use cache.
assert(plan3.isInstanceOf[InMemoryRelation])
// auth once only.
- assert(plan3.getTagValue(KYUUBI_AUTHZ_TAG).getOrElse(false))
+ assert(plan3.getTagValue(KYUUBI_AUTHZ_TAG).nonEmpty)
})
}
}
@@ -952,4 +952,48 @@ class HiveCatalogRangerSparkExtensionSuite extends
RangerSparkExtensionSuite {
}
}
}
+
+ test("[KYUUBI #5503][AUTHZ] Check plan auth checked should not set tag to
all child nodes") {
+ assume(isSparkV32OrGreater, "Spark 3.1 not support lateral subquery.")
+ val db1 = defaultDb
+ val table1 = "table1"
+ val perm_view = "perm_view"
+ withSingleCallEnabled {
+ withCleanTmpResources(
+ Seq((s"$db1.$table1", "table"), (s"$db1.$perm_view", "view"))) {
+ doAs(admin, sql(s"CREATE TABLE IF NOT EXISTS $db1.$table1 (id int,
scope int)"))
+ doAs(admin, sql(s"CREATE VIEW $db1.$perm_view AS SELECT * FROM
$db1.$table1"))
+ interceptContains[AccessControlException](
+ doAs(
+ someone,
+ sql(
+ s"""
+ |SELECT t1.id
+ |FROM $db1.$table1 t1,
+ |LATERAL (
+ | SELECT *
+ | FROM $db1.$perm_view t2
+ | WHERE t1.id = t2.id
+ |)
+ |""".stripMargin).show()))(
+ s"does not have [select] privilege on " +
+ s"[$db1/$perm_view/id,$db1/$perm_view/scope]")
+ interceptContains[AccessControlException](
+ doAs(
+ permViewOnlyUser,
+ sql(
+ s"""
+ |SELECT t1.id
+ |FROM $db1.$table1 t1,
+ |LATERAL (
+ | SELECT *
+ | FROM $db1.$perm_view t2
+ | WHERE t1.id = t2.id
+ |)
+ |""".stripMargin).show()))(
+ s"does not have [select] privilege on " +
+ s"[$db1/$table1/id,$db1/$table1/scope]")
+ }
+ }
+ }
}
