fix(share): keep downstream access from surviving a delegate who leaves

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();

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<string> = 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:<uid>`, 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<boolean> {
+    ): 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(