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
keep PolarisPrincipal in AuthorizationRequest
keep AuthorizationRequest as one principal plus List
make AuthorizationIntent a sealed hierarchy of:
- TargetlessAuthorizationIntent
- SingleTargetAuthorizationIntent
- PairwiseTargetAuthorizationIntent
remove nullable parent shape accessors from AuthorizationIntent
keep shape-specific access only on the concrete intent subtypes
preserve batching as multiple AuthorizationIntents within one AuthorizationRequest

The benefit of this change is that it allows the authorization model, and the authorize SPI, to explicitly encapsulate the relationship between the operation and the target shape without leaving it to the PolarisAuthorizer implementation to interpret nullable parent accessors.

Batching

In this model, batching consists of a single PolarisPrincipal and multiple AuthorizationIntents inside one AuthorizationRequest, 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:

intents in one request are AND-combined
batch resolveAuthorizationInputs(...) is implementation-defined
built-in RBAC and OPA implementations resolve once per request via shared resolveAll()
built-in authorize(...) implementations evaluate intents sequentially and short-circuit on the first deny
OPA now evaluates multi-intent requests sequentially (this is a notable non-breaking, performance impacting change)

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

Example authorize method call shapes:

TargetlessAuthorizationIntent

    principal
      + request[
          intent(op=LIST_CATALOGS)
        ]

SingleTargetAuthorizationIntent

    principal
      + request[
          intent(op=GET_CATALOG, target=catalog1)
        ]

