roryqi commented on PR #12231: URL: https://github.com/apache/gravitino/pull/12231#issuecomment-5238012276
One more correctness concern about the partial-load semantics: This PR intentionally keeps resolved policies in the enforcer even when the role is incomplete, and only avoids recording the role in `loadedRoles`. That preserves availability for resolved privileges, but it may weaken DENY semantics. Example: ```text Role A grants ALLOW SELECT on table1. Role B grants DENY SELECT on table1. User has both roles. During Role B load, table1 cannot be resolved, so the DENY row is not added to denyEnforcer. Role B is not marked loaded, but partialRoleLoadBackoff suppresses immediate retry. A later SELECT table1 request resolves table1 successfully. allowEnforcer has Role A allow, denyEnforcer is missing Role B deny. Result may become allowed, although DENY should have won. ``` So the current partial-load behavior is not strictly fail-closed. Missing ALLOW rows usually cause false denies, but missing DENY rows can cause false allows. Can we clarify the intended invariant here? If DENY must remain conservative, I think an incomplete role should probably not apply any ALLOW rows until the role is fully resolved, or partial loads should otherwise fail closed for the whole role. -- 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]
