fix: let an owner retire the seats of a deleted team, and only those

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.
This commit is contained in:
Juan Castro
2026-09-15 13:22:13 -04:00
parent 27dd0f8ce6
commit 230241d6d2
4 changed files with 69 additions and 10 deletions
@@ -353,8 +353,8 @@ export class TeamController extends PuterController {
async deleteMember(req: Request, res: Response): Promise<void> {
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);
@@ -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 () => {
+20 -8
View File
@@ -228,11 +228,14 @@ export class TeamService extends PuterService {
async requireOrgAccount(
teamUid: string,
targetUserId: number,
opts: { includeDeleted?: boolean } = {},
): Promise<TeamMemberRow> {
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<TeamRow> {
@@ -1094,8 +1097,11 @@ export class TeamService extends PuterService {
actorUserId: number,
targetUserId: number,
): Promise<void> {
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({
+16
View File
@@ -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<TeamMemberRow | null> {
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<boolean> {
return (await this.getMembership(teamUid, userId)) !== null;