yashmayya opened a new pull request, #19209:
URL: https://github.com/apache/pinot/pull/19209
The single-stage engine authorizes the table in the query's `FROM` clause.
The `lookup()` transform function names its dimension table with a string
literal argument instead of through the `FROM` clause, so that table is not
part of the query's data source and never reached the table-level access checks:
```sql
SELECT lookup('someDimTable', 'someColumn', 'primaryKey', factCol) FROM
someFactTable
```
Here only `someFactTable` was checked, and `someDimTable` was read without
the principal needing access to it. `BaseSingleStageBrokerRequestHandler`
already had a `// LOOKUP function looks up another table's schema, skip the
check for now.` marker on the related column-resolution path.
The multi-stage engine is not affected — it does not register `lookup()` as
a function (`TransformFunctionType.LOOKUP` has no return type inference, so
`PinotOperatorTable` skips it), and the equivalent lookup join references the
dimension table as a regular table scan, which its all-tables authorization
already covers.
## Changes
- Dimension tables named in `lookup()` are collected while the query is
compiled (`extractLookupTableNames`), covering the select list, filter, group
by, order by and having clauses, calls nested inside other expressions, and a
`lookup()` nested inside another one's join value.
- They are authorized alongside the queried table in `doHandleRequest`, so
both the physical and the logical table paths are covered. `IN_SUBQUERY`
subqueries already recurse through the same handler and are covered as well.
- Names are collected exactly as written, which is how
`LookupTransformFunction` resolves them on the server, so the table that gets
authorized is always the one actually read.
- Row-level security: `lookup()` resolves a row by primary key against the
dimension table's in-memory data and never evaluates a filter against that
table, so an RLS filter on it cannot be applied. Such a query is now rejected
rather than silently returning unfiltered rows.
## Behavior change
Queries using `lookup()` now require access to the dimension table as well:
- A principal not authorized for the dimension table now gets a 403 where
the query previously succeeded.
- With `pinot.broker.enable.row.column.level.auth` enabled, a query is
rejected when the principal has an RLS filter configured on a table it reaches
through `lookup()`.
The check goes through `hasTableAccess`, the same path logical tables and
the multi-stage engine already use, so a custom `AccessControl` implementation
should implement `authorize(RequesterIdentity, Set<String>)`. All in-tree
implementations (`AllowAll`, `BasicAuth`, `ZkBasicAuth`) already do.
## Testing
- `BaseSingleStageBrokerRequestHandlerTest#testExtractLookupTableNames`
covers each clause, nesting, function-name casing, and multiple dimension
tables in one query.
- `LookupTableAccessControlIntegrationTest` runs a broker with Basic auth
and asserts that a principal scoped to the fact table is denied, that a
principal authorized for both tables still gets the looked-up values, and that
queries without `lookup()` are unaffected.
--
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]
