Hi Laurent,

Good idea about the feature flag!
I would make it overridable at catalog level (like the other
polaris.config.* flags), so an operator can disable it for a specific
catalog.
When disabled, Polaris should behave like today: no parsing, no validation,
no chain in the events, and an empty referencedBy().
I would keep it enabled by default (with strict validation), as soon as
it's in CHANGELOG/documentation.

About the engine's responsibility (ensuring that all entries in the chain
pertain to this catalog), I agree: Polaris can't verify anything about
other catalogs. That's also what the Trino PR does afair: only Iceberg
views of the same catalog are sent, and a foreign DEFINER view in the
middle of the chain fails the load by default. It's also what makes the
duplicate check correct: with entries from several catalogs, two different
views with the same name would be rejected as a cycle.

My point was only about the javadoc :) the referencedBy() entries are names
asserted by the client, interpreted in the target catalog, and not resolved
by Polaris. An authorizer that needs to check that they exist (or that they
are actually view) has to resolve them in resolveAuthorizationInputs, as
you described. I just wanted to propose a couple of sentences in the
javadoc to prevent authorizers from treating the chain as verified entities
;)

About the cross catalog definer case (viewB definer in catalog 1, viewC in
catalog 2): I agree it's a limitation if referenced-by itself, not
something Polaris can solve on its own. Evaluating viewC as the query user
is the right default imho: worst case is an access denied, not a data leak.
I would document it as a known limitation (we can bring the discussion on
the Iceberg dev mailing list).

Thanks again for the proposal, and, as I said in my previous email, I think
we can focus on part 1 only for now.

Regards
JB

On Thu, Nov 5, 2026 at 4:48 PM Laurent Goujon via dev <
[email protected]> wrote:

> Hello JB,
>
> - Today Polaris ignores the parameter, even if the Iceberg client sends it
> > since 1.12. With the PR, a request with a malformed chain in the
> parameter,
> > a repeated entry or more than 10 entries gets a 400 (even if the
> authorizer
> > ignores the chain). I'm completely ok with "strict" validation but it
> means
> > users "might" see it.
>
>
> We could add a configuration flag to ignore the referenced-by chain. When
> the flag is on, the referenced-by chain will not propagate to the SPI
> though.
>
> - The spec identifiers have no catalog part, so the PR maps each entry to a
> > securable in the target catalog. A entry may not exist, or may match an
> > unrelated view with the same name if an engine includes a view from
> another
> > catalog. I would state this clearly in the referencedBy() javadoc, so
> > authorizers don't treat the chain as a list of verified entities.
>
>
> IMHO, it is the engine's responsibility to ensure that all entries in the
> chain pertain to this catalog since we don't have any visibility on other
> catalogs. We are also touching on the limit of referenced-by which does not
> work cross catalog. Let's say the chain is viewA -> view B -> view C ->
> table X where viewA, viewB and table X belong to catalog 1, and view C to
> catalog 2. If viewB is a definer view, there's no way to express that the
> security evaluation context has changed (for example a different user/role)
> and should be used to check access on view C (assuming that they share the
> same auth infra). The fallback would be to query view C as the original
> query user (which would be safe/no data leakage) and only send complete
> chains relative to a single catalog but this may cause access denied
> scenarios. This situation has been flagged by both Spark [1] and Trino [2]
> communities as well.
>
> [1] https://github.com/apache/iceberg/pull/15895
> [2] https://github.com/trinodb/trino/pull/31542
>
> On Wed, Nov 4, 2026 at 11:58 PM Jean-Baptiste Onofré <[email protected]>
> wrote:
>
> > Hi Laurent,
> >
> > Thanks for the proposal!
> >
> > I'm +1 on part 1. Polaris accepts referenced-by today and drops it as you
> > said, so giving authorizers access to the chain makes sense to me.
> > Regarding the SPI, Polaris is in charge of parsing, validating and
> > forwarding the chain to the authorizers. Each authorizer decides what to
> do
> > with it, possibly ignoring it.
> >
> > I propose to focus on part 1 for now, I believe it will be easy to get
> > consensus on that.
> >
> > I have 3 comments on part 1:
> > - Today Polaris ignores the parameter, even if the Iceberg client sends
> it
> > since 1.12. With the PR, a request with a malformed chain in the
> parameter,
> > a repeated entry or more than 10 entries gets a 400 (even if the
> authorizer
> > ignores the chain). I'm completely ok with "strict" validation but it
> means
> > users "might" see it.
> > - The spec identifiers have no catalog part, so the PR maps each entry to
> > a securable in the target catalog. A entry may not exist, or may match an
> > unrelated view with the same name if an engine includes a view from
> another
> > catalog. I would state this clearly in the referencedBy() javadoc, so
> > authorizers don't treat the chain as a list of verified entities.
> > - I think OPA is a good first consumer candidate. We can add the chain to
> > the OPA input (in the resource, with targets and secondaries). It's just
> > additive, needs no new privilege or persistence change. It's a good
> "PoC",
> > also helping to shape the SPI with a real use case. That's a good first
> > step to part 2.
> >
> > Regards
> > JB
> >
> > On Wed, Nov 4, 2026 at 9:19 PM Laurent Goujon via dev <
> > [email protected]> wrote:
> >
> >> Hi all,
> >>
> >> The Iceberg REST spec has an optional `referenced-by` query parameter on
> >> loadTable, loadView and loadCredentials (apache/iceberg#13810). When an
> >> engine loads a table or view while evaluating a view, it can pass the
> >> chain
> >> of views it went through, outermost first. It was added to make definer
> >> views possible. Polaris accepts the parameter but drops it today, so no
> >> authorizer can act on it.
> >>
> >> I wrote a proposal and opened a draft PR for the first part:
> >>
> >> - Proposal: https://github.com/apache/polaris/issues/5979
> >> - Draft PR (part 1): https://github.com/apache/polaris/pull/5980
> >>
> >> The proposal has two parts:
> >>
> >> 1. SPI. Pass the parsed chain to authorizers through a new
> >> `SingleTargetAuthorizationIntent#referencedBy()` accessor. A chain that
> >> cannot be parsed, is deeper than a configurable limit (10 by default) or
> >> repeats an entry is rejected with a 400. The chain is added to the load
> >> events. Existing authorizers (built-in, OPA, Ranger) are unchanged, and
> >> the
> >> PR does not change any decision.
> >> 2. Built-in authorizer. An administrator binds a view to a catalog role
> (a
> >> new `CATALOG_ROLE_EXECUTE_AS` privilege, with the view as the grantee),
> >> and
> >> a new catalog privilege marks the callers that may assert a chain. The
> >> built-in authorizer then evaluates the chain, switching to the bound
> role
> >> at each bound view. This part is not in the PR.
> >>
> >> Since part 1 changes an extension point, I would like to agree on it
> here
> >> before taking the PR out of draft.
> >>
> >> Re part 2: the proposal includes it so that part 1 comes with a proof of
> >> concept for how an authorizer would use the chain. I can also work on a
> PR
> >> for it, or we could focus on a different authorizer instead (I'm open to
> >> that).
> >>
> >> Thanks,
> >> Laurent
> >>
> >
>

Reply via email to