feat(auth): authorize query-auth reads and carry the grant on the split - #758
feat(auth): authorize query-auth reads and carry the grant on the split#758plusplusjiajia wants to merge 1 commit into
Conversation
e324972 to
f67b381
Compare
f67b381 to
96e1832
Compare
|
|
||
| /// Whether the server says this table is `query-auth.enabled` right now: the | ||
| /// handle's schema is a snapshot, and a cached `false` would skip the check. | ||
| pub(crate) async fn server_query_auth_enabled(&self) -> Result<bool> { |
There was a problem hiding this comment.
[P1] Apply the live server check to direct search APIs too
This helper closes the stale-handle gap for TableScan, but the direct scored/search entry points still call only CoreOptions::ensure_read_authorized() on the schema cached when the handle was loaded. In particular, BatchVectorSearchBuilder::execute reads the snapshot/index manifest directly, and VectorSearchBuilder::execute_scored, FullTextSearchBuilder::execute_scored, and HybridSearchBuilder::execute_scored reach those direct paths without an authorized TableScan.
Therefore: load a REST table while query auth is false, enable restricted query auth on the server, then reuse the handle for one of these searches. The cached guard passes and row IDs/scores derived from protected data are returned without the auth exchange. Please route every out-of-band search entry through this async server-state check and reject when query auth is enabled (these paths cannot apply masking/filtering), with stale-handle regressions analogous to the new scan test.
| } | ||
| let canonical = schema_fields | ||
| .iter() | ||
| .any(|f| f.id() == field.id() && f.name() == field.name()); |
There was a problem hiding this comment.
[P1] Validate the full nested field shape, not only the top-level pair
Both this guard and the old-file check above compare only the top-level (id, name). That leaves a concrete disclosure path after nested schema evolution: suppose the authorized current schema contains profile ROW<public>, while a live older file has the same top-level field id/name but profile ROW<public, secret>. A caller can use the public ReadBuilder::with_read_type with that old Row type; this check passes, and data_file_reader::prune_data_type recursively selects the requested old child by id, so profile.secret is decoded even though it is absent from the schema/column set the server authorized. The planning check at lines 87-92 also passes the old file for the same reason.
Please validate canonical fields recursively (including Row children and nested Array/Map/Multiset element types, allowing only explicitly safe evolution), and make the old-file containment check recursive too. An end-to-end test with a dropped nested field and a crafted old read type should be rejected.
| core_options.ensure_type_paimon_served(&self.table.identifier().full_name())?; | ||
| // The handle's flag is a snapshot; the marker survives a round-trip. | ||
| let required = core_options.query_auth_enabled() | ||
| || data_splits.iter().any(|s| s.query_auth_required()); |
There was a problem hiding this comment.
[P1] Do not accept an unmarked split from a stale REST handle
When a REST handle was loaded while query auth was false, core_options.query_auth_enabled() stays false. If the server later enables restricted query auth, a caller that bypasses TableScan can still pass an old unmarked plan—or a split made through the public DataSplitBuilder—to this synchronous to_arrow boundary. required is false, so the method returns rows without any auth RPC or grant. This contradicts the PR’s stated stale-handle guarantee; the new test covers planning after the toggle, but not this public read path.
Please make REST reads require evidence that the split was planned after a live server-state check even when that check said query auth was disabled (for example, a catalog-session-bound checked-plan capability distinct from query_auth_required), or make materialization able to re-authorize. Unmarked splits should remain acceptable only where no REST authorization boundary exists. Add a regression for: load with false, retain/build an unmarked split, enable a restricted response, then call to_arrow and require refusal.
Purpose
A query-auth.enabled table makes the server return a per-user row filter and column masking that the client is expected to apply. This client cannot apply them yet, so it refuses to read such a table at all — even for a user the server reports as unrestricted. This slice fetches the authorization at scan-plan time and carries it to the read, so that user can read. A user with rules gets the same refusal as before.
Brief change log
TableScan::plan authorizes once and stamps the result on every split, as Java wraps each split in a QueryAuthSplit. TableRead::to_arrow then decides from the splits: each must carry a grant, from this handle,
and unrestricted — per split, since split lists can be concatenated across plans.
Whether a table is query-auth comes from the server, not the loaded handle: the option can be set after a load, and a cached false would skip authorization entirely. Sync read boundaries cannot ask, so they
read a marker the splits carry. Unlike the grant it survives serialization, so a round-tripped split still demands authorization and, having lost the grant, fails closed.
Four refusals are deliberate. A restricted grant fails at planning, since a plan carries row counts and min/max that engines answer COUNT/MIN/MAX from without reading a row. A time-travelled, branch, or
decorated (db.t$branch_x, db.t$files) handle is refused, since the server rules on the current schema while those read other files. A plan is refused when a data file still carries statistics for a column the
current schema no longer has — value_stats and write_cols are public on every split. And the request names no columns, which the server expands to the real schema fields — Java sends the read type's names
instead. Naming a reserved system column would fail the server's permission check and deny an otherwise authorized user, so a read that reaches one is refused client-side instead; the cost is that a user
authorized for only a subset of columns is still refused, as today.
AuthTableQueryResponse rejects unknown fields, unlike every other response: an absent field reads as "no rule", so protocol drift would look like an unrestricted grant.