mirror of
https://github.com/HeyPuter/puter.git
synced 2026-09-20 20:26:21 +00:00
feat: delete a team seat for good, once it is disabled
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.
This commit is contained in:
@@ -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();
|
||||
|
||||
|
||||
@@ -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<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);
|
||||
const target = await this.#requireTargetUserId(req);
|
||||
|
||||
await this.services.team.deleteMember(uid, userId, target);
|
||||
res.json({ success: true });
|
||||
}
|
||||
|
||||
// -- Audit --------------------------------------------------------
|
||||
|
||||
@Get('/:uid/audit', {
|
||||
|
||||
@@ -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;
|
||||
|
||||
@@ -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<void> {
|
||||
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<void> {
|
||||
await this.stores.user.update(userId, {
|
||||
|
||||
@@ -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.
|
||||
|
||||
Reference in New Issue
Block a user