Repository navigation
fix(user): never serialise password digests or SSO tokens - #341
Merged
Merged
Conversation
A raw `User#to_json` included `password_digest`, `access_token`, `refresh_token`, `expires_at`, `expires` and any unsaved plain text `password`. Any API route that returns a raw `User` therefore leaked them; rest-api has had to fix this route by route (#461, group_users). Mark those attributes `ignore_serialize: true`. Only output is affected: - They are still read from JSON, so changefeeds (which rebuild models via `from_trusted_json`) keep receiving them. - History diffs and snapshots are built from `attributes_json` / `changes_json`, which don't use `JSON::Field`, so password changes are still recorded as `[REDACTED]`. - Persistence goes through `DB::Field` and is unchanged. Specs add the raw `to_json` view to the sensitive field serialisation checks, plus checks for an unsaved plain text password and that secrets are still read from trusted JSON. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
User#to_jsonincluded these fields:password_digestaccess_token/refresh_token(stored SSO tokens)expires_at/expirespassword, if it was set but not yet saved#340 made sure the
to_public_*,to_admin_*andto_group_*views neverinclude these fields, but plain
to_jsonstill did. So any API route thatreturns a raw
Userexposed them. rest-api has been closing these routesone at a time:
/usersin PlaceOS/rest-api#461, and/group_usersin afollow-up.
Fix
Those six attributes now have
ignore_serialize: true. Theattributemacro passes the option through to
JSON::FieldandYAML::Field, so itonly affects output:
to_json(and any futureto_yaml)from_json/from_trusted_jsonrow_to_json)attributes_json/changes_json)[REDACTED]DB::Field)Tests
In the existing
sensitive field serialisationblock ofspec/user_spec.cr:rawview (user.to_json). It goes through the same checks asthe other views: no secret keys, and no digest or token values. It
failed before the fix, leaking 5 keys.
to_json.After merge
placeos-modelsin rest-api (and any otherservice that renders users).
OPENAPI_DOC.yml, since thePlaceOS.Model.Userschema loses these fields.