yuqi1129 opened a new pull request, #13606: URL: https://github.com/apache/gravitino/pull/13606
### What changes were proposed in this pull request? Keep every JCasbin check of a request at least as complete as the role policies the request loaded: - `JcasbinAuthorizer` records a per-role clear generation whenever a role's policies are cleared without being replaced in the same critical section (loaded-role eviction, `handleRolePrivilegeChange`). Clears that remove nothing, and the clear inside `replaceRolePolicies`, are not recorded. - The request records the generation before loading its roles and the ids of its bound roles (`AuthorizationRequestContext`). - Before each `enforce()` (allow, deny, narrowed) and the `hasDenyPolicy` scan, the check verifies under the read lock that no bound role was cleared since. If one was, it reloads the cleared roles outside the lock via `versionCheckAndLoadRoles`, then checks again. - After 3 reloads, or if a reload fails, the check fails closed: allow returns `false`, deny and `hasDenyPolicy` return `true`. Earlier checks stay valid because they ran against complete policies, so there is no whole-expression retry, no new context, and no lock held across DB lookups. Every entry point (REST, list filtering, Iceberg, Lance) is covered without changes. ### Why are the changes needed? A loaded-role eviction or privilege change between two checks of one request clears the role's `p` rows while its `g` row stays. The request never reloads the role, so later checks deny granted privileges or miss deny policies. Fix: #13605 ### Does this PR introduce _any_ user-facing change? No. Requests that race with an eviction now see the role's policies. A WARN is logged if a check fails closed. ### How was this patch tested? New tests in `TestJcasbinAuthorizer` (all except the empty-role guard fail before this change): - allow policies evicted, or the role privilege changed, between two checks → later check allowed - deny policies evicted between checks → deny still applies; `hasDenyPolicy` still true - narrowed (active roles) check after eviction - eviction during the request's own role load - unrelated role eviction → no reload; empty role → no reload - role deleted mid-request → denied without looping - repeated evictions → allow and deny fail closed - evictor thread racing 1000 composite decisions → no wrong deny, no wrong allow Plus `TestAuthorizationRequestContext` for the new fields. `./gradlew :server-common:test :core:test --tests 'org.apache.gravitino.authorization.*' :server:test --tests '*uthoriz*' -PskipITs` -- 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]
