EnxDev commented on code in PR #44017:
URL: https://github.com/apache/superset/pull/44017#discussion_r4121859970
##########
superset/subjects/utils.py:
##########
@@ -206,7 +206,42 @@ 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. The cache can miss a subject created earlier in
+ the same request (the create paths in ``commands/utils.py`` read back
through
+ here), but that never grants access to anyone else, in either direction. A
+ cache that misses a just-created subject: the subject is not yet listed in
+ any resource's editors or viewers, so an ``is_editor``/``is_viewer`` check
+ still returns the same answer. A cache that retains a subject removed
+ earlier in the same request: the only path that reads it back is
+ ``ensure_no_lockout`` in ``commands/utils.py``, where a stale membership
can
+ at most let the caller lock themselves out -- annoying, not a security
+ hole. Neither direction flips a decision for a third party, and the
+ staleness window is bounded by the request.
Review Comment:
This makes `ensure_no_lockout` sound like the only reader, but `is_editor`,
`is_viewer`, `tasks/utils.py:98` and the bootstrap in `views/base.py` all read
through here too.
What actually closes the removal direction is that membership only changes
in the Admin user, role and group endpoints or in login-time role sync, and
none of those requests read this back afterwards. Could the sentence say that
instead?
```suggestion
still returns the same answer. A cache that retains a subject removed
earlier in the same request: membership only changes in the Admin user,
role and group endpoints or in login-time role sync, and none of those
requests read this back afterwards. Neither direction flips a decision
for a third party, and the staleness window is bounded by the request.
```
--
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]
