feat(share): keep recipients consistent when a shared item changes

This commit is contained in:
Juan Castro
2026-08-12 18:12:30 -04:00
parent f9c9daea87
commit 6b5d6f80ba
2 changed files with 227 additions and 3 deletions
@@ -503,6 +503,121 @@ describe('ShareService', () => {
expect(rows).toEqual([]);
});
it('retires grants when the file is removed through the FS', 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',
});
expect(await canRead(recipient.actor, file.path)).toBe(true);
// The real delete path, not the hook. FSService emits without
// awaiting, so the cleanup lands shortly after `remove` resolves.
await server.services.fs.remove(owner.user.id, { entry: file });
let rows = [] as unknown[];
for (let attempt = 0; attempt < 50; attempt++) {
rows = await server.stores.permission.readLinkedUserUserPerms(
recipient.user.id,
[`fs:${file.uuid}:read`],
);
if (rows.length === 0) break;
await new Promise((resolve) => setTimeout(resolve, 20));
}
expect(rows).toEqual([]);
expect(await canRead(recipient.actor, file.path)).toBe(false);
});
describe('keeping recipients in sync', () => {
/** Collect one GUI event's audiences for the life of the callback. */
const captureAudiences = async (
event:
| 'outer.gui.item.removed'
| 'outer.gui.item.moved'
| 'outer.gui.item.updated',
uuid: string,
fn: () => Promise<void>,
) => {
const seen: number[][] = [];
server.clients.event.on(event, (_key, data) => {
const payload = data as {
user_id_list?: number[];
response?: { uuid?: string };
};
if (payload.response?.uuid !== uuid) return;
seen.push(payload.user_id_list ?? []);
});
await fn();
for (let i = 0; i < 50 && seen.length === 0; i++) {
await new Promise((resolve) => setTimeout(resolve, 20));
}
return seen;
};
it('tells a recipient when a shared file is deleted', 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',
});
// Without this the recipient's open window shows a file that is
// gone until their next request happens to fail.
const audiences = await captureAudiences(
'outer.gui.item.removed',
file.uuid,
async () => {
await server.services.fs.remove(owner.user.id, {
entry: file,
});
},
);
expect(audiences.flat()).toContain(recipient.user.id);
});
it('tells a recipient when a shared file moves', 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 audiences = await captureAudiences(
'outer.gui.item.moved',
file.uuid,
async () => {
await server.clients.event.emitAndWait(
'fs.move.node',
{
node: file,
fromPath: file.path,
toPath: `${file.path}-moved`,
},
{},
);
},
);
expect(audiences.flat()).toContain(recipient.user.id);
// The owner already gets their own event from the FS layer;
// announcing again here would double it up.
expect(audiences.flat()).not.toContain(owner.user.id);
});
});
it('paginates what has been shared with me', async () => {
const owner = await makeUser();
const recipient = await makeUser();
+112 -3
View File
@@ -83,6 +83,109 @@ export const DEFAULT_DAILY_SHARE_LIMIT = 200;
export class ShareService extends PuterService {
declare protected services: LayerInstances<typeof puterServices>;
/**
* FS mutations only notify the owner, leaving a recipient's open window
* stale. Handled here rather than per controller so the audience logic
* lives in one place, and over the event bus because fs is constructed
* first and cannot depend on this service.
*/
override onServerStart(): void {
this.clients.event.on('fs.remove.node', (_key, data) => {
const entry = (data as { node?: FSEntry })?.node;
if (!entry?.uuid) return;
// Returned so an `emitAndWait` caller can observe the cleanup; the
// FS path uses plain `emit`, where it stays best-effort.
return this.#onEntryRemoved(entry).catch((err) => {
console.warn(
'[ShareService] failed to retire grants for a deleted entry:',
entry.uuid,
err,
);
});
});
this.clients.event.on('fs.move.node', (_key, data) => {
const { node, fromPath } = (data ?? {}) as {
node?: FSEntry;
fromPath?: string;
};
if (!node?.uuid) return;
return this.#fanOutToHolders(node, 'outer.gui.item.moved', {
...node,
from_path: fromPath,
from_new_service: true,
}).catch(() => {
// A stale window is better than a failed move.
});
});
this.clients.event.on('fs.write.file', (_key, data) => {
const entry = (data as { node?: FSEntry })?.node;
if (!entry?.uuid) return;
return this.#fanOutToHolders(entry, 'outer.gui.item.updated', {
...entry,
from_new_service: true,
}).catch(() => {
// Same — never fail a write over its notification.
});
});
}
/**
* Retire the grants, then tell the recipients. The revoke reports exactly
* who lost access, which the index can no longer answer — its rows cascade
* away with the fsentry.
*/
async #onEntryRemoved(entry: FSEntry): Promise<void> {
const removed = await this.onEntryDeleted(entry.uuid);
const holders = [
...new Set(removed.map((row) => Number(row.holder_user_id))),
].filter((id) => Number.isFinite(id) && id !== entry.userId);
if (holders.length === 0) return;
await this.#emitGui('outer.gui.item.removed', holders, {
...entry,
from_new_service: true,
});
}
async #fanOutToHolders(
entry: FSEntry,
event: 'outer.gui.item.moved' | 'outer.gui.item.updated',
response: Record<string, unknown>,
): Promise<void> {
const rows = await this.stores.share.listByFsentry(entry.id);
const holders = [
...new Set(
rows.map((row: { holder_user_id: number }) =>
Number(row.holder_user_id),
),
),
].filter((id) => Number.isFinite(id) && id !== entry.userId);
if (holders.length === 0) return;
await this.#emitGui(event, holders, response);
}
async #emitGui(
event:
| 'outer.gui.item.removed'
| 'outer.gui.item.moved'
| 'outer.gui.item.updated',
userIds: number[],
response: Record<string, unknown>,
): Promise<void> {
try {
await this.clients.event.emit(
event,
{ user_id_list: userIds, response },
{},
);
} catch {
// Non-critical.
}
}
// -- Writes -------------------------------------------------------
/**
@@ -238,9 +341,15 @@ export class ShareService extends PuterService {
return { revoked };
}
/** Retire the grants pointing at a node that no longer exists. */
async onEntryDeleted(entryUid: string): Promise<void> {
await this.stores.permission.deleteUserUserPermsByPermissionPrefix(
/**
* 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
* cascade away with the fsentry.
*/
async onEntryDeleted(
entryUid: string,
): Promise<Array<{ holder_user_id: number; issuer_user_id: number }>> {
return this.stores.permission.deleteUserUserPermsByPermissionPrefix(
`fs:${entryUid}`,
);
}