SEC2-003: An administrator can join any group unnoticed: members cannot see who is in their group, and the log does not name the change #945

New issue

Closed

opened 2026-10-17 10:37:46 +00:00 by tiagoagueda · 0 comments

tiagoagueda commented

2026-10-17 10:37:46 +00:00

Owner

Found on 2026-10-16 in a read-only security review of postulo at 56fc9d531, checked by a second, independent pass.

Severity: Low · Category: Authentication/Authorization · Reference: CWE-778 (Insufficient Logging); CWE-306 (Missing Authentication for Critical Function); OWASP A09:2021 (Security Logging and Monitoring Failures)

Location:

src/postulo/core/views_access.py:84-103 (GroupsView.post, action save)
src/postulo/accounts/views.py:670-693 (YourGroupsView)
src/postulo/templates/accounts/groups.html:22-25 (the member count)

Issue

Group membership is the authorisation primitive of #896: every member of a group may see, change, share and delete what the group owns and everything shared with the group (core/sharing.py:59-71). Server settings → Groups lets any staff account rewrite any group's member list, themselves included:

            chosen = [int(pk) for pk in request.POST.getlist("members") if pk.isdigit()]
            ...
                    group.user_set.set(get_user_model().objects.filter(pk__in=chosen))
            ...
            logger.warning("Group %r set by %s", group.name, request.user.username)

Three things make this quieter than the threat model's promise that a grant is "explicitly, for a stated scope, revocably, and visibly to them at any time":

the log line names the group and the actor but not who was added or removed, so an administrator who joins a group, reads or edits its records, and leaves again leaves two identical-looking lines;
Your groups shows members only as a count (members = group.user_set.count(), drawn as "N members"), so the people whose records are reached have no page on which the new member appears;
the write needs no fresh authentication (compare #740 and PersonAdminView), although it confers read/write/manage over records.

The threat model does say an administrator is trusted with the instance, and Server settings → People already shows every account; this is about the trace rather than the capability.

Impact

A staff administrator who is not the operator, or whoever holds such a session, can reach every group-owned company, contact and listing (and what is shared with the group) without the group's members being able to notice from their own pages, and with no log entry that says so afterwards. Notes, applications and documents stay out of reach, as promised.

Fix

Log the delta: compute before = set(group.user_set.values_list("pk", flat=True)) and write the added/removed usernames into the warning line (usernames, never addresses).
On Your groups, list the members' usernames (the username is what "others on a shared instance see" per User's docstring), so a new member is visible to every other one.
Decorate GroupsView.post with reauthentication_required, for consistency with PersonAdminView.
Optionally record membership changes in the record trail or notify members (notifications.service.notify) when someone joins.
Done when a test posts a membership change and asserts the log line names the added and removed usernames, and a unit or browser test asserts accounts:groups lists the members of the viewer's group.

#896 (closed) - introduced the page and promises visibility of grants; member visibility and the audit line are not discussed. #740 (closed) - re-authentication of administrative actions; groups did not exist then. #95 (closed) - "an administrator can decide a plugin for a person — and must not be able to decide it quietly" is the same principle applied to plugin policy, which is shown on the person's own settings page; group membership has no such mirror. #268 (open) - background: its Part 3 says "An administrator who could grant themselves access is the whole promise gone", which #896's design and the updated threat model superseded for companies, contacts and listings.

Found on 2026-10-16 in a read-only security review of `postulo` at `56fc9d531`, checked by a second, independent pass. **Severity:** Low · **Category:** Authentication/Authorization · **Reference:** CWE-778 (Insufficient Logging); CWE-306 (Missing Authentication for Critical Function); OWASP A09:2021 (Security Logging and Monitoring Failures) **Location:** - `src/postulo/core/views_access.py:84-103` (`GroupsView.post`, action `save`) - `src/postulo/accounts/views.py:670-693` (`YourGroupsView`) - `src/postulo/templates/accounts/groups.html:22-25` (the member count) ## Issue Group membership is the authorisation primitive of #896: every member of a group may see, change, share and delete what the group owns and everything shared with the group (`core/sharing.py:59-71`). *Server settings → Groups* lets any staff account rewrite any group's member list, themselves included: ```python chosen = [int(pk) for pk in request.POST.getlist("members") if pk.isdigit()] ... group.user_set.set(get_user_model().objects.filter(pk__in=chosen)) ... logger.warning("Group %r set by %s", group.name, request.user.username) ``` Three things make this quieter than the threat model's promise that a grant is "explicitly, for a stated scope, revocably, and visibly to them at any time": - the log line names the group and the actor but not who was added or removed, so an administrator who joins a group, reads or edits its records, and leaves again leaves two identical-looking lines; - *Your groups* shows members only as a count (`members = group.user_set.count()`, drawn as "N members"), so the people whose records are reached have no page on which the new member appears; - the write needs no fresh authentication (compare #740 and `PersonAdminView`), although it confers read/write/manage over records. The threat model does say an administrator is trusted with the instance, and *Server settings → People* already shows every account; this is about the trace rather than the capability. ## Impact A staff administrator who is not the operator, or whoever holds such a session, can reach every group-owned company, contact and listing (and what is shared with the group) without the group's members being able to notice from their own pages, and with no log entry that says so afterwards. Notes, applications and documents stay out of reach, as promised. ## Fix 1. Log the delta: compute `before = set(group.user_set.values_list("pk", flat=True))` and write the `added`/`removed` usernames into the warning line (usernames, never addresses). 2. On *Your groups*, list the members' usernames (the username is what "others on a shared instance see" per `User`'s docstring), so a new member is visible to every other one. 3. Decorate `GroupsView.post` with `reauthentication_required`, for consistency with `PersonAdminView`. 4. Optionally record membership changes in the record trail or notify members (`notifications.service.notify`) when someone joins. Done when a test posts a membership change and asserts the log line names the added and removed usernames, and a unit or browser test asserts `accounts:groups` lists the members of the viewer's group. ## Related #896 (closed) - introduced the page and promises visibility of grants; member visibility and the audit line are not discussed. #740 (closed) - re-authentication of administrative actions; groups did not exist then. #95 (closed) - "an administrator can decide a plugin for a person — and must not be able to decide it quietly" is the same principle applied to plugin policy, which is shown on the person's own settings page; group membership has no such mirror. #268 (open) - background: its Part 3 says "An administrator who could grant themselves access is the whole promise gone", which #896's design and the updated threat model superseded for companies, contacts and listings.

tiagoagueda added the

bug

security

labels

2026-10-17 10:37:47 +00:00

tiagoagueda added this to the 0.6.0 milestone

2026-10-17 10:37:48 +00:00

tiagoagueda referenced this issue

2026-10-17 10:41:22 +00:00

SEC2 index: security review of 2026-10-16 (86 issues across the core, plugins and extensions) #992

tiagoagueda referenced this issue from a commit

2026-10-17 15:33:37 +00:00