auth: Refactor `AuthorizationRequest` Into Explicit Shapes by sungwy · Pull Request #4409 · apache/polaris

sungwy · 2026-05-12T02:30:35Z

This PR introduces a breaking change to the the new authorization SPI request model to make authorization intent explicit and to separate single-request shape from batching. Handler call sites on main have not migrated to the new SPI yet, so these changes are safe to introduce. This is an alternative proposal to #4201

Changes are as follows:

remove AuthorizationTargetBinding
remove PolarisPrincipal from AuthorizationRequest (only has single operation + no target, single target or pairwise targets)
make AuthorizationRequest a sealed hierarchy of:
- UntargetedAuthorizationRequest
- SingleTargetAuthorizationRequest
- PairwiseTargetAuthorizationRequest
model batching as List for one principal, rather than one request with multiple bindings

The benefit of this change is that it allows the AuthorizationRequest, and the authorize SPI to explicitly encapsulate the relationship between the operation and the targets without leaving it to the PolarisAuthorizer implementation to interpret it.

Batching

In order to support this model, a batching SPI needs to be introduced. Batching consists of a single PolarisPrincipal and multiple AuthorizationRequests, and can represent:

one operation across multiple targets
genuinely multiple authorization intents in one handler flow (e.g. granular authorization for updateTable)

Current batch behavior is intentionally conservative:

batch resolveAuthorizationInputs(...) is implementation-defined
built-in RBAC and OPA implementations resolve once per batch via shared resolveAll()
batch authorize(...) defaults to sequential evaluation
OPA now evaluates batches sequentially (this is a notable non-breaking, performance impacting change)

This preserves authorization outcomes while leaving room for PolarisAuthorizer implementation to introduce batching semantics in the downstream contract in the future.

Example authorize method call shapes:

SingleTargetAuthorizationRequest

  principal
    + request(op=GET_CATALOG, target=catalog1)

Batch SingleTargetAuthorizationRequest (same operation)

  principal
    + COMMIT_TRANSACTION
    + targets[
        catalog1/ns1/table1,
        catalog1/ns1/table2,
        ...
      ]

Batch SingleTargetAuthorizationRequest (different operations - e.g. updateTable)

  principal
    + batch[
        request(op=SET_TABLE_PROPERTIES, target=catalog1/ns1/table1),
        request(op=SET_TABLE_SNAPSHOT_REF, target=catalog1/ns1/table1)
      ]

Checklist

🛡️ Don't disclose security issues! (contact security@apache.org)
🔗 Clearly explained why the changes are needed, or linked related issues: Fixes #
🧪 Added/updated tests with good coverage, or manually tested (and explained how)
💡 Added comments for complex logic
🧾 Updated CHANGELOG.md (if needed)
📚 Updated documentation in site/content/in-dev/unreleased (if needed)

sungwy · 2026-05-13T12:21:40Z

+    Preconditions.checkNotNull(operation, "operation must be non-null");
+    Preconditions.checkState(
+        target != null || secondary != null,
+        "PairwiseTargetAuthorizationRequest must contain a target or secondary");


note to reviewer: a stronger verification would be to check that both target and secondary are != null, but we are not doing that today as there are valid PairwiseTargetAuthorizationRequest like ADD_ROOT_GRANT_TO_PRINCIPAL_ROLE that have null target but a non null secondary.

Copilot

Pull request overview

This PR refactors the new authorization SPI request model to make authorization intent explicit by splitting AuthorizationRequest into a sealed hierarchy (untargeted, single-target, pairwise-target) and moving principal/batching concerns out of the request itself and into the PolarisAuthorizer API.

Changes:

Replaces AuthorizationRequest (principal + target bindings) with explicit request shapes: UntargetedAuthorizationRequest, SingleTargetAuthorizationRequest, and PairwiseTargetAuthorizationRequest.
Updates the PolarisAuthorizer SPI to take an explicit PolarisPrincipal argument and adds first-class batch methods using List<AuthorizationRequest>.
Updates RBAC/OPA implementations and tests to use the new request factories and batch behavior.

Reviewed changes

Copilot reviewed 12 out of 12 changed files in this pull request and generated 2 comments.

Show a summary per file

