Merge pull request #4091 from HeyPuter/juancastro/teams-cleanup

Teams cleanup: pagination, resumable delete, and credentials that stop leaving by mail
This commit is contained in:
Juan Fernando Castro authored and GitHub committed 2026-10-06 19:27:09 -04:00
commit 0e2f9663b2
13 files changed
+377 -74

No files matched your search

+19 -4
View File
@@ -352,10 +352,10 @@ support@puter.com immediately.
html: `
<p>Hi there,</p>
<p>{{team_name}} has created a Puter account for you.</p>
<p>Username: <b>{{username}}</b><br>
Temporary password: <b>{{temporary_password}}</b></p>
<p>You will be asked to choose your own password the first time you sign in.
This temporary one stops working then, and it expires on its own if unused.</p>
<p>Username: <b>{{username}}</b></p>
<p>{{team_name}} will give you a temporary password. You will be asked to choose
your own the first time you sign in. The temporary one stops working then, and it
expires on its own if unused.</p>
<p>What this means:</p>
<ul>
<li>This account belongs to {{team_name}}. They pay for it and can close it.</li>
@@ -364,6 +364,21 @@ This temporary one stops working then, and it expires on its own if unused.</p>
as you. Choose your own password promptly.</li>
</ul>
<p>Sincerely,</p>
<p>Puter</p>
`,
},
team_account_reissued: {
subject: 'A new temporary password for your {{team_name}} account',
html: `
<p>Hi there,</p>
<p>{{team_name}} has issued a new temporary password for your Puter account
<b>{{username}}</b>. Any temporary password you were given before this one has
stopped working.</p>
<p>{{team_name}} will give you the new one. You will be asked to choose your own
password the first time you sign in; the temporary one stops working then, and
it expires on its own if unused.</p>
<p>If you were not expecting this, ask {{team_name}} -- only they can issue it.</p>
<p>Sincerely,</p>
<p>Puter</p>
`,
},
@@ -303,6 +303,21 @@ describe('team endpoints over HTTP', () => {
expect(afterBody.items.some((t) => t.uid === team.uid)).toBe(true);
});
it('does not count a team it hides from an app token', async () => {
const { team } = await makeServiceTeam();
const appToken = await makeAppToken(env.users.user);
const res = await call('GET', '/teams?includeTotal=true', appToken);
expect(res.status).toBe(200);
const body = (await res.json()) as {
items: { uid: string }[];
total: number;
};
expect(body.items.some((t) => t.uid === team.uid)).toBe(false);
// A count over rows the page withholds reports the hidden team.
expect(body.total).toBe(body.items.length);
});
it('still lists a team to its owner session with the directory off', async () => {
const { team } = await makeServiceTeam();
const res = await call('GET', '/teams', env.users.user.token);
+14 -12
View File
@@ -111,16 +111,22 @@ export class TeamController extends PuterController {
async listTeams(req: Request, res: Response): Promise<void> {
const userId = this.#requireUserId(req);
await this.#requireTeamsAvailable(req, userId);
const teams = await this.stores.team.listTeamsForUser(userId);
// An app only sees a team whose owner opened the directory to apps;
// a team that hasn't is indistinguishable from no team at all.
const visible = isAccountContext(req.actor)
? teams
: teams.filter((t) => this.services.team.isDirectoryOpen(t));
// In SQL, or cursor and total describe teams the app cannot see.
const page = await this.stores.team.listTeamsForUser(userId, {
limit: req.query.limit,
cursor:
typeof req.query.cursor === 'string'
? req.query.cursor
: undefined,
includeTotal: req.query.includeTotal === 'true',
openOnly: !isAccountContext(req.actor),
});
res.json({
items: visible.map((t) =>
items: page.items.map((t) =>
toClientTeam(t, t.owner_user_id === userId),
),
...(page.cursor ? { cursor: page.cursor } : {}),
...(page.total === undefined ? {} : { total: page.total }),
});
}
@@ -236,11 +242,7 @@ export class TeamController extends PuterController {
});
}
/**
* Admits an app actor, like `listTeams` and `listMembers`, once the team
* has opted in. Discloses nothing a colleague cannot already read through
* `/members`.
*/
/** App actors too, once opted in; carries the `uuid` `/members` hides. */
@Get('/:uid/directory', {
subdomain: 'api',
requireVerified: true,
@@ -293,6 +293,38 @@ describe('ShareService', () => {
expect(await canRead(squatter.actor, file.path)).toBe(true);
});
it('lets an unconfirmed claim on an address refuse nothing', async () => {
const owner = await makeUser();
const squatter = await makeUser();
await server.stores.user.update(squatter.user.id, {
email_confirmed: false,
});
await server.services.share.setBlockAllSenders(squatter.actor, true);
const file = await makeFile(owner.user);
// Otherwise one unconfirmed row blocks every invite to that mailbox.
const result = await share(owner.actor, {
uid: file.uuid,
recipient: { email: squatter.email },
mode: 'read',
});
expect(result.pending).toBe(true);
// Confirming it makes the claim an identity, and the block real.
await server.stores.user.update(squatter.user.id, {
email_confirmed: true,
});
await expect(
share(owner.actor, {
uid: (await makeFile(owner.user)).uuid,
recipient: { email: squatter.email },
mode: 'read',
}),
).rejects.toMatchObject({
legacyCode: 'recipient_not_accepting_shares',
});
});
// `+` is only an alias separator where the domain says so.
it('does not hand a plus-addressed share to the base account', async () => {
const owner = await makeUser();
+3 -1
View File
@@ -3085,7 +3085,9 @@ export class ShareService extends PuterService {
const mayReach =
(await this.stores.user.findEmailOwner(email)) ??
(await this.stores.user.getByCleanEmail(abuseKey(email)));
if (mayReach) await this.#assertNotBlocked(issuerId, mayReach);
// Confirmed only: a claim must not decide who may write to an address.
if (mayReach?.email_confirmed)
await this.#assertNotBlocked(issuerId, mayReach);
// Stored canonicalized, because claiming matches on it: the confirmed
// address arrives in whatever form the signup normalized to, and an
@@ -191,6 +191,24 @@ describe('team billing events', () => {
});
});
it('does not re-charge a seat that was already disabled before the delete', async () => {
const team = await makeTeam();
const a = await provision(team);
const b = await provision(team);
await service.disableMember(team.uid, owner.id, a.userId);
seen.length = 0;
await service.deleteTeam(team.uid, owner.id);
// Only one `enabled` will ever close a charge, so only one may open it.
const disabled = of('team.account.disabled');
expect(disabled).toHaveLength(1);
expect(disabled[0].data.user_id).toBe(b.userId);
// Still both seats: the team is losing two accounts either way.
expect(of('team.deleted')[0].data).toMatchObject({ account_count: 2 });
});
it('emits account-deleted, which a post-delete listener could not', async () => {
const team = await makeTeam();
const seat = await provision(team);
@@ -1259,7 +1259,7 @@ describe('TeamService', () => {
return { sent, restore: () => (client.sendRaw = original) };
};
it('emails the credential when an address is given', async () => {
it('notifies the member without putting the credential in the mail', async () => {
const { team } = await makeTeam();
const username = `mail_${Math.random().toString(36).slice(2, 9)}`;
@@ -1277,8 +1277,8 @@ describe('TeamService', () => {
expect(mail.sent).toHaveLength(1);
expect(mail.sent[0].to).toBe(`${username}@test.local`);
expect(mail.sent[0].html).toContain(username);
// The point of the address: without it this is the only copy.
expect(mail.sent[0].html).toContain(created.temporaryPassword);
// Nobody confirmed this address; the owner has the credential already.
expect(mail.sent[0].html).not.toContain(created.temporaryPassword);
});
it('sends nothing when no address is given', async () => {
@@ -1294,7 +1294,7 @@ describe('TeamService', () => {
expect(mail.sent).toHaveLength(0);
});
it('emails the fresh credential on re-issue, not the old one', async () => {
it('keeps the re-issued credential out of the mail too', async () => {
const { team } = await makeTeam();
const username = `re_${Math.random().toString(36).slice(2, 9)}`;
const first = await service.provisionAccount(team.uid, owner.id, {
@@ -1315,8 +1315,12 @@ describe('TeamService', () => {
}
expect(mail.sent).toHaveLength(1);
expect(mail.sent[0].html).toContain(again.temporaryPassword);
// The re-sendable path, so the one that made Puter a usable mailer.
expect(mail.sent[0].html).not.toContain(again.temporaryPassword);
expect(mail.sent[0].html).not.toContain(first.temporaryPassword);
// Its own notice, not a second `created` mail saying nothing new.
expect(mail.sent[0].subject).toMatch(/new temporary password/iu);
expect(mail.sent[0].html).toMatch(/stopped working/iu);
});
it('tells a member their account was disabled', async () => {
+83 -37
View File
@@ -27,11 +27,15 @@ import { isReservedUsername } from '../../util/reservedUsernames.js';
import type { EmailTemplateName } from '../../clients/email/templates.js';
import { subscriptionSatisfies } from '../metering/enforcement.js';
import { ORG_SEAT_FREE_SUBSCRIPTION } from '../metering/consts.js';
import { runWithConcurrencyLimitSettled } from '../../util/concurrency.js';
// A free team is small on purpose; paying widens it. Both overridable in config.
const FREE_SEAT_CAP = 4;
const PAID_SEAT_CAP = 40;
// Seat teardown is independent per member, so a delete need not be serial.
const TEARDOWN_CONCURRENCY = 8;
import type {
EventMap,
TeamBillingContext,
@@ -535,10 +539,7 @@ export class TeamService extends PuterService {
}
}
/**
* The member list as an app may read it, once the team has opted in. The
* page carries only what a colleague already sees through `/members`.
*/
/** The roster an app may read once opted in; that opt-in grants `uuid`. */
async listDirectory(
teamUid: string,
actorUserId: number,
@@ -636,32 +637,12 @@ export class TeamService extends PuterService {
// Otherwise they keep working, unreachable through a deleted team.
let page = await this.stores.team.listMembers(teamUid, { limit: 200 });
for (;;) {
for (const member of page.items) {
if (Number(member.org_owned) !== 1) continue;
await this.stores.team.appendAudit({
teamId: team.id,
userId: member.user_id,
actorUserId,
action: 'disable',
reason: 'team_deleted',
});
const held = await this.#heldBytes(member.user_id);
await this.#suspend(member.user_id);
disabled++;
// Per seat, not one bulk event: the byte charge is per account.
this.#emitBilling('team.account.disabled', {
...billing,
user_id: member.user_id,
user_uuid: member.uuid,
username: member.username,
held_bytes: held,
});
// The team notice covers the disabling, so a member is
// told once rather than twice about the same event.
await this.#notifyMember(member.user_id, 'team_closed', team);
}
disabled += await this.#disableSeatsForDelete(
page.items.filter((m) => Number(m.org_owned) === 1),
team,
actorUserId,
billing,
);
if (!page.cursor) break;
page = await this.stores.team.listMembers(teamUid, {
limit: 200,
@@ -683,6 +664,74 @@ export class TeamService extends PuterService {
});
}
/**
* One page of seats, as `disableMember` does one member. Everything that
* answers for the whole page -- who is already off, what they hold, the
* audit -- goes out once; what is genuinely per account stays per account.
* Returns how many of them end up disabled.
*/
async #disableSeatsForDelete(
seats: TeamMemberRow[],
team: TeamRow,
actorUserId: number,
billing: TeamBillingContext,
): Promise<number> {
if (seats.length === 0) return 0;
// From the primary: a cached `suspended` would re-charge on a retry.
const suspended = await this.stores.user.getSuspendedByIds(
seats.map((m) => m.user_id),
);
const todo = seats.filter((m) => !suspended.get(m.user_id));
// Already off, and still accounts the team is losing.
if (todo.length === 0) return seats.length;
const ids = todo.map((m) => m.user_id);
const held = await this.stores.fsEntry.getHeldBytesForUsers(ids);
const rows = await this.stores.user.getByIds(ids);
// Recorded first, so a failed append cannot leave an unlogged suspension.
await this.stores.team.appendAuditMany(
todo.map((m) => ({
teamId: team.id,
userId: m.user_id,
actorUserId,
action: 'disable',
reason: 'team_deleted',
})),
);
// Bounded: what is left per seat is a write and a mail send.
const outcomes = await runWithConcurrencyLimitSettled(
todo,
TEARDOWN_CONCURRENCY,
async (member) => {
await this.#suspend(member.user_id);
// Per seat, not one bulk event: the byte charge is per account.
this.#emitBilling('team.account.disabled', {
...billing,
user_id: member.user_id,
user_uuid: member.uuid,
username: member.username,
held_bytes: held.get(member.user_id) ?? 0,
});
// The team notice covers the disabling, so a member is told once.
await this.#notifyUser(
rows.get(member.user_id),
'team_closed',
team,
);
},
);
const done = outcomes.filter((o) => o.status === 'fulfilled').length;
// Settled: a rejection cannot cancel its peers, so finish the page.
const failed = outcomes.find((o) => o.status === 'rejected');
if (failed) throw failed.reason;
return seats.length - todo.length + done;
}
/** Team owner only. Readable after deletion -- that is the point of it. */
async listAudit(
teamUid: string,
@@ -976,11 +1025,9 @@ export class TeamService extends PuterService {
action: 'provision',
});
// Returned once; forced change on first use is what bounds it.
// Not mailed: nobody confirmed the address the owner typed in.
const temporaryPassword = await this.#issueTemporaryPassword(user.id);
await this.#notifyUser(user, 'team_account_created', team, {
temporary_password: temporaryPassword,
});
await this.#notifyUser(user, 'team_account_created', team);
// Last: the seat is only chargeable once it exists and can be used.
this.#emitBilling('team.account.created', {
@@ -1023,9 +1070,8 @@ export class TeamService extends PuterService {
});
const temporaryPassword =
await this.#issueTemporaryPassword(targetUserId);
await this.#notifyUser(user, 'team_account_created', team, {
temporary_password: temporaryPassword,
});
// Its own notice; the created one omits the password just invalidated.
await this.#notifyUser(user, 'team_account_reissued', team);
return { temporaryPassword };
}
+32
View File
@@ -3344,6 +3344,38 @@ export class FSEntryStore extends PuterStore {
return Number(rows[0]?.totalUsage ?? 0);
}
/**
* Held bytes for several accounts in one grouped read, as `getHeldBytes`
* does for one.
*/
async getHeldBytesForUsers(
userIds: number[],
): Promise<Map<number, number>> {
const out = new Map<number, number>();
const unique = [
...new Set(
(Array.isArray(userIds) ? userIds : []).filter(
(id): id is number => typeof id === 'number',
),
),
];
for (const id of unique) out.set(id, 0);
for (let offset = 0; offset < unique.length; offset += 200) {
const chunk = unique.slice(offset, offset + 200);
const placeholders = chunk.map(() => '?').join(', ');
const rows = (await this.clients.db.read(
`SELECT user_id, COALESCE(SUM(size), 0) AS ${this.clients.db.quoteIdentifier('totalUsage')} ` +
`FROM fsentries WHERE user_id IN (${placeholders}) GROUP BY user_id`,
chunk,
)) as { user_id: number; totalUsage: number }[];
for (const row of rows) {
out.set(Number(row.user_id), Number(row.totalUsage ?? 0));
}
}
return out;
}
/**
* `consistentRead` reads both queries from the primary instead of a replica
* — used for the exact re-check when a replica-backed read would reject a
+34 -3
View File
@@ -467,8 +467,8 @@ describe('TeamStore', () => {
await store.addMember(a.uid, member.id, { orgOwned: true });
await store.addMember(b.uid, member.id, { orgOwned: true });
const teams = await store.listTeamsForUser(member.id);
expect(teams.map((t) => t.uid).sort()).toEqual([a.uid, b.uid].sort());
const { items } = await store.listTeamsForUser(member.id);
expect(items.map((t) => t.uid).sort()).toEqual([a.uid, b.uid].sort());
});
it('drops a soft-deleted team from the user\'s list', async () => {
@@ -481,10 +481,41 @@ describe('TeamStore', () => {
await store.addMember(team.uid, member.id, { orgOwned: true });
await store.softDelete(team.uid);
expect(await store.listTeamsForUser(member.id)).toEqual([]);
expect((await store.listTeamsForUser(member.id)).items).toEqual([]);
await expect(store.isMember(team.uid, member.id)).resolves.toBe(false);
});
it('pages the user team list and counts on request', async () => {
const member = await makeUser();
const made = [];
for (let i = 0; i < 3; i++) {
const team = await store.create({
ownerUserId: owner.id,
name: `Paged ${i}`,
handle: freeHandle(),
});
await store.addMember(team.uid, member.id, { orgOwned: true });
made.push(team.uid);
}
const first = await store.listTeamsForUser(member.id, {
limit: 2,
includeTotal: true,
});
expect(first.items).toHaveLength(2);
expect(first.total).toBe(3);
expect(first.cursor).toBeTruthy();
const second = await store.listTeamsForUser(member.id, {
cursor: first.cursor,
});
expect(second.items).toHaveLength(1);
expect(second.cursor).toBeUndefined();
expect(
[...first.items, ...second.items].map((t) => t.uid).sort(),
).toEqual([...made].sort());
});
it('refuses to add a member to a group that is not a team', async () => {
const [seeded] = (await server.clients.db.read(
'SELECT `uid` FROM `group` WHERE `kind` IS NULL LIMIT 1',
+83 -11
View File
@@ -522,11 +522,8 @@ export class TeamStore extends PuterStore {
return rows[0] ?? null;
}
/** Members to announce a team share to; bounded, or the send is too. */
/**
* Short-lived, and deliberately only over reads that are not authorization.
* A stale entry here costs a notification, never access -- `isMember` and
* `getByUid` are left uncached for that reason.
* Short-lived, and safe for `getByUid`/`getMembership`: mutations bust it.
*
* `jct_user_group.user_id` is ON DELETE CASCADE, so deleting an account
* changes membership without passing through this store. The TTL is the
@@ -609,15 +606,58 @@ export class TeamStore extends PuterStore {
return rows.map((r) => Number(r.user_id));
}
/** Teams this user belongs to, oldest first. */
async listTeamsForUser(userId: number): Promise<TeamRow[]> {
const rows = await this.clients.db.read(
/** A user's teams, oldest first; `openOnly` is the page an app may see. */
async listTeamsForUser(
userId: number,
opts: {
limit?: unknown;
cursor?: string;
includeTotal?: boolean;
openOnly?: boolean;
} = {},
): Promise<PageResult<TeamRow>> {
const limit =
normalizeLimit(opts.limit, { cap: MEMBER_PAGE_CAP }) ??
MEMBER_PAGE_SIZE;
const page = decodeCursor(opts.cursor, 'team cursor');
const after = typeof page?.id === 'number' ? page.id : null;
const open = opts.openOnly ? ' AND g.`directory_enabled` = 1' : '';
const rows = (await this.clients.db.read(
'SELECT g.* FROM `group` g ' +
'JOIN `jct_user_group` ug ON ug.`group_id` = g.`id` ' +
`WHERE ug.\`user_id\` = ? AND g.${this.#live()} ORDER BY g.\`id\``,
[userId, TEAM_KIND],
);
return rows as unknown as TeamRow[];
`WHERE ug.\`user_id\` = ? AND g.${this.#live()}` +
open +
(after === null ? '' : ' AND g.`id` > ?') +
' ORDER BY g.`id` LIMIT ?',
after === null
? [userId, TEAM_KIND, limit + 1]
: [userId, TEAM_KIND, after, limit + 1],
)) as unknown as TeamRow[];
const items = rows.slice(0, limit);
const cursor =
rows.length > limit
? encodeCursor({ id: items[items.length - 1].id })
: undefined;
let total: number | undefined;
if (opts.includeTotal) {
const totals = (await this.clients.db.read(
'SELECT COUNT(*) AS n FROM `group` g ' +
'JOIN `jct_user_group` ug ON ug.`group_id` = g.`id` ' +
`WHERE ug.\`user_id\` = ? AND g.${this.#live()}` +
open,
[userId, TEAM_KIND],
)) as unknown as Array<{ n: number | string }>;
total = Number(totals[0]?.n ?? 0);
}
return {
items,
...(cursor ? { cursor } : {}),
...(total === undefined ? {} : { total }),
};
}
/** `orgOwned` decides who pays: 1 team-created, 0 the team owner. */
@@ -780,6 +820,38 @@ export class TeamStore extends PuterStore {
);
}
/** The same rows as `appendAudit`, in one statement per chunk. */
async appendAuditMany(
entries: Array<{
teamId: number;
userId: number;
actorUserId: number;
action: string;
reason?: string | null;
}>,
): Promise<void> {
if (entries.length === 0) return;
for (let offset = 0; offset < entries.length; offset += 200) {
const chunk = entries.slice(offset, offset + 200);
const values = chunk.map(() => '(?, ?, ?, ?, ?, ?, ?)').join(', ');
const params = chunk.flatMap((e) => [
e.teamId,
e.teamId,
e.userId,
e.userId,
e.actorUserId,
e.action,
e.reason ?? null,
]);
await this.clients.db.write(
'INSERT INTO `audit_team_membership` ' +
'(`group_id`, `group_id_keep`, `user_id`, `user_id_keep`, ' +
`\`actor_user_id\`, \`action\`, \`reason\`) VALUES ${values}`,
params,
);
}
}
/** The whole team's audit, newest first, keyset-paginated on `id`. */
async listAudit(
teamId: number,
+34
View File
@@ -248,6 +248,40 @@ export class UserStore extends PuterStore {
return this.getByProperty('referral_code', normalized, opts);
}
/**
* Which of these accounts are suspended, read from the primary.
*
* Deliberately not `getByIds`, which answers from cache: a caller deciding
* whether to suspend an account needs the row as it is now, not as it was
* when something last cached it.
*/
async getSuspendedByIds(ids: number[]): Promise<Map<number, boolean>> {
const out = new Map<number, boolean>();
const unique = [
...new Set(
(Array.isArray(ids) ? ids : []).filter(
(id): id is number => typeof id === 'number',
),
),
];
for (
let offset = 0;
offset < unique.length;
offset += BULK_QUERY_CHUNK_SIZE
) {
const chunk = unique.slice(offset, offset + BULK_QUERY_CHUNK_SIZE);
const placeholders = chunk.map(() => '?').join(', ');
const rows = (await this.clients.db.pread(
`SELECT \`id\`, \`suspended\` FROM \`user\` WHERE \`id\` IN (${placeholders})`,
chunk,
)) as Array<{ id: number; suspended: unknown }>;
for (const row of rows) {
out.set(Number(row.id), Boolean(row.suspended));
}
}
return out;
}
/**
* Batched lookup by id. Dedupes input ids, reads cache via a pipelined
* MGET, and resolves remaining misses with a single `SELECT … WHERE id IN
+1 -1
View File
@@ -617,7 +617,7 @@ const en = {
teams_account_of: 'This account belongs to %%',
teams_add_account: 'Add an account',
teams_email_optional: 'Email (optional)',
teams_add_account_email_hint: 'If you add an address, we email the username and temporary password to it. Otherwise the password below is the only copy.',
teams_add_account_email_hint: 'If you add an address, we email them their username — never the password. The password below is the only copy, so pass it on yourself.',
teams_add_account_hint:
'Puter creates the account and gives you a one-time password to pass on. The username has to be free across all of Puter.',
teams_plan_change_for: 'Which plan for {{username}}?',