Route table-like authorization through authorize(state, request) - #5170
Open
ZephyrYWZhou wants to merge 1 commit into
Open
Route table-like authorization through authorize(state, request)#5170ZephyrYWZhou wants to merge 1 commit into
ZephyrYWZhou wants to merge 1 commit into
Conversation
Migrate authorizeResolvedBasicTableLikeOperationOrThrow in CatalogHandler to obtain its authorization decision from the decision-native PolarisAuthorizer.authorize(AuthorizationState, AuthorizationRequest) path (via the throwing authorizeOrThrow(state, request) convenience wrapper) instead of the legacy resolved-path authorizeOrThrow overload. This is a small, behavior-preserving preparatory step toward migrating all callers off the legacy authorizeOrThrow overloads so those overloads can be removed. The resolution manifest is already populated by resolveBasicTableLikeTargetOrThrow, so the authorizer re-resolves the same table-like securable from the request intent. The existing null-check guard is retained so a missing entity still surfaces as a not-found (404) response rather than a server error from the authorizer's re-resolution. All table-like operations that flow through this method (loadTable, loadCredentials, loadView, dropTable, etc.) still throw ForbiddenException on denial, so behavior is unchanged. Update IcebergCatalogHandlerTest#loadCredentialsFallbackResolvesOnceThenAuthorizesReadDelegation to stub/verify the decision-native authorizeOrThrow(state, request) path (matching the request intent's operation) instead of the legacy overload.
Author
|
Tagging @dimas-b for review on this PR as the first preparatory PR of the migration efforts. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
This is the first preparatory PR to migrate
authorizeOrThrowtoauthorize.This PR migrates
CatalogHandler.authorizeResolvedBasicTableLikeOperationOrThrowto obtain its authorization decision from the decision-nativePolarisAuthorizer.authorize(AuthorizationState, AuthorizationRequest)path (via the throwingauthorizeOrThrow(state, request)convenience wrapper) instead of the legacy resolved-pathauthorizeOrThrow(principal, activatedEntities, op, target, secondary)overload.Why this method first
authorizeResolvedBasicTableLikeOperationOrThrowis the most widely used table-like authorization method —loadTable,loadCredentials,loadView,dropTable,commitView, and others all flow through it (directly or viaresolveAndAuthorizeBasicTableLikeOperationOrThrow).The resolution manifest is already populated by
resolveBasicTableLikeTargetOrThrowbefore this method runs, so the authorizer re-resolves the same table-like securable from the request intent with no manifest changes required. This makes it the safest, highest-leverage starting point.Changes
CatalogHandler.java—authorizeResolvedBasicTableLikeOperationOrThrownow builds anAuthorizationRequest(singleSingleTargetAuthorizationIntent) and callsauthorizer().authorizeOrThrow(state, request). The existing null-check guard is retained so a missing entity still surfaces as a not-found (404) response rather than a server error from the authorizer's re-resolution.IcebergCatalogHandlerTest.java— updatedloadCredentialsFallbackResolvesOnceThenAuthorizesReadDelegationto stub/verify the decision-nativeauthorizeOrThrow(state, request)path (matched by the request intent's operation) instead of the legacy overload; added a smallrequestWithOperation(...)argument-matcher helper.Behavior
No behavioral change. Every table-like operation that flows through this method still throws
ForbiddenExceptionon denial (the wrapper delegates toauthorize(...)and throws when not allowed), andloadTable/loadCredentialskeep their existing write-delegation-probe → read-delegation-fallback control flow.Testing
IcebergCatalogHandlerTest(mock-based) passes.PolarisAuthorizerImplauthz suites that exercise allow/deny through this method all pass with no changes required:IcebergCatalogHandlerAuthzTest(6921),IcebergCatalogHandlerNoSqlAuthzTest(6921),IcebergCatalogHandlerFineGrainedDisabledTest(107),PolarisGenericTableCatalogHandlerAuthzTest(538),PolicyCatalogHandlerAuthzTest(1518) — 0 failures.Checklist
IcebergCatalogHandlerauthorize()path