File	Description
polaris-core/src/test/java/org/apache/polaris/core/auth/PolarisAuthorizerImplTest.java	Updates authorizer tests for explicit principal argument and adds batch-semantics tests.
polaris-core/src/test/java/org/apache/polaris/core/auth/AuthorizationRequestTest.java	Updates request-shape tests to validate the new sealed request variants and factories.
polaris-core/src/main/java/org/apache/polaris/core/auth/UntargetedAuthorizationRequest.java	Adds explicit request type for operations without securable targets.
polaris-core/src/main/java/org/apache/polaris/core/auth/SingleTargetAuthorizationRequest.java	Adds explicit request type for operations with exactly one target.
polaris-core/src/main/java/org/apache/polaris/core/auth/PairwiseTargetAuthorizationRequest.java	Adds explicit request type for operations with optional (target, secondary) pairing.
polaris-core/src/main/java/org/apache/polaris/core/auth/AuthorizationTargetBinding.java	Removes the prior binding abstraction.
polaris-core/src/main/java/org/apache/polaris/core/auth/AuthorizationRequest.java	Converts `AuthorizationRequest` to a sealed interface with factories and normalized accessors.
polaris-core/src/main/java/org/apache/polaris/core/auth/PolarisAuthorizer.java	Updates SPI signatures to accept explicit principal and adds batch authorize/throw helpers.
polaris-core/src/main/java/org/apache/polaris/core/auth/PolarisAuthorizerImpl.java	Updates RBAC authorizer to new SPI signatures and delegates batch authorize sequentially.
extensions/auth/ranger/impl/src/main/java/org/apache/polaris/extension/auth/ranger/RangerPolarisAuthorizer.java	Updates Ranger authorizer stubs to match the new SPI signatures.
extensions/auth/opa/impl/src/main/java/org/apache/polaris/extension/auth/opa/OpaPolarisAuthorizer.java	Updates OPA authorizer to new SPI signatures and adds sequential batch authorize implementation.
extensions/auth/opa/impl/src/test/java/org/apache/polaris/extension/auth/opa/OpaPolarisAuthorizerTest.java	Updates OPA tests for explicit principal arg and adds batch evaluation tests.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

sungwy · 2026-05-13T12:45:02Z

    return allowed
        ? AuthorizationDecision.allow()
        : AuthorizationDecision.deny(
-            "OPA denied authorization for " + request.formatForAuthorizationMessage());
+            "OPA denied authorization for principal="
+                + polarisPrincipal.getName()
+                + " operation="
+                + request.getOperation());


Thanks Copilot. This was intentional, as it was suggested in a previous discussion that hiding internal security semantics may be beneficial.

dimas-b · 2026-05-13T13:31:45Z

@sneethiraj: FYI

dimas-b · 2026-05-13T13:46:28Z

  void resolveAuthorizationInputs(
-      @Nonnull AuthorizationState authzState, @Nonnull AuthorizationRequest request);
+      @Nonnull AuthorizationState authzState,
+      @Nonnull PolarisPrincipal polarisPrincipal,


I'm hesitant about using PolarisPrincipal as an explicit parameter.

PolarisPrincipal is generally a request-scoped contextual piece of data. Non-authentication code cannot and should not do anything with PolarisPrincipal aside from passing it through... the latter can be done by the CDI framework more cleanly.

Making it an explicit parameter may feel compelling as a closure of all relevant AuthZ inputs. On the other hand if we look at it from the caller side, explicitly referencing PolarisPrincipal is a burden, IMHO.

What might work for both sides (impl. and callers) is a two-layer approach similar to RealmConfig, where the interface does not have an explicit realm parameter, but RealmConfigImpl has it.

WDYT about using PolarisAuthorizer as a caller-side API (no principal parameters) and adding a PolarisAuthorizerBackend as an SPI for plugins (with principal parameters). A shim will exits in-between as a request-scoped CDI bean for extracting PolarisPrincipal from the CDI context?

This is just a idea for consideration... not a blocker for this PR.

Hi Dmitri, that's a good suggestion and I agree with the direction. Splitting the caller-facing API from the implementation/plugin SPI seems clean.

One question I had while thinking about it: if we do that for PolarisPrincipal, do you think we should apply the same principle to RealmContext as well? Some authorizers may also need realm information as part of the effective auth context, so I’m curious whether you think those should be treated consistently or not.

That said, I agree this is probably better left out of scope for this PR and handled in a follow-up.

Yes, for RealmContext too.

... although, let's hold RealmContext a bit... it might belong in the authZ factory instead... we can discuss in details later, when implementation time comes :)

I'd push back on lifting PolarisPrincipal out of AuthorizationRequest. (subject, action, resource) is the canonical tuple for authorization, the principal is the "who" in "who did what on what" and belongs with the operation and target as part of one authorization question, not as a parallel argument threaded alongside.

