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 9be2a9c50 [KYUUBI #5503][FOLLOWUP][AUTHZ] Authz should skip inner plan
that have been verified
9be2a9c50 is described below
commit 9be2a9c508882d81d99e736a814bce457f2082b5
Author: Angerszhuuuu <[email protected]>
AuthorDate: Tue Oct 31 16:35:31 2023 +0800
[KYUUBI #5503][FOLLOWUP][AUTHZ] Authz should skip inner plan that have been
verified
### _Why are the changes needed?_
To close #5503
For sql such as lateral join in test `[KYUUBI #5503][AUTHZ] Check plan auth
checked should not set tag to all child nodes`, it will first verify subquery
in `lateral` then verify whole plan, if there is a view, when verify the whole
plan, the `PermanentViewMarker` will be remove by spark's optimizer.
Then it will verify both source table `table1` and `table2`.
So I think we need to do 3 things:
1. Mark all PermanentViewMarker's children's all nodes as checked and
Subquery's all child marks as checked.
2. `isAuthChecked` should only check the first level of the plan to avoid
skipping the check of the whole plan in the demo test
3. in `buildQuery`, if the current node has the tag, we just skip it.
Without this pr, the SQL in test will both check `table1` and `table2`
### _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 #5563 from AngersZhuuuu/KYUUBI-5503-FOLLOWUP.
Closes #5503
c1a427f58 [Angerszhuuuu] Update Authorization.scala
d6b2899db [Angerszhuuuu] update
633bc91e0 [Angerszhuuuu] Update Authorization.scala
7a006b136 [Angerszhuuuu] [KYUUBI #5503][FOLLOWUP][AUTHZ] Authz should skip
inner plan that have been verified
Authored-by: Angerszhuuuu <[email protected]>
Signed-off-by: Cheng Pan <[email protected]>
---
.../plugin/spark/authz/PrivilegesBuilder.scala | 3 +
.../plugin/spark/authz/rule/Authorization.scala | 29 ++++----
.../spark/authz/gen/PolicyJsonFileGenerator.scala | 13 ++++
.../src/test/resources/sparkSql_hive_jenkins.json | 77 ++++++++++++++++++----
.../plugin/spark/authz/RangerTestResources.scala | 1 +
.../authz/ranger/RangerSparkExtensionSuite.scala | 42 +++++++++++-
6 files changed, 137 insertions(+), 28 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 07ed71011..833499280 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
@@ -26,6 +26,7 @@ import org.slf4j.LoggerFactory
import org.apache.kyuubi.plugin.spark.authz.OperationType.OperationType
import org.apache.kyuubi.plugin.spark.authz.PrivilegeObjectActionType._
+import org.apache.kyuubi.plugin.spark.authz.rule.Authorization.KYUUBI_AUTHZ_TAG
import
org.apache.kyuubi.plugin.spark.authz.rule.permanentview.PermanentViewMarker
import org.apache.kyuubi.plugin.spark.authz.serde._
import org.apache.kyuubi.plugin.spark.authz.util.AuthZUtils._
@@ -80,6 +81,8 @@ object PrivilegesBuilder {
}
plan match {
+ case p if p.getTagValue(KYUUBI_AUTHZ_TAG).nonEmpty =>
+
case p: Project => buildQuery(p.child, privilegeObjects, p.projectList,
conditionList, spark)
case j: Join =>
diff --git
a/extensions/spark/kyuubi-spark-authz/src/main/scala/org/apache/kyuubi/plugin/spark/authz/rule/Authorization.scala
b/extensions/spark/kyuubi-spark-authz/src/main/scala/org/apache/kyuubi/plugin/spark/authz/rule/Authorization.scala
index db50873b3..6c7d0afa6 100644
---
a/extensions/spark/kyuubi-spark-authz/src/main/scala/org/apache/kyuubi/plugin/spark/authz/rule/Authorization.scala
+++
b/extensions/spark/kyuubi-spark-authz/src/main/scala/org/apache/kyuubi/plugin/spark/authz/rule/Authorization.scala
@@ -18,7 +18,7 @@
package org.apache.kyuubi.plugin.spark.authz.rule
import org.apache.spark.sql.SparkSession
-import org.apache.spark.sql.catalyst.plans.logical.LogicalPlan
+import org.apache.spark.sql.catalyst.plans.logical.{LogicalPlan, Subquery}
import org.apache.spark.sql.catalyst.rules.Rule
import org.apache.spark.sql.catalyst.trees.TreeNodeTag
@@ -42,20 +42,27 @@ object Authorization {
val KYUUBI_AUTHZ_TAG = TreeNodeTag[Unit]("__KYUUBI_AUTHZ_TAG")
+ private def markAllNodesAuthChecked(plan: LogicalPlan): LogicalPlan = {
+ plan.transformDown { case p =>
+ p.setTagValue(KYUUBI_AUTHZ_TAG, ())
+ p
+ }
+ }
+
protected def markAuthChecked(plan: LogicalPlan): LogicalPlan = {
- plan match {
- case _: PermanentViewMarker =>
- plan.transformUp { case p =>
- p.setTagValue(KYUUBI_AUTHZ_TAG, ())
- p
- }
- case _ =>
- plan.setTagValue(KYUUBI_AUTHZ_TAG, ())
+ plan.setTagValue(KYUUBI_AUTHZ_TAG, ())
+ plan transformDown {
+ case pvm: PermanentViewMarker =>
+ markAllNodesAuthChecked(pvm)
+ case subquery: Subquery =>
+ markAllNodesAuthChecked(subquery)
}
- plan
}
protected def isAuthChecked(plan: LogicalPlan): Boolean = {
- plan.find(_.getTagValue(KYUUBI_AUTHZ_TAG).nonEmpty).nonEmpty
+ plan match {
+ case subquery: Subquery => isAuthChecked(subquery.child)
+ case p => p.getTagValue(KYUUBI_AUTHZ_TAG).nonEmpty
+ }
}
}
diff --git
a/extensions/spark/kyuubi-spark-authz/src/test/gen/scala/org/apache/kyuubi/plugin/spark/authz/gen/PolicyJsonFileGenerator.scala
b/extensions/spark/kyuubi-spark-authz/src/test/gen/scala/org/apache/kyuubi/plugin/spark/authz/gen/PolicyJsonFileGenerator.scala
index 7faddd0c7..7996c8ecc 100644
---
a/extensions/spark/kyuubi-spark-authz/src/test/gen/scala/org/apache/kyuubi/plugin/spark/authz/gen/PolicyJsonFileGenerator.scala
+++
b/extensions/spark/kyuubi-spark-authz/src/test/gen/scala/org/apache/kyuubi/plugin/spark/authz/gen/PolicyJsonFileGenerator.scala
@@ -108,6 +108,7 @@ class PolicyJsonFileGenerator extends AnyFunSuite {
policyAccessForDefaultBobUse,
policyAccessForDefaultBobSelect,
policyAccessForPermViewAccessOnly,
+ policyAccessForTable2AccessOnly,
// row filter
policyFilterForSrcTableKeyLessThan20,
policyFilterForPermViewKeyLessThan20,
@@ -345,4 +346,16 @@ class PolicyJsonFileGenerator extends AnyFunSuite {
users = List(permViewOnlyUser),
accesses = allowTypes(select),
delegateAdmin = true)))
+
+ private val policyAccessForTable2AccessOnly = KRangerPolicy(
+ name = "someone_access_table2",
+ resources = Map(
+ databaseRes(defaultDb),
+ tableRes("table2"),
+ allColumnRes),
+ policyItems = List(
+ KRangerPolicyItem(
+ users = List(table2OnlyUser),
+ accesses = allowTypes(select),
+ delegateAdmin = true)))
}
diff --git
a/extensions/spark/kyuubi-spark-authz/src/test/resources/sparkSql_hive_jenkins.json
b/extensions/spark/kyuubi-spark-authz/src/test/resources/sparkSql_hive_jenkins.json
index 6c160d321..76d8c788a 100644
---
a/extensions/spark/kyuubi-spark-authz/src/test/resources/sparkSql_hive_jenkins.json
+++
b/extensions/spark/kyuubi-spark-authz/src/test/resources/sparkSql_hive_jenkins.json
@@ -544,6 +544,55 @@
"isEnabled" : true,
"version" : 1,
"service" : "hive_jenkins",
+ "name" : "someone_access_table2",
+ "policyType" : 0,
+ "policyPriority" : 0,
+ "description" : "",
+ "isAuditEnabled" : true,
+ "resources" : {
+ "database" : {
+ "values" : [ "default" ],
+ "isExcludes" : false,
+ "isRecursive" : false
+ },
+ "column" : {
+ "values" : [ "*" ],
+ "isExcludes" : false,
+ "isRecursive" : false
+ },
+ "table" : {
+ "values" : [ "table2" ],
+ "isExcludes" : false,
+ "isRecursive" : false
+ }
+ },
+ "conditions" : [ ],
+ "policyItems" : [ {
+ "accesses" : [ {
+ "type" : "select",
+ "isAllowed" : true
+ } ],
+ "users" : [ "user_table2_only" ],
+ "groups" : [ ],
+ "roles" : [ ],
+ "conditions" : [ ],
+ "delegateAdmin" : true
+ } ],
+ "denyPolicyItems" : [ ],
+ "allowExceptions" : [ ],
+ "denyExceptions" : [ ],
+ "dataMaskPolicyItems" : [ ],
+ "rowFilterPolicyItems" : [ ],
+ "options" : { },
+ "validitySchedules" : [ ],
+ "policyLabels" : [ ],
+ "isDenyAllElse" : false
+ }, {
+ "id" : 9,
+ "guid" : "45c48cce-2e2d-3fbd-aa1a-fc51c7c6ad26",
+ "isEnabled" : true,
+ "version" : 1,
+ "service" : "hive_jenkins",
"name" : "src_key_less_than_20",
"policyType" : 2,
"policyPriority" : 0,
@@ -586,8 +635,8 @@
"policyLabels" : [ ],
"isDenyAllElse" : false
}, {
- "id" : 9,
- "guid" : "45c48cce-2e2d-3fbd-aa1a-fc51c7c6ad26",
+ "id" : 10,
+ "guid" : "d3d94468-02a4-3259-b55d-38e6d163e820",
"isEnabled" : true,
"version" : 1,
"service" : "hive_jenkins",
@@ -633,8 +682,8 @@
"policyLabels" : [ ],
"isDenyAllElse" : false
}, {
- "id" : 10,
- "guid" : "d3d94468-02a4-3259-b55d-38e6d163e820",
+ "id" : 11,
+ "guid" : "6512bd43-d9ca-36e0-ac99-0b0a82652dca",
"isEnabled" : true,
"version" : 1,
"service" : "hive_jenkins",
@@ -685,8 +734,8 @@
"policyLabels" : [ ],
"isDenyAllElse" : false
}, {
- "id" : 11,
- "guid" : "6512bd43-d9ca-36e0-ac99-0b0a82652dca",
+ "id" : 12,
+ "guid" : "c20ad4d7-6fe9-3759-aa27-a0c99bff6710",
"isEnabled" : true,
"version" : 1,
"service" : "hive_jenkins",
@@ -737,8 +786,8 @@
"policyLabels" : [ ],
"isDenyAllElse" : false
}, {
- "id" : 12,
- "guid" : "c20ad4d7-6fe9-3759-aa27-a0c99bff6710",
+ "id" : 13,
+ "guid" : "c51ce410-c124-310e-8db5-e4b97fc2af39",
"isEnabled" : true,
"version" : 1,
"service" : "hive_jenkins",
@@ -789,8 +838,8 @@
"policyLabels" : [ ],
"isDenyAllElse" : false
}, {
- "id" : 13,
- "guid" : "c51ce410-c124-310e-8db5-e4b97fc2af39",
+ "id" : 14,
+ "guid" : "aab32389-22bc-325a-af60-6eb525ffdc56",
"isEnabled" : true,
"version" : 1,
"service" : "hive_jenkins",
@@ -841,8 +890,8 @@
"policyLabels" : [ ],
"isDenyAllElse" : false
}, {
- "id" : 14,
- "guid" : "aab32389-22bc-325a-af60-6eb525ffdc56",
+ "id" : 15,
+ "guid" : "9bf31c7f-f062-336a-96d3-c8bd1f8f2ff3",
"isEnabled" : true,
"version" : 1,
"service" : "hive_jenkins",
@@ -893,8 +942,8 @@
"policyLabels" : [ ],
"isDenyAllElse" : false
}, {
- "id" : 15,
- "guid" : "9bf31c7f-f062-336a-96d3-c8bd1f8f2ff3",
+ "id" : 16,
+ "guid" : "c74d97b0-1eae-357e-84aa-9d5bade97baf",
"isEnabled" : true,
"version" : 1,
"service" : "hive_jenkins",
diff --git
a/extensions/spark/kyuubi-spark-authz/src/test/scala/org/apache/kyuubi/plugin/spark/authz/RangerTestResources.scala
b/extensions/spark/kyuubi-spark-authz/src/test/scala/org/apache/kyuubi/plugin/spark/authz/RangerTestResources.scala
index 468c2c50b..4f870d504 100644
---
a/extensions/spark/kyuubi-spark-authz/src/test/scala/org/apache/kyuubi/plugin/spark/authz/RangerTestResources.scala
+++
b/extensions/spark/kyuubi-spark-authz/src/test/scala/org/apache/kyuubi/plugin/spark/authz/RangerTestResources.scala
@@ -28,6 +28,7 @@ object RangerTestUsers {
val createOnlyUser = "create_only_user"
val defaultTableOwner = "default_table_owner"
val permViewOnlyUser = "user_perm_view_only"
+ val table2OnlyUser = "user_table2_only"
// non-authorized users
val invisibleUser = "i_am_invisible"
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 672d7208f..d3f190687 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
@@ -957,12 +957,17 @@ class HiveCatalogRangerSparkExtensionSuite extends
RangerSparkExtensionSuite {
assume(isSparkV32OrGreater, "Spark 3.1 not support lateral subquery.")
val db1 = defaultDb
val table1 = "table1"
+ val table2 = "table2"
val perm_view = "perm_view"
withSingleCallEnabled {
withCleanTmpResources(
- Seq((s"$db1.$table1", "table"), (s"$db1.$perm_view", "view"))) {
+ Seq(
+ (s"$db1.$table1", "table"),
+ (s"$db1.$table2", "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"))
+ doAs(admin, sql(s"CREATE TABLE IF NOT EXISTS $db1.$table2 (id int,
scope int)"))
+ doAs(admin, sql(s"CREATE VIEW $db1.$perm_view AS SELECT * FROM
$db1.$table2"))
interceptContains[AccessControlException](
doAs(
someone,
@@ -992,7 +997,38 @@ class HiveCatalogRangerSparkExtensionSuite extends
RangerSparkExtensionSuite {
|)
|""".stripMargin).show()))(
s"does not have [select] privilege on " +
- s"[$db1/$table1/id,$db1/$table1/scope]")
+ s"[$db1/$table1/id]")
+
+ interceptContains[AccessControlException](
+ doAs(
+ someone,
+ sql(
+ s"""
+ |SELECT t1.id
+ |FROM $db1.$table1 t1,
+ |LATERAL (
+ | SELECT *
+ | FROM $db1.$table2 t2
+ | WHERE t1.id = t2.id
+ |)
+ |""".stripMargin).show()))(
+ s"does not have [select] privilege on " +
+ s"[$db1/$table2/id,$db1/$table2/scope]")
+ interceptContains[AccessControlException](
+ doAs(
+ table2OnlyUser,
+ sql(
+ s"""
+ |SELECT t1.id
+ |FROM $db1.$table1 t1,
+ |LATERAL (
+ | SELECT *
+ | FROM $db1.$table2 t2
+ | WHERE t1.id = t2.id
+ |)
+ |""".stripMargin).show()))(
+ s"does not have [select] privilege on " +
+ s"[$db1/$table1/id]")
}
}
}