Batch SingleTargetAuthorizationIntent (same operation)

    principal
      + request[
          intent(op=GET_TABLE, target=catalog1/ns1/table1),
          intent(op=GET_TABLE, target=catalog1/ns1/table2)
        ]

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

    principal
      + request[
          intent(op=REMOVE_TABLE_PROPERTIES, target=catalog1/ns1/table1),
          intent(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.

@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).

I’m not sure CDI should drive the shape of the authorization model here. To me, AuthorizationRequest should be self contained and represent the full authorization question, which includes the principal.

It is fine for callers to obtain the principal from request context, but once we construct an AuthorizationRequest, I think it should carry the subject, operation, and target together. Otherwise we are really modeling an AuthorizationIntent, not a full authorization request.

I also don’t really see the need to split PolarisAuthorizer into a front end and back end interface. That feels like implementation complexity leaking into the core authorization model.

Thanks for the comments @flyrain and @dimas-b

I think the shape @flyrain proposed makes sense, so that the subject is still included in the AuthorizationRequest.

I think the CDI integration can be introduced incrementally, since the SPI shape will still remain the same if we implement this through an API/SPI split as @dimas-b suggests.

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 🤔

flyrain

Thanks for the change, @sungwy . I think that's the right direction. Left one comment on the interface AuthorizationIntent, so that we could really use the power of subclasses sealed interface.

flyrain · 2026-05-20T00:23:57Z

+  PolarisAuthorizableOperation getOperation();
+
+  @Nullable
+  PolarisSecurable getTarget();


Thanks for the change. I think AuthorizationIntent as a sealed hierarchy is the right shape for these requests. Want to flag something about the parent interface, because I don't think we're actually getting the benefit of sealing it.

The parent exposes @Nullable getTarget() and @Nullable getSecondary(), plus hasSecurableType and containsType. Both callers, OPA's authorize and PolarisAuthorizerImpl.authorizeIntent, just pull the nullable accessors off and null-check at runtime; neither switches on the subtype. And hasSecurableType / containsType aren't called from anywhere in production code, only by their own tests. So we've ended up with three records, three factory overloads, two nullable parent accessors, and two unused helpers, but the ergonomic at the call site is identical to what AuthorizationTargetBinding gave us: a thing with two nullables, check them.

What worries me more is what happens next. Say six months from now we add a TripleTargetAuthorizationIntent(op, t1, t2, t3), merge-style ops eventually want something like that. With the current shape we'd either have to bolt getTertiary() onto the parent (every existing variant returns null for it, every caller grows another nullable branch) or pretend t3 fits inside one of the accessors we already have. The parent interface ends up accumulating one nullable accessor per shape we've ever supported, and the seal stops paying its way entirely.

I think the seal earns its keep if we make the parent thin, just operation(), and push typed access onto the subtypes:

switch (intent) { case TargetlessAuthorizationIntent t -> ... case SingleTargetAuthorizationIntent s -> ... s.target() ... // @Nonnull case PairwiseTargetAuthorizationIntent p -> ... p.target(), p.secondary() ... }

Then SingleTarget.target() is non-null by type, so callers stop null-checking what the seal already guarantees. The 3-arg of(op, target, secondary) ambiguity quietly dissolves, no nullable parent contract to satisfy, no reason to overload. Adding TripleTarget later becomes a compile error at every switch site, which is exactly the property sealed types are there to give you. I'd suggest to push down the subtype switch as much as possible. If some methods are not ready for the change(e.g., the old method authorizeOrThrow still takes targets and secondaries, I'm OK to change it as a followup.

WDYT?

Thanks for the thoughtful comment, @flyrain. I share a similar concern.

One thing I do want to optimize for here though is compatibility across PolarisAuthorizer implementations. I’m a bit hesitant to require every downstream consumer to know the full set of AuthorizationIntent children and switch over them in order to support the SPI.

In hindsight, keeping target / secondary as separate parent accessors has a very similar tradeoff in the case of the example you described, so I agree that is not a great long-term answer either.

One idea I had was to introduce a compatibility accessor like asAuthorizationTuple() on AuthorizationIntent, implemented by each subtype. The idea would be that each intent can project itself into a normalized tuple shape that downstream consumers can use for payload construction without having to switch on the subtype directly.

My hope is that this gives us a better way to take advantage of the sealed hierarchy for construction, while still keeping the downstream consumer contract relatively stable. Curious what you think.

+1 to asAuthorizationTuple() or shorter asTuple().

However with the tuple approach we do not really gain much by having concrete sub-classes. All data could be represented by operation type (enum) + list(tuple of securables), I think.

IIRC, the idea for sub-types was mainly to allow exact semantics to be specified in terms of java access methods, which, I assume, would be different in each sub-class. However, my impression now is that we're gravitating towards a more generic representation at the java level. So, I think tuples should work just fine and each operation type will have to describe (javadoc) the tuple structure it goes along with.

That said, please feel free to keep sub-types if you think they are useful.

I'm OK to have a asAuthorizationTuple() for backward compatibilities, if we are aimed to remove it later. Besides, I'm also concerned it's structure if we are trying to keep it as a long-term solution, is it a list of securables or two lists of securables? Neither of them seems generic enough to cover the different semantic we may put into subclasses of AuthorizationIntent.

Thanks again for both of your thoughts @dimas-b and @flyrain I thought a bit more about the tradeoff you pointed out, and I think I understand your concerns more clearly now.

What clicked for me is that there are probably two different consumer models here. For PolarisAuthorizerImpl and likely RangerPolarisAuthorizer, the implementation really owns the authorization semantics locally, so reacting to new AuthorizationIntent shapes explicitly is actually desirable (like TertiaryAuthorizationIntent. In that case, switch-case seems desirable rather than an operational burden, because we want the introduction of a new subtype to force PolarisAuthorizer logic to be reviewed and updated.

OpaPolarisAuthorizer feels a bit different because it is thinner and the effective policy logic lives in Rego outside Polaris. That initially made me wonder whether there was value in preserving a simpler projection for such consumers even if the core model stays sealed and typed. But the more I thought about it, the more marginal that compatibility benefit seemed if authorizers like PolarisAuthorizerImpl and RangerPolarisAuthorizer still need explicit handling through switch-case anyway.

So I'm beginning to learn towards not having Nullable getTarget() and getSecondary() in the sealed interface, and having those methods only exposed in the subtypes that have those attributes, and rely on the consumers to use explicit switch case handling instead, aligning with @flyrain 's suggestion:

switch (intent) { case TargetlessAuthorizationIntent t -> ... case SingleTargetAuthorizationIntent s -> ... s.target() ... // @Nonnull case PairwiseTargetAuthorizationIntent p -> ... p.target(), p.secondary() ... }

@sungwy : that proposal LGTM!

I think it also aligns with what @adutra proposed for sub-types (which might have been in our video sync call)

flyrain · 2026-05-20T00:23:57Z

   *
   * <p>Implementations should rely on any required state in {@link AuthorizationState} and the
-   * intent captured by {@link AuthorizationRequest} (principal, operation, and target securables).
+   * request captured by {@link AuthorizationRequest}.


Worth documenting the new batch contract here: intents in one request are AND-combined and evaluation short-circuits on the first deny. Important for downstream PolarisAuthorizer authors.

I think that's a great addition. Thanks @flyrain

dimas-b · 2026-05-21T14:43:17Z

side note: we should move this concrete Authorizer impl. into a different package to avoid confusion with SPI classes.

dimas-b

LGTM 👍

dimas-b · 2026-05-25T14:25:07Z

+
+  boolean hasSecurableType(PolarisEntityType... types);
+
+  static boolean containsType(PolarisSecurable securable, PolarisEntityType... types) {


nit: I tent to read two-arg methods like verb(p1, p1) as p1 VERB p1. In that perspective current method name and params are a bit unconfortable... How about containsType(PolarisEntityType... types, PolarisSecurable securable)?

... or isTypeInList(PolarisSecurable securable, PolarisEntityType... types)?

verb(p1, p1) as p1 VERB p1: did you mean to say verb(p1, p2)?

dimas-b · 2026-05-26T00:35:35Z

      List<PolarisResolvedPathWrapper> resolvedTargets;
-      if (targets.isEmpty()) {
+      List<PolarisResolvedPathWrapper> resolvedSecondaries;
+      if (intent instanceof TargetlessAuthorizationIntent) {


nit: why not switch here too?

This is because polaris-core compiles with Java 17. The subtype matching syntax via switch is supported in Java 21+:

polaris/polaris-core/build.gradle.kts

Line 21 in 43c8d15

id("polaris-client")

polaris/build-logic/src/main/kotlin/polaris-client.gradle.kts

Lines 22 to 24 in 43c8d15

plugins { id("polaris-java") }

tasks.withType(JavaCompile::class.java).configureEach { options.release = 17 }

flyrain

LGTM. Thanks a lot for working on it @sungwy ! Left minor comments. They are not blockers.

flyrain · 2026-05-27T16:41:26Z

-      }
-    }
-    return false;
+  public static AuthorizationRequest of(


Not a blocker: Looks like only tests call to it. I assume we could remove it once we finish all refactors, right?

flyrain · 2026-05-27T16:43:10Z

-      }
-    }
-    return false;
+  public boolean hasSecurableType(PolarisEntityType type) {


not a blocker: Is it mainly used by a test class? In that case, should we move the method to the test class itself?

Hi @flyrain - that's a great call out. This method is expected to be used once we update the OpaPolarisAuthorizer to skip OPA based checks and deny a request based on the type of the PolarisSecurable. The expected usage is noted in the test, and I assume we'll adopt it soon

Thanks for the context. That's reasonable. Do we expect ranger and native authorizer need the same thing?

I don't think native authorizer will need it, but Ranger probably should follow the same pattern in ignoring native RBAC entities (like PRINCIPAL, PRINCIPAL_ROLE, etc)

I was wondering if filtering on operation makes more sense. In that case, we don't need extra method, while operation is more reliable (given some operations doesn't have securable at all) and provide a more deterministic semantic that certain operations are not supported in certain authorizer.

flyrain · 2026-05-27T16:44:05Z

+
+  @NonNull PolarisAuthorizableOperation getOperation();
+
+  boolean hasSecurableType(PolarisEntityType type);


Same comment here. We may not need it in interface.

flyrain · 2026-05-27T16:44:41Z

+    return new SingleTargetAuthorizationIntent(operation, target);
+  }
+
+  static AuthorizationIntent of(


Same here. I assume we could remove it once the refactor was done.

flyrain · 2026-05-27T16:44:46Z

+    return new TargetlessAuthorizationIntent(operation);
+  }
+
+  static AuthorizationIntent of(


flyrain · 2026-05-27T16:45:43Z

+    permits TargetlessAuthorizationIntent,
+        SingleTargetAuthorizationIntent,
+        PairwiseTargetAuthorizationIntent {
+  static AuthorizationIntent of(@NonNull PolarisAuthorizableOperation operation) {


Same here. Only a test needs it.

These factory methods are intended for use by the handler call sites when we switch over to using AuthorizationRequest based authorize calls.

This is definitely preemptive, and we can remove them, if we want to introduce them we are ready to use them in production instead.

adutra · 2026-05-27T17:13:50Z

+ * <p>The primary target may be omitted for legacy root-scoped flows that rely on an implicit root
+ * primary plus an explicit secondary target.
+ */
+public record PairwiseTargetAuthorizationIntent(


I wonder if we could be more specific here: "pairwise" is still fairly abstract and doesn't capture the relationship between target and secondary.

For example, for rename operations, we could have a RenameAuthorizationIntent(PolarisAuthorizableOperation op, PolarisSecurable from, PolarisSecurable to) – that would be more expressive in terms of relationship between the two securables.

Other similar intents requiring two or more securables could be introduced later, to express different relationships – sealed interfaces are perfect for that.

Sorry if this was raised before – I know I'm late to the party :-) – I don't consider this as a blocker though.

No worries, and thanks for the review. I had initially been cautious about how much subtype-specific handling this would push into PolarisAuthorizer implementations, but I’m increasingly convinced there is value in making the intent semantics as explicit as possible and letting authorizers handle those semantics directly.

See also: #4409 (comment)

Let me explore this a bit more and get a better sense of how many concrete intent types we would actually need to model the current authorization shapes semantically.

Re: RenameAuthorizationIntent - the "rename" part overlaps with the meaning of the "operation" a bit. Will following this line of thinking to the extreme lead to having as many pair-wise intent classes as we have pair-wise operations?

To be clear: I'm not against replacing the operation enum with a rich auth intent class hierarchy.

+1 on @dimas-b 's point. I think the operation part explicitly tell an intent like rename. I'm not convinced that we need to split the pair intent to multiple subclasses, like rename intent, grant policy intent.

Re: RenameAuthorizationIntent - the "rename" part overlaps with the meaning of the "operation" a bit. Will following this line of thinking to the extreme lead to having as many pair-wise intent classes as we have pair-wise operations?

That's precisely why I didn't suggest TableRenameAuthorizationIntent – that's too specific. But a rename operation can affect any "renamable" securable.

We need to strike a balance between intent expressiveness and intent reusability.

Thanks for your thoughts everyone.

One follow-up direction I explored is to keep operation as describing the action, and use the intent subtype to describe the relationship between the pairwise securables. I think the main value in splitting PairwiseTargetAuthorizationIntent further would be to replace generic target / secondary with names that fully express the relationship between the PolarisSecurables.

That would likely mean replacing PairwiseTargetAuthorizationIntent with a small set like:

RenameAuthorizationIntent(operation, from, to)

PolicyAttachmentAuthorizationIntent(operation, policy, attachedTo)

RoleAssignmentAuthorizationIntent(operation, role, assignee)

PrivilegeGrantAuthorizationIntent(operation, grantTarget, grantee)

possibly RootPrivilegeGrantAuthorizationIntent(operation, grantee) for the current root-scoped special case

I see the value in this form, because it allows PolarisAuthorizableOperation to continue carrying the action, while the intent subtype expresses how the PolarisSecurables relate to each other.

Let me know what you folks think about this approach

Nice suggestion, @sungwy ! +1

I'm fine with splitting. The major concern is that each authorizer impl. need to switch between them, which seems fine. I also think @dimas-b 's suggestion of having a PairwiseAuthorizationIntent is worth to explore.

Thank you @dimas-b and @flyrain :)

@flyrain - I thought about that too, but I wasn’t sure what value a PairwiseAuthorizationIntent parent would add if we are not also introducing generic accessors like target / secondary. Without that, it seems like it would mainly serve as a grouping layer rather than adding new API or behavior.

I agreed that is a syntax sugar for grouping. I think we could add it later if needed.

flyrain · 2026-05-27T20:46:01Z

Thanks a lot for working on it, @sungwy . Thanks @dimas-b and @adutra for the review. I'm OK to merge it as is. We could fix non-blockers in followup PRs. WDYT?

gracechen09

lgtm!

gracechen09 · 2026-05-29T17:56:00Z

-    return securables.stream()
-        .map(PolarisSecurable::formatForAuthorizationMessage)
-        .collect(Collectors.joining(", ", "[", "]"));
+  public static AuthorizationRequest of(


Just curious if this is intended to make the code less verbose in the call site, or if it's just used for testing? if it's the former, would this method duplicate what AuthorizationRequest of( @NonNull PolarisPrincipal principal, @NonNull AuthorizationIntent intent) does ?

gracechen09 · 2026-05-29T20:05:00Z

+          queryOpa(
+              buildOpaAuthorizationInput(request.principal(), operation, targets, secondaries));
+      if (!allowed) {
+        return AuthorizationDecision.deny(


I'm wondering if we could keep request.formatForAuthorizationMessage() in the deny log for better observability

gracechen09 · 2026-05-29T20:43:18Z

-    // using binding tuples like [(target, secondary), ...].
+    // Keep the existing OPA input shape by always emitting target and secondary lists, using
+    // empty lists when an intent does not carry that slot. Future work can revisit the payload
+    // shape if OPA starts consuming intent subtype distinctions directly.


I might have missed some previous discussions on this. I'm wondering if we've thought about substituting empty target with the root target on the Polaris side instead of letting OPA to handle the empty target?
Sending the empty list may become a bit confusing for the OPA policy maintainers. For example, they'll need to know that LIST_CATALOG auth request has an empty target and maintain an OPA policy to handle it.

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 previously 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

use authorizationIntent

9354d3c

sungwy dismissed dimas-b’s stale review via 9354d3c May 19, 2026 20:31

flyrain reviewed May 20, 2026

View reviewed changes

sungwy added 3 commits May 19, 2026 22:00

update doc - thanks yufei

bb91c05

merge from main

772ced5

fix ci

65b582c

dimas-b reviewed May 21, 2026

View reviewed changes

sungwy added 2 commits May 24, 2026 17:54

adopt feedback on AuthorizationRequest shape

657c4d3

adopt feedback on AuthorizationRequest shape

cbc5e06

dimas-b previously approved these changes May 26, 2026

View reviewed changes

Merge branch 'main' into auth-op-model-spike

820fe2e

sungwy dismissed dimas-b’s stale review via 820fe2e May 27, 2026 14:19

adopt nits

0f91ce6

sungwy requested review from dimas-b and flyrain May 27, 2026 16:43

flyrain approved these changes May 27, 2026

View reviewed changes

adutra reviewed May 27, 2026

View reviewed changes

gracechen09 reviewed May 29, 2026

View reviewed changes


		boolean hasSecurableType(PolarisEntityType... types);

		static boolean containsType(PolarisSecurable securable, PolarisEntityType... types) {

	plugins { id("polaris-java") }

	tasks.withType(JavaCompile::class.java).configureEach { options.release = 17 }


		@NonNull PolarisAuthorizableOperation getOperation();

		boolean hasSecurableType(PolarisEntityType type);

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!

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!

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!

flyrain left a comment

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 20, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

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

dimas-b May 20, 2026 •

edited

Loading