From a60a4a98b41d07dd51ff69c981c783e4a85d5c48 Mon Sep 17 00:00:00 2001 From: Juan Castro Date: Thu, 3 Sep 2026 23:50:54 -0400 Subject: [PATCH] feat: delete a team seat for good, once it is disabled MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A disabled account persists indefinitely. Removing it is an explicit request, never a timer, and it is refused on a live account with `account_must_be_disabled_first` — which puts a reversible step in front of the only irreversible operation in the feature. The audit row is written before `cascadeDelete` runs. The FKs are ON DELETE SET NULL and the `_keep` columns carry the identifiers, so the record of what was done survives the account it names. No second billing emit here: `cascadeDelete` already captures the seat and fires `team.account.deleted` through UserAccountService, and emitting again would close the storage charge twice. Disabling closed the per-account charge; this closes the storage one, and it is the only thing that does. There is no restore window, and none was wanted: the reversible step already exists earlier at disable, a disabled account costs only the bytes it holds so nothing pressures a hasty delete, and a restore promise means retaining data the team explicitly asked to be rid of. Published in rate-limits-and-quotas.md alongside the team-deletion note, since the two are easy to confuse and only one of them frees a seat. Closes PUT-1732. --- .../team/TeamController.http.test.ts | 19 +++++++++ .../controllers/team/TeamController.ts | 17 ++++++++ src/backend/services/team/TeamEvents.test.ts | 39 +++++++++++++++++++ src/backend/services/team/TeamService.ts | 35 +++++++++++++++++ src/docs/src/rate-limits-and-quotas.md | 2 + 5 files changed, 112 insertions(+) diff --git a/src/backend/controllers/team/TeamController.http.test.ts b/src/backend/controllers/team/TeamController.http.test.ts index 2c4b061d9..0c1f4e84d 100644 --- a/src/backend/controllers/team/TeamController.http.test.ts +++ b/src/backend/controllers/team/TeamController.http.test.ts @@ -153,6 +153,25 @@ describe('team endpoints over HTTP', () => { expect(res.status).toBe(404); }); + it('deletes a seat over HTTP only once it is disabled', async () => { + const { team, memberUsername } = await makeTeam(); + const path = `/teams/${team.uid}/members/${memberUsername}`; + + const live = await call('DELETE', path, env.users.user.token); + expect(live.status).toBe(409); + expect(await live.json()).toMatchObject({ + code: 'account_must_be_disabled_first', + }); + + await call('POST', `${path}/disable`, env.users.user.token); + const gone = await call('DELETE', path, env.users.user.token); + + expect(gone.status).toBe(200); + expect( + await env.server.stores.user.getByUsername(memberUsername), + ).toBeFalsy(); + }); + it('lists members with org_owned distinguishing the owner', async () => { const { team, memberUsername } = await makeTeam(); diff --git a/src/backend/controllers/team/TeamController.ts b/src/backend/controllers/team/TeamController.ts index a54d0cdd7..f3269578a 100644 --- a/src/backend/controllers/team/TeamController.ts +++ b/src/backend/controllers/team/TeamController.ts @@ -308,6 +308,23 @@ export class TeamController extends PuterController { res.json({ success: true }); } + @Delete('/:uid/members/:username', { + subdomain: 'api', + requireUserActor: true, + requireVerified: true, + rateLimit: TEAM_LIMIT, + }) + async deleteMember(req: Request, res: Response): Promise { + const userId = this.#requireUserId(req); + const uid = this.#param(req, 'uid'); + // Authority first, or resolving `:username` is an existence oracle. + await this.services.team.requireOwner(uid, userId); + const target = await this.#requireTargetUserId(req); + + await this.services.team.deleteMember(uid, userId, target); + res.json({ success: true }); + } + // -- Audit -------------------------------------------------------- @Get('/:uid/audit', { diff --git a/src/backend/services/team/TeamEvents.test.ts b/src/backend/services/team/TeamEvents.test.ts index 84d39ec60..090d31243 100644 --- a/src/backend/services/team/TeamEvents.test.ts +++ b/src/backend/services/team/TeamEvents.test.ts @@ -280,6 +280,45 @@ describe('team billing events', () => { ).not.toContain(seat.userId); }); + it('refuses to delete a live account', async () => { + const team = await makeTeam(); + const seat = await provision(team); + + await expect( + service.deleteMember(team.uid, owner.id, seat.userId), + ).rejects.toMatchObject({ + statusCode: 409, + legacyCode: 'account_must_be_disabled_first', + }); + expect(await server.stores.user.getById(seat.userId)).toBeTruthy(); + }); + + it('deletes a disabled account and closes the storage charge once', async () => { + const team = await makeTeam(); + const seat = await provision(team); + await service.disableMember(team.uid, owner.id, seat.userId); + seen.length = 0; + + await service.deleteMember(team.uid, owner.id, seat.userId); + + expect(await server.stores.user.getById(seat.userId)).toBeFalsy(); + expect(of('team.account.deleted')).toHaveLength(1); + }); + + it('keeps the audit trail attributable after the account is gone', async () => { + const team = await makeTeam(); + const seat = await provision(team); + await service.disableMember(team.uid, owner.id, seat.userId); + await service.deleteMember(team.uid, owner.id, seat.userId); + + const rows = await server.stores.team.listAudit(team.id); + const mine = rows.items.filter( + (r) => Number(r.user_id_keep) === seat.userId, + ); + // The FKs are ON DELETE SET NULL, so `_keep` is the only thing left. + expect(mine.map((r) => r.action)).toContain('delete_account'); + }); + it('says nothing about an account no team pays for', async () => { const outsider = await makeUser(); seen.length = 0; diff --git a/src/backend/services/team/TeamService.ts b/src/backend/services/team/TeamService.ts index 0743fad38..3845d8fc0 100644 --- a/src/backend/services/team/TeamService.ts +++ b/src/backend/services/team/TeamService.ts @@ -78,6 +78,9 @@ const CAP_LOCK_TTL_SECONDS = 10; export const AUDIT_RESET_PASSWORD = 'reset_member_password'; export const AUDIT_ACTIVATE = 'activate'; +/** Written before the row it names goes; `_keep` is what preserves it. */ +export const AUDIT_DELETE_ACCOUNT = 'delete_account'; + /** Not an audit row: synthesised from `sessions` for the member's own view. */ export const SIGN_IN_ACTION = 'sign_in'; @@ -986,6 +989,38 @@ export class TeamService extends PuterService { }); } + /** Disable is the reversible step in front of the irreversible one. */ + async deleteMember( + teamUid: string, + actorUserId: number, + targetUserId: number, + ): Promise { + const team = await this.requireOwner(teamUid, actorUserId); + await this.requireOrgAccount(teamUid, targetUserId); + + // Forced, as `enableMember` is: a cached row predates the disable. + const user = await this.stores.user.getByProperty('id', targetUserId, { + force: true, + }); + if (!user?.suspended) { + throw new HttpError(409, 'Disable the account before deleting it', { + legacyCode: 'account_must_be_disabled_first', + }); + } + + // Written first; `_keep` is what makes it outlive the account. + await this.stores.team.appendAudit({ + teamId: team.id, + userId: targetUserId, + actorUserId, + action: AUDIT_DELETE_ACCOUNT, + }); + + // `cascadeDelete` emits `team.account.deleted` itself; a second emit + // here would close the storage charge twice. + await this.services.userAccount.cascadeDelete(targetUserId); + } + /** The three columns together; `suspended` is the one that gates requests. */ async #suspend(userId: number): Promise { await this.stores.user.update(userId, { diff --git a/src/docs/src/rate-limits-and-quotas.md b/src/docs/src/rate-limits-and-quotas.md index ce5e7a32f..3595dd1b8 100644 --- a/src/docs/src/rate-limits-and-quotas.md +++ b/src/docs/src/rate-limits-and-quotas.md @@ -200,6 +200,8 @@ A reset returns a temporary password once and never again. It stops working 24 h Deleting a team frees the owner's slot, but it does **not** free the seats: the accounts it created still exist, still hold their files, and keep their usernames. They are disabled, not removed — deleting a team is not a way to stop paying for the accounts in it. +Removing a seat for good is a separate, explicit request, and it is refused unless the account is already disabled (`account_must_be_disabled_first`). That ordering puts a reversible step in front of the only irreversible operation in the feature. **There is no restore window**: deletion removes the files, returns the username to the pool, and invalidates every credential. Nothing expires a disabled account on a timer — it persists, costing only the bytes it holds, until someone asks for it to go. + Lowering the seat limit never disables anyone. A team already above a reduced limit keeps every account it has and is simply refused new ones until it is back under. Both limits are per deployment (`max_teams_per_user`, `max_seats_per_team`) rather than per team, so raising them moves every team at once.