dennismdejong opened a new pull request, #13707: URL: https://github.com/apache/gravitino/pull/13707
### What changes were proposed in this pull request? `JcasbinAuthorizer` now tracks which roles currently hold incomplete policies in a new `incompleteRolePolicyIds` set, and `hasDenyPolicy` reports a deny for any user bound to such a role. ### Why are the changes needed? `hasDenyPolicy` answers an existence question — "can a DENY hide one of the listed objects?" — by scanning the deny enforcer's rows for the user's roles. A role load that cannot resolve a securable object installs only the rows it could resolve, so the missing deny row makes the scan return `false`, and `MetadataAuthzHelper` may then take the parent-scope short-circuit and skip the per-object checks that would have caught the deny. `partialRoleLoadBackoff` was the only available evidence of an incomplete load, and it cannot carry that answer: it is a retry throttle whose expiry is unrelated to the installed rows, and it is not even re-armed when the retry cannot read the role entity. The new marker is added and removed only together with the policy rows it describes, so the two lifetimes always coincide. Deliberately conservative: the marker is role-scoped, not privilege-scoped, so a role that stays unresolvable (e.g. it still references a dropped table) keeps the list short-circuit disabled for its holders. The per-object authorization path is unchanged. Fix: #13691 ### Does this PR introduce _any_ user-facing change? Yes, in the safe direction: a partially loaded role no longer lets list authorization skip the per-object checks, so objects that a not-yet-resolved DENY is meant to hide stay hidden. List calls from users holding such a role fall back to per-object authorization and may be slower. ### How was this patch tested? Added regression tests in `TestJcasbinAuthorizer`; all four fail without the fix: - `testHasDenyPolicyReportsMissingDenyOfIncompleteRoleLoad`: initial partial load; asserts the deny row is genuinely absent from the deny enforcer; then expires the retry throttle and makes the retry unreadable, so nothing re-arms the throttle - the deny must still be reported. - `testHasDenyPolicyRecoversAfterCompleteRoleLoad`: recovery after a complete load (marker cleared, answer goes back to `false` for an ALLOW-only role), and a partial reload over a complete load re-arms the marker while the resolvable rows stay installed. - `testCompleteResultAfterPartialLoadClearsIncompleteMarker`: complete-after-partial replacement, plus explicit `handleRolePrivilegeChange` invalidation clearing the marker. - `testPartialResultCannotOverwriteCompletedConcurrentLoad` (extended): a late partial result must not mark a role that a concurrent complete load already finished. `./gradlew :server-common:spotlessCheck :server-common:test --tests "org.apache.gravitino.server.authorization.jcasbin.TestJcasbinAuthorizer"` passes (68 tests, 0 failures). -- 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]