Batching feels like an internal concern of the request model, not a reason to split principal out. The targeted shapes are really (action, resource). They don't need to know about the subject. So I'd factor that into its own interface and let AuthorizationRequest compose principal on top:

// (action, resource) public sealed interface AuthorizationIntent { PolarisAuthorizableOperation operation(); record Untargeted(PolarisAuthorizableOperation operation) implements AuthorizationIntent {} record SingleTarget(PolarisAuthorizableOperation operation, PolarisSecurable target) implements AuthorizationIntent {} record PairwiseTarget( PolarisAuthorizableOperation operation, @Nullable PolarisSecurable target, @Nullable PolarisSecurable secondary) implements AuthorizationIntent {} } // (subject, intents), single shape, batching is just N >= 1 record AuthorizationRequest( PolarisPrincipal principal, List<AuthorizationIntent> intents) {}

If a future flow ever needs mixed-principal batches, that's a new sealed variant rather than an SPI signature change.

I think it's worth getting this right before handlers migrate, since walking the SPI shape back later is much harder than now. Curious what you and others think.

Hi @flyrain - I really like this suggestion. I think the AuthorizationIntent / AuthorizationRequest(subject, intents) split is a cleaner model, especially since the sealed hierarchy is really describing action/resource shape rather than the full authorization request.

I agree that this is a great time to review the shape with scrutiny, as we have an opportunity to come to a strong consensus before we migrate the handler call sites.

If we are in agreement, I can go ahead and adopt this suggestion.

@dimas-b curious to hear your thoughts on this as well. If we are largely in agreement I can move forward with this suggestion

Callers of PolarisAuthorizer in current OSS servers operate in a context with an implied PolarisPrincipal. The principal is established at a higher level than Polaris services (API endpoints). Those callers cannot create or alter PolarisPrincipal (at least they should not).

I do not see a need to force the service code to pass PolarisPrincipal through to PolarisAuthorizer. Service code does not have control over PolarisPrincipal anyway.

AuthorizationIntent captures that situation well.

Making PolarisPrincipal an explicit parameter for the "Authorizer Backend" SPI makes sense, though.

Like I said in my first message, this is not a concern for current PR, though. It's food for thought for future SPI polishing.

If we do want to resolve this now, I propose implementing the API/SPI split as I hinted in my previous messages. In that case AuthorizationIntent will be an API-side concept, AuthorizationRequest an SPI-side concept. The middle "shim" will be constructing AuthorizationRequest from an explicit AuthorizationIntent parameter plus implicit context data containing PolarisPrincipal. RealmContext will also be an SPI-side concern to be handled by the "shim" and/or factory code (to be figured out).

If can have consensus on that approach, I think actual Authorizer Backend instances will become Application-scoped beans, which will enable them to keep long-running state (if they have to).

Side note: the need to make PolarisPrincipal and RealmContext explicit in the Authorizer Backend SPI is based purely on our standing design principle that polaris-core should be neutral to CDI. The API/SPI split I mentioned above is basically a manifestation of CDI context in terms of polaris-core classes. If we adopted CDI throughout the Polaris codebase, this design could be simplified (but I do not mean to make this change now... just sharing my POV).

dimas-b · 2026-05-13T13:48:33Z

