From 9f615cc632b9af09fbb8ea122a2a0596581a275c Mon Sep 17 00:00:00 2001 From: Juan Castro Date: Wed, 2 Sep 2026 18:43:52 -0400 Subject: [PATCH 1/3] feat: add the two-team fixture and group permission grants MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two tickets, together because the fixture has no behaviour of its own — the grant tests are its first real use, and "a grant to team A must not resolve for B's members" is exactly a two-team assertion. PUT-1719. Every query resolving team-scoped data has to take the team as a parameter rather than infer it. One that forgets returns the right rows against a database holding a single team, so a one-team fixture does not give weaker coverage — it gives false confidence. The fixture provides two teams, each with its own master and two activated seats, plus a user in neither. Each team having its own master is also what the shipped `max_teams_per_user: 1` requires, so it runs against the real cap rather than lifting it the way the existing team suites have to. Seats are activated before use: provisioning leaves `requires_email_confirmation` set and `requireVerified` rejects them outright, so an unactivated seat cannot call anything and every authorization assertion built on one would be vacuous. PUT-1725. The write half of group grants; the read half already worked, since `#scanUserGroup` joins `jct_user_group` itself and resolves for whoever is a member at scan time. PermissionStore resolveGroupId, listGroupMemberUuids, upsertUserGroupPerm, deleteUserGroupPerm, auditUserGroupPerm PermissionService grantUserGroupPermission, revokeUserGroupPermission Both rewrite the permission first, so `fs:/path:mode` collapses to `fs::mode` and a revoke names the same string the grant wrote; both gate on `canManagePermission`; both audit into the existing `audit_user_to_group_permissions`. The delete is scoped to the issuer, so one issuer's revoke cannot drop another's identical grant — the PK is (user_id, group_id, permission) and user_id is the issuer. Cache invalidation goes through `bumpCacheGenerations` with every member uuid at once. That already announces to peer regions in a single event, so the fan-out costs one broadcast rather than one per member. There is no flat-KV equivalent for group grants, so unlike the user-to-user path there is no second write to keep consistent and no second invalidation surface. Closes PUT-1719 and PUT-1725. --- .../services/permission/GroupGrants.test.ts | 362 ++++++++++++++++++ .../services/permission/PermissionService.ts | 139 +++++++ .../stores/permission/PermissionStore.ts | 98 ++++- src/backend/testFixtures/twoTeams.ts | 170 ++++++++ 4 files changed, 768 insertions(+), 1 deletion(-) create mode 100644 src/backend/services/permission/GroupGrants.test.ts create mode 100644 src/backend/testFixtures/twoTeams.ts diff --git a/src/backend/services/permission/GroupGrants.test.ts b/src/backend/services/permission/GroupGrants.test.ts new file mode 100644 index 000000000..d5955696e --- /dev/null +++ b/src/backend/services/permission/GroupGrants.test.ts @@ -0,0 +1,362 @@ +/** + * Copyright (C) 2024-present Puter Technologies Inc. + * + * This file is part of Puter. + * + * Puter is free software: you can redistribute it and/or modify + * it under the terms of the GNU Affero General Public License as published + * by the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Affero General Public License for more details. + * + * You should have received a copy of the GNU Affero General Public License + * along with this program. If not, see . + */ + +import { afterAll, beforeAll, describe, expect, it } from 'vitest'; +import type { Actor } from '../../core/actor'; +import { + setupTwoTeams, + type TwoTeams, +} from '../../testFixtures/twoTeams.js'; + +describe('group grants', () => { + let fx: TwoTeams; + + /** A plain user actor, which is what a grant is issued as. */ + const actorFor = async (userId: number): Promise => { + const user = await fx.env.server.stores.user.getById(userId); + return { user } as unknown as Actor; + }; + + /** Written directly: these tests are about granting, not about writing. */ + const makeFile = async (ownerId: number) => { + const uid = crypto.randomUUID(); + const name = `f_${uid.slice(0, 8)}.txt`; + const owner = await fx.env.server.stores.user.getById(ownerId); + const path = `/${owner!.username}/${name}`; + await fx.env.server.clients.db.write( + 'INSERT INTO `fsentries` (`uuid`, `name`, `path`, `user_id`, `is_dir`, `modified`) ' + + 'VALUES (?, ?, ?, ?, ?, ?)', + [ + uid, + name, + path, + ownerId, + // `is_dir` is a real boolean on postgres. + fx.env.server.clients.db.booleanValue(false), + Math.floor(Date.now() / 1000), + ], + ); + return { path, uid }; + }; + + const permissions = () => fx.env.server.services.permission; + const store = () => fx.env.server.stores.permission; + + beforeAll(async () => { + fx = await setupTwoTeams(); + }, 180_000); + + afterAll(async () => { + await fx?.shutdown(); + }); + + // -- the fixture itself ------------------------------------------- + + it('builds two teams, each with its own owner and two seats', async () => { + expect(fx.a.uid).not.toBe(fx.b.uid); + expect(fx.a.owner.userId).not.toBe(fx.b.owner.userId); + expect(fx.a.seats).toHaveLength(2); + expect(fx.b.seats).toHaveLength(2); + + // Distinct throughout, or it cannot tell this team from any. + const ids = [ + fx.a.owner.userId, + fx.b.owner.userId, + ...fx.a.seats.map((s) => s.userId), + ...fx.b.seats.map((s) => s.userId), + fx.outsider.userId, + ]; + expect(new Set(ids).size).toBe(ids.length); + }); + + it('runs against the shipped one-team-per-user cap', async () => { + // Lifting the cap here would stop it resembling production. + const cfg = fx.env.server.services.team.config as { + max_teams_per_user?: number; + }; + expect(cfg.max_teams_per_user ?? 1).toBe(1); + }); + + it('seats can actually call the API', async () => { + // An inert token would make every assertion below vacuous. + const res = await fx.call('GET', `/teams/${fx.a.uid}`, fx.a.seats[0].token); + expect(res.status).toBe(200); + }); + + // -- grants resolve for members, and only members ----------------- + + it('resolves a grant for every member of the team it was given to', async () => { + const file = await makeFile(fx.outsider.userId); + const permission = `fs:${file.uid}:read`; + + await permissions().grantUserGroupPermission( + await actorFor(fx.outsider.userId), + fx.a.uid, + permission, + ); + + for (const seat of fx.a.seats) { + const rows = await store().readUserGroupPerms(seat.userId, [ + permission, + ]); + expect(rows, `seat ${seat.username}`).toHaveLength(1); + } + }); + + it('does not resolve for the other team', async () => { + const file = await makeFile(fx.outsider.userId); + const permission = `fs:${file.uid}:read`; + + await permissions().grantUserGroupPermission( + await actorFor(fx.outsider.userId), + fx.a.uid, + permission, + ); + + // Why the fixture has two: with one, an unscoped query still passes. + for (const seat of fx.b.seats) { + const rows = await store().readUserGroupPerms(seat.userId, [ + permission, + ]); + expect(rows, `B seat ${seat.username}`).toHaveLength(0); + } + }); + + it('does not resolve for a user in no team', async () => { + const file = await makeFile(fx.a.owner.userId); + const permission = `fs:${file.uid}:read`; + + await permissions().grantUserGroupPermission( + await actorFor(fx.a.owner.userId), + fx.a.uid, + permission, + ); + + const rows = await store().readUserGroupPerms(fx.outsider.userId, [ + permission, + ]); + expect(rows).toHaveLength(0); + }); + + // -- revoke -------------------------------------------------------- + + it('revoking removes it for every member', async () => { + const file = await makeFile(fx.outsider.userId); + const permission = `fs:${file.uid}:read`; + const issuer = await actorFor(fx.outsider.userId); + + await permissions().grantUserGroupPermission( + issuer, + fx.a.uid, + permission, + ); + const removed = await permissions().revokeUserGroupPermission( + issuer, + fx.a.uid, + permission, + ); + + expect(removed).toBe(true); + for (const seat of fx.a.seats) { + expect( + await store().readUserGroupPerms(seat.userId, [permission]), + ).toHaveLength(0); + } + }); + + it('announces the revoke for every member, so their watches get settled', async () => { + const file = await makeFile(fx.outsider.userId); + const permission = `fs:${file.uid}:read`; + const issuer = await actorFor(fx.outsider.userId); + const announced: number[] = []; + const bus = fx.env.server.clients.event; + const listen = ((_k: string, d: { holderUserId: number }) => { + announced.push(d.holderUserId); + }) as never; + bus.on('permission.revoked', listen); + + try { + await permissions().grantUserGroupPermission( + issuer, + fx.a.uid, + permission, + ); + announced.length = 0; + await permissions().revokeUserGroupPermission( + issuer, + fx.a.uid, + permission, + ); + } finally { + bus.off?.('permission.revoked', listen); + } + + for (const seat of fx.a.seats) { + expect(announced).toContain(seat.userId); + } + }); + + it('reports false when there was nothing to revoke', async () => { + const file = await makeFile(fx.outsider.userId); + const removed = await permissions().revokeUserGroupPermission( + await actorFor(fx.outsider.userId), + fx.a.uid, + `fs:${file.uid}:read`, + ); + // Matching nothing is not an error, but a caller must be able to tell. + expect(removed).toBe(false); + }); + + it('one issuer revoking does not drop another issuer identical grant', async () => { + // Revoker owns the file; the rival grant is written directly. + const file = await makeFile(fx.a.owner.userId); + const permission = `fs:${file.uid}:write`; + const groupId = (await store().resolveGroupId(fx.a.uid))!; + + // The only shape where the DELETE's issuer scoping decides anything. + await permissions().grantUserGroupPermission( + await actorFor(fx.a.owner.userId), + fx.a.uid, + permission, + ); + await store().upsertUserGroupPerm( + groupId, + fx.outsider.userId, + permission, + {}, + ); + expect( + await store().readUserGroupPerms(fx.a.seats[0].userId, [permission]), + ).toHaveLength(2); + + await permissions().revokeUserGroupPermission( + await actorFor(fx.a.owner.userId), + fx.a.uid, + permission, + ); + + // The owner's row is gone; the other issuer's survives. + const left = await store().readUserGroupPerms(fx.a.seats[0].userId, [ + permission, + ]); + expect(left).toHaveLength(1); + expect(left[0].user_id).toBe(fx.outsider.userId); + }); + + // -- authorization and shape -------------------------------------- + + it('refuses a grant the issuer has no authority over', async () => { + const file = await makeFile(fx.b.owner.userId); + + await expect( + permissions().grantUserGroupPermission( + await actorFor(fx.outsider.userId), + fx.a.uid, + `fs:${file.uid}:write`, + ), + ).rejects.toMatchObject({ statusCode: 403 }); + }); + + it('404s on a group that does not exist', async () => { + const file = await makeFile(fx.outsider.userId); + await expect( + permissions().grantUserGroupPermission( + await actorFor(fx.outsider.userId), + '00000000-0000-4000-8000-000000000000', + `fs:${file.uid}:read`, + ), + ).rejects.toMatchObject({ statusCode: 404 }); + }); + + it('round-trips a path-form permission through grant and revoke', async () => { + const owner = await fx.env.server.stores.user.getById( + fx.outsider.userId, + ); + const file = await makeFile(fx.outsider.userId); + const issuer = await actorFor(fx.outsider.userId); + + // Both must collapse to the same `fs::` string, or nothing matches. + const pathPerm = `fs:${file.path}:read`; + await permissions().grantUserGroupPermission( + issuer, + fx.a.uid, + pathPerm, + ); + expect(owner).toBeTruthy(); + + const stored = await store().readUserGroupPerms(fx.a.seats[0].userId, [ + `fs:${file.uid}:read`, + ]); + expect(stored).toHaveLength(1); + + expect( + await permissions().revokeUserGroupPermission( + issuer, + fx.a.uid, + pathPerm, + ), + ).toBe(true); + }); + + it('audits both the grant and the revoke', async () => { + const file = await makeFile(fx.outsider.userId); + const permission = `fs:${file.uid}:read`; + const issuer = await actorFor(fx.outsider.userId); + + await permissions().grantUserGroupPermission( + issuer, + fx.a.uid, + permission, + ); + await permissions().revokeUserGroupPermission( + issuer, + fx.a.uid, + permission, + ); + + const rows = (await fx.env.server.clients.db.read( + 'SELECT `action` FROM `audit_user_to_group_permissions` ' + + 'WHERE `permission` = ? ORDER BY `id`', + [permission], + )) as { action: string }[]; + expect(rows.map((r) => r.action)).toEqual(['grant', 'revoke']); + }); + + it('stops resolving once the team is soft-deleted', async () => { + const file = await makeFile(fx.outsider.userId); + const permission = `fs:${file.uid}:read`; + await permissions().grantUserGroupPermission( + await actorFor(fx.outsider.userId), + fx.b.uid, + permission, + ); + const seat = fx.b.seats[0].userId; + expect( + await store().readUserGroupPerms(seat, [permission]), + ).toHaveLength(1); + + // Deletion suspends the seats but leaves memberships and grants, and + // the uid stops resolving -- so this access would be unwithdrawable. + await fx.env.server.stores.team.softDelete(fx.b.uid); + + expect( + await store().readUserGroupPerms(seat, [permission]), + ).toHaveLength(0); + }); +}); diff --git a/src/backend/services/permission/PermissionService.ts b/src/backend/services/permission/PermissionService.ts index 346702991..00b1d9c9f 100644 --- a/src/backend/services/permission/PermissionService.ts +++ b/src/backend/services/permission/PermissionService.ts @@ -885,6 +885,145 @@ export class PermissionService extends PuterService { if (user.uuid) await this.#bumpUserCacheGeneration(user.uuid); } + // -- Group grants ---- the write half; `#scanUserGroup` reads them ---- + + /** Resolved once here so neither grant nor revoke holds SQL. */ + async #requireGroupId(groupUid: string): Promise { + const groupId = await this.stores.permission.resolveGroupId(groupUid); + if (groupId === null) { + throw new HttpError(404, `group_does_not_exist: ${groupUid}`, { + legacyCode: 'subject_does_not_exist', + }); + } + return groupId; + } + + /** Batched: one event for the whole group, not one per member. */ + async #bumpGroupCacheGeneration(groupId: number): Promise { + const uuids = + await this.stores.permission.listGroupMemberUuids(groupId); + if (uuids.length === 0) return; + await this.stores.permission.bumpCacheGenerations( + uuids.map((uuid) => `user:${uuid}`), + ); + } + + async grantUserGroupPermission( + actor: Actor, + groupUid: string, + permission: string, + extra: Record = {}, + meta: GrantMeta = {}, + ): Promise { + // First: the rewrite decides the row's width and what a revoke matches. + permission = await this.rewritePermission(permission); + if (permission.length > PERMISSION_MAX_LEN) { + throw new HttpError(400, 'permission is too long', { + legacyCode: 'bad_request', + }); + } + const groupId = await this.#requireGroupId(groupUid); + + if (!(await this.canManagePermission(actor, permission))) { + throw new HttpError(403, `permission_denied: ${permission}`, { + legacyCode: 'permission_denied', + }); + } + if (!actor.user?.id) { + throw new HttpError(403, 'actor must be a user', { + legacyCode: 'forbidden', + }); + } + const issuerId = actor.user.id; + + await this.stores.permission.upsertUserGroupPerm( + groupId, + issuerId, + permission, + extra, + ); + + // Off the critical path, but a silent drop makes the log untrustworthy. + this.stores.permission + .auditUserGroupPerm({ + group_id: groupId, + issuer_user_id: issuerId, + permission, + action: 'grant', + reason: meta.reason ?? 'granted via PermissionService', + extra: this.#auditActorContext(actor), + }) + .catch((err) => { + console.warn( + '[PermissionService] failed to audit user-group grant:', + err, + ); + }); + + await this.#bumpGroupCacheGeneration(groupId); + } + + /** Scoped to this issuer's grant; returns whether one was removed. */ + async revokeUserGroupPermission( + actor: Actor, + groupUid: string, + permission: string, + meta: GrantMeta = {}, + ): Promise { + // Same rewrite as the grant, or this matches nothing and says it did. + permission = await this.rewritePermission(permission); + const groupId = await this.#requireGroupId(groupUid); + + if (!actor.user?.id) { + throw new HttpError(403, 'actor must be a user', { + legacyCode: 'forbidden', + }); + } + const issuerId = actor.user.id; + + if (!(await this.canManagePermission(actor, permission))) { + throw new HttpError(403, `permission_denied: ${permission}`, { + legacyCode: 'permission_denied', + }); + } + + const revoked = await this.stores.permission.deleteUserGroupPerm( + groupId, + issuerId, + permission, + ); + + this.stores.permission + .auditUserGroupPerm({ + group_id: groupId, + issuer_user_id: issuerId, + permission, + action: 'revoke', + reason: meta.reason ?? 'revoked via PermissionService', + extra: this.#auditActorContext(actor), + }) + .catch((err) => { + console.warn( + '[PermissionService] failed to audit user-group revoke:', + err, + ); + }); + + // Bumped even when nothing matched: a cached allow must not survive. + await this.#bumpGroupCacheGeneration(groupId); + + // Nothing but the membership names the holders, so without this their + // watches outlive the revoke. + if (revoked) { + for (const memberId of await this.stores.permission.listGroupMemberIds( + groupId, + )) { + this.#announceRevoked(memberId, null, permission); + } + } + return revoked; + } + /** * Remove the grant `actor` issued, or the one named by `opts.issuerUserId` * when the caller has established authority over another issuer's grant (a diff --git a/src/backend/stores/permission/PermissionStore.ts b/src/backend/stores/permission/PermissionStore.ts index c9af8282c..49b464141 100644 --- a/src/backend/stores/permission/PermissionStore.ts +++ b/src/backend/stores/permission/PermissionStore.ts @@ -959,10 +959,13 @@ export class PermissionStore extends PuterStore { if (permissions.length === 0) return []; let permClause = permissions.map(() => 'p.permission = ?').join(' OR '); if (permissions.length > 1) permClause = `(${permClause})`; + // Deletion leaves memberships and grants, so a deleted team would + // otherwise keep resolving access nothing can withdraw. const rows = await this.clients.db.read( 'SELECT p.permission, p.user_id, p.group_id, p.extra FROM `user_to_group_permissions` p ' + 'JOIN `jct_user_group` ug ON p.group_id = ug.group_id ' + - `WHERE ug.user_id = ? AND ${permClause}`, + 'JOIN `group` g ON g.`id` = ug.group_id ' + + `WHERE ug.user_id = ? AND g.\`deleted_at\` IS NULL AND ${permClause}`, [userId, ...permissions], ); return rows.map((row) => @@ -970,6 +973,99 @@ export class PermissionStore extends PuterStore { ); } + /** Any group, seeded or team: a grant does not care which kind it is. */ + async resolveGroupId(groupUid: string): Promise { + const rows = (await this.clients.db.read( + 'SELECT `id` FROM `group` WHERE `uid` = ? LIMIT 1', + [groupUid], + )) as { id: number }[]; + return rows[0]?.id ?? null; + } + + /** Whose cached readings a grant to this group invalidates. */ + async listGroupMemberUuids(groupId: number): Promise { + const rows = (await this.clients.db.read( + 'SELECT u.`uuid` FROM `jct_user_group` ug ' + + 'JOIN `user` u ON u.`id` = ug.`user_id` ' + + 'WHERE ug.`group_id` = ?', + [groupId], + )) as { uuid: string | null }[]; + return rows + .map((r) => r.uuid) + .filter((uuid): uuid is string => Boolean(uuid)); + } + + /** Whose standing access a grant to this group settles. */ + async listGroupMemberIds(groupId: number): Promise { + const rows = (await this.clients.db.read( + 'SELECT `user_id` FROM `jct_user_group` WHERE `group_id` = ?', + [groupId], + )) as { user_id: number }[]; + return rows.map((r) => Number(r.user_id)); + } + + /** `user_id` is the issuer; `group_id` is who receives it. */ + async upsertUserGroupPerm( + groupId: number, + issuerUserId: number, + permission: string, + extra: Record, + ): Promise { + const upsertClause = this.clients.db.upsertClause( + ['user_id', 'group_id', 'permission'], + ['extra'], + ); + await this.clients.db.write( + 'INSERT INTO `user_to_group_permissions` (`user_id`, `group_id`, `permission`, `extra`) ' + + `VALUES (?, ?, ?, ?) ${upsertClause}`, + [ + issuerUserId, + groupId, + permission, + JSON.stringify(extra), + JSON.stringify(extra), + ], + ); + } + + /** Scoped to the issuer: one issuer's revoke must not drop another's. */ + async deleteUserGroupPerm( + groupId: number, + issuerUserId: number, + permission: string, + ): Promise { + const result = await this.clients.db.write( + 'DELETE FROM `user_to_group_permissions` ' + + 'WHERE `group_id` = ? AND `user_id` = ? AND `permission` = ?', + [groupId, issuerUserId, permission], + ); + return result.anyRowsAffected; + } + + async auditUserGroupPerm( + entry: AuditEntry & { + group_id: number; + issuer_user_id: number; + permission: string; + }, + ): Promise { + await this.clients.db.write( + 'INSERT INTO `audit_user_to_group_permissions` (' + + '`user_id`, `user_id_keep`, `group_id`, `group_id_keep`, ' + + '`permission`, `extra`, `action`, `reason`) VALUES (?, ?, ?, ?, ?, ?, ?, ?)', + [ + entry.issuer_user_id, + entry.issuer_user_id, + entry.group_id, + entry.group_id, + entry.permission, + entry.extra ? JSON.stringify(entry.extra) : null, + entry.action, + entry.reason, + ], + ); + } + // -- SQL: access token permissions ------------------------------- async hasAccessTokenPerm( diff --git a/src/backend/testFixtures/twoTeams.ts b/src/backend/testFixtures/twoTeams.ts new file mode 100644 index 000000000..96c281112 --- /dev/null +++ b/src/backend/testFixtures/twoTeams.ts @@ -0,0 +1,170 @@ +/* + * Copyright (C) 2024-present Puter Technologies Inc. + * + * This file is part of Puter. + * + * Puter is free software: you can redistribute it and/or modify + * it under the terms of the GNU Affero General Public License as published + * by the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Affero General Public License for more details. + * + * You should have received a copy of the GNU Affero General Public License + * along with this program. If not, see . + */ + +/** + * Two teams side by side, plus a user in neither: an unscoped query still + * returns the right rows when only one exists, so one gives false confidence. + */ + +import { v4 as uuidv4 } from 'uuid'; +import type { IConfig } from '../types'; +import { setupPuterTestEnv, type PuterTestEnv } from '../testUtil.js'; + +/** An account that can call the API: verified, with a session token. */ +export type FixtureUser = { + userId: number; + username: string; + token: string; +}; + +export type FixtureTeam = { + uid: string; + handle: string; + name: string; + /** Owns the team and pays for it; `org_owned = 0`. */ + owner: FixtureUser; + /** Provisioned seats, activated so they can make requests. */ + seats: FixtureUser[]; +}; + +export type TwoTeams = { + env: PuterTestEnv; + a: FixtureTeam; + b: FixtureTeam; + /** Signed in, in no team at all. */ + outsider: FixtureUser; + call: ( + method: string, + path: string, + token: string, + body?: unknown, + ) => Promise; + shutdown: () => Promise; +}; + +const rand = () => Math.random().toString(36).slice(2, 10); + +/** Seats per team. Two is enough to tell "this one" from "all of them". */ +const SEATS_PER_TEAM = 2; + +/** Own owner per team, so this runs against the real cap. */ +export const setupTwoTeams = async ( + configOverrides: Partial = {}, +): Promise => { + const env = await setupPuterTestEnv({ + teams_enabled: true, + ...configOverrides, + } as IConfig); + + const call = ( + method: string, + path: string, + token: string, + body?: unknown, + ) => + fetch(new URL(path, env.apiOrigin), { + method, + headers: { + 'content-type': 'application/json', + authorization: `Bearer ${token}`, + }, + ...(body === undefined ? {} : { body: JSON.stringify(body) }), + }); + + const tokenFor = async (userId: number): Promise => { + const row = await env.server.stores.user.getById(userId); + const { token } = await env.server.services.auth.createSessionToken( + row!, + ); + return token; + }; + + const makeUser = async (): Promise => { + const username = `fx_${rand()}`; + const created = (await env.server.stores.user.create({ + username, + uuid: uuidv4(), + password: null, + email: `${username}@test.local`, + // `requireVerified` rejects an unconfirmed account outright. + email_confirmed: true, + })) as unknown as { id: number }; + return { + userId: created.id, + username, + token: await tokenFor(created.id), + }; + }; + + /** Provisioning leaves the seat unconfirmed and mid-password-change. */ + const activate = async (username: string): Promise => { + const seat = await env.server.stores.user.getByUsername(username); + await env.server.stores.user.update(seat!.id, { + email_confirmed: 1, + requires_email_confirmation: 0, + requires_password_change: 0, + }); + return { + userId: seat!.id, + username, + token: await tokenFor(seat!.id), + }; + }; + + const expectOk = async (res: Response, what: string) => { + if (res.status !== 200) { + throw new Error( + `fixture: ${what} failed with ${res.status}: ${await res.text()}`, + ); + } + return res; + }; + + const makeTeam = async (name: string): Promise => { + const owner = await makeUser(); + const handle = `ws-${rand()}`; + + const res = await expectOk( + await call('POST', '/teams', owner.token, { name, handle }), + `creating ${name}`, + ); + const team = (await res.json()) as { uid: string }; + + const seats: FixtureUser[] = []; + for (let i = 0; i < SEATS_PER_TEAM; i++) { + const username = `st_${rand()}`; + await expectOk( + await call('POST', `/teams/${team.uid}/members`, owner.token, { + username, + email: `${username}@test.local`, + }), + `provisioning into ${name}`, + ); + seats.push(await activate(username)); + } + + return { uid: team.uid, handle, name, owner, seats }; + }; + + const a = await makeTeam('Team A'); + const b = await makeTeam('Team B'); + const outsider = await makeUser(); + + return { env, a, b, outsider, call, shutdown: () => env.shutdown() }; +}; From da680744013774f05e2879825e986b54b7158469 Mon Sep 17 00:00:00 2001 From: Juan Castro Date: Wed, 2 Sep 2026 19:18:49 -0400 Subject: [PATCH 2/3] feat: share with a team MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Covers PUT-1726, PUT-1727 and PUT-1729. Together because a recipient without the listing work produces a share that is created correctly, resolves correctly, and never appears in "Shared with me" — a state nobody would ship. The recipient. `ShareRecipient` gains `team` (uid) and `teamHandle`, resolved before the email and username branches and never falling through to them. Passing both is an error rather than a precedence rule, so a call site always shows which was chosen: handles are released on soft delete and can be reclaimed by an unrelated team, and a scripted share to a handle would silently retarget. There is no bare-string spelling, since that would change how existing strings are interpreted. ACLService.setUserGroup mirrors setUserUser: same read-modify-write, same one-mode-per-node rule, under a node lock keyed on the group. One grant against the team, not one per member, so membership changes apply without touching the grant and a share spends one unit of the daily quota however many members there are. A member who joins afterwards gets access, which is asserted. The index row carries `holder_group_id` and leaves `holder_user_id` NULL, so `0077`'s group index constrains it rather than the user-holder one. Listing. `listByHolder` and `countByHolder` union the caller's teams into the same keyset page — `ORDER BY id` still holds — and `#grantEvidence` gains group grants as a third source. Without that third source the share is filtered out of every listing as dead: nothing errors, the share simply is not there, which is the one place a user would look for it. Unsharing revokes the grant as well as deleting the index row. Deleting the row alone would hide the share while leaving every member holding real access. Closes PUT-1726, PUT-1727 and PUT-1729. --- .../controllers/share/ShareController.ts | 37 +- src/backend/controllers/share/clientShare.ts | 2 + .../team/TeamController.http.test.ts | 109 ++++ src/backend/services/acl/ACLService.ts | 130 ++++ .../services/permission/PermissionService.ts | 21 +- src/backend/services/share/ShareService.ts | 520 ++++++++++++++- src/backend/services/share/TeamShare.test.ts | 596 ++++++++++++++++++ .../stores/permission/PermissionStore.ts | 68 ++ src/backend/stores/share/ShareStore.js | 181 +++++- src/backend/stores/team/TeamStore.ts | 20 + .../FileSystem/operations/operations.test.js | 49 ++ .../FileSystem/operations/shareUtil.js | 14 + src/puter-js/src/modules/FileSystem/types.js | 10 +- 13 files changed, 1712 insertions(+), 45 deletions(-) create mode 100644 src/backend/services/share/TeamShare.test.ts diff --git a/src/backend/controllers/share/ShareController.ts b/src/backend/controllers/share/ShareController.ts index f9246743b..e914f1bda 100644 --- a/src/backend/controllers/share/ShareController.ts +++ b/src/backend/controllers/share/ShareController.ts @@ -144,7 +144,7 @@ export class ShareController extends PuterController { const results: ShareOutcome[] = await Promise.all( settled.map(async (outcome, index) => { const { recipient, item } = pairs[index]; - const label = recipient.email ?? recipient.username ?? ''; + const label = this.#recipientLabel(recipient); if (outcome.status === 'fulfilled') { // The whole share, not just an acknowledgement, so a caller // needn't re-read to learn what it created. @@ -227,7 +227,7 @@ export class ShareController extends PuterController { ); const results: ShareOutcome[] = settled.map((outcome, index) => { const { recipient, item } = pairs[index]; - const label = recipient.email ?? recipient.username ?? ''; + const label = this.#recipientLabel(recipient); if (outcome.status === 'fulfilled') { return { recipient: label, @@ -554,9 +554,13 @@ export class ShareController extends PuterController { const unique: T[] = []; const seen = new Map(); const indexOf = pairs.map((pair) => { + // Every recipient field, or two teams key alike and collapse + // into one -- the second reporting the first's outcome as its own. const key = JSON.stringify([ pair.recipient.email ?? null, pair.recipient.username ?? null, + pair.recipient.team ?? null, + pair.recipient.teamHandle ?? null, pair.item.uid ?? null, pair.item.path ?? null, ]); @@ -638,6 +642,17 @@ export class ShareController extends PuterController { return value === NO_APP ? null : value; } + /** Echoes back the identifier the caller named, so results are matchable. */ + #recipientLabel(recipient: ShareRecipient): string { + return ( + recipient.email ?? + recipient.username ?? + recipient.teamHandle ?? + recipient.team ?? + '' + ); + } + #recipients(body: Record): ShareRecipient[] { const raw = body.recipients ?? body.recipient; const list = Array.isArray(raw) ? raw : [raw]; @@ -659,6 +674,24 @@ export class ShareController extends PuterController { typeof rec.email === 'string' ? rec.email.trim() : ''; const username = typeof rec.username === 'string' ? rec.username.trim() : ''; + const team = + typeof rec.team === 'string' ? rec.team.trim() : ''; + const teamHandle = + typeof rec.teamHandle === 'string' + ? rec.teamHandle.trim() + : ''; + // Ambiguous rather than a precedence rule to memorise. + if (team && teamHandle) { + throw new HttpError( + 400, + 'pass `team` or `teamHandle`, not both', + { legacyCode: 'bad_request' }, + ); + } + if (team || teamHandle) { + out.push(team ? { team } : { teamHandle }); + continue; + } if (email || username) { out.push(email ? { email } : { username }); } diff --git a/src/backend/controllers/share/clientShare.ts b/src/backend/controllers/share/clientShare.ts index 8f0e813df..b26137acd 100644 --- a/src/backend/controllers/share/clientShare.ts +++ b/src/backend/controllers/share/clientShare.ts @@ -63,6 +63,8 @@ export async function toClientShare( is_dir: share.isDir, issuer: share.issuer.username, holder: share.holder.username, + // Named, or a team share reads as a share with nobody. + ...(share.holderTeam ? { holder_team: share.holderTeam } : {}), created_at: share.createdAt, issued_by_app: share.issuedByApp ?? null, inherited_from: share.inheritedFrom ?? null, diff --git a/src/backend/controllers/team/TeamController.http.test.ts b/src/backend/controllers/team/TeamController.http.test.ts index ba084c71e..76cc916ed 100644 --- a/src/backend/controllers/team/TeamController.http.test.ts +++ b/src/backend/controllers/team/TeamController.http.test.ts @@ -236,6 +236,115 @@ describe('team endpoints over HTTP', () => { ); expect(theirs.status).toBe(404); }); + + // -- sharing with a team ------------------------------------- + + it('names the team it shared with instead of an empty recipient', async () => { + const { team } = await makeTeam(); + const file = `/${env.users.user.username}/Documents/label-${Math.random() + .toString(36) + .slice(2, 8)}.txt`; + const written = await call('POST', '/fs/write', env.users.user.token, { + fileMetadata: { path: file, size: 3, contentType: 'text/plain' }, + fileContent: 'abc', + }); + expect(written.status).toBe(200); + + const shared = await call('POST', '/share', env.users.user.token, { + recipients: [{ team: team.uid }], + items: [file], + mode: 'read', + }); + expect(shared.status).toBe(200); + const body = (await shared.json()) as { + results: { recipient: string; status: string }[]; + }; + + expect(body.results[0].status).toBe('success'); + // Echoes the identifier the caller named; '' would leave a client with + // nothing to render or to match its request against. + expect(body.results[0].recipient).toBe(team.uid); + }); + + it('shares with two teams rather than collapsing them into one', async () => { + const second = await call('POST', '/teams', env.users.user.token, { + name: 'Second', + handle: randomHandle(), + }); + expect(second.status).toBe(200); + const teamB = (await second.json()) as { uid: string }; + const { team: teamA } = await makeTeam(); + + const file = `/${env.users.user.username}/Documents/two-${Math.random() + .toString(36) + .slice(2, 8)}.txt`; + const written = await call('POST', '/fs/write', env.users.user.token, { + fileMetadata: { path: file, size: 3, contentType: 'text/plain' }, + fileContent: 'abc', + }); + const fileUid = ((await written.json()) as { fsEntry: { uid: string } }) + .fsEntry.uid; + + const shared = await call('POST', '/share', env.users.user.token, { + recipients: [{ team: teamA.uid }, { team: teamB.uid }], + items: [file], + mode: 'read', + }); + expect(shared.status).toBe(200); + const body = (await shared.json()) as { + results: { recipient: string; status: string }[]; + }; + + expect(body.results).toHaveLength(2); + expect(body.results.every((r) => r.status === 'success')).toBe(true); + + // The response shape alone proves nothing: `results` is built from the + // request's pairs, so a collapsed pair still reports two successes with + // the right labels. Only the shares that exist afterwards show it. + const mine = await call( + 'GET', + '/share/shared-by-me?limit=100', + env.users.user.token, + ); + expect(mine.status).toBe(200); + const listing = (await mine.json()) as { + items: { uid_entry?: string; holder_team?: { uid: string } }[]; + }; + const holders = listing.items + .filter((i) => i.uid_entry === fileUid) + .map((i) => i.holder_team?.uid) + .filter(Boolean) + .sort(); + expect(holders).toEqual([teamA.uid, teamB.uid].sort()); + }); + + it('names the team by handle when the caller shared by handle', async () => { + const handle = randomHandle(); + const res = await call('POST', '/teams', env.users.user.token, { + name: 'Byhandle', + handle, + }); + expect(res.status).toBe(200); + + const file = `/${env.users.user.username}/Documents/handle-${Math.random() + .toString(36) + .slice(2, 8)}.txt`; + await call('POST', '/fs/write', env.users.user.token, { + fileMetadata: { path: file, size: 3, contentType: 'text/plain' }, + fileContent: 'abc', + }); + + const shared = await call('POST', '/share', env.users.user.token, { + recipients: [{ teamHandle: handle }], + items: [file], + mode: 'read', + }); + expect(shared.status).toBe(200); + const body = (await shared.json()) as { + results: { recipient: string }[]; + }; + expect(body.results[0].recipient).toBe(handle); + }); }); describe('team endpoints with teams_enabled off', () => { diff --git a/src/backend/services/acl/ACLService.ts b/src/backend/services/acl/ACLService.ts index 87031b23b..89017f914 100644 --- a/src/backend/services/acl/ACLService.ts +++ b/src/backend/services/acl/ACLService.ts @@ -280,6 +280,136 @@ export class ACLService extends PuterService { * * Caller (controller) validates that both actors are user-type. */ + // -- Group holders ---- one grant, resolved per member at scan time ---- + + /** The group analogue of `statUserUser`; no holder actor to validate. */ + async statUserGroup( + issuer: Actor, + groupUid: string, + resource: ResourceDescriptor, + ): Promise { + if (issuer.app || issuer.accessToken) + throw new HttpError(403, 'issuer must be a user actor', { + legacyCode: 'forbidden', + }); + + const out: StatPermissionsResult = {}; + const ancestors = await resource.resolveAncestors(); + for (const ancestor of ancestors) { + // Both namespaces: `manage:fs:` sits outside `fs:`. + const prefixes = [ + PermissionUtil.join('fs', ancestor.uid), + PermissionUtil.join(MANAGE_PERM_PREFIX, 'fs', ancestor.uid), + ]; + const perms = ( + await Promise.all( + prefixes.map((prefix) => + this.services.permission.queryIssuerGroupPermissionsByPrefix( + issuer, + groupUid, + prefix, + ), + ), + ) + ).flat(); + if (perms.length > 0) out[ancestor.path] = perms; + } + return out; + } + + /** Same read-modify-write and one-mode-per-node rule as `setUserUser`. */ + async setUserGroup( + issuer: Actor, + groupUid: string, + resource: ResourceDescriptor, + mode: AclMode, + options: { onlyIfHigher?: boolean } = {}, + ): Promise { + if (issuer.app || issuer.accessToken) + throw new HttpError(403, 'issuer must be a user actor', { + legacyCode: 'forbidden', + }); + + const ancestors = await resource.resolveAncestors(); + const self = ancestors[0]; + if (!self) + throw new HttpError( + 400, + 'resource has no ancestor chain (is it root?)', + { legacyCode: 'bad_request' }, + ); + + return this.#withNodeLock( + `${issuer.user.id}:group:${groupUid}:${self.uid}`, + () => + this.#setUserGroupLocked( + issuer, + groupUid, + resource, + mode, + self.uid, + options, + ), + ); + } + + async #setUserGroupLocked( + issuer: Actor, + groupUid: string, + resource: ResourceDescriptor, + mode: AclMode, + uid: string, + options: { onlyIfHigher?: boolean } = {}, + ): Promise { + const stat = await this.statUserGroup(issuer, groupUid, resource); + const existing = stat[resource.path] ?? []; + + const existingModes = existing.map((p) => + PermissionUtil.isManage(p) + ? MANAGE_PERM_PREFIX + : PermissionUtil.split(p).at(-1), + ); + + if (existingModes.includes(mode)) return false; + + if (options.onlyIfHigher) { + const higher = MODES_ABOVE[mode] ?? [mode]; + if ( + existingModes.some( + (m) => + m === MANAGE_PERM_PREFIX || + (m && higher.includes(m as AclMode)), + ) + ) { + return false; + } + } + + const newPerm = + mode === MANAGE_PERM_PREFIX + ? PermissionUtil.join(MANAGE_PERM_PREFIX, 'fs', uid) + : PermissionUtil.join('fs', uid, mode); + await this.services.permission.grantUserGroupPermission( + issuer, + groupUid, + newPerm, + ); + + // One mode per node per issuer/holder — higher modes supersede lower. + for (const perm of existing) { + const existingMode = PermissionUtil.isManage(perm) + ? MANAGE_PERM_PREFIX + : PermissionUtil.split(perm).at(-1); + if (existingMode === mode) continue; + await this.services.permission.revokeUserGroupPermission( + issuer, + groupUid, + perm, + ); + } + return true; + } + async statUserUser( issuer: Actor, holder: Actor, diff --git a/src/backend/services/permission/PermissionService.ts b/src/backend/services/permission/PermissionService.ts index 00b1d9c9f..4288c199b 100644 --- a/src/backend/services/permission/PermissionService.ts +++ b/src/backend/services/permission/PermissionService.ts @@ -908,6 +908,22 @@ export class PermissionService extends PuterService { ); } + /** The group analogue of `queryIssuerHolderPermissionsByPrefix`. */ + async queryIssuerGroupPermissionsByPrefix( + issuer: Actor, + groupUid: string, + prefix: string, + ): Promise { + if (!issuer.user?.id) return []; + const groupId = await this.stores.permission.resolveGroupId(groupUid); + if (groupId === null) return []; + return this.stores.permission.queryIssuerGroupPermsByPrefix( + issuer.user.id, + groupId, + prefix, + ); + } + async grantUserGroupPermission( actor: Actor, groupUid: string, @@ -969,6 +985,7 @@ export class PermissionService extends PuterService { groupUid: string, permission: string, meta: GrantMeta = {}, + opts: { issuerUserId?: number } = {}, ): Promise { // Same rewrite as the grant, or this matches nothing and says it did. permission = await this.rewritePermission(permission); @@ -979,7 +996,9 @@ export class PermissionService extends PuterService { legacyCode: 'forbidden', }); } - const issuerId = actor.user.id; + // Whose grant to clear; authority still comes from `actor`, so an owner + // can withdraw a delegate's grant without impersonating them. + const issuerId = opts.issuerUserId ?? actor.user.id; if (!(await this.canManagePermission(actor, permission))) { throw new HttpError(403, `permission_denied: ${permission}`, { diff --git a/src/backend/services/share/ShareService.ts b/src/backend/services/share/ShareService.ts index fcfcb5148..80eb5c757 100644 --- a/src/backend/services/share/ShareService.ts +++ b/src/backend/services/share/ShareService.ts @@ -30,6 +30,7 @@ import { } from '../../util/email.js'; import type { FSEntry } from '../../stores/fs/FSEntry'; import type { UserUserAuditFilter } from '../../stores/permission/PermissionStore'; +import { MEMBER_PAGE_CAP, type TeamRow } from '../../stores/team/TeamStore'; import type { UserRow } from '../../stores/user/UserStore'; import type { AclMode } from '../acl/ACLService'; import { @@ -38,14 +39,21 @@ import { resolveSharePath, } from '../fs/sharePathMask'; import { MANAGE_PERM_PREFIX } from '../permission/consts'; +import { PermissionUtil } from '../permission/permissionUtil.js'; import { PuterService } from '../types'; // -- Types ------------------------------------------------------------ -/** A recipient named by whichever identifier the caller had. */ +/** + * A recipient named by whichever identifier the caller had. `teamHandle` is + * separate from `team` so the call site shows a handle was used: handles are + * released on soft delete and can be reclaimed by another team. + */ export interface ShareRecipient { email?: string; username?: string; + team?: string; + teamHandle?: string; } export interface ShareTarget { @@ -70,6 +78,8 @@ interface ShareIndexRow { */ interface OutboundShareRow extends Omit { holder_user_id: number | null; + /** Set instead of `holder_user_id` when the holder is a team. */ + holder_group_id?: number | null; recipient_email?: string; } @@ -82,6 +92,8 @@ export interface ShareInput extends ShareTarget { interface GrantEvidence { /** Issuers with a live, attributable grant. */ issuers: Set; + /** Issuers reaching via a group; they back no user-to-user row. */ + groupIssuers: Set; /** A live grant that names no issuer — a legacy flat entry. */ unattributed: boolean; /** The holder owns the entry outright, so no grant is needed. */ @@ -146,6 +158,10 @@ export interface ResolvedShare { size: number | null; /** Set by `share()` only: who to notify. Never sent to a client. */ holderId?: number; + /** The team this went to, when the recipient was one. */ + holderTeam?: { uid: string; name: string | null; handle: string | null }; + /** Internal group id; the notification fan-out reads it. Never sent out. */ + holderGroupId?: number; /** Whether this call created reach that didn't exist before. */ isNew?: boolean; /** @@ -255,7 +271,7 @@ const EMAIL_SHAPE = /^[^\s@]+@[^\s@]+\.[^\s@]+$/u; const BLOCK_ALL_SHARES_KEY = 'blockAllShares'; /** Whether this account refuses shares from everyone. */ -const blocksAllShares = (user: Pick | null): boolean => +export const blocksAllShares = (user: Pick | null): boolean => Boolean(user?.metadata?.[BLOCK_ALL_SHARES_KEY]); /** @@ -819,10 +835,12 @@ export class ShareService extends PuterService { .filter((node) => typeof node.id === 'number') .map((node) => [node.id, node]), ); - return { - rows: await this.stores.share.listReaching([...nodesById.keys()]), - nodesById, - }; + const ids = [...nodesById.keys()]; + const [direct, viaGroup] = await Promise.all([ + this.stores.share.listReaching(ids), + this.stores.share.listGroupReachingMembers(ids), + ]); + return { rows: [...direct, ...viaGroup], nodesById }; } async #emitGui( @@ -865,6 +883,22 @@ export class ShareService extends PuterService { if (resolved.kind === 'pending') { return this.#invite(actor, issuerId, entry, resolved.email, mode); } + if (resolved.kind === 'team') { + // Handles are public, so without this anyone could push files into + // every member's inbox by naming one. + if (!(await this.stores.team.isMember(resolved.team.uid, issuerId))) { + throw new HttpError(404, 'Team does not exist', { + legacyCode: 'team_not_found', + }); + } + return this.#shareWithTeam( + actor, + issuerId, + entry, + resolved.team, + mode, + ); + } const holder = resolved.user; if (holder.id === issuerId) { @@ -996,12 +1030,17 @@ export class ShareService extends PuterService { permission, })), ); - const [linked, flat] = await Promise.all([ + const [linked, flat, viaGroup] = await Promise.all([ this.stores.permission.readLinkedUserUserPermsForHolders( refs.map((ref) => ref.holderUserId), refs.map((ref) => ref.permission), ), this.stores.permission.getFlatUserPermsForRefs(refs), + // Third source, or a team share is listed as dead. + this.stores.permission.readUserGroupPermsForHolders( + refs.map((ref) => ref.holderUserId), + refs.map((ref) => ref.permission), + ), ]); // Both reads folded onto (holder, permission); the linked read spans @@ -1009,21 +1048,28 @@ export class ShareService extends PuterService { // they simply go unread below. const byHolderPerm = new Map< string, - { issuers: Set; unattributed: boolean } + { + issuers: Set; + groupIssuers: Set; + unattributed: boolean; + } >(); const record = ( holderId: number, permission: string, issuer: unknown, + viaGroup = false, ) => { const key = `${holderId}:${permission}`; const found = byHolderPerm.get(key) ?? { issuers: new Set(), + groupIssuers: new Set(), unattributed: false, }; const issuerId = Number(issuer); - if (Number.isFinite(issuerId)) found.issuers.add(issuerId); - else found.unattributed = true; + if (!Number.isFinite(issuerId)) found.unattributed = true; + else if (viaGroup) found.groupIssuers.add(issuerId); + else found.issuers.add(issuerId); byHolderPerm.set(key, found); }; for (const row of linked) { @@ -1037,10 +1083,21 @@ export class ShareService extends PuterService { if (value.deleted) continue; record(ref.holderUserId, ref.permission, value.issuer_user_id); } + // Kept apart: a group grant from this issuer does not make their + // withdrawn user-to-user share live again. + for (const row of viaGroup) { + record( + Number(row.holder_user_id), + row.permission, + row.user_id, + true, + ); + } for (const [key, { holderId, entry }] of unique) { const merged: GrantEvidence = { issuers: new Set(), + groupIssuers: new Set(), unattributed: false, owned: entry.userId === holderId, }; @@ -1048,6 +1105,8 @@ export class ShareService extends PuterService { const found = byHolderPerm.get(`${holderId}:${permission}`); if (!found) continue; for (const issuer of found.issuers) merged.issuers.add(issuer); + for (const issuer of found.groupIssuers) + merged.groupIssuers.add(issuer); merged.unattributed ||= found.unattributed; } evidence.set(key, merged); @@ -1067,7 +1126,14 @@ export class ShareService extends PuterService { for (const entry of entries) { const found = evidence.get(`${holderId}:${entry.id}`); if (!found) continue; - if (found.owned || found.unattributed || found.issuers.size > 0) { + // Group reach counts here: the question is whether they reach it at + // all, not whose row backs it. + if ( + found.owned || + found.unattributed || + found.issuers.size > 0 || + found.groupIssuers.size > 0 + ) { live.add(entry.uuid); } } @@ -1106,7 +1172,14 @@ export class ShareService extends PuterService { ); const live = new Set(); for (const [key, found] of evidence) { - if (found.owned || found.unattributed || found.issuers.size > 0) { + // Reach is reach, however it arrived -- unlike `#reachingGrants`, + // which asks whose grant backs one row. + if ( + found.owned || + found.unattributed || + found.issuers.size > 0 || + found.groupIssuers.size > 0 + ) { live.add(key); } } @@ -1169,6 +1242,10 @@ export class ShareService extends PuterService { await this.#assertCanManage(actor, entry); return this.#cancelInvite(entry, resolved.email, issuerId); } + if (resolved.kind === 'team') { + await this.#assertCanManage(actor, entry); + return this.#unshareTeam(actor, issuerId, entry, resolved.team); + } const holder = resolved.user; // Dropping your own access needs no authority over the node — only @@ -1377,11 +1454,31 @@ export class ShareService extends PuterService { ...new Set(removed.map((row) => Number(row.holder_user_id))), ].filter((id) => Number.isFinite(id)); const holders = await this.stores.user.getByIds(holderIds); - await this.stores.permission.bumpCacheGenerations( - [...holders.values()] - .filter((user) => user.uuid) - .map((user) => `user:${user.uuid}`), - ); + + // Group grants on the same node, and their members' caches: without + // this they outlive the file and keep answering "allowed". + const removedGroups = + await this.stores.permission.deleteUserGroupPermsByPermissionPrefixes( + uids.flatMap((uid) => [`fs:${uid}`, `manage:fs:${uid}`]), + ); + const memberUuids = ( + await Promise.all( + [...new Set(removedGroups.map((row) => Number(row.group_id)))] + .filter((id) => Number.isFinite(id)) + .map((id) => + this.stores.permission.listGroupMemberUuids(id), + ), + ) + ).flat(); + + await this.stores.permission.bumpCacheGenerations([ + ...new Set([ + ...[...holders.values()] + .filter((user) => user.uuid) + .map((user) => `user:${user.uuid}`), + ...memberUuids.map((uuid) => `user:${uuid}`), + ]), + ]); return removed; } @@ -1461,9 +1558,12 @@ export class ShareService extends PuterService { total?: number; }> { const holderId = this.#requireUserId(actor); + // Resolved once; team shares ride the caller's own page. + const groupIds = await this.stores.team.listGroupIdsForUser(holderId); const page = await this.stores.share.listByHolder(holderId, { limit: opts.limit, cursor: opts.cursor, + groupIds, }); const entries = await this.stores.fsEntry.getEntriesByIds( @@ -1510,7 +1610,11 @@ export class ShareService extends PuterService { items, ...(page.cursor ? { cursor: page.cursor } : {}), ...(opts.includeTotal - ? { total: await this.stores.share.countByHolder(holderId) } + ? { + total: await this.stores.share.countByHolder(holderId, { + groupIds, + }), + } : {}), }; } @@ -1570,6 +1674,24 @@ export class ShareService extends PuterService { const nodeById = new Map( [...entries.values()].map((entry) => [entry.id, entry]), ); + // Named so the listing can say which team, not just that it is one. + const teamsById = new Map( + ( + await Promise.all( + [ + ...new Set( + rows + .filter((row) => row.holder_group_id) + .map((row) => Number(row.holder_group_id)), + ), + ].map((id) => + this.stores.team.getByIdIncludingDeleted(id), + ), + ) + ) + .filter((team): team is TeamRow => team !== null) + .map((team) => [team.id, team]), + ); // Three bounds. A claimed row needs the grant *this issuer* made to // still be there — on the pair alone, a grant withdrawn outside // `unshare` stays listed while anyone else grants the same holder the @@ -1577,24 +1699,33 @@ export class ShareService extends PuterService { // would grant, or a revoked delegate keeps reading the entry's name // and size out of invites that can never be claimed. And an app sees // only the part of any of it the credential reaches in its own right. - const [stillReaches, pendingAllowed, reachable] = await Promise.all([ - this.#reachingGrants(rows, nodeById), - this.#pendingStillAuthorized( - rows.filter((row) => !row.holder_user_id), - nodeById, - users, - ), - this.#reachableBy(actor, [...entries.values()]), - ]); + const [stillReaches, pendingAllowed, reachable, liveGroupGrants] = + await Promise.all([ + this.#reachingGrants(rows, nodeById), + this.#pendingStillAuthorized( + // Group rows have no holder user but are not invites. + rows.filter( + (row) => !row.holder_user_id && !row.holder_group_id, + ), + nodeById, + users, + ), + this.#reachableBy(actor, [...entries.values()]), + this.#liveGroupGrants(rows, nodeById), + ]); const items: ResolvedShare[] = []; for (const row of rows) { const entry = entries.get(Number(row.fsentry_id)); if (!entry) continue; if (!reachable.has(entry.uuid)) continue; - const pending = !row.holder_user_id; + // A group row has no holder user either, and is not an invite. + const pending = !row.holder_user_id && !row.holder_group_id; if (pending && !pendingAllowed.has(row.uid)) continue; - if ( + // Its liveness is the grant, not a holder's reach. + if (row.holder_group_id) { + if (!liveGroupGrants.has(row.uid)) continue; + } else if ( !pending && !stillReaches.has( `${Number(row.holder_user_id)}:${entry.id}:${Number(row.issuer_user_id)}`, @@ -1602,8 +1733,22 @@ export class ShareService extends PuterService { ) { continue; } + const team = row.holder_group_id + ? teamsById.get(Number(row.holder_group_id)) + : undefined; items.push( - this.#resolvedShareRow(row, entry, users, { entryMeta: true }), + this.#resolvedShareRow(row, entry, users, { + entryMeta: true, + ...(team + ? { + holderTeam: { + uid: team.uid, + name: team.name, + handle: team.handle, + }, + } + : {}), + }), ); } @@ -1719,6 +1864,22 @@ export class ShareService extends PuterService { } if (!(await this.#hasOwnReach(actor, entry, 'see'))) throw notFound(); + // Not an invite: deleting the row alone leaves every member's access. + if (row.holder_group_id) { + const team = await this.stores.team.getByIdIncludingDeleted( + Number(row.holder_group_id), + ); + if (!team) throw notFound(); + await this.#assertCanManage(actor, entry); + return this.#unshareTeam( + actor, + userId, + entry, + team, + Number(row.issuer_user_id), + ); + } + // An invite is just its row — including one whose address has since // been registered but never claimed: no holder, no grant, so // recipient-addressed revocation would never find it. @@ -1881,6 +2042,14 @@ export class ShareService extends PuterService { const pendingRows = await this.stores.share.listPendingOnFsentry( entry.id, ); + // Ancestors too: a team can reach this through a folder above it. + const groupRows = ( + await Promise.all( + [entry.id, ...viaById.keys()].map((id) => + this.stores.share.listGroupOnFsentry(id), + ), + ) + ).flat(); const userIds = [ ...[...rows, ...inherited.map((i) => i.row)].flatMap( (row: { issuer_user_id: number; holder_user_id: number }) => [ @@ -1891,6 +2060,9 @@ export class ShareService extends PuterService { ...pendingRows.map((row: { issuer_user_id: number }) => Number(row.issuer_user_id), ), + ...groupRows.map((row: { issuer_user_id: number }) => + Number(row.issuer_user_id), + ), ]; const users = await this.stores.user.getByIds(userIds); const maskedPath = maskEntryPath(entry); @@ -1934,7 +2106,43 @@ export class ShareService extends PuterService { }), ); - return inheritedShares.concat(own, pending); + // The team itself, so whoever manages the node can see it is shared + // with one and take it back from here. + const liveGroup = await this.#liveGroupGrants(groupRows, nodeById); + const teamsById = new Map( + ( + await Promise.all( + [ + ...new Set( + groupRows.map((row) => + Number(row.holder_group_id), + ), + ), + ].map((id) => this.stores.team.getByIdIncludingDeleted(id)), + ) + ) + .filter((team): team is TeamRow => team !== null) + .map((team) => [team.id, team]), + ); + const groups: ResolvedShare[] = groupRows + .filter((row) => liveGroup.has(String(row.uid))) + .map((row: OutboundShareRow) => { + const team = teamsById.get(Number(row.holder_group_id)); + return this.#resolvedShareRow(row, entry, users, { + path: maskedPath, + ...(team + ? { + holderTeam: { + uid: team.uid, + name: team.name, + handle: team.handle, + }, + } + : {}), + }); + }); + + return inheritedShares.concat(own, groups, pending); } /** Whether each of the caller's own `entries` is shared, keyed by uuid. */ @@ -2259,6 +2467,178 @@ export class ShareService extends PuterService { * Spends daily quota: an invite is reach the issuer is handing out, and * exempting it would make the limit optional. */ + /** Whether this issuer already grants this team anything here. */ + async #hasGroupGrantFrom( + entry: FSEntry, + groupUid: string, + issuer: Actor, + ): Promise { + const perms = await Promise.all( + [ + PermissionUtil.join('fs', entry.uuid), + PermissionUtil.join(MANAGE_PERM_PREFIX, 'fs', entry.uuid), + ].map((prefix) => + this.services.permission.queryIssuerGroupPermissionsByPrefix( + issuer, + groupUid, + prefix, + ), + ), + ); + return perms.flat().length > 0; + } + + /** One grant, resolved per member at scan time; one unit of quota. */ + async #shareWithTeam( + actor: Actor, + issuerId: number, + entry: FSEntry, + team: TeamRow, + mode: AclMode, + ): Promise { + // No self / owner check: a team is not a person. + const userActor = userRelatedActor(actor); + const hadAccess = await this.#hasGroupGrantFrom( + entry, + team.uid, + userActor, + ); + const releaseQuota = hadAccess + ? null + : await this.#reserveDailyQuota(issuerId); + + try { + await this.services.acl.setUserGroup( + userActor, + team.uid, + this.#descriptorFor(entry), + mode, + ); + + const row = await this.stores.share.upsertActiveGroup({ + issuerUserId: issuerId, + holderGroupId: team.id, + fsentryId: entry.id, + mode, + issuerAppUid: this.#actingAppUid(actor), + }); + return { + ...this.#resolve(row, entry, actor, { username: null }), + holderTeam: { + uid: team.uid, + name: team.name ?? null, + handle: team.handle ?? null, + }, + holderGroupId: team.id, + isNew: !hadAccess, + }; + } catch (err) { + await releaseQuota?.(); + // Undo only reach this call created, as the user path does. + if (!hadAccess) { + try { + // `manage` is written prefix-first; the naive join misses it. + await this.services.permission.revokeUserGroupPermission( + userActor, + team.uid, + entryPermissionForMode(entry.uuid, mode), + ); + } catch { + // Best effort; the throw below is what the caller sees. + } + } + throw err; + } + } + + /** An owner clears any issuer's grant; anyone else only their own. */ + async #unshareTeam( + actor: Actor, + issuerId: number, + entry: FSEntry, + team: TeamRow, + onlyIssuer?: number, + ): Promise<{ revoked: number }> { + const isOwner = entry.userId === issuerId; + const issuers = onlyIssuer !== undefined + ? [onlyIssuer] + : isOwner + ? [ + ...new Set( + ( + await this.stores.share.listGroupSharesByFsentry( + entry.id, + ) + ) + .filter( + (row: { holder_group_id: number }) => + Number(row.holder_group_id) === team.id, + ) + .map((row: { issuer_user_id: number }) => + Number(row.issuer_user_id), + ), + ), + issuerId, + ] + : [issuerId]; + + // Authority is the caller's throughout, as on the user-to-user path: + // impersonating a delegate who has since lost it throws 403 and aborts + // the whole unshare, including the caller's own grant. + const me = userRelatedActor(actor); + + // Whatever a member re-shared goes with them, as on the user path, and + // first: clearing the group grant would strip the `manage` it needs. + let revoked = 0; + const members = await this.stores.team.listMembers(team.uid, { + limit: MEMBER_PAGE_CAP, + }); + const issuerSet = new Set(issuers); + for (const member of members.items) { + const memberId = Number(member.user_id); + // The owner's own grants do not derive from this one, and an + // issuer's are handled by the revoke loop below. + if (memberId === entry.userId || issuerSet.has(memberId)) continue; + revoked += await this.#revokeDownstream(me, entry, memberId); + } + + const permissions = entryPermissions(entry.uuid); + const manageable = await Promise.all( + permissions.map((permission) => + this.services.permission.canManagePermission(me, permission), + ), + ); + + for (const id of new Set(issuers)) { + for (let i = 0; i < permissions.length; i++) { + if (!manageable[i]) continue; + if ( + await this.services.permission.revokeUserGroupPermission( + me, + team.uid, + permissions[i], + { reason: 'unshared' }, + { issuerUserId: id }, + ) + ) { + revoked++; + } + } + await this.stores.share.deleteActiveGroup({ + holderGroupId: team.id, + fsentryId: entry.id, + issuerUserId: id, + }); + } + return { revoked }; + } + + /** An actor for another issuer, so an owner can clear their grant. */ + async #issuerActor(userId: number): Promise { + const user = await this.stores.user.getById(userId); + return user ? this.#actorFor(user) : null; + } + async #invite( actor: Actor, issuerId: number, @@ -2343,11 +2723,13 @@ export class ShareService extends PuterService { entryMeta?: boolean; provenance?: boolean; holderUsername?: string | null; + holderTeam?: { uid: string; name: string | null; handle: string | null }; via?: string | null; path?: string; } = {}, ): ResolvedShare { - const pending = !row.holder_user_id; + // A group row has no holder user and is not an invite. + const pending = !row.holder_user_id && !row.holder_group_id; return { uid: String(row.uid), mode: String(row.mode), @@ -2388,6 +2770,7 @@ export class ShareService extends PuterService { ?.invitedAddress ?? row.recipient_email, } : {}), + ...(opts.holderTeam ? { holderTeam: opts.holderTeam } : {}), createdAt: row.created_at, ...(opts.provenance === false ? {} @@ -2409,6 +2792,43 @@ export class ShareService extends PuterService { * The owner's invites are theirs by definition; only a delegate's cost a * check, and those are rare on any page. */ + /** + * Group rows on this page whose grant is still there. The index row and the + * grant are separate writes, so a row can outlive what it records. + */ + async #liveGroupGrants( + rows: OutboundShareRow[], + nodeById: Map, + ): Promise> { + const live = new Set(); + const groupRows = rows.filter((row) => row.holder_group_id); + if (groupRows.length === 0) return live; + + await Promise.all( + groupRows.map(async (row) => { + const entry = nodeById.get(Number(row.fsentry_id)); + if (!entry) return; + const prefixes = [ + PermissionUtil.join('fs', entry.uuid), + PermissionUtil.join(MANAGE_PERM_PREFIX, 'fs', entry.uuid), + ]; + for (const prefix of prefixes) { + const found = + await this.stores.permission.queryIssuerGroupPermsByPrefix( + Number(row.issuer_user_id), + Number(row.holder_group_id), + prefix, + ); + if (found.length > 0) { + live.add(row.uid); + return; + } + } + }), + ); + return live; + } + async #pendingStillAuthorized( rows: OutboundShareRow[], nodeById: Map, @@ -2511,11 +2931,45 @@ export class ShareService extends PuterService { * Who the share is for. An unconfirmed address resolves to an invite rather * than a failure; a username cannot be invited, there is nothing to reach. */ + /** + * Null when the caller named no team; throws when they named a bad + * one. + */ + async #resolveTeamRecipient( + recipient: ShareRecipient, + ): Promise { + const uid = recipient?.team?.trim(); + const handle = recipient?.teamHandle?.trim(); + if (!uid && !handle) return null; + if (uid && handle) { + throw new HttpError(400, 'pass `team` or `teamHandle`, not both', { + legacyCode: 'bad_request', + }); + } + + const team = uid + ? await this.stores.team.getByUid(uid) + : // Filters soft-deleted, so a released handle 404s until reclaimed. + await this.stores.team.getByHandle(handle!); + if (!team) { + throw new HttpError(404, 'Team does not exist', { + legacyCode: 'team_not_found', + }); + } + return team; + } + async #resolveRecipient( recipient: ShareRecipient, ): Promise< - { kind: 'user'; user: UserRow } | { kind: 'pending'; email: string } + | { kind: 'user'; user: UserRow } + | { kind: 'pending'; email: string } + | { kind: 'team'; team: TeamRow } > { + // Before email and username, and never falling through to them. + const team = await this.#resolveTeamRecipient(recipient); + if (team) return { kind: 'team', team }; + const email = recipient?.email?.trim(); const username = recipient?.username?.trim(); // Case and provider aliases resolve to the account; see #addressOwner. diff --git a/src/backend/services/share/TeamShare.test.ts b/src/backend/services/share/TeamShare.test.ts new file mode 100644 index 000000000..5f1b0a212 --- /dev/null +++ b/src/backend/services/share/TeamShare.test.ts @@ -0,0 +1,596 @@ +/** + * Copyright (C) 2024-present Puter Technologies Inc. + * + * This file is part of Puter. + * + * Puter is free software: you can redistribute it and/or modify + * it under the terms of the GNU Affero General Public License as published + * by the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Affero General Public License for more details. + * + * You should have received a copy of the GNU Affero General Public License + * along with this program. If not, see . + */ + +import { afterAll, beforeAll, describe, expect, it } from 'vitest'; +import type { Actor } from '../../core/actor'; +import { + setupTwoTeams, + type TwoTeams, +} from '../../testFixtures/twoTeams.js'; + +describe('sharing with a team', () => { + let fx: TwoTeams; + + const actorFor = async (userId: number): Promise => { + const user = await fx.env.server.stores.user.getById(userId); + return { user } as unknown as Actor; + }; + + const shares = () => fx.env.server.services.share; + + /** A file the owner of team A owns, ready to be shared. */ + const makeFile = async (ownerId: number) => { + const uid = crypto.randomUUID(); + const name = `f_${uid.slice(0, 8)}.txt`; + const owner = await fx.env.server.stores.user.getById(ownerId); + const path = `/${owner!.username}/${name}`; + await fx.env.server.clients.db.write( + 'INSERT INTO `fsentries` (`uuid`, `name`, `path`, `user_id`, `is_dir`, `modified`) ' + + 'VALUES (?, ?, ?, ?, ?, ?)', + [ + uid, + name, + path, + ownerId, + fx.env.server.clients.db.booleanValue(false), + Math.floor(Date.now() / 1000), + ], + ); + return { path, uid }; + }; + + /** A directory and a file inside it, both as real fsentry rows. */ + const makeNestedFile = async (ownerId: number) => { + const owner = await fx.env.server.stores.user.getById(ownerId); + const dirUid = crypto.randomUUID(); + const dirName = `d_${dirUid.slice(0, 8)}`; + const dirPath = `/${owner!.username}/${dirName}`; + const insert = (uid: string, name: string, path: string, isDir: boolean) => + fx.env.server.clients.db.write( + 'INSERT INTO `fsentries` (`uuid`, `name`, `path`, `user_id`, `is_dir`, `modified`) ' + + 'VALUES (?, ?, ?, ?, ?, ?)', + [ + uid, + name, + path, + ownerId, + fx.env.server.clients.db.booleanValue(isDir), + Math.floor(Date.now() / 1000), + ], + ); + await insert(dirUid, dirName, dirPath, true); + const fileUid = crypto.randomUUID(); + const fileName = `f_${fileUid.slice(0, 8)}.txt`; + await insert(fileUid, fileName, `${dirPath}/${fileName}`, false); + return { dirPath, dirUid, fileUid }; + }; + + const shareWithTeam = async ( + ownerId: number, + path: string, + recipient: Record, + mode = 'read', + ) => + shares().share(await actorFor(ownerId), { + path, + recipient, + mode, + } as never); + + const inbox = async (userId: number) => + shares().listSharedWithMe(await actorFor(userId), { limit: 100 }); + + beforeAll(async () => { + fx = await setupTwoTeams(); + }, 180_000); + + afterAll(async () => { + await fx?.shutdown(); + }); + + // -- the recipient ------------------------------------------------- + + it('shares with a team addressed by uid', async () => { + const file = await makeFile(fx.a.owner.userId); + + const res = await shareWithTeam(fx.a.owner.userId, file.path, { + team: fx.a.uid, + }); + + expect(res.holderTeam?.uid).toBe(fx.a.uid); + expect(res.isNew).toBe(true); + }); + + it('accepts the handle under its own field', async () => { + const file = await makeFile(fx.a.owner.userId); + + const res = await shareWithTeam(fx.a.owner.userId, file.path, { + teamHandle: fx.a.handle, + }); + + expect(res.holderTeam?.uid).toBe(fx.a.uid); + }); + + it('refuses both fields at once rather than picking one', async () => { + const file = await makeFile(fx.a.owner.userId); + + await expect( + shareWithTeam(fx.a.owner.userId, file.path, { + team: fx.a.uid, + teamHandle: fx.a.handle, + }), + ).rejects.toMatchObject({ statusCode: 400 }); + }); + + it('404s on a team that does not exist', async () => { + const file = await makeFile(fx.a.owner.userId); + + await expect( + shareWithTeam(fx.a.owner.userId, file.path, { + team: '00000000-0000-4000-8000-000000000000', + }), + ).rejects.toMatchObject({ statusCode: 404 }); + }); + + it('does not read a team handle out of a bare string recipient', async () => { + const file = await makeFile(fx.a.owner.userId); + + // A bare string is an email or a username, never a team — the + // SDK contract that keeps this additive. + await expect( + shareWithTeam(fx.a.owner.userId, file.path, { + username: fx.a.handle, + } as never), + ).rejects.toMatchObject({ statusCode: 404 }); + }); + + // -- who it reaches ------------------------------------------------ + + it('reaches every member of the team', async () => { + const file = await makeFile(fx.a.owner.userId); + await shareWithTeam(fx.a.owner.userId, file.path, { team: fx.a.uid }); + + for (const seat of fx.a.seats) { + const items = (await inbox(seat.userId)).items; + expect( + items.map((i) => i.entryUid), + `seat ${seat.username}`, + ).toContain(file.uid); + } + }); + + it('does not reach the other team', async () => { + const file = await makeFile(fx.a.owner.userId); + await shareWithTeam(fx.a.owner.userId, file.path, { team: fx.a.uid }); + + for (const seat of fx.b.seats) { + const items = (await inbox(seat.userId)).items; + expect(items.map((i) => i.entryUid)).not.toContain(file.uid); + } + }); + + it('reaches someone who joins the team afterwards', async () => { + const file = await makeFile(fx.a.owner.userId); + await shareWithTeam(fx.a.owner.userId, file.path, { team: fx.a.uid }); + + // The grant is one row against the team; the scan resolves it for + // whoever is a member at the time. + const username = `late_${Math.random().toString(36).slice(2, 9)}`; + const created = await fx.env.server.services.team.provisionAccount( + fx.a.uid, + fx.a.owner.userId, + { username, email: `${username}@test.local` }, + ); + + const items = (await inbox(created.userId)).items; + expect(items.map((i) => i.entryUid)).toContain(file.uid); + }); + + // -- listing: the silent failure this phase guards ---------------- + + it('appears in a member inbox, which is what `#liveGrants` decides', async () => { + const file = await makeFile(fx.a.owner.userId); + await shareWithTeam(fx.a.owner.userId, file.path, { team: fx.a.uid }); + + const items = (await inbox(fx.a.seats[0].userId)).items; + const row = items.find((i) => i.entryUid === file.uid); + // Without group evidence the share resolves correctly and is filtered + // out of the listing as dead — nothing errors, it is simply absent. + expect(row).toBeDefined(); + expect(row!.mode).toBe('read'); + }); + + it('counts team shares in the total, not just the page', async () => { + const file = await makeFile(fx.a.owner.userId); + await shareWithTeam(fx.a.owner.userId, file.path, { team: fx.a.uid }); + + const seat = fx.a.seats[0]; + const res = await shares().listSharedWithMe( + await actorFor(seat.userId), + { limit: 100, includeTotal: true }, + ); + expect(res.total).toBeGreaterThanOrEqual(res.items.length); + }); + + // -- revoke -------------------------------------------------------- + + it('unsharing removes it from every member inbox', async () => { + const file = await makeFile(fx.a.owner.userId); + await shareWithTeam(fx.a.owner.userId, file.path, { team: fx.a.uid }); + + await shares().unshare(await actorFor(fx.a.owner.userId), { + path: file.path, + recipient: { team: fx.a.uid }, + } as never); + + for (const seat of fx.a.seats) { + const items = (await inbox(seat.userId)).items; + expect(items.map((i) => i.entryUid)).not.toContain(file.uid); + } + }); + + it('unsharing revokes the grant, not just the index row', async () => { + const file = await makeFile(fx.a.owner.userId); + await shareWithTeam(fx.a.owner.userId, file.path, { team: fx.a.uid }); + + await shares().unshare(await actorFor(fx.a.owner.userId), { + path: file.path, + recipient: { team: fx.a.uid }, + } as never); + + // Deleting the row alone would hide the share while leaving every + // member holding real access — worse than a listing bug. + const perms = await fx.env.server.stores.permission.readUserGroupPerms( + fx.a.seats[0].userId, + [`fs:${file.uid}:read`], + ); + expect(perms).toHaveLength(0); + }); + + // -- coexistence with user shares --------------------------------- + + it('leaves an external share working', async () => { + const file = await makeFile(fx.a.owner.userId); + const outsider = await fx.env.server.stores.user.getById( + fx.outsider.userId, + ); + + // A member sharing outside the team is permitted; if that ever + // stops working it should fail here rather than for a customer. + const res = await shares().share( + await actorFor(fx.a.owner.userId), + { + path: file.path, + recipient: { username: outsider!.username }, + mode: 'read', + } as never, + ); + expect(res.holderId).toBe(fx.outsider.userId); + + const items = (await inbox(fx.outsider.userId)).items; + expect(items.map((i) => i.entryUid)).toContain(file.uid); + }); + + it('keeps a user share when the team share is revoked', async () => { + const file = await makeFile(fx.a.owner.userId); + const outsider = await fx.env.server.stores.user.getById( + fx.outsider.userId, + ); + + await shareWithTeam(fx.a.owner.userId, file.path, { team: fx.a.uid }); + await shares().share(await actorFor(fx.a.owner.userId), { + path: file.path, + recipient: { username: outsider!.username }, + mode: 'read', + } as never); + + await shares().unshare(await actorFor(fx.a.owner.userId), { + path: file.path, + recipient: { team: fx.a.uid }, + } as never); + + // The two holders are independent rows and independent grants. + const stillThere = (await inbox(fx.outsider.userId)).items; + expect(stillThere.map((i) => i.entryUid)).toContain(file.uid); + const gone = (await inbox(fx.a.seats[0].userId)).items; + expect(gone.map((i) => i.entryUid)).not.toContain(file.uid); + }); + + // -- the index row ------------------------------------------------- + + it('writes a group-held index row, not a user-held one', async () => { + const file = await makeFile(fx.a.owner.userId); + await shareWithTeam(fx.a.owner.userId, file.path, { team: fx.a.uid }); + + const rows = (await fx.env.server.clients.db.read( + 'SELECT `holder_user_id`, `holder_group_id` FROM `share` ' + + 'WHERE `fsentry_id` = (SELECT `id` FROM `fsentries` WHERE `uuid` = ?)', + [file.uid], + )) as { holder_user_id: number | null; holder_group_id: number | null }[]; + + expect(rows).toHaveLength(1); + expect(rows[0].holder_user_id).toBeFalsy(); + expect(rows[0].holder_group_id).toBeTruthy(); + }); + + it('re-sharing the same team does not spend quota twice', async () => { + const file = await makeFile(fx.a.owner.userId); + + const first = await shareWithTeam(fx.a.owner.userId, file.path, { + team: fx.a.uid, + }); + const second = await shareWithTeam(fx.a.owner.userId, file.path, { + team: fx.a.uid, + }); + + expect(first.isNew).toBe(true); + expect(second.isNew).toBe(false); + }); + + // -- a group row is not a pending invite --------------------------- + // + // `holder_user_id IS NULL` meant "unclaimed invite" everywhere until a + // team share gave that column a second reason to be null. + + it('lists a team share as a real share, not a pending invite', async () => { + const file = await makeFile(fx.a.owner.userId); + await shareWithTeam(fx.a.owner.userId, file.path, { team: fx.a.uid }); + + const page = await shares().listSharedByMe( + await actorFor(fx.a.owner.userId), + { limit: 100 }, + ); + const row = page.items.find((i) => i.entryUid === file.uid); + + expect(row).toBeTruthy(); + expect(row?.pending).toBeFalsy(); + expect(row?.holderTeam?.uid).toBe(fx.a.uid); + }); + + it('does not show a phantom invite on the item it was shared with', async () => { + const file = await makeFile(fx.a.owner.userId); + await shareWithTeam(fx.a.owner.userId, file.path, { team: fx.a.uid }); + + const rows = await shares().listSharesOf( + await actorFor(fx.a.owner.userId), + { uid: file.uid } as never, + ); + + // A blank-addressed pending row here is the group row misread. + expect( + rows.some((r) => r.pending && !r.recipientEmail), + ).toBe(false); + + // And at the source: the group row must not be in the invite feed at + // all. Asserted separately because `#resolvedShareRow` would mask it. + const entry = await fx.env.server.stores.fsEntry.getEntryByUuid( + file.uid, + ); + const invites = + await fx.env.server.stores.share.listPendingOnFsentry(entry!.id); + expect(invites.filter((r) => r.holder_group_id)).toHaveLength(0); + }); + + it('revoking from the outbound listing withdraws the grant, not just the row', async () => { + const file = await makeFile(fx.a.owner.userId); + await shareWithTeam(fx.a.owner.userId, file.path, { team: fx.a.uid }); + + const page = await shares().listSharedByMe( + await actorFor(fx.a.owner.userId), + { limit: 100 }, + ); + const row = page.items.find((i) => i.entryUid === file.uid); + expect(row?.uid).toBeTruthy(); + + await shares().revokeSharedByMe( + await actorFor(fx.a.owner.userId), + row!.uid, + ); + + // Probing the inbox would pass either way: deleting the index row alone + // also empties the listing. The grant is what members actually resolve. + const permission = `fs:${file.uid}:read`; + for (const seat of fx.a.seats) { + const rows = await fx.env.server.stores.permission.readUserGroupPerms( + seat.userId, + [permission], + ); + expect(rows, `seat ${seat.username}`).toHaveLength(0); + } + }); + + it('clears the caller own grant even when another issuer lost authority', async () => { + const file = await makeFile(fx.a.owner.userId); + const owner = await actorFor(fx.a.owner.userId); + await shareWithTeam(fx.a.owner.userId, file.path, { team: fx.a.uid }); + + // A second index row attributed to a seat, who cannot manage this node. + // Impersonating them to revoke throws 403 and aborts the whole unshare. + const team = await fx.env.server.stores.team.getByUid(fx.a.uid); + const entry = await fx.env.server.stores.fsEntry.getEntryByUuid( + file.uid, + ); + await fx.env.server.stores.share.upsertActiveGroup({ + issuerUserId: fx.a.seats[0].userId, + holderGroupId: team.id, + fsentryId: entry.id, + mode: 'read', + }); + + await shares().unshare(owner, { + path: file.path, + recipient: { team: fx.a.uid }, + }); + + const rows = await fx.env.server.stores.permission.readUserGroupPerms( + fx.a.seats[1].userId, + [`fs:${file.uid}:read`], + ); + expect(rows).toHaveLength(0); + }); + + it('counts members as reached, so they get live events on a shared folder', async () => { + const file = await makeFile(fx.a.owner.userId); + await shareWithTeam(fx.a.owner.userId, file.path, { team: fx.a.uid }); + const entry = await fx.env.server.stores.fsEntry.getEntryByUuid( + file.uid, + ); + + // The feed behind `outer.gui.item.*`: without group rows a member sees + // the folder but never a change inside it, so it goes stale silently. + const rows = + await fx.env.server.stores.share.listGroupReachingMembers([ + entry.id, + ]); + const reached = rows.map((r) => Number(r.holder_user_id)); + for (const seat of fx.a.seats) { + expect(reached, `seat ${seat.username}`).toContain(seat.userId); + } + }); + + it('clears the group grant when the entry itself is deleted', async () => { + const file = await makeFile(fx.a.owner.userId); + await shareWithTeam(fx.a.owner.userId, file.path, { team: fx.a.uid }); + const permission = `fs:${file.uid}:read`; + const seat = fx.a.seats[0].userId; + expect( + await fx.env.server.stores.permission.readUserGroupPerms(seat, [ + permission, + ]), + ).toHaveLength(1); + + await shares().onEntryDeleted([file.uid]); + + // Otherwise the grant outlives the file and keeps answering "allowed". + expect( + await fx.env.server.stores.permission.readUserGroupPerms(seat, [ + permission, + ]), + ).toHaveLength(0); + }); + + it('does not let a team grant keep a revoked direct share listed', async () => { + const file = await makeFile(fx.a.owner.userId); + const seat = fx.a.seats[0]; + const owner = await actorFor(fx.a.owner.userId); + + // Same issuer reaches the same holder two ways: directly, and through + // the team they share. + await shares().share(owner, { + path: file.path, + recipient: { username: seat.username }, + mode: 'read', + }); + await shareWithTeam(fx.a.owner.userId, file.path, { team: fx.a.uid }); + + // The direct grant is withdrawn outside `unshare`, leaving its index + // row behind. The group grant must not stand in for it. + await fx.env.server.stores.permission.deleteUserUserPermsByPermissionPrefixes( + [`fs:${file.uid}`, `manage:fs:${file.uid}`], + ); + + const page = await shares().listSharedByMe(owner, { limit: 100 }); + const direct = page.items.filter( + (i) => i.entryUid === file.uid && !i.holderTeam, + ); + expect(direct).toHaveLength(0); + }); + + it('shows the team in the share dialog for that item', async () => { + const file = await makeFile(fx.a.owner.userId); + await shareWithTeam(fx.a.owner.userId, file.path, { team: fx.a.uid }); + + const rows = await shares().listSharesOf( + await actorFor(fx.a.owner.userId), + { uid: file.uid }, + ); + + // Neither the holder feed nor the invite feed matches a group row, so + // without its own read the dialog shows nothing to revoke. + const team = rows.filter((r) => r.holderTeam?.uid === fx.a.uid); + expect(team).toHaveLength(1); + expect(team[0].pending).toBeFalsy(); + }); + // -- review findings ------------------------------------------------ + + it('refuses to share into a team the sharer does not belong to', async () => { + const file = await makeFile(fx.outsider.userId); + + await expect( + shares().share(await actorFor(fx.outsider.userId), { + path: file.path, + recipient: { teamHandle: fx.a.handle }, + mode: 'read', + }), + ).rejects.toMatchObject({ statusCode: 404 }); + }); + + it('revoking one row leaves another issuer grant on the same team', async () => { + const file = await makeFile(fx.a.owner.userId); + const owner = await actorFor(fx.a.owner.userId); + await shareWithTeam(fx.a.owner.userId, file.path, { team: fx.a.uid }); + + const team = await fx.env.server.stores.team.getByUid(fx.a.uid); + const entry = await fx.env.server.stores.fsEntry.getEntryByUuid(file.uid); + await fx.env.server.stores.share.upsertActiveGroup({ + issuerUserId: fx.a.seats[0].userId, + holderGroupId: team.id, + fsentryId: entry.id, + mode: 'read', + }); + + const rows = await fx.env.server.stores.share.listGroupOnFsentry(entry.id); + const delegateRow = rows.find( + (r) => Number(r.issuer_user_id) === fx.a.seats[0].userId, + ); + await shares().revokeSharedByMe(owner, String(delegateRow.uid)); + + // Row-addressed revocation takes that row's issuer only. + const left = await fx.env.server.stores.permission.readUserGroupPerms( + fx.a.seats[1].userId, + [`fs:${file.uid}:read`], + ); + expect(left.length).toBeGreaterThan(0); + }); + + it('names the issuer of a team-only share in the dialog', async () => { + const file = await makeFile(fx.a.owner.userId); + await shareWithTeam(fx.a.owner.userId, file.path, { team: fx.a.uid }); + + const rows = await shares().listSharesOf( + await actorFor(fx.a.owner.userId), + { uid: file.uid }, + ); + const team = rows.find((r) => r.holderTeam?.uid === fx.a.uid); + + // Without the issuer resolved the dialog reads "shared by nobody". + expect(team?.issuer?.username).toBeTruthy(); + }); + + it('lists a team reaching a file through a shared ancestor', async () => { + const { dirPath, fileUid } = await makeNestedFile(fx.a.owner.userId); + await shareWithTeam(fx.a.owner.userId, dirPath, { team: fx.a.uid }); + + const rows = await shares().listSharesOf( + await actorFor(fx.a.owner.userId), + { uid: fileUid }, + ); + expect(rows.some((r) => r.holderTeam?.uid === fx.a.uid)).toBe(true); + }); +}); diff --git a/src/backend/stores/permission/PermissionStore.ts b/src/backend/stores/permission/PermissionStore.ts index 49b464141..571238745 100644 --- a/src/backend/stores/permission/PermissionStore.ts +++ b/src/backend/stores/permission/PermissionStore.ts @@ -487,6 +487,33 @@ export class PermissionStore extends PuterStore { } /** As above, for several prefixes in one scan. */ + /** The group analogue: a deleted node's group grants must go with it. */ + async deleteUserGroupPermsByPermissionPrefixes( + permissions: string[], + ): Promise> { + if (permissions.length === 0) return []; + const where = permissions + .map(() => "(`permission` = ? OR `permission` LIKE ? ESCAPE '!')") + .join(' OR '); + const params = permissions.flatMap((permission) => [ + permission, + `${permission.replace(/([!%_])/g, '!$1')}:%`, + ]); + + const rows = (await this.clients.db.read( + 'SELECT `group_id`, `permission` FROM `user_to_group_permissions` ' + + `WHERE ${where}`, + params, + )) as Array<{ group_id: number; permission: string }>; + if (rows.length === 0) return []; + + await this.clients.db.write( + `DELETE FROM \`user_to_group_permissions\` WHERE ${where}`, + params, + ); + return rows; + } + async deleteUserUserPermsByPermissionPrefixes( permissions: string[], ): Promise< @@ -1004,6 +1031,47 @@ export class PermissionStore extends PuterStore { return rows.map((r) => Number(r.user_id)); } + /** Batched `readUserGroupPerms`, tagged with the user each row reached. */ + async readUserGroupPermsForHolders( + userIds: number[], + permissions: string[], + ): Promise< + Array<{ holder_user_id: number; permission: string; user_id: number }> + > { + const holders = [...new Set(userIds)]; + const perms = [...new Set(permissions)]; + if (holders.length === 0 || perms.length === 0) return []; + const rows = await this.clients.db.read( + 'SELECT ug.`user_id` AS `holder_user_id`, p.`permission`, p.`user_id` ' + + 'FROM `user_to_group_permissions` p ' + + 'JOIN `jct_user_group` ug ON p.`group_id` = ug.`group_id` ' + + 'JOIN `group` g ON g.`id` = ug.`group_id` ' + + `WHERE ug.\`user_id\` IN (${holders.map(() => '?').join(', ')}) ` + + 'AND g.`deleted_at` IS NULL ' + + `AND p.\`permission\` IN (${perms.map(() => '?').join(', ')})`, + [...holders, ...perms], + ); + return rows as unknown as Array<{ + holder_user_id: number; + permission: string; + user_id: number; + }>; + } + + /** What this issuer already granted this group under a prefix. */ + async queryIssuerGroupPermsByPrefix( + issuerUserId: number, + groupId: number, + prefix: string, + ): Promise { + const rows = await this.clients.db.read( + 'SELECT permission FROM `user_to_group_permissions` ' + + 'WHERE `user_id` = ? AND `group_id` = ? AND permission LIKE ?', + [issuerUserId, groupId, `${prefix}%`], + ); + return rows.map((r) => String(r.permission)); + } + /** `user_id` is the issuer; `group_id` is who receives it. */ async upsertUserGroupPerm( groupId: number, diff --git a/src/backend/stores/share/ShareStore.js b/src/backend/stores/share/ShareStore.js index 5ff0ae4e5..3e187a272 100644 --- a/src/backend/stores/share/ShareStore.js +++ b/src/backend/stores/share/ShareStore.js @@ -77,16 +77,27 @@ export class ShareStore extends PuterStore { * Returns rows only; the caller hydrates fsentries (batched) and drops any * whose entry it can't resolve. Paths are deliberately not stored — a move * or rename would strand them. + * + * @param {number} holderUserId + * @param {{ limit?: number; cursor?: string; groupIds?: number[] }} [opts] */ - async listByHolder(holderUserId, { limit, cursor } = {}) { + async listByHolder(holderUserId, { limit, cursor, groupIds = [] } = {}) { const size = this.#pageSize(limit); const afterId = this.#afterId(cursor); + const groups = [...new Set(groupIds)].filter(Boolean); + + // Same keyset page: `ORDER BY id` holds whatever the holder is. + const holderClause = groups.length + ? `(\`holder_user_id\` = ? OR \`holder_group_id\` IN (${groups + .map(() => '?') + .join(', ')}))` + : '`holder_user_id` = ?'; // One extra row tells us whether another page exists. const rows = await this.clients.db.read( - 'SELECT * FROM `share` WHERE `holder_user_id` = ? AND `id` > ? ' + + `SELECT * FROM \`share\` WHERE ${holderClause} AND \`id\` > ? ` + 'ORDER BY `id` LIMIT ?', - [holderUserId, afterId, size + 1], + [holderUserId, ...groups, afterId, size + 1], ); const hasMore = rows.length > size; @@ -331,6 +342,32 @@ export class ShareStore extends PuterStore { return rows.map((r) => this.#normalizeRow(r)); } + /** + * Team shares reaching these nodes, one row per member. Members hold + * through the group, so without this a shared folder never pushes them a + * change and goes stale until they refresh. + * + * @param {number[]} fsentryIds + */ + async listGroupReachingMembers(fsentryIds) { + if (fsentryIds.length === 0) return []; + const placeholders = fsentryIds.map(() => '?').join(', '); + const rows = await this.clients.db.read( + 'SELECT `share`.*, `ug`.`user_id` AS `member_user_id` FROM `share` ' + + 'JOIN `jct_user_group` `ug` ON `ug`.`group_id` = `share`.`holder_group_id` ' + + 'JOIN `group` `g` ON `g`.`id` = `share`.`holder_group_id` ' + + `WHERE \`share\`.\`fsentry_id\` IN (${placeholders}) ` + + 'AND `share`.`holder_group_id` IS NOT NULL ' + + 'AND `g`.`deleted_at` IS NULL ORDER BY `share`.`id`', + fsentryIds, + ); + // Shaped as a holder row, so the caller's fan-out needs no group branch. + return rows.map((r) => ({ + ...this.#normalizeRow(r), + holder_user_id: Number(r.member_user_id), + })); + } + /** * Which of `fsentryIds` carry a share, pending invites included. * @@ -357,10 +394,21 @@ export class ShareStore extends PuterStore { return shared; } - async countByHolder(holderUserId) { + /** + * @param {number} holderUserId + * @param {{ groupIds?: number[] }} [opts] Same union as `listByHolder`, or + * `includeTotal` undercounts a member's team shares. + */ + async countByHolder(holderUserId, { groupIds = [] } = {}) { + const groups = [...new Set(groupIds)].filter(Boolean); + const holderClause = groups.length + ? `(\`holder_user_id\` = ? OR \`holder_group_id\` IN (${groups + .map(() => '?') + .join(', ')}))` + : '`holder_user_id` = ?'; const rows = await this.clients.db.read( - 'SELECT COUNT(*) AS `count` FROM `share` WHERE `holder_user_id` = ?', - [holderUserId], + `SELECT COUNT(*) AS \`count\` FROM \`share\` WHERE ${holderClause}`, + [holderUserId, ...groups], ); return Number(rows[0]?.count ?? 0); } @@ -382,16 +430,40 @@ export class ShareStore extends PuterStore { return rows.map((r) => this.#normalizeRow(r)); } + /** + * Team shares on one node. Neither `listByFsentry` (holder rows) nor + * the invite feed matches them, so without this the share dialog shows + * nothing for a file shared with a team. + * + * @param {number} fsentryId + */ + async listGroupOnFsentry(fsentryId) { + const rows = await this.clients.db.read( + 'SELECT `share`.* FROM `share` ' + + 'JOIN `group` `g` ON `g`.`id` = `share`.`holder_group_id` ' + + 'WHERE `share`.`fsentry_id` = ? ' + + 'AND `share`.`holder_group_id` IS NOT NULL ' + + 'AND `g`.`deleted_at` IS NULL ORDER BY `share`.`id`', + [fsentryId], + ); + return rows.map((r) => this.#normalizeRow(r)); + } + /** * Unclaimed invites on one node, whoever sent them. What someone managing * the node needs to see who has been asked but has not arrived. * + * A team share also has no `holder_user_id` -- its holder is the group + * -- so both columns are checked, or every team share is listed here + * as an invite to a blank address. + * * @param {number} fsentryId */ async listPendingOnFsentry(fsentryId) { const rows = await this.clients.db.read( 'SELECT * FROM `share` WHERE `fsentry_id` = ? AND ' + - '`holder_user_id` IS NULL ORDER BY `id`', + '`holder_user_id` IS NULL AND `holder_group_id` IS NULL ' + + 'ORDER BY `id`', [fsentryId], ); return rows.map((r) => this.#normalizeRow(r)); @@ -541,6 +613,98 @@ export class ShareStore extends PuterStore { return this.getActive({ holderUserId, fsentryId, issuerUserId }); } + /** + * The team-holder form; `holder_user_id` stays NULL, so `0077`'s group + * index constrains these rows rather than the user-holder one. + * + * @param {object} input + * @param {number} input.issuerUserId + * @param {number} input.holderGroupId + * @param {number} input.fsentryId + * @param {string} input.mode + * @param {string | null} [input.issuerAppUid] + */ + async upsertActiveGroup({ + issuerUserId, + holderGroupId, + fsentryId, + mode, + issuerAppUid = null, + }) { + if (!issuerUserId || !holderGroupId || !fsentryId || !mode) { + throw new Error( + 'upsertActiveGroup: issuerUserId, holderGroupId, fsentryId and mode are required', + ); + } + const data = JSON.stringify( + issuerAppUid ? { issuedByApp: issuerAppUid } : {}, + ); + await this.clients.db.write( + 'INSERT INTO `share` (`uid`, `issuer_user_id`, `recipient_email`, ' + + '`holder_group_id`, `fsentry_id`, `mode`, `data`, `applied_at`) ' + + 'VALUES (?, ?, ?, ?, ?, ?, ?, CURRENT_TIMESTAMP) ' + + this.clients.db.upsertClause( + ['holder_group_id', 'fsentry_id', 'issuer_user_id'], + ['mode', 'data'], + ), + [ + uuidv4(), + issuerUserId, + // NOT NULL since `0067`; a team has no address. + '', + holderGroupId, + fsentryId, + mode, + data, + mode, + data, + ], + ); + return this.getActiveGroup({ holderGroupId, fsentryId, issuerUserId }); + } + + /** + * @param {object} input + * @param {number} input.holderGroupId + * @param {number} input.fsentryId + * @param {number} input.issuerUserId + */ + async getActiveGroup({ holderGroupId, fsentryId, issuerUserId }) { + const rows = await this.clients.db.read( + 'SELECT * FROM `share` WHERE `holder_group_id` = ? AND ' + + '`fsentry_id` = ? AND `issuer_user_id` = ? LIMIT 1', + [holderGroupId, fsentryId, issuerUserId], + ); + return this.#normalizeRow(rows[0]) ?? null; + } + + /** Every team-held share of this node, whoever issued it. */ + async listGroupSharesByFsentry(fsentryId) { + const rows = await this.clients.db.read( + 'SELECT * FROM `share` WHERE `fsentry_id` = ? AND `holder_group_id` IS NOT NULL', + [fsentryId], + ); + return rows.map((row) => this.#normalizeRow(row)).filter(Boolean); + } + + /** + * @param {object} input + * @param {number} input.holderGroupId + * @param {number} input.fsentryId + * @param {number | null} [input.issuerUserId] + */ + async deleteActiveGroup({ holderGroupId, fsentryId, issuerUserId = null }) { + const scoped = issuerUserId !== null && issuerUserId !== undefined; + const result = await this.clients.db.write( + 'DELETE FROM `share` WHERE `holder_group_id` = ? AND `fsentry_id` = ?' + + (scoped ? ' AND `issuer_user_id` = ?' : ''), + scoped + ? [holderGroupId, fsentryId, issuerUserId] + : [holderGroupId, fsentryId], + ); + return result.anyRowsAffected; + } + /** * @param {object} input * @param {number} input.holderUserId @@ -646,7 +810,10 @@ export class ShareStore extends PuterStore { ') ' + 'SELECT `share`.`uid` FROM `share` ' + 'JOIN `subtree` ON `share`.`fsentry_id` = `subtree`.`id` ' + + // Group rows also have no holder user; deleting one here would + // drop the index row and leave its grant standing. 'WHERE `share`.`holder_user_id` IS NULL AND ' + + '`share`.`holder_group_id` IS NULL AND ' + '`share`.`issuer_user_id` = ?', [fsentryId, issuerUserId], ); diff --git a/src/backend/stores/team/TeamStore.ts b/src/backend/stores/team/TeamStore.ts index bd969e768..50138d4b1 100644 --- a/src/backend/stores/team/TeamStore.ts +++ b/src/backend/stores/team/TeamStore.ts @@ -338,6 +338,26 @@ export class TeamStore extends PuterStore { return { items, cursor }; } + /** Internal ids of this user's live teams, for the share listing. */ + async listGroupIdsForUser(userId: number): Promise { + const rows = (await this.clients.db.read( + 'SELECT g.`id` FROM `group` g ' + + 'JOIN `jct_user_group` ug ON ug.`group_id` = g.`id` ' + + `WHERE ug.\`user_id\` = ? AND g.${this.#live()}`, + [userId, TEAM_KIND], + )) as { id: number }[]; + return rows.map((r) => Number(r.id)); + } + + /** By internal id, soft-deleted included: those shares stay revocable. */ + async getByIdIncludingDeleted(id: number): Promise { + const rows = (await this.clients.db.read( + 'SELECT * FROM `group` WHERE `id` = ? AND `kind` = ?', + [id, TEAM_KIND], + )) as unknown as TeamRow[]; + return rows[0] ?? null; + } + /** Teams this user belongs to, oldest first. */ async listTeamsForUser(userId: number): Promise { const rows = await this.clients.db.read( diff --git a/src/puter-js/src/modules/FileSystem/operations/operations.test.js b/src/puter-js/src/modules/FileSystem/operations/operations.test.js index ce9802d44..cf764861b 100644 --- a/src/puter-js/src/modules/FileSystem/operations/operations.test.js +++ b/src/puter-js/src/modules/FileSystem/operations/operations.test.js @@ -429,6 +429,55 @@ describe('stat', () => { expect(item.is_shared).toBe(true); }); + it('sends a team recipient through instead of dropping it', async () => { + FakeXHR.respondWith = () => ({ status: 'success', results: [] }); + await fs.share({ + path: '/a/file.txt', + recipient: { team: 'ws-uid-1' }, + mode: 'read', + }); + // Dropped before this mapping existed, so the call shared with nobody + // and still reported success. + expect(lastBody().recipients).toEqual([{ team: 'ws-uid-1' }]); + }); + + it('sends a team handle under its own field', async () => { + FakeXHR.respondWith = () => ({ status: 'success', results: [] }); + await fs.share({ + path: '/a/file.txt', + recipient: { teamHandle: 'acme' }, + mode: 'read', + }); + expect(lastBody().recipients).toEqual([{ teamHandle: 'acme' }]); + }); + + it('publishes the team a share reached', async () => { + FakeXHR.respondWith = () => ({ + uid: 'u1', + is_dir: false, + is_shared: true, + shares: [ + { + uid: 's1', + mode: 'read', + uid_entry: 'u1', + is_dir: false, + holder: null, + holder_team: { uid: 'ws-uid-1', name: 'Acme', handle: 'acme' }, + }, + ], + }); + const item = await fs.stat('/a/file.txt', { returnShares: true }); + // `holder` is null for a team share, so without this the client + // is told the file is shared with nobody. + expect(item.shares[0].holderTeam).toEqual({ + uid: 'ws-uid-1', + name: 'Acme', + handle: 'acme', + }); + expect(item.shares[0].holder).toBe(null); + }); + it('re-reads a cached item after a share is withdrawn', async () => { FakeXHR.respondWith = () => ({ uid: 'u1', is_dir: false, is_shared: true }); await fs.stat('/a/file.txt'); diff --git a/src/puter-js/src/modules/FileSystem/operations/shareUtil.js b/src/puter-js/src/modules/FileSystem/operations/shareUtil.js index b9eaaaa9e..d50b1b4d3 100644 --- a/src/puter-js/src/modules/FileSystem/operations/shareUtil.js +++ b/src/puter-js/src/modules/FileSystem/operations/shareUtil.js @@ -25,9 +25,15 @@ export const toShareRecipients = (value) => { : { username: trimmed }; } const record = /** @type {Record} */ (entry); + // Object form only: a bare-string spelling would reinterpret + // strings that already mean something. return { ...(record.email ? { email: String(record.email) } : {}), ...(record.username ? { username: String(record.username) } : {}), + ...(record.team ? { team: String(record.team) } : {}), + ...(record.teamHandle + ? { teamHandle: String(record.teamHandle) } + : {}), }; }); }; @@ -72,6 +78,14 @@ export const toShare = (row) => ({ owner: /** @type {string | null} */ (row.owner ?? null), issuer: /** @type {string | null} */ (row.issuer ?? null), holder: /** @type {string | null} */ (row.holder ?? null), + // `holder` is null for a team share; this names who it reached. + ...(row.holder_team || row.holderTeam + ? { + holderTeam: /** @type {{ uid: string, name: string | null, handle: string | null }} */ ( + row.holder_team ?? row.holderTeam + ), + } + : {}), inheritedFrom: /** @type {string | null} */ (row.inherited_from ?? null), issuedByApp: /** @type {string | null} */ (row.issued_by_app ?? null), ...(row.status === 'pending' || row.pending === true diff --git a/src/puter-js/src/modules/FileSystem/types.js b/src/puter-js/src/modules/FileSystem/types.js index adc96cd2c..9bfd53393 100644 --- a/src/puter-js/src/modules/FileSystem/types.js +++ b/src/puter-js/src/modules/FileSystem/types.js @@ -310,7 +310,10 @@ * Who a share is for. Give an `email` or a `username`; a bare string is read * as an email when it contains `@` and a username otherwise. * - * @typedef {string | { email?: string, username?: string }} ShareRecipient + * A team is named by `team` (uid) or `teamHandle`, never as a bare + * string -- that spelling already means an email or a username. + * + * @typedef {string | { email?: string, username?: string, team?: string, teamHandle?: string }} ShareRecipient */ /** @@ -331,7 +334,10 @@ * @property {string | null} owner Username of the item's owner. Only set by * the listings, `listShared()` and `listSharedByMe()`. * @property {string | null} issuer Username of whoever granted it. - * @property {string | null} holder Username of whoever received it. + * @property {string | null} holder Username of whoever received it. Null for a + * team share, which has no individual holder -- see `holderTeam`. + * @property {{ uid: string, name: string | null, handle: string | null }} [holderTeam] + * The team this was shared with, when it was shared with one. * @property {string | null} [inheritedFrom] Shared ancestor this access comes from, if any. * @property {string | null} [issuedByApp] UID of the app that asked for this * share, or `null` when a person made it directly. From 7a02fa7741478f5111f5e2b5409d9069cead6d27 Mon Sep 17 00:00:00 2001 From: Juan Castro Date: Thu, 3 Sep 2026 17:30:18 -0400 Subject: [PATCH 3/3] feat: tell every team member when something is shared with them `notifyShared` keyed every map off `ResolvedShare.holderId`. A team share has no individual holder -- the grant is one row against the group and `holder_user_id` is NULL -- so it was skipped everywhere and the call returned having told nobody. Nothing errored: the share was created, resolved and listed correctly. The failure mode was silence. `#recipientsOf` now expands a team share to its live members at announcement time, so the grant stays one row and only the telling fans out. `#announce` and `#emailHolder` are both inside the per-recipient loop, so members get the in-app notification and the email digest. Bounded by `NOTIFY_FANOUT_CAP`, which logs when it bites rather than truncating silently -- a shortened fan-out otherwise reads as "everyone knows". The existing per-recipient budgets absorb the volume from there. Wording is unchanged: "shared N items with you", not the team name. A member who joins later resolves the grant through the scan but is not retroactively told; announcements describe a moment, not a state. --- .../share/ShareNotificationService.ts | 112 ++++++- .../services/share/TeamShareNotify.test.ts | 317 ++++++++++++++++++ src/backend/services/team/TeamEvents.test.ts | 63 ++++ src/backend/services/team/TeamService.ts | 7 + .../services/user/UserAccountService.ts | 2 + src/backend/stores/team/TeamStore.ts | 130 +++++++ 6 files changed, 621 insertions(+), 10 deletions(-) create mode 100644 src/backend/services/share/TeamShareNotify.test.ts diff --git a/src/backend/services/share/ShareNotificationService.ts b/src/backend/services/share/ShareNotificationService.ts index ff32dae9c..23144de09 100644 --- a/src/backend/services/share/ShareNotificationService.ts +++ b/src/backend/services/share/ShareNotificationService.ts @@ -40,7 +40,8 @@ import { shareDeepLink, sharedViewLink, } from './shareDeepLink'; -import type { ResolvedShare } from './ShareService'; +import { NOTIFY_FANOUT_CAP } from '../../stores/team/TeamStore'; +import { blocksAllShares, type ResolvedShare } from './ShareService'; /** * How long one sharer stays quiet after reaching a recipient, and how long a @@ -264,27 +265,29 @@ export class ShareNotificationService extends PuterService { const counts = new Map(); const named = new Map(); const targets = new Map(); - for (const share of shares) { - if (share.pending) continue; - if (!share.isNew || !share.holderId) continue; - if (share.holderId === issuerId) continue; - counts.set(share.holderId, (counts.get(share.holderId) ?? 0) + 1); + // A team share has no holder of its own, so it is expanded here. + for (const { holderId, share } of await this.#recipientsOf( + shares, + issuerId, + )) { + counts.set(holderId, (counts.get(holderId) ?? 0) + 1); const item = this.#digestItem(share); if (item) { - const items = named.get(share.holderId) ?? []; + const items = named.get(holderId) ?? []; if (items.length < DIGEST_NAMES_PER_SENDER) items.push(item); - named.set(share.holderId, items); + named.set(holderId, items); } // Only a lone item is worth pointing at; a second nulls it. const path = this.#targetPath(share); targets.set( - share.holderId, - targets.has(share.holderId) || !path + holderId, + targets.has(holderId) || !path ? null : { path, name: share.name as string }, ); } + // Each recipient fails alone: one refused send must not cost the next // person their notification. Failures are logged, never thrown. await Promise.allSettled( @@ -325,6 +328,95 @@ export class ShareNotificationService extends PuterService { } } + /** + * Who each share announces to. A user share is its own holder; a team + * share is every live member except the issuer, expanded at announcement + * time so the grant stays one row and only the telling fans out. + * + * A member who joins later resolves the grant through the scan but is never + * told: access follows the team, announcements describe a moment. + */ + async #recipientsOf( + shares: ResolvedShare[], + issuerId: number, + ): Promise> { + const out: Array<{ holderId: number; share: ResolvedShare }> = []; + const byGroup = new Map(); + const allowedByGroup = new Map(); + for (const share of shares) { + if (share.pending || !share.isNew) continue; + + if (share.holderId) { + if (share.holderId === issuerId) continue; + out.push({ holderId: share.holderId, share }); + continue; + } + if (!share.holderGroupId) continue; + + // Cached per group: N items shared with one team is one read. + let members = byGroup.get(share.holderGroupId); + if (!members) { + try { + members = await this.stores.team.listMemberIdsByGroupId( + share.holderGroupId, + ); + } catch (err) { + // Silence is this path's failure mode, so say so out loud. + console.warn( + '[share-notify] could not expand team', + share.holderGroupId, + err, + ); + continue; + } + byGroup.set(share.holderGroupId, members); + } + // The store reads one past the cap, so `>` means it really did + // truncate; exactly the cap did not. + if (members.length > NOTIFY_FANOUT_CAP) { + members = members.slice(0, NOTIFY_FANOUT_CAP); + byGroup.set(share.holderGroupId, members); + console.warn( + '[share-notify] team fan-out hit the cap; some members were not told', + { groupId: share.holderGroupId, cap: NOTIFY_FANOUT_CAP }, + ); + } + // A block refuses contact, and this is the contact: the group grant + // is one row so it cannot exclude a member, but the telling can. + // Cached with the member list -- block state is per pair, not per item. + let allowed = allowedByGroup.get(share.holderGroupId); + if (!allowed) { + allowed = await this.#unblocked(members, issuerId); + allowedByGroup.set(share.holderGroupId, allowed); + } + for (const holderId of allowed) { + if (holderId === issuerId) continue; + out.push({ holderId, share }); + } + } + return out; + } + + /** Members who have not refused shares from this issuer. */ + async #unblocked(members: number[], issuerId: number): Promise { + const kept = await Promise.all( + members.map(async (id) => { + try { + const user = await this.stores.user.getById(id); + if (blocksAllShares(user)) return null; + return (await this.stores.userBlock.isBlocked(id, issuerId)) + ? null + : id; + } catch (err) { + // Failing open would announce to someone who refused it. + console.warn('[share-notify] block check failed', id, err); + return null; + } + }), + ); + return kept.filter((id): id is number => id !== null); + } + /** * Fold this batch into the notification the recipient hasn't dealt with, or * start one. Written either way — a suppressed interruption must not lose diff --git a/src/backend/services/share/TeamShareNotify.test.ts b/src/backend/services/share/TeamShareNotify.test.ts new file mode 100644 index 000000000..84b8e62eb --- /dev/null +++ b/src/backend/services/share/TeamShareNotify.test.ts @@ -0,0 +1,317 @@ +/* + * Copyright (C) 2024-present Puter Technologies Inc. + * + * This file is part of Puter. + * + * Puter is free software: you can redistribute it and/or modify + * it under the terms of the GNU Affero General Public License as published + * by the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Affero General Public License for more details. + * + * You should have received a copy of the GNU Affero General Public License + * along with this program. If not, see . + */ + +import { + afterAll, + afterEach, + beforeAll, + beforeEach, + describe, + expect, + it, + vi, +} from 'vitest'; +import type { Actor } from '../../core/actor'; +import { + setupTwoTeams, + type TwoTeams, +} from '../../testFixtures/twoTeams.js'; + +/** + * A team share names no holder, so every map in `notifyShared` used to skip + * it and the call succeeded having told nobody. The failure mode is silence: + * the share is created, resolves and lists correctly, and nothing errors. Only + * a test that counts notifications catches it. + */ +describe('announcing a team share', () => { + let fx: TwoTeams; + + const actorFor = async (userId: number): Promise => { + const user = await fx.env.server.stores.user.getById(userId); + return { user } as unknown as Actor; + }; + + const makeFile = async (ownerId: number) => { + const uid = crypto.randomUUID(); + const name = `n_${uid.slice(0, 8)}.txt`; + const owner = await fx.env.server.stores.user.getById(ownerId); + const path = `/${owner!.username}/${name}`; + await fx.env.server.clients.db.write( + 'INSERT INTO `fsentries` (`uuid`, `name`, `path`, `user_id`, `is_dir`, `modified`) ' + + 'VALUES (?, ?, ?, ?, ?, ?)', + [ + uid, + name, + path, + ownerId, + fx.env.server.clients.db.booleanValue(false), + Math.floor(Date.now() / 1000), + ], + ); + return path; + }; + + /** Share and announce, the way `ShareController` does on the response path. */ + const shareAndNotify = async ( + issuerId: number, + path: string, + recipient: Record, + ) => { + const actor = await actorFor(issuerId); + const share = await fx.env.server.services.share.share(actor, { + path, + recipient, + mode: 'read', + } as never); + await fx.env.server.services.shareNotification.notifyShared(actor, [ + share, + ]); + return share; + }; + + const notificationsFor = async (userId: number) => { + const rows = await fx.env.server.stores.notification.listByUserId( + userId, + { filter: 'unacknowledged' }, + ); + return rows as unknown as Array<{ value?: unknown }>; + }; + + const titlesFor = async (userId: number): Promise => + (await notificationsFor(userId)).map((row) => { + const value = ( + typeof row.value === 'string' + ? JSON.parse(row.value) + : (row.value ?? {}) + ) as { title?: string }; + return value.title ?? ''; + }); + + let sent: Array<{ to: string; subject: string }>; + + beforeAll(async () => { + fx = await setupTwoTeams({ + // A transport must exist for the service to try at all; `sendRaw` + // is spied, so nothing leaves the process. + email: { + from: '"Puter (test)" ', + host: '127.0.0.1', + port: 1, + }, + // Digests flush almost at once rather than after a real minute. + share_notify_limits: { emailBatchSeconds: 0.05 }, + } as never); + }, 180_000); + + beforeEach(() => { + sent = []; + vi.spyOn(fx.env.server.clients.email, 'sendRaw').mockImplementation( + async (options: { to: string; subject: string }) => { + sent.push({ to: options.to, subject: options.subject }); + return null; + }, + ); + }); + + afterEach(() => { + vi.restoreAllMocks(); + }); + + afterAll(async () => { + await fx?.shutdown(); + }); + + it('tells every member of the team', async () => { + const path = await makeFile(fx.a.owner.userId); + await shareAndNotify(fx.a.owner.userId, path, { team: fx.a.uid }); + + for (const seat of fx.a.seats) { + const titles = await titlesFor(seat.userId); + expect(titles.length).toBeGreaterThan(0); + } + }); + + it('does not tell the issuer about their own share', async () => { + const before = (await notificationsFor(fx.a.owner.userId)).length; + const path = await makeFile(fx.a.owner.userId); + await shareAndNotify(fx.a.owner.userId, path, { team: fx.a.uid }); + + expect((await notificationsFor(fx.a.owner.userId)).length).toBe(before); + }); + + it('does not tell members of a team it was not shared with', async () => { + const before = await Promise.all( + fx.b.seats.map(async (s) => (await notificationsFor(s.userId)).length), + ); + const path = await makeFile(fx.a.owner.userId); + await shareAndNotify(fx.a.owner.userId, path, { team: fx.a.uid }); + + const after = await Promise.all( + fx.b.seats.map(async (s) => (await notificationsFor(s.userId)).length), + ); + expect(after).toEqual(before); + }); + + it('does not retroactively tell a member who joins afterwards', async () => { + const path = await makeFile(fx.a.owner.userId); + await shareAndNotify(fx.a.owner.userId, path, { team: fx.a.uid }); + + // The outsider joins after the announcement; the scan will resolve the + // grant for them, but announcements describe a moment, not a state. + const team = await fx.env.server.stores.team.getByUid(fx.a.uid); + await fx.env.server.stores.team.addMember( + fx.a.uid, + fx.outsider.userId, + { orgOwned: false }, + ); + expect(team).toBeTruthy(); + + expect(await titlesFor(fx.outsider.userId)).toHaveLength(0); + }); + + it('emails every member too, not just the in-app notification', async () => { + const path = await makeFile(fx.a.owner.userId); + await shareAndNotify(fx.a.owner.userId, path, { team: fx.a.uid }); + + // The digest is held briefly, so wait for the flush rather than the + // queueing -- asserting too early passes for the wrong reason. + await vi.waitFor( + () => { + for (const seat of fx.a.seats) { + expect( + sent.some((mail) => mail.to === `${seat.username}@test.local`), + ).toBe(true); + } + }, + { timeout: 10_000, interval: 100 }, + ); + + // And nobody outside the team was mailed about it. Checked against + // team B's owner, who never joins A -- the `outsider` is admitted + // to A by the test above, so by here they are a member. + expect( + sent.some((mail) => mail.to.includes(fx.b.owner.username)), + ).toBe(false); + }); + + it('does not announce to a member who blocked the sharer', async () => { + // A member nobody has notified yet, so a new row is detectable. Reusing + // a seat would not be: a second share *folds* into its open + // notification, leaving the row count unchanged either way. + const username = `blk_${Math.random().toString(36).slice(2, 9)}`; + const fresh = await fx.env.server.stores.user.create({ + username, + uuid: crypto.randomUUID(), + password: null, + email: `${username}@test.local`, + }); + await fx.env.server.stores.team.addMember(fx.a.uid, fresh.id, { + orgOwned: false, + }); + const blocker = { userId: fresh.id, username }; + const other = fx.a.seats[1]; + await fx.env.server.stores.userBlock.create( + blocker.userId, + fx.a.owner.userId, + ); + + const before = (await notificationsFor(blocker.userId)).length; + expect(before).toBe(0); + const path = await makeFile(fx.a.owner.userId); + await shareAndNotify(fx.a.owner.userId, path, { team: fx.a.uid }); + + // The grant is one row against the group so it still reaches them; the + // contact the block refuses is the telling. + // + // Asserted on notifications rather than mail: by this point in the file + // the per-pair interruption budget is spent, so no digest opens and an + // email assertion would pass for the wrong reason. Notifications are + // still written when the budget is spent -- that is the documented + // split between what it says and whether it may interrupt. + expect((await notificationsFor(blocker.userId)).length).toBe(before); + expect( + (await notificationsFor(other.userId)).length, + ).toBeGreaterThan(0); + + await fx.env.server.stores.userBlock.deleteByPair( + blocker.userId, + fx.a.owner.userId, + ); + }); + + it('reads the member list once however many items are shared', async () => { + const spy = vi.spyOn( + fx.env.server.stores.team, + 'listMemberIdsByGroupId', + ); + const actor = await actorFor(fx.a.owner.userId); + const paths = [ + await makeFile(fx.a.owner.userId), + await makeFile(fx.a.owner.userId), + await makeFile(fx.a.owner.userId), + ]; + const created = []; + for (const path of paths) { + created.push( + await fx.env.server.services.share.share(actor, { + path, + recipient: { team: fx.a.uid }, + mode: 'read', + }), + ); + } + spy.mockClear(); + + await fx.env.server.services.shareNotification.notifyShared( + actor, + created, + ); + + // One team, so one read -- not one per item. + expect(spy).toHaveBeenCalledTimes(1); + }); + it('checks block state once per team, not once per item', async () => { + const spy = vi.spyOn(fx.env.server.stores.userBlock, 'isBlocked'); + const actor = await actorFor(fx.a.owner.userId); + const created = []; + for (let i = 0; i < 3; i++) { + created.push( + await fx.env.server.services.share.share(actor, { + path: await makeFile(fx.a.owner.userId), + recipient: { team: fx.a.uid }, + mode: 'read', + }), + ); + } + spy.mockClear(); + + await fx.env.server.services.shareNotification.notifyShared( + actor, + created, + ); + + // Block state is per (member, issuer) pair -- constant across the items + // in one call, so 3 items must not triple the queries. + const team = await fx.env.server.stores.team.getByUid(fx.a.uid); + const members = await fx.env.server.stores.team.listMemberIdsByGroupId( + team.id, + ); + expect(spy.mock.calls.length).toBeLessThanOrEqual(members.length); + }); +}); diff --git a/src/backend/services/team/TeamEvents.test.ts b/src/backend/services/team/TeamEvents.test.ts index 1748a4c02..84d39ec60 100644 --- a/src/backend/services/team/TeamEvents.test.ts +++ b/src/backend/services/team/TeamEvents.test.ts @@ -217,6 +217,69 @@ describe('team billing events', () => { expect(await server.stores.team.getOrgSeat(seat.userId)).toBeNull(); }); + // -- the authorization cache ------------------------------------------ + // `isMember` and `getByUid` gate every team route, so a stale entry is + // access, not a stray notification. Each bust gets its own case. + + it('stops answering member once the account is removed', async () => { + const team = await makeTeam(); + const seat = await provision(team); + expect(await server.stores.team.isMember(team.uid, seat.userId)).toBe(true); + + await server.stores.team.removeMember(team.uid, seat.userId); + + expect(await server.stores.team.isMember(team.uid, seat.userId)).toBe(false); + }); + + it('stops answering member once the account is deleted out from under it', async () => { + const team = await makeTeam(); + const seat = await provision(team); + expect(await server.stores.team.isMember(team.uid, seat.userId)).toBe(true); + + // Cascades the membership row without passing through TeamStore. + await server.services.userAccount.cascadeDelete(seat.userId); + + expect(await server.stores.team.isMember(team.uid, seat.userId)).toBe(false); + }); + + it('stops resolving the team, and its memberships, once it is deleted', async () => { + const team = await makeTeam(); + const seat = await provision(team); + expect(await server.stores.team.getByUid(team.uid)).toBeTruthy(); + expect(await server.stores.team.isMember(team.uid, seat.userId)).toBe(true); + + await server.stores.team.softDelete(team.uid); + + expect(await server.stores.team.getByUid(team.uid)).toBeNull(); + expect(await server.stores.team.isMember(team.uid, seat.userId)).toBe(false); + }); + + it('serves the renamed team, not the name it was read at', async () => { + const team = await makeTeam(); + expect((await server.stores.team.getByUid(team.uid))?.name).toBe('Billing Co'); + + await server.stores.team.update(team.uid, { name: 'Renamed Co' }); + + expect((await server.stores.team.getByUid(team.uid))?.name).toBe('Renamed Co'); + }); + + it('does not serve a stale member list after the roster changes', async () => { + const team = await makeTeam(); + const before = await server.stores.team.listMemberIdsByGroupId(team.id); + + const seat = await provision(team); + const after = await server.stores.team.listMemberIdsByGroupId(team.id); + expect(after).toContain(seat.userId); + expect(after.length).toBe(before.length + 1); + + // The membership row cascades away with the account, never through + // TeamStore -- the case a TTL alone would get wrong. + await server.services.userAccount.cascadeDelete(seat.userId); + expect( + await server.stores.team.listMemberIdsByGroupId(team.id), + ).not.toContain(seat.userId); + }); + 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 6f8045407..53dd1c2ac 100644 --- a/src/backend/services/team/TeamService.ts +++ b/src/backend/services/team/TeamService.ts @@ -113,6 +113,13 @@ export class TeamService extends PuterService { } } + /** The membership cascaded away, so the cached reads over it must go too. */ + async forgetSeat(seat: TeamBillingEvent): Promise { + const team = await this.stores.team.getByUid(seat.team_uid); + await this.stores.team.bustMembership(team?.id ?? -1, seat.user_id); + await this.stores.team.bustMember(seat.team_uid, seat.user_id); + } + /** Paired with `captureSeatForBilling`, once the account is really gone. */ emitSeatDeleted(seat: TeamBillingEvent | null): void { if (seat) this.#emitBilling('team.account.deleted', seat); diff --git a/src/backend/services/user/UserAccountService.ts b/src/backend/services/user/UserAccountService.ts index 539b02104..fe0a07799 100644 --- a/src/backend/services/user/UserAccountService.ts +++ b/src/backend/services/user/UserAccountService.ts @@ -100,6 +100,8 @@ export class UserAccountService extends PuterService { } // Tells prod to stop charging the owner for this seat. this.services.team.emitSeatDeleted(seat); + // The membership row cascaded away, so anything keyed on it is stale. + if (seat) await this.services.team.forgetSeat(seat); } /** diff --git a/src/backend/stores/team/TeamStore.ts b/src/backend/stores/team/TeamStore.ts index 50138d4b1..d75331756 100644 --- a/src/backend/stores/team/TeamStore.ts +++ b/src/backend/stores/team/TeamStore.ts @@ -40,6 +40,9 @@ export interface TeamRow { export const TEAM_KIND = 'team'; +/** Short: a stale read here costs a notification, not access. */ +const TEAM_CACHE_TTL_SECONDS = 60; + /** A membership row, joined to the member's username. */ /** A seat, joined to the team that pays for it. */ export interface OrgSeatRow { @@ -77,6 +80,13 @@ export const MEMBER_PAGE_CAP = 200; export const AUDIT_PAGE_SIZE = 50; export const AUDIT_PAGE_CAP = 200; +/** + * Ceiling on a share announcement's fan-out. Well above the default seat cap, + * so it bounds a pathological team rather than a real one; the caller logs + * when it bites, because a silently shortened fan-out reads as "everyone knows". + */ +export const NOTIFY_FANOUT_CAP = 500; + /** Longest handle mysql can store — `varchar(64)` in mysql_mig_28. */ export const HANDLE_MAX_LENGTH = 64; export const HANDLE_MIN_LENGTH = 3; @@ -173,6 +183,10 @@ export class TeamStore extends PuterStore { /** The team with this uid, or null. Soft-deleted ones are excluded. */ async getByUid(uid: string): Promise { + return this.#cached(`team:row:${uid}`, () => this.#readByUid(uid)); + } + + async #readByUid(uid: string): Promise { const rows = await this.clients.db.read( `SELECT * FROM \`group\` WHERE \`uid\` = ? AND ${this.#live()}`, [uid, TEAM_KIND], @@ -270,15 +284,27 @@ export class TeamStore extends PuterStore { `UPDATE \`group\` SET ${sets.join(', ')} WHERE \`uid\` = ? AND ${this.#live()}`, [...params, uid, TEAM_KIND], ); + await this.#bustRow(uid); return this.getByUid(uid); } /** Releases the handle, since nothing addresses by it; keeps `name`. */ async softDelete(uid: string): Promise { + // Both resolved first: once `deleted_at` is set neither lookup finds it. + const team = await this.getByUid(uid); + const memberIds = team + ? await this.#readMemberIds(team.id, NOTIFY_FANOUT_CAP) + : []; const result = await this.clients.db.write( `UPDATE \`group\` SET \`deleted_at\` = CURRENT_TIMESTAMP, \`handle\` = NULL ` + `WHERE \`uid\` = ? AND ${this.#live()}`, [uid, TEAM_KIND], ); + if (result.anyRowsAffected && team) { + // Bounded by the fan-out cap, and this runs once per deletion. + await this.bustMembership(team.id); + await this.#bustRow(uid); + await Promise.all(memberIds.map((id) => this.bustMember(uid, id))); + } return result.anyRowsAffected; } @@ -288,6 +314,15 @@ export class TeamStore extends PuterStore { async getMembership( teamUid: string, userId: number, + ): Promise { + return this.#cached(`team:member:${teamUid}:${userId}`, () => + this.#readMembership(teamUid, userId), + ); + } + + async #readMembership( + teamUid: string, + userId: number, ): Promise { const rows = await this.clients.db.read( 'SELECT ug.`id`, ug.`user_id`, ug.`group_id`, ug.`org_owned`, ' + @@ -358,6 +393,87 @@ 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. + * + * `jct_user_group.user_id` is ON DELETE CASCADE, so deleting an account + * changes membership without passing through this store. The TTL is the + * backstop for that; the explicit busts cover everything else. + */ + async #cached(key: string, read: () => Promise): Promise { + try { + const hit = await this.clients.redis.get(key); + if (hit !== null) return JSON.parse(hit) as T; + } catch { + return read(); + } + const value = await read(); + try { + await this.clients.redis.set( + key, + JSON.stringify(value), + 'EX', + TEAM_CACHE_TTL_SECONDS, + ); + } catch { + /* a cache that cannot be written is still correct */ + } + return value; + } + + async #bust(...keys: string[]): Promise { + try { + await Promise.all(keys.map((k) => this.clients.redis.del(k))); + } catch { + /* the TTL clears it */ + } + } + + /** Busts everything keyed on this team's membership. */ + async bustMembership(groupId: number, userId?: number): Promise { + await this.#bust( + `team:members:${groupId}`, + ...(userId === undefined ? [] : [`team:seat:${userId}`]), + ); + } + + /** + * One membership pair. Authorization reads this, so every path that can + * change it busts here -- the TTL is not the mechanism. + */ + async bustMember(teamUid: string, userId: number): Promise { + await this.#bust(`team:member:${teamUid}:${userId}`); + } + + async #bustRow(uid: string): Promise { + await this.#bust(`team:row:${uid}`); + } + + async listMemberIdsByGroupId( + groupId: number, + limit = NOTIFY_FANOUT_CAP, + ): Promise { + if (limit !== NOTIFY_FANOUT_CAP) + return this.#readMemberIds(groupId, limit); + return this.#cached(`team:members:${groupId}`, () => + this.#readMemberIds(groupId, limit), + ); + } + + async #readMemberIds(groupId: number, limit: number): Promise { + const rows = (await this.clients.db.read( + 'SELECT ug.`user_id` FROM `jct_user_group` ug ' + + 'JOIN `group` g ON g.`id` = ug.`group_id` ' + + `WHERE ug.\`group_id\` = ? AND g.${this.#live()} ` + + 'ORDER BY ug.`id` LIMIT ?', + [groupId, TEAM_KIND, limit + 1], + )) as { user_id: number }[]; + return rows.map((r) => Number(r.user_id)); + } + /** Teams this user belongs to, oldest first. */ async listTeamsForUser(userId: number): Promise { const rows = await this.clients.db.read( @@ -396,6 +512,7 @@ export class TeamStore extends PuterStore { // Not `booleanValue`: it yields a real boolean on postgres. [userId, opts.orgOwned ? 1 : 0, teamUid, TEAM_KIND], ); + if (result.anyRowsAffected) await this.#bustForTeamUid(teamUid, userId); return result.anyRowsAffected; } @@ -433,6 +550,12 @@ export class TeamStore extends PuterStore { /** Soft-deleted teams count: their seats still hold billable bytes. */ async getOrgSeat(userId: number): Promise { + return this.#cached(`team:seat:${userId}`, () => + this.#readOrgSeat(userId), + ); + } + + async #readOrgSeat(userId: number): Promise { const rows = (await this.clients.db.read( 'SELECT ug.`id`, ug.`user_id`, u.`uuid`, u.`username`, ' + 'g.`uid` AS `team_uid`, g.`owner_user_id` ' + @@ -529,7 +652,14 @@ export class TeamStore extends PuterStore { `(SELECT \`id\` FROM \`group\` WHERE \`uid\` = ? AND ${this.#live()})`, [userId, teamUid, TEAM_KIND], ); + if (result.anyRowsAffected) await this.#bustForTeamUid(teamUid, userId); return result.anyRowsAffected; } + /** The membership row is gone by now, so the team is resolved by uid. */ + async #bustForTeamUid(teamUid: string, userId: number): Promise { + const team = await this.getByUidIncludingDeleted(teamUid); + await this.bustMembership(team?.id ?? -1, userId); + await this.bustMember(teamUid, userId); + } }