From 0509247d928377c3e6e8eaac640178a1b3b03123 Mon Sep 17 00:00:00 2001 From: viv-4 Date: Thu, 12 Nov 2026 13:49:53 +1100 Subject: [PATCH] fix(user): never serialise password digests or SSO tokens 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 --- spec/user_spec.cr | 29 +++++++++++++++++++++++++++++ src/placeos-models/user.cr | 13 +++++++------ 2 files changed, 36 insertions(+), 6 deletions(-) diff --git a/spec/user_spec.cr b/spec/user_spec.cr index 8d7f5877..d5ee66dc 100644 --- a/spec/user_spec.cr +++ b/spec/user_spec.cr @@ -316,6 +316,7 @@ module PlaceOS::Model end views = { + "raw" => ->(u : User) { u.to_json }, "public" => ->(u : User) { u.to_public_json }, "public_struct" => ->(u : User) { u.to_public_struct.to_json }, "public_meta" => ->(u : User) { u.to_public_metadata_json }, @@ -351,6 +352,34 @@ module PlaceOS::Model end end + it "never serialises a plain text password set before saving" do + user = Generator.user + user.password = "uns4ved-p@ssw0rd" + + user.to_json.should_not contain("uns4ved-p@ssw0rd") + end + + # Changefeeds rebuild models from the database's JSON, so secrets must + # still be read even though they are never written + it "still reads secrets from trusted JSON" do + user = User.from_trusted_json({ + name: "Changefeed User", + email: "changefeed@example.com", + authority_id: "authority-12345", + password_digest: "$2a$11$digest", + access_token: "feed-access", + refresh_token: "feed-refresh", + expires_at: 1234_i64, + expires: true, + }.to_json) + + user.password_digest.should eq "$2a$11$digest" + user.access_token.should eq "feed-access" + user.refresh_token.should eq "feed-refresh" + user.expires_at.should eq 1234_i64 + user.expires.should be_true + end + {"public", "public_struct", "public_meta", "public_meta_st"}.each do |view| it "hides admin only fields from the #{view} view" do user = secret_user.call diff --git a/src/placeos-models/user.cr b/src/placeos-models/user.cr index fd12af64..3e02983c 100644 --- a/src/placeos-models/user.cr +++ b/src/placeos-models/user.cr @@ -56,19 +56,20 @@ module PlaceOS::Model attribute department : String?, sanitize: :text attribute preferred_language : String? - attribute password_digest : String?, mass_assignment: false + # Secrets are never serialised (they're still read, e.g. by changefeeds) + attribute password_digest : String?, mass_assignment: false, ignore_serialize: true attribute email_digest : String?, mass_assignment: false attribute card_number : String?, mass_assignment: false attribute deleted : Bool = false, mass_assignment: false attribute groups : Array(String) = [] of String, mass_assignment: false - attribute access_token : String?, mass_assignment: false - attribute refresh_token : String?, mass_assignment: false - attribute expires_at : Int64?, mass_assignment: false - attribute expires : Bool = false, mass_assignment: false + attribute access_token : String?, mass_assignment: false, ignore_serialize: true + attribute refresh_token : String?, mass_assignment: false, ignore_serialize: true + attribute expires_at : Int64?, mass_assignment: false, ignore_serialize: true + attribute expires : Bool = false, mass_assignment: false, ignore_serialize: true - attribute password : String? + attribute password : String?, ignore_serialize: true attribute sys_admin : Bool = false, mass_assignment: false attribute support : Bool = false, mass_assignment: false