+    for (AuthorizationRequest request : requests) {
+      AuthorizationDecision decision = authorize(authzState, polarisPrincipal, request);
+      if (!decision.isAllowed()) {
+        return decision;


This may not work well for list filtering... in that case the caller will need a decision for each target, I think 🤔

I know list filtering a future feature, but IIRC the general consensus is that it's worth implementing, so we should probably consider it in the AuthZ SPI.

I agree. I don’t think the current batch authorize shape is the right model for list filtering, since that likely needs a different response shape entirely rather than a binary decision.

I’d prefer to leave that out of scope for this PR and introduce it as a separate dedicated API. However, if we think we need to reach consensus on that before moving this SPI forward, I’m okay slowing this down and thinking it through first. My current view, though, is that authorize and filterTargets should be separate SPIs.

I'm ok with a gradual approach to supporting list filtering 👍

dimas-b · 2026-05-13T13:58:40Z

-        .map(PolarisSecurable::formatForAuthorizationMessage)
-        .collect(Collectors.joining(", ", "[", "]"));
-  }
+  List<PolarisSecurable> getSecondaries();


The interface breakdown (PairwiseTargetAuthorizationRequest, etc.) LGTM 👍

However, returning a list of targets and a list of secondaries appears to contradict that interface hierarchy 🤔 it mixes pair-wise associations into a list-to-list association... effectively making PairwiseTargetAuthorizationRequest pretty useless, it seems 🤔

WDYT about using a visitor approach here?

AuthorizationRequest.forEachRelation(visitor) -- called by AuthZ implementations

Visitor.process(operation) Visitor.process(operation, PolarisSecurable target) Visitor.process(operation, PolarisSecurable target, PolarisSecurable secondary)

Then AuthorizationRequest can probably have only one concrete private impl. for now without sub-interfaces.

Adding new relation types (shapes) will break existing AuthZ implementations, but I think it would be a good break - it will force implementors to analyze the new data shape and think about supporting it property (rather than ignoring through omission).

Alternatively we could have AuthorizationRequest.asRelations() returning Stream<Tuple<Operation, Target, Secondary>>

Tuple being a new class (do suggest a better name).

This opens a way for supporting more complex input shape via the relational algebra approach without addition interface changes.

On a second thought, I think we might be over-designing this 😅

I guess we do not have any existing use cases for multiple targets/secondaries per AuthorizationRequest.

I personally gravitate towards starting simple and just have one triplet: <operation, target, secondary> per AuthorizationRequest, the latter two parameters being optional.

I think, for the sake of making progress, it might be best to have a simple SPI now and worry about more complex cases later, if we ever have to. WDYT?

Yeah, I think this is a good idea, and thanks for the catch.

I’m a bit on the fence on whether optional/null is better than throwing on an invalid accessor, for example getTarget() on UntargetedAuthorizationRequest. But I’m fine keeping it simple for now and moving forward with the optional variant.

With throwing the caller has to examine the request's sub-type to avoid hitting those exceptions... this might promote a lot of if/else/switch code 🤔 Optional will allow more generic processing relying only on the presence of data.... just sharing my view :)

dimas-b · 2026-05-13T18:07:04Z

+        SingleTargetAuthorizationRequest,
+        PairwiseTargetAuthorizationRequest {
+  static AuthorizationRequest of(@Nonnull PolarisAuthorizableOperation operation) {
+    return new UntargetedAuthorizationRequest(operation);


UntargetedAuthorizationRequest is only for realm-level actions, right? Even a catalog is not in the picture in this case, right?

That's right. These are operations like CREATE_CATALOG that are at the root level.

Here are examples from the Ranger tests that would use UntargetedAuthorizationRequest: https://github.com/apache/polaris/blob/main/extensions/auth/ranger/impl/src/test/resources/authz_tests/tests_authz_root.json

dimas-b · 2026-05-13T23:11:37Z

+  }
+
+  @Override
+  public @Nullable PolarisSecurable getTarget() {


nit: I guess we might be allowed to override this annotation with @Nonnull 🤔

sungwy added 3 commits May 6, 2026 22:25

propose new shapes for AuthorizationRequest

4960773

additional tests

51dc649

minor changes

ff44952

github-project-automation Bot added this to Basic Kanban Board May 12, 2026

github-project-automation Bot moved this to PRs In Progress in Basic Kanban Board May 12, 2026

sungwy changed the title ~~Auth op model spike~~ auth: Refactor AuthorizationRequest Into Explicit Shapes May 12, 2026

sungwy commented May 13, 2026

View reviewed changes

sungwy marked this pull request as ready for review May 13, 2026 12:25

Copilot AI review requested due to automatic review settings May 13, 2026 12:25

Copilot started reviewing on behalf of sungwy May 13, 2026 12:26 View session

Copilot AI reviewed May 13, 2026

View reviewed changes

test nit

633cb88

sungwy requested review from dimas-b and flyrain May 13, 2026 12:51

dimas-b reviewed May 13, 2026

View reviewed changes

adopt feedback - thanks dmitri

96623d5

dimas-b approved these changes May 13, 2026

View reviewed changes

github-project-automation Bot moved this from PRs In Progress to Ready to merge in Basic Kanban Board May 13, 2026

Conversation

sungwy commented May 12, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Batching

Example authorize method call shapes:

Checklist

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

dimas-b commented May 13, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dimas-b May 13, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dimas-b May 14, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dimas-b May 14, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dimas-b May 13, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

sungwy commented May 12, 2026 •

edited

Loading

dimas-b May 13, 2026 •

edited

Loading

dimas-b May 14, 2026 •

edited

Loading

dimas-b May 14, 2026 •

edited

Loading

dimas-b May 13, 2026 •

edited

Loading