dennismdejong opened a new pull request, #13715:
URL: https://github.com/apache/gravitino/pull/13715

   …cks of one request
   
   A request loads its JCasbin role policies once 
(AuthorizationRequestContext#loadRole) and only takes the read lock around each 
single enforce(). A loaded-role TTL or size eviction, or a privilege change on 
the role, can clear the role's p rows between two checks while the user's g row 
stays. Later checks of that request then evaluate without the role: they deny a 
privilege the role grants, or miss a deny the role carries.
   
   Keep every 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. A reload that cannot read a 
role's entity (other than because the role was dropped) fails the check closed, 
so missing rows are never taken as "no deny".
   - The clear-generation map is capped at the role cache size; pruning raises 
a watermark, and requests older than it reload once, so pruning never hides a 
clear.
   
   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.
   
   <!--
   1. Title: [#<issue>] <type>(<scope>): <subject>
      Examples:
        - "[#123] feat(operator): Support xxx"
        - "[#233] fix: Check null before access result in xxx"
        - "[MINOR] refactor: Fix typo in variable name"
        - "[MINOR] docs: Fix typo in README"
        - "[#255] test: Fix flaky test NameOfTheTest"
      Reference: https://www.conventionalcommits.org/en/v1.0.0/
   2. If the PR is unfinished, please mark this PR as draft.
   -->
   
   ### What changes were proposed in this pull request?
   
   (Please outline the changes and how this PR fixes the issue.)
   
   ### Why are the changes needed?
   
   (Please clarify why the changes are needed. For instance,
     1. If you propose a new API, clarify the use case for a new API.
     2. If you fix a bug, describe the bug.)
   
   Fix: #(issue)
   
   ### Does this PR introduce _any_ user-facing change?
   
   (Please list the user-facing changes introduced by your change, including
     1. Change in user-facing APIs.
     2. Addition or removal of property keys.)
   
   ### How was this patch tested?
   
   (Please test your changes, and provide instructions on how to test it:
     1. If you add a feature or fix a bug, add a test to cover your changes.
     2. If you fix a flaky test, repeat it for many times to prove it works.)
   


-- 
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]

Reply via email to