From 230241d6d289617d6012a652ffb85de120d545ed Mon Sep 17 00:00:00 2001 From: Juan Castro Date: Tue, 15 Sep 2026 13:22:13 -0400 Subject: [PATCH] fix: let an owner retire the seats of a deleted team, and only those MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Deleting a team suspends every provisioned seat, but every member route resolved live teams only — so the suspended seats could never be deleted afterwards, stranding the accounts and, on the billing side, their paused subscriptions. deleteMember now resolves the team soft-deleted or not, matching the audit reader. It also gains the guard enableMember always had: only the team's own suspension qualifies. A platform-suspended seat could previously be cascade-deleted by the owner, destroying an account Puter had frozen. --- .../controllers/team/TeamController.ts | 4 +-- src/backend/services/team/TeamEvents.test.ts | 31 +++++++++++++++++++ src/backend/services/team/TeamService.ts | 28 ++++++++++++----- src/backend/stores/team/TeamStore.ts | 16 ++++++++++ 4 files changed, 69 insertions(+), 10 deletions(-) diff --git a/src/backend/controllers/team/TeamController.ts b/src/backend/controllers/team/TeamController.ts index af54dad94..2ae90e7c1 100644 --- a/src/backend/controllers/team/TeamController.ts +++ b/src/backend/controllers/team/TeamController.ts @@ -353,8 +353,8 @@ export class TeamController extends PuterController { 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); + // Authority first (anti-oracle); deleted team included, see the service. + await this.services.team.requireOwnedTeam(uid, userId); const target = await this.#requireTargetUserId(req); await this.services.team.deleteMember(uid, userId, target); diff --git a/src/backend/services/team/TeamEvents.test.ts b/src/backend/services/team/TeamEvents.test.ts index c8629e1bd..7358c8581 100644 --- a/src/backend/services/team/TeamEvents.test.ts +++ b/src/backend/services/team/TeamEvents.test.ts @@ -319,6 +319,37 @@ describe('team billing events', () => { expect(mine.map((r) => r.action)).toContain('delete_account'); }); + it('deletes a suspended seat after its team is deleted', async () => { + const team = await makeTeam(); + const seat = await provision(team); + await service.deleteTeam(team.uid, owner.id); + seen.length = 0; + + // The deletion suspended the seat; this is the only way to retire it. + await service.deleteMember(team.uid, owner.id, seat.userId); + + expect(await server.stores.user.getById(seat.userId)).toBeFalsy(); + // The billing stop still fires: `getOrgSeat` includes deleted teams. + expect(of('team.account.deleted')).toHaveLength(1); + }); + + it('refuses to delete a seat Puter suspended', async () => { + const team = await makeTeam(); + const seat = await provision(team); + await server.stores.user.update(seat.userId, { + suspended: 1, + suspended_at: Math.floor(Date.now() / 1000), + suspended_reason: 'abuse_review', + }); + await server.stores.user.invalidateById(seat.userId); + + // As `enableMember` holds: a platform suspension is not the team's. + await expect( + service.deleteMember(team.uid, owner.id, seat.userId), + ).rejects.toMatchObject({ statusCode: 409, legacyCode: 'conflict' }); + expect(await server.stores.user.getById(seat.userId)).toBeTruthy(); + }); + // -- the directory -------------------------------------------------- it('offers only members who have actually taken up their account', async () => { diff --git a/src/backend/services/team/TeamService.ts b/src/backend/services/team/TeamService.ts index 5addd614a..961447c02 100644 --- a/src/backend/services/team/TeamService.ts +++ b/src/backend/services/team/TeamService.ts @@ -228,11 +228,14 @@ export class TeamService extends PuterService { async requireOrgAccount( teamUid: string, targetUserId: number, + opts: { includeDeleted?: boolean } = {}, ): Promise { - const membership = await this.stores.team.getMembership( - teamUid, - targetUserId, - ); + const membership = opts.includeDeleted + ? await this.stores.team.getMembershipIncludingDeleted( + teamUid, + targetUserId, + ) + : await this.stores.team.getMembership(teamUid, targetUserId); // Tested explicitly, never inferred from NULL. if (!membership || Number(membership.org_owned) !== 1) { throw new HttpError(404, 'Not an account of this team', { @@ -566,7 +569,7 @@ export class TeamService extends PuterService { actorUserId: number, opts: { limit?: unknown; cursor?: string } = {}, ) { - const team = await this.#requireOwnedTeam(teamUid, actorUserId); + const team = await this.requireOwnedTeam(teamUid, actorUserId); return this.#withUsernames( await this.stores.team.listAudit(team.id, opts), ); @@ -664,7 +667,7 @@ export class TeamService extends PuterService { } /** Resolves a team the caller owns, soft-deleted or not. */ - async #requireOwnedTeam( + async requireOwnedTeam( teamUid: string, actorUserId: number, ): Promise { @@ -1094,8 +1097,11 @@ export class TeamService extends PuterService { actorUserId: number, targetUserId: number, ): Promise { - const team = await this.requireOwner(teamUid, actorUserId); - await this.requireOrgAccount(teamUid, targetUserId); + // Deleted team included: the only self-serve way to retire its seats. + const team = await this.requireOwnedTeam(teamUid, actorUserId); + await this.requireOrgAccount(teamUid, targetUserId, { + includeDeleted: true, + }); // Forced, as `enableMember` is: a cached row predates the disable. const user = await this.stores.user.getByProperty('id', targetUserId, { @@ -1106,6 +1112,12 @@ export class TeamService extends PuterService { legacyCode: 'account_must_be_disabled_first', }); } + // Only the team's own suspension may be deleted, as `enableMember` holds. + if (user.suspended_reason !== DISABLED_BY_TEAM) { + throw new HttpError(409, 'That account was suspended by Puter', { + legacyCode: 'conflict', + }); + } // Written first; `_keep` is what makes it outlive the account. await this.stores.team.appendAudit({ diff --git a/src/backend/stores/team/TeamStore.ts b/src/backend/stores/team/TeamStore.ts index bcadaa461..c26a90e74 100644 --- a/src/backend/stores/team/TeamStore.ts +++ b/src/backend/stores/team/TeamStore.ts @@ -346,6 +346,22 @@ export class TeamStore extends PuterStore { return (rows[0] as unknown as TeamMemberRow) ?? null; } + /** As `getMembership`, deleted team included; uncached — deletion is rare. */ + async getMembershipIncludingDeleted( + teamUid: string, + userId: number, + ): Promise { + const rows = await this.clients.db.read( + 'SELECT ug.`id`, ug.`user_id`, ug.`group_id`, ug.`org_owned`, ' + + 'ug.`created_at`, u.`username`, u.`uuid` FROM `jct_user_group` ug ' + + 'JOIN `user` u ON u.`id` = ug.`user_id` ' + + 'JOIN `group` g ON g.`id` = ug.`group_id` ' + + 'WHERE g.`uid` = ? AND ug.`user_id` = ? AND g.`kind` = ?', + [teamUid, userId, TEAM_KIND], + ); + return (rows[0] as unknown as TeamMemberRow) ?? null; + } + /** Whether this user belongs to this team. */ async isMember(teamUid: string, userId: number): Promise { return (await this.getMembership(teamUid, userId)) !== null;