Skip to content

fix(group_users): don't return sensitive fields - #462

Merged
stakach merged 2 commits into
masterfrom
fix/group-users-sensitive-fields
Nov 12, 2026
Merged

stakach merged 2 commits into
masterfrom
fix/group-users-sensitive-fields

Conversation

@viv-4

@viv-4 viv-4 commented Nov 12, 2026

Copy link
Copy Markdown
Contributor

Problem

GET /api/engine/v2/group_users/ adds the group and the user to each row, so the
frontend can show a member list without fetching each user separately.
The user it adds was the raw Model::User, and that object's JSON includes:

  • password_digest
  • access_token / refresh_token (the stored SSO tokens)
  • expires_at / expires

This isn't limited to admins. Any group manager sees these fields for every
member of the groups they manage.

#461 fixed the same leak in the users controller but didn't touch this one.

Fix

The embedded user is now User::PublicResponse (to_public_struct): the same
allow-listed public view that /users gives non-admins. It's the public view
for every caller, admins included, so these rows also no longer include
admin-only fields (sys_admin, support, misc, login_name, card_number,
ui_theme).

Breaking change for clients

The user object in group_users index rows now contains only the
PUBLIC_DATA fields plus id. Any client that read other fields from it
will stop getting them. Users can still get their own full profile from
GET /users/current.

Tests

New sensitive fields specs in spec/controllers/groups/user_spec.cr run as
both a group manager and a sys_admin. Each one adds a member that has a password
and SSO tokens, lists the group, and checks that:

  • the embedded user has no secret keys and no admin-only keys
  • the response body doesn't contain the digest or token values anywhere

Both new specs failed before the fix and pass with it. The 4 existing
group_users specs still pass.

OpenAPI

OPENAPI_DOC.yml is regenerated. GroupUser.user now points to
PlaceOS.Model.User.PublicResponse.

It also includes the /users response schema changes from #461, which never
regenerated the doc: create, update and the search endpoints now return the
response-struct union instead of PlaceOS.Model.User.

The group_users index hydrates each row with its user so the frontend
can render a member list without an N+1 fetch. It embedded the raw
`Model::User`, whose default JSON includes `password_digest`,
`access_token`, `refresh_token`, `expires_at` and `expires`. Any group
manager (not only admins) could read them for every member.

Embed `User::PublicResponse` (`to_public_struct`) instead, matching
what #461 did for the users controller.

OPENAPI_DOC.yml is regenerated: `GroupUser.user` now references
`PlaceOS.Model.User.PublicResponse`, and it also picks up the /users
response schema changes from #461, which were never regenerated.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@viv-4
viv-4 requested a review from stakach November 12, 2026 02:17
@github-actions github-actions Bot added the type: bug something isn't working label Nov 12, 2026
- chatgpt: show, destroy and websocket resume only resolve the caller's
  own chat; anyone else's is a 404 (resume is checked before the upgrade)
- oauth_apps: index returns a secret-free view to non-admins
- flux: refuse guest tokens, any signed in user can still query
- emails: index and show are support / admin only
- api_keys: embed the owner as User::AdminResponse instead of the raw
  model, which serialised the password digest and SSO tokens

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@github-actions github-actions Bot added type: bug something isn't working and removed type: bug something isn't working labels Nov 12, 2026
@stakach
stakach merged commit f0bad50 into master Nov 12, 2026
11 checks passed
@stakach
stakach deleted the fix/group-users-sensitive-fields branch November 12, 2026 05:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

type: bug something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants