dimas-b commented on code in PR #4409:
URL: https://github.com/apache/polaris/pull/4409#discussion_r3243423951
##########
polaris-core/src/main/java/org/apache/polaris/core/auth/PolarisAuthorizer.java:
##########
@@ -38,26 +39,85 @@ public interface PolarisAuthorizer {
* <p>This method should not perform authorization decisions directly.
*/
void resolveAuthorizationInputs(
- @Nonnull AuthorizationState authzState, @Nonnull AuthorizationRequest
request);
+ @Nonnull AuthorizationState authzState,
+ @Nonnull PolarisPrincipal polarisPrincipal,
Review Comment:
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 it 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).
--
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]
