Server settings → People → Plugins: the account lookup runs before the staff check, so the status code says whether an account exists #373

Closed
opened 2026-10-01 10:46:13 +00:00 by tiagoagueda · 0 comments
Owner

Found on 2026-09-29 in a read-only review of postulo at 95348908d, reading the Server settings views and checking each person view's order of lookup and permission check with the test client.

Checked again on 2026-10-01 against main at 0a5cad587: still present. The code is now at core/server_views.py:1189-1191 (PersonPluginsView.dispatch); core/mixins.py:74 has not moved, and the test client gives the same answers.

PersonPluginsView looks the account up in dispatch before it calls super().dispatch, and super().dispatch is where StaffRequiredMixin (LoginRequiredMixin plus UserPassesTestMixin) runs its checks. The lookup therefore answers visitors who are not signed in and people who are not administrators: a missing account gets 404, an existing one the sign-in redirect or 403. Every other person view looks the account up in get or post, after the checks, and answers the same whichever account number is asked for.

  • Where: core/server_views.py:1188-1190, PersonPluginsView.dispatch; the checks it runs ahead of are StaffRequiredMixin in core/mixins.py:74.
  • What happens: a GET of /server/people/<pk>/plugins/ for an account that exists answers 302 to sign-in when nobody is signed in, and 403 to a signed-in person who is not an administrator. For an account number that does not exist it answers 404 to both. server:person_username, by comparison, answers 302 to an anonymous visitor either way. Checked with the test client against the snapshot: anonymous 302 for an existing account and 404 for a missing one; non-administrator 403 and 404.
  • Who can trigger it: anybody who can reach the instance, with no account.
  • Why it matters: account keys are sequential integers, so walking the range tells an outsider how many accounts the instance holds and which have been deleted. That is little on its own, but it is the difference in answers the project's rule against confirming records by status code exists to prevent, and this is the one person view where the order is reversed.
  • Fix: look the person up only after the checks: remove the dispatch override and fetch the person in get_context_data and post (through a small _person() helper), as PersonDeleteView and PersonRecoveryView do.

Done when

  • A test in tests/test_server_settings.py asks for server:person_plugins with an existing and a missing account number, anonymously and as a non-administrator, and gets the same status code for both in each case (302, then 403); it fails today with 404 for the missing number.
Found on 2026-09-29 in a read-only review of postulo at 95348908d, reading the Server settings views and checking each person view's order of lookup and permission check with the test client. Checked again on 2026-10-01 against `main` at `0a5cad587`: still present. The code is now at `core/server_views.py:1189-1191` (`PersonPluginsView.dispatch`); `core/mixins.py:74` has not moved, and the test client gives the same answers. `PersonPluginsView` looks the account up in `dispatch` before it calls `super().dispatch`, and `super().dispatch` is where `StaffRequiredMixin` (`LoginRequiredMixin` plus `UserPassesTestMixin`) runs its checks. The lookup therefore answers visitors who are not signed in and people who are not administrators: a missing account gets 404, an existing one the sign-in redirect or 403. Every other person view looks the account up in `get` or `post`, after the checks, and answers the same whichever account number is asked for. - **Where:** `core/server_views.py:1188-1190`, `PersonPluginsView.dispatch`; the checks it runs ahead of are `StaffRequiredMixin` in `core/mixins.py:74`. - **What happens:** a GET of `/server/people/<pk>/plugins/` for an account that exists answers 302 to sign-in when nobody is signed in, and 403 to a signed-in person who is not an administrator. For an account number that does not exist it answers 404 to both. `server:person_username`, by comparison, answers 302 to an anonymous visitor either way. Checked with the test client against the snapshot: anonymous 302 for an existing account and 404 for a missing one; non-administrator 403 and 404. - **Who can trigger it:** anybody who can reach the instance, with no account. - **Why it matters:** account keys are sequential integers, so walking the range tells an outsider how many accounts the instance holds and which have been deleted. That is little on its own, but it is the difference in answers the project's rule against confirming records by status code exists to prevent, and this is the one person view where the order is reversed. - **Fix:** look the person up only after the checks: remove the `dispatch` override and fetch the person in `get_context_data` and `post` (through a small `_person()` helper), as `PersonDeleteView` and `PersonRecoveryView` do. ## Done when - A test in `tests/test_server_settings.py` asks for `server:person_plugins` with an existing and a missing account number, anonymously and as a non-administrator, and gets the same status code for both in each case (302, then 403); it fails today with 404 for the missing number.
tiagoagueda added this to the 0.5.0 milestone 2026-10-01 10:46:13 +00:00
Sign in to join this conversation.
No milestone
No project
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set

Reference
Postulo/postulo#373
No description provided.