[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 <angers.zhu@gmail.com> Signed-off-by: Kent Yao <yao@apache.org>
This commit is contained in:
Angerszhuuuu
Kent Yao
parent
1224b0d2c9
commit
9e2fcc3f1f
@ -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))
|
||||
|
||||
|
||||
@ -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)
|
||||
}
|
||||
}
|
||||
|
||||
@ -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
|
||||
|
||||
@ -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 {
|
||||
|
||||
Loading…
Reference in New Issue
Block a user