alexandrefimov opened a new pull request, #7637:
URL: https://github.com/apache/kyuubi/pull/7637
### Why are the changes needed?
Closes #7623.
`AuthzConfigurationChecker` guards the exclusion by matching a `SetCommand`
in the logical plan (`AuthzConfigurationChecker.scala:42-45`), so the
protection covers the `SET` syntax only - which is also how the docs describe
it (`docs/security/authorization/spark/overview.rst:106`, "A set statement with
key equal to ..."). Every channel that writes the config without producing a
plan keeps working: `spark.conf.set`, the Spark Connect Config RPC
(`SparkConnectConfigHandler.handleSet` calls `conf.set` directly), or the key
passed in a JDBC connection string. Once `RuleAuthorization` is named there,
`Optimizer.batches` drops it - extension rules are not in
`SparkOptimizer.nonExcludableRules` - and the rest of the session runs
unauthorized.
Reproduced on Spark 4.0.3 with `kyuubi-spark-authz` and a Ranger plugin that
denies by default: `create` denied, `SET
spark.sql.optimizer.excludedRules=...RuleAuthorization` rejected by the
checker, the same key accepted over the Connect Config RPC, the next `create`
allowed.
The documented mitigation, `kyuubi.session.conf.restrict.list`
(`docs/security/authorization/spark/overview.rst:85-93`), does reject such a
JDBC connection - I checked that too - but it is enforced in the server's
`SessionManager`, so it does not reach a client that talks to the engine
directly.
This patch reads the value in effect on every plan instead of matching the
statement. Check rules are not filtered by `excludedRules`, which only applies
to optimizer batches, so this check cannot be removed the same way.
Two points a reviewer may want to decide differently:
- The value check matches `org.apache.kyuubi.plugin.spark.authz.ranger`, the
prefix the existing `SET` case uses. The plugin also injects optimizer rules
from `org.apache.kyuubi.plugin.spark.authz.rule` (`RuleEliminateMarker` and its
neighbours), which neither the old nor the new check covers. I left the prefix
as is rather than widen the scope here.
- The check is fail closed for the whole session: with the exclusion already
in the session conf, every plan is rejected, not just the `SET`. That is the
intent, but it is a visible behaviour change for a session that set the key
before this patch.
### How was this patch tested?
New test in `AuthzConfigurationCheckerSuite`: the config is written through
`spark.conf.set` - the same write path the Connect Config RPC takes - and the
next plan is rejected, while excluding a non-authz rule (`ConstantFolding`)
stays allowed.
```
build/mvn test -pl extensions/spark/kyuubi-spark-authz -Dtest=none \
-DwildcardSuites=org.apache.kyuubi.plugin.spark.authz.rule.AuthzConfigurationCheckerSuite
```
All three tests in the suite pass; with the new check removed from `apply`,
exactly the new test fails.
The existing test needed one line: `sql("set
spark.sql.optimizer.excludedRules=...")` applies the value to the shared
session before the rule is invoked by hand, so the test now unsets it -
otherwise the effective-value check rejects every later plan in that session.
### Was this patch authored or co-authored using generative AI tooling?
Assisted-by: Claude:claude-opus-5
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
