[I] Deduplicate intent-to-resolved-path resolution shared by PolarisAuthorizerImpl and RangerPolarisAuthorizer [polaris]

["ZephyrYWZhou (via GitHub)" <gi...@apache.org>]

ZephyrYWZhou opened a new issue, #5485:
URL: https://github.com/apache/polaris/issues/5485

   ## Background
   
   After #5194, each `PolarisAuthorizer` implementation resolves an `AuthorizationRequest`'s intents into resolved entity paths on its own, so it can choose what to resolve. As a result, the mechanical mapping from an `AuthorizationIntent` to its resolved target/secondary paths is now duplicated **verbatim** in two authorizers:
   
   - `PolarisAuthorizerImpl` (built-in RBAC, `polaris-core`)
   - `RangerPolarisAuthorizer` (Apache Ranger extension)
   
   Both contain the same per-intent-type dispatch over the 7 sealed `AuthorizationIntent` types plus identical private helpers `getResolvedSecurable(...)` and `getPathNamesWithinCatalog(...)`.
   
   This was raised while reviewing #5170 (which was superseded by #5194); the suggestion there was to file a follow-up PR to deduplicate the shared resolution logic.
   
   ## Problem
   
   The intent-to-resolved-path translation is correctness-sensitive and drift-prone:
   
   - `getResolvedSecurable` encodes which resolution-manifest lookup applies per entity type (`CATALOG` → reference catalog, top-level → top-level entity, else → resolved path), and `getPathNamesWithinCatalog` strips the catalog segment before lookup.
   - Because the logic is copy-pasted, a change to the resolution rules or the addition of a new intent type can be applied to one authorizer and missed in the other, producing an authorization inconsistency between the built-in and Ranger paths. This has already shown up as a maintenance cost: the `CATALOG` branch in `getResolvedSecurable` had to be added to both copies.
   
   ## Proposal
   
   Extract the shared translation into a single `AuthorizationIntentResolver` in `org.apache.polaris.core.auth`:
   
   ```
   ResolvedIntent resolve(PolarisResolutionManifest manifest,
                          AuthorizationIntent intent,
                          boolean prependRootContainer)
   ```
   
   - Authorizer-specific concerns stay in each authorizer: the operation-semantics / rooting decision (each authorizer computes and passes its own `prependRootContainer`), the terminal decision call (`authorizeRbacOrThrow` vs `authorizeRangerOrThrow`), and exception handling.
   - The change is behavior-preserving; the existing authorization test suites are the safety net.
   
   `OpaPolarisAuthorizer` is **out of scope**: it maps intents to its own `ResourceEntity` model and ships them to an external OPA server, so it does not perform resolution-manifest path resolution.
   
   ## Acceptance Criteria
   
   - A shared `AuthorizationIntentResolver` in `polaris-core` is used by both `PolarisAuthorizerImpl` and `RangerPolarisAuthorizer`; the duplicated private helpers are removed.
   - No behavior change: existing authz suites pass (`PolarisAuthorizerImplTest`, the runtime-service catalog handler authz tests, the Ranger unit tests + `intTest`, and the OPA tests).
   - `polaris-core` remains Java 17 source-compatible.
   
   ## Possible follow-up (out of scope for this issue)
   
   Consider moving securable naming onto the `AuthorizationIntent` types themselves (each intent exposes its own target/secondary securables). That would remove the per-type dispatch entirely and provide compile-time exhaustiveness for future intent types, without requiring a Java 21 pattern `switch` (which `polaris-core` cannot use since it compiles with `--release 17`).
   
   Related: #5170, #5194, #5171, #5034
   


-- 
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: issues-unsubscribe@polaris.apache.org.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

Re: [I] Deduplicate intent-to-resolved-path resolution shared by PolarisAuthorizerImpl and RangerPolarisAuthorizer [polaris]

["ZephyrYWZhou (via GitHub)" <gi...@apache.org>]

ZephyrYWZhou commented on issue #5485:
URL: https://github.com/apache/polaris/issues/5485#issuecomment-5614087614

   Follow-up note (from the #5486 review): @flyrain suggested passing the resolved targets/secondaries down into `PolarisAuthorizerImpl.findMissingPrivileges()`, since the structure of targets and secondaries may change in the future. Capturing it here so it isn't missed — to be picked up right after #5486 merges, alongside the "let each `AuthorizationIntent` expose its own securables" refactor already noted above.


-- 
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: issues-unsubscribe@polaris.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

Re: [I] Deduplicate intent-to-resolved-path resolution shared by PolarisAuthorizerImpl and RangerPolarisAuthorizer [polaris]

["flyrain (via GitHub)" <gi...@apache.org>]

flyrain closed issue #5485: Deduplicate intent-to-resolved-path resolution shared by PolarisAuthorizerImpl and RangerPolarisAuthorizer
URL: https://github.com/apache/polaris/issues/5485


-- 
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: issues-unsubscribe@polaris.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org