diff --git a/src/backend/services/share/ShareService.test.ts b/src/backend/services/share/ShareService.test.ts index 755eef8ad..6768a1519 100644 --- a/src/backend/services/share/ShareService.test.ts +++ b/src/backend/services/share/ShareService.test.ts @@ -135,6 +135,25 @@ describe('ShareService', () => { expect(listed.items.map((i) => i.entryUid)).toContain(file.uuid); }); + it('carries the entry metadata the file browser renders', async () => { + const owner = await makeUser(); + const recipient = await makeUser(); + const file = await makeFile(owner.user); + + await share(owner.actor, { + uid: file.uuid, + recipient: { email: recipient.email }, + mode: 'read', + }); + + const [listed] = ( + await server.services.share.listSharedWithMe(recipient.actor) + ).items; + expect(listed.modified).toBe(file.modified); + expect(Number.isFinite(listed.modified)).toBe(true); + expect(listed.size).toBe(file.size); + }); + it('resolves a recipient by username as well as email', async () => { const owner = await makeUser(); const recipient = await makeUser(); @@ -323,6 +342,66 @@ describe('ShareService', () => { ).toBe(dir.path); }); + it('takes downstream access with a delegate who leaves', 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); + + // What "Remove from Shared" calls. The delegate's grant goes, and with + // it the authority behind everything they issued. + await unshare(delegate.actor, { + uid: file.uuid, + recipient: { username: delegate.user.username }, + }); + + expect(await canRead(delegate.actor, file.path)).toBe(false); + expect(await canRead(third.actor, file.path)).toBe(false); + }); + + it('keeps the index row when the actor could not revoke anything', 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', + }); + await unshare(delegate.actor, { + uid: file.uuid, + recipient: { username: delegate.user.username }, + }); + + // A grant the owner cannot see is a grant nobody can withdraw, so the + // owner's view must not go quiet while access is still live. + const rows = await server.services.share.listSharesOf(owner.actor, { + uid: file.uuid, + }); + const stillListed = rows.map((r) => r.holder.username); + expect(stillListed).not.toContain(third.user.username); + expect(await canRead(third.actor, file.path)).toBe(false); + }); + it('lets a delegate clear only what it 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 0b660774e..db860302a 100644 --- a/src/backend/services/share/ShareService.ts +++ b/src/backend/services/share/ShareService.ts @@ -55,6 +55,8 @@ export interface ResolvedShare { createdAt: unknown; /** Set when the access comes from a shared ancestor, not this node. */ inheritedFrom?: string | null; + modified: number; + size: number | null; } const SHAREABLE_MODES: ReadonlySet = new Set([ @@ -325,26 +327,27 @@ export class ShareService extends PuterService { ] : [issuerId]; - let revoked = 0; + // Whatever the holder re-shared goes with them, and this has to run + // first: when the holder is the actor, clearing their own grants would + // strip the very `manage` the cascade needs to do it. + let revoked = await this.#revokeDownstream(actor, entry, holder.id); + for (const issuer of issuers) { - const didRevoke = await this.#revokeFor( + const { revoked: didRevoke, authorized } = await this.#revokeFor( actor, entry, holder.username, issuer as number, ); if (didRevoke) revoked++; - await this.stores.share.deleteActive({ - holderUserId: holder.id, - fsentryId: entry.id, - issuerUserId: issuer as number, - }); + if (authorized) { + await this.stores.share.deleteActive({ + holderUserId: holder.id, + fsentryId: entry.id, + 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 }; } @@ -382,21 +385,20 @@ export class ShareService extends PuterService { seen, ); - if ( - await this.#revokeFor( - actor, - entry, - downstream.username, - issuerId, - ) - ) { - revoked++; + const { revoked: didRevoke, authorized } = await this.#revokeFor( + actor, + entry, + downstream.username, + issuerId, + ); + if (didRevoke) revoked++; + if (authorized) { + await this.stores.share.deleteActive({ + holderUserId: holderId, + fsentryId: entry.id, + issuerUserId: issuerId, + }); } - await this.stores.share.deleteActive({ - holderUserId: holderId, - fsentryId: entry.id, - issuerUserId: issuerId, - }); } return revoked; } @@ -459,6 +461,8 @@ export class ShareService extends PuterService { issuer: { username: issuer?.username ?? null }, holder: { username: actor.user.username ?? null }, createdAt: row.created_at, + modified: entry.modified, + size: entry.size, }); } @@ -522,6 +526,8 @@ export class ShareService extends PuterService { }, createdAt: row.created_at, inheritedFrom: via, + modified: entry.modified, + size: entry.size, }), ); @@ -548,6 +554,8 @@ export class ShareService extends PuterService { }, createdAt: row.created_at, inheritedFrom: null, + modified: entry.modified, + size: entry.size, }), ); return inheritedShares.concat(own); @@ -570,6 +578,8 @@ export class ShareService extends PuterService { issuer: { username: issuer.user.username ?? null }, holder: { username: holder.username ?? null }, createdAt: row.created_at, + modified: entry.modified, + size: entry.size, }; } @@ -703,13 +713,16 @@ export class ShareService extends PuterService { * `manage` grant needs `manage:manage:fs:`, which only the owner * holds, so a delegate withdrawing a plain `read` would otherwise fail on * reaching the manage form. + * + * `authorized` is false when it could manage none of them — the caller must + * then leave the index row alone, or it hides a grant that is still live. */ async #revokeFor( actor: Actor, entry: FSEntry, username: string, issuerUserId: number, - ): Promise { + ): Promise<{ revoked: boolean; authorized: boolean }> { const permissions = [ `fs:${entry.uuid}:see`, `fs:${entry.uuid}:list`, @@ -742,7 +755,7 @@ export class ShareService extends PuterService { ); if (didRevoke) revoked = true; } - return revoked; + return { revoked, authorized: manageable.some(Boolean) }; } async #revokeQuietly(