From 06248b667accd86269dc020212947039261aa69f Mon Sep 17 00:00:00 2001 From: Juan Castro Date: Thu, 13 Aug 2026 13:31:50 -0400 Subject: [PATCH] fix(share): withdraw what a removed recipient re-shared --- .../services/share/ShareService.test.ts | 34 ++++++ src/backend/services/share/ShareService.ts | 100 +++++++++++++++++- src/docs/src/FS/unshare.md | 2 + 3 files changed, 134 insertions(+), 2 deletions(-) diff --git a/src/backend/services/share/ShareService.test.ts b/src/backend/services/share/ShareService.test.ts index 9055fb23a..9bb74e5ec 100644 --- a/src/backend/services/share/ShareService.test.ts +++ b/src/backend/services/share/ShareService.test.ts @@ -288,6 +288,40 @@ describe('ShareService', () => { expect(await canRead(fourth.actor, file.path)).toBe(true); }); + it('revoking a delegate also revokes what they re-shared', async () => { + const owner = await makeUser(); + const delegate = await makeUser(); + const third = await makeUser(); + const file = await makeFile(owner.user); + + await share(owner.actor, { + uid: file.uuid, + recipient: { email: delegate.email }, + mode: 'manage', + }); + await share(delegate.actor, { + uid: file.uuid, + recipient: { email: third.email }, + mode: 'read', + }); + expect(await canRead(third.actor, file.path)).toBe(true); + + await unshare(owner.actor, { + uid: file.uuid, + recipient: { email: delegate.email }, + }); + + // The delegate's authority to grant came from access the owner has + // now withdrawn, so what they granted cannot outlive it. + expect(await canRead(delegate.actor, file.path)).toBe(false); + expect(await canRead(third.actor, file.path)).toBe(false); + expect( + await server.services.share.listSharesOf(owner.actor, { + uid: file.uuid, + }), + ).toEqual([]); + }); + it('lets the owner clear a grant a delegate issued', async () => { const owner = await makeUser(); const delegate = await makeUser(); diff --git a/src/backend/services/share/ShareService.ts b/src/backend/services/share/ShareService.ts index a8a8fa2d7..6916074c5 100644 --- a/src/backend/services/share/ShareService.ts +++ b/src/backend/services/share/ShareService.ts @@ -53,6 +53,8 @@ export interface ResolvedShare { issuer: { username: string | null }; holder: { username: string | null }; createdAt: unknown; + /** Set when the access comes from a shared ancestor, not this node. */ + inheritedFrom?: string | null; } const SHAREABLE_MODES: ReadonlySet = new Set([ @@ -338,9 +340,67 @@ export class ShareService extends PuterService { issuerUserId: issuer as number, }); } + + // Whatever the holder re-shared goes with them. Their authority to + // grant came from this access, so leaving those behind would let + // access outlive the permission it was derived from. + revoked += await this.#revokeDownstream(actor, entry, holder.id); return { revoked }; } + /** + * Withdraw everything `issuerId` granted on this node, and everything those + * recipients granted in turn. + * + * `seen` guards the walk: two delegates can each have granted the other, + * and without it the recursion would not terminate. + */ + async #revokeDownstream( + actor: Actor, + entry: FSEntry, + issuerId: number, + seen: Set = new Set(), + ): Promise { + if (seen.has(issuerId)) return 0; + seen.add(issuerId); + + const rows = (await this.stores.share.listByFsentry(entry.id)).filter( + (row: { issuer_user_id: number }) => + Number(row.issuer_user_id) === issuerId, + ); + + let revoked = 0; + for (const row of rows) { + const holderId = Number(row.holder_user_id); + const downstream = await this.stores.user.getById(holderId); + if (!downstream?.username) continue; + + revoked += await this.#revokeDownstream( + actor, + entry, + holderId, + seen, + ); + + if ( + await this.#revokeFor( + actor, + entry, + downstream.username, + issuerId, + ) + ) { + revoked++; + } + await this.stores.share.deleteActive({ + holderUserId: holderId, + fsentryId: entry.id, + issuerUserId: issuerId, + }); + } + return revoked; + } + /** * Retire the grants pointing at a node that no longer exists. Returns the * rows removed, which is the only record of who had access — the index rows @@ -422,8 +482,22 @@ export class ShareService extends PuterService { const entry = await this.#resolveEntry(target); await this.#assertCanManage(actor, entry); + // Access is inherited down the tree, so a node's own rows are only + // half the answer — without the ancestors' the caller is told nobody + // can reach a file that several people can. + const ancestors = await this.services.fs.getAncestorChain(entry.path); + const inherited: Array<{ row: Record; via: string }> = + []; + for (const ancestor of ancestors.slice(1)) { + const node = await this.stores.fsEntry.getEntryByUuid(ancestor.uid); + if (!node) continue; + for (const row of await this.stores.share.listByFsentry(node.id)) { + inherited.push({ row, via: ancestor.path }); + } + } + const rows = await this.stores.share.listByFsentry(entry.id); - const userIds = rows.flatMap( + const userIds = [...rows, ...inherited.map((i) => i.row)].flatMap( (row: { issuer_user_id: number; holder_user_id: number }) => [ Number(row.issuer_user_id), Number(row.holder_user_id), @@ -431,7 +505,27 @@ export class ShareService extends PuterService { ); const users = await this.stores.user.getByIds(userIds); - return rows.map( + const inheritedShares: ResolvedShare[] = inherited.map( + ({ row, via }) => ({ + uid: String(row.uid), + mode: String(row.mode), + path: entry.path, + entryUid: entry.uuid, + isDir: Boolean(entry.isDir), + issuer: { + username: + users.get(Number(row.issuer_user_id))?.username ?? null, + }, + holder: { + username: + users.get(Number(row.holder_user_id))?.username ?? null, + }, + createdAt: row.created_at, + inheritedFrom: via, + }), + ); + + const own: ResolvedShare[] = rows.map( (row: { uid: string; mode: string; @@ -453,8 +547,10 @@ export class ShareService extends PuterService { users.get(Number(row.holder_user_id))?.username ?? null, }, createdAt: row.created_at, + inheritedFrom: null, }), ); + return inheritedShares.concat(own); } // -- Internals ---------------------------------------------------- diff --git a/src/docs/src/FS/unshare.md b/src/docs/src/FS/unshare.md index 44337fc6b..6f49b0da9 100644 --- a/src/docs/src/FS/unshare.md +++ b/src/docs/src/FS/unshare.md @@ -45,6 +45,8 @@ A `Promise` that resolves to `{ revoked }`, where `revoked` is how many grants w An item's owner cannot be removed from their own item. +Withdrawing someone's access also withdraws whatever **they** re-shared of that item. Their authority to grant came from the access being removed, so it cannot outlive it. + ## Examples Stop sharing a file