fix(api): unsuspend only restores suspension-disabled users (#917 L-5)#1182
Merged
Conversation
Partner unsuspend did `UPDATE users SET status='active' WHERE partner_id=? AND status='disabled'` and re-enabled EVERY disabled user under the partner — including users disabled for compromise, off-boarding, or a manual admin action. A suspend/unsuspend cycle would silently resurrect accounts an operator had deliberately locked. Fix: record WHY a user is disabled. - New nullable `users.disabled_reason` column (migration adds it + backfills users currently disabled under a still-suspended partner as 'partner_suspended', so in-flight suspensions still unsuspend correctly). - Suspend stamps `disabled_reason='partner_suspended'` on the users it disables, and now skips users already disabled so it never re-stamps an other-reason disable. Session/JWT/OAuth revocation still covers ALL partner users via the separate affectedUserIds set, so the security posture is unchanged. - Unsuspend re-enables only `status='disabled' AND disabled_reason= 'partner_suspended'`, clearing the marker; users disabled for any other reason stay disabled. The disable/re-enable queries are extracted into two small exported helpers so the handler and an integration test share the exact SQL. The integration test (real Postgres) proves the row-filtering semantics: a user disabled for another reason survives a suspend (not re-stamped) and is left disabled after unsuspend. Tests: new partnerUnsuspendScope.integration.test.ts (2); abuse unit suite (26) and autoMigrate ordering guard (43) green; typecheck + lint clean. Addresses L-5 of #917 (other sub-items tracked separately). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Deploying breeze with
|
| Latest commit: |
ad08aa7
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://0ba152f7.breeze-9te.pages.dev |
| Branch Preview URL: | https://fix-917-l5-unsuspend-scope.breeze-9te.pages.dev |
…er on manual status change - Suspend now disables only `status='active'` users (was `<> 'disabled'`). An invited (unaccepted) user is left invited rather than disabled+stamped — the partner-suspension gate already blocks them, and stamping them made unsuspend silently promote an unaccepted invite into a full 'active' account. The marker is now only ever on users whose correct restore status is 'active'. - Admin PATCH /users/:id clears disabled_reason on any status change, so a manual disable reads as "other reason" (never swept by unsuspend) and reactivation leaves no stale marker. - Integration test now also asserts invited and platform-admin users are untouched by suspend, and that an invited user stays invited across a full suspend/unsuspend cycle. - Migration documents the backfill's known over-tag limitation for partners suspended via the non-abuse orgs.ts path (tiny blast radius, never worse than pre-fix). Tests: integration (3) + abuse/users unit (74) green; typecheck + lint clean. Co-Authored-By: Claude Opus 4.8 (1M context) <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
1 participant
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.
Summary
Addresses L-5 of the #917 hardening cluster. Partner unsuspend (
apps/api/src/routes/admin/abuse.ts) did:— re-enabling every disabled user under the partner, regardless of why they were disabled. So a suspend → unsuspend cycle would silently resurrect users an operator had deliberately disabled for compromise, off-boarding, or a manual admin action.
Fix — record why a user is disabled
2026-06-09-users-disabled-reason.sql: adds a nullableusers.disabled_reason(NULL = "disabled for some non-suspension reason") and backfills users currently disabled under a still-suspendedpartner as'partner_suspended', so in-flight suspensions still unsuspend correctly. Idempotent; adding a column to the already-RLS-forceduserstable needs no policy change.disabled_reason='partner_suspended'on the users it disables, and now skips users alreadydisabledso it never re-stamps an other-reason disable. Session/JWT/OAuth/remote-session revocation still covers all partner users (via the separateaffectedUserIdsset), so the suspension's security posture is unchanged.status='disabled' AND disabled_reason='partner_suspended', clearing the marker. Users disabled for any other reason stay disabled.The two user-state queries are extracted into small exported helpers (
disablePartnerUsersForSuspension,reEnableSuspensionDisabledUsers) so the handler and the integration test share the exact SQL.Tests
partnerUnsuspendScope.integration.test.ts(real Postgres) proves the row-filtering semantics end-to-end: a user disabled for another reason is not re-stamped by suspend and is left disabled after unsuspend, while suspension-disabled users are restored.autoMigrateordering guard (43) green;tsc+ lint clean.Addresses L-5 of #917 (other sub-items tracked separately).
🤖 Generated with Claude Code