EnxDev commented on code in PR #44017:
URL: https://github.com/apache/superset/pull/44017#discussion_r4079619319
##########
superset/subjects/utils.py:
##########
@@ -206,7 +206,33 @@ def get_user_subject_ids(user_id: int) -> list[int]:
1. The user's own USER-type subject
2. ROLE-type subjects for all direct and group-derived roles the user has
3. GROUP-type subjects for all groups the user belongs to
+
+ Memoised for the duration of the request, keyed by user id. Authorization
+ calls this once per object checked -- ``is_editor``/``is_viewer`` run it
for
+ every chart on a dashboard. Nothing reads a user's subjects after changing
+ them within a single request, so the cached set cannot go stale in place.
Review Comment:
`commands/utils.py:127` creates the user's USER subject (`get_user_subject`
there is `get_or_create_user_subject`, aliased at line 40) and line 133 reads
back through this function, so "nothing reads a user's subjects after changing
them" isn't quite the rule. The `get_user_group_subjects` backfill does the
same on the create paths.
It still holds up, just for a different reason: a subject created
mid-request isn't referenced by any resource's editors or viewers yet, so a
cache that's missing it can't flip `is_editor`. Could we write that reason down
instead? As worded it reads like something a future create-then-check path can
lean on.
##########
tests/unit_tests/subjects/test_utils.py:
##########
@@ -611,3 +612,63 @@ def test_compute_subjects_all_variants(mock_compute):
ensure_no_lockout=True,
field_name="editors",
)
+
+
+def test_get_user_subject_ids_memoises_within_a_request(app) -> None:
Review Comment:
Following on from the other note, none of these pin the create-then-read
case, which is the one carrying the security argument.
Something like: patch the query to return `[7]`, read it, switch the patch
to `[7, 99]`, and assert the same request still sees `[7]`. That makes the
staleness window a deliberate contract instead of a docstring claim.
--
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
