mirror of
https://github.com/mauriceboe/TREK.git
synced 2026-10-18 21:16:47 +00:00
Move packing onto the decorators and de-duplicate its privacy fan-out
Nine more methods, so the registry owns 24 of 113. The #858 privacy logic existed three times: PackingService had the primitives, the controller layered emitToViewers and broadcastUpdate on top privately, and the plugin deps factory carried a standalone copy of all four, with a comment asking for them to be kept in lockstep by hand. Migrating packing meant either copying them a fourth time or fixing that, so the two private controller methods moved into PackingService and the factory copy is gone. Controller and PackingRpc now call the same code, and the controller suite proves it by passing with all 45 assertions unchanged. The four public/private transitions get one test each, because each one is a way to leak a private item to the whole trip room, plus a test that the privacy is read before the write rather than after: reading it afterwards would report "already private", skip the room delete, and leave the item on every member's screen.
This commit is contained in:
Maurice
@@ -109,21 +109,10 @@ export class PackingController {
|
||||
this.requireEdit(trip, user);
|
||||
// checked arrives as boolean or legacy 0/1 — the service coerces by truthiness.
|
||||
const item = this.packing.createItem(tripId, { name: body.name, category: body.category, checked: body.checked === undefined ? undefined : !!body.checked, is_private: body.is_private, visibility: body.visibility, recipient_ids: body.recipient_ids }, user.id);
|
||||
this.emitToViewers(tripId, 'packing:created', { item }, item, socketId);
|
||||
this.packing.emitToViewers(tripId, 'packing:created', { item }, item, socketId);
|
||||
return { item };
|
||||
}
|
||||
|
||||
/** Deliver an item event to exactly the people who can see it (#858): the whole
|
||||
* room for a Common item, or owner + recipients for a restricted one. */
|
||||
private emitToViewers<E extends TrekWsTripEventName>(tripId: string, event: E, payload: TrekWsPayload<E>, item: PackingItemRow, socketId: string | undefined): void {
|
||||
const viewers = this.packing.viewersOf(item);
|
||||
if (viewers === null) {
|
||||
this.packing.broadcast(tripId, event, payload, socketId);
|
||||
} else {
|
||||
this.packing.broadcastToViewers(tripId, event, payload, viewers, socketId);
|
||||
}
|
||||
}
|
||||
|
||||
@Put('reorder')
|
||||
reorder(
|
||||
@CurrentUser() user: User,
|
||||
@@ -163,41 +152,10 @@ export class PackingController {
|
||||
if (isUpdateConflict(updated)) {
|
||||
throw new HttpException({ error: 'conflict', server: updated.server }, 409);
|
||||
}
|
||||
this.broadcastUpdate(tripId, id, updated as PackingItemRow, !!before?.is_private, socketId);
|
||||
this.packing.broadcastUpdate(tripId, id, updated as PackingItemRow, !!before?.is_private, socketId);
|
||||
return { item: updated };
|
||||
}
|
||||
|
||||
/**
|
||||
* Routes a packing-item update over WebSocket so private items (#858) stay
|
||||
* scoped to their owner across the four public↔private transitions:
|
||||
* - stays private → owner-only update
|
||||
* - public→private → drop it from the whole room, re-add for the owner
|
||||
* - private→public → create for members who lacked it, then update for all
|
||||
* - stays public → plain update to all
|
||||
*/
|
||||
private broadcastUpdate(
|
||||
tripId: string,
|
||||
id: string,
|
||||
item: PackingItemRow,
|
||||
wasPrivate: boolean,
|
||||
socketId: string | undefined,
|
||||
): void {
|
||||
const nowPrivate = !!item.is_private;
|
||||
if (nowPrivate) {
|
||||
if (wasPrivate) {
|
||||
this.packing.broadcastItem(tripId, 'packing:updated', { item }, item, socketId);
|
||||
} else {
|
||||
this.packing.broadcast(tripId, 'packing:deleted', { itemId: Number(id) }, socketId);
|
||||
this.packing.broadcastItem(tripId, 'packing:created', { item }, item, socketId);
|
||||
}
|
||||
} else {
|
||||
if (wasPrivate) {
|
||||
this.packing.broadcast(tripId, 'packing:created', { item }, socketId);
|
||||
}
|
||||
this.packing.broadcast(tripId, 'packing:updated', { item }, socketId);
|
||||
}
|
||||
}
|
||||
|
||||
@Delete(':id')
|
||||
remove(
|
||||
@CurrentUser() user: User,
|
||||
@@ -212,7 +170,7 @@ export class PackingController {
|
||||
throw new HttpException({ error: 'Item not found' }, 404);
|
||||
}
|
||||
// Scope the delete to the people who could see it (owner + recipients, #858).
|
||||
this.emitToViewers(tripId, 'packing:deleted', { itemId: Number(id) }, deleted as PackingItemRow, socketId);
|
||||
this.packing.emitToViewers(tripId, 'packing:deleted', { itemId: Number(id) }, deleted as PackingItemRow, socketId);
|
||||
return { success: true };
|
||||
}
|
||||
|
||||
@@ -236,7 +194,7 @@ export class PackingController {
|
||||
// The viewer set just changed: drop the item from the whole room, then re-add
|
||||
// it for whoever can now see it (owner + recipients, or everyone if Common).
|
||||
this.packing.broadcast(tripId, 'packing:deleted', { itemId: Number(id) }, socketId);
|
||||
this.emitToViewers(tripId, 'packing:created', { item: updated }, updated as PackingItemRow, socketId);
|
||||
this.packing.emitToViewers(tripId, 'packing:created', { item: updated }, updated as PackingItemRow, socketId);
|
||||
return { item: updated };
|
||||
}
|
||||
|
||||
@@ -255,7 +213,7 @@ export class PackingController {
|
||||
throw new HttpException({ error: 'Item not found' }, 404);
|
||||
}
|
||||
// The clone is personal to the caller — only their sockets need it.
|
||||
this.emitToViewers(tripId, 'packing:created', { item }, item, socketId);
|
||||
this.packing.emitToViewers(tripId, 'packing:created', { item }, item, socketId);
|
||||
return { item };
|
||||
}
|
||||
|
||||
|
||||
@@ -2,6 +2,9 @@ import { Module } from '@nestjs/common';
|
||||
import { PackingController } from './packing.controller';
|
||||
import { PackingMcp } from './packing.mcp';
|
||||
import { PackingService } from './packing.service';
|
||||
import { PackingRpc } from './packing.rpc';
|
||||
import { PluginGuardsModule } from '../plugins/host/plugin-guards.module';
|
||||
import { RealtimeModule } from '../realtime/realtime.module';
|
||||
import { PermissionsModule } from '../permissions/permissions.module';
|
||||
import { AuthModule } from '../auth/auth.module';
|
||||
|
||||
@@ -9,9 +12,9 @@ import { AuthModule } from '../auth/auth.module';
|
||||
* Exports PackingService for in-container consumers (TripsService bundle,
|
||||
* PluginHostDepsFactory). */
|
||||
@Module({
|
||||
imports: [PermissionsModule, AuthModule],
|
||||
imports: [PermissionsModule, AuthModule, RealtimeModule, PluginGuardsModule],
|
||||
controllers: [PackingController],
|
||||
providers: [PackingService, PackingMcp],
|
||||
providers: [PackingService, PackingMcp, PackingRpc],
|
||||
exports: [PackingService],
|
||||
})
|
||||
export class PackingModule {}
|
||||
|
||||
@@ -0,0 +1,144 @@
|
||||
import { packingCreateItemRequestSchema, packingUpdateItemRequestSchema } from '@trek/shared';
|
||||
import { PluginController, PluginMethod } from '../plugins/host/rpc-kit/decorators';
|
||||
import { PluginGuards } from '../plugins/host/plugin-guards.service';
|
||||
import { BadParams, ForbiddenResource } from '../plugins/host/rpc-errors';
|
||||
import { asPayload, num } from '../plugins/host/rpc-params';
|
||||
import type { PluginRpcContext } from '../plugins/host/rpc-kit/types';
|
||||
import { RealtimeService } from '../realtime/realtime.service';
|
||||
import { PackingService } from './packing.service';
|
||||
import { isUpdateConflict } from '../common/conflictResult';
|
||||
|
||||
/** Packing rides on the app's own 'packing_edit' permission, exactly like the REST path. */
|
||||
const PACKING_EDIT_ACTION = 'packing_edit';
|
||||
|
||||
type PrivacyItem = { is_private?: number; owner_id?: number | null; recipients?: { user_id: number }[] };
|
||||
|
||||
/**
|
||||
* The packing surface a plugin may reach (#plugins).
|
||||
*
|
||||
* The privacy-scoped broadcasts (#858) go through PackingService, the same methods
|
||||
* the REST controller calls. They used to be duplicated inside the plugin deps
|
||||
* factory, with a comment asking for both copies to be kept in lockstep by hand;
|
||||
* that copy is gone. Getting this wrong leaks a private item to the whole trip room,
|
||||
* which is why `wasPrivate` is read BEFORE the write and why the four transitions
|
||||
* live in exactly one place.
|
||||
*
|
||||
* Bags carry no privacy, so they broadcast to the room unfiltered.
|
||||
*/
|
||||
@PluginController()
|
||||
export class PackingRpc {
|
||||
constructor(
|
||||
private readonly packing: PackingService,
|
||||
private readonly realtime: RealtimeService,
|
||||
private readonly guards: PluginGuards,
|
||||
) {}
|
||||
|
||||
@PluginMethod('packing.list', { permission: 'db:read:packing' })
|
||||
list(params: Record<string, unknown>, ctx: PluginRpcContext): unknown {
|
||||
// Scoped to the acting user so the #858 visibility filter applies: a plugin must
|
||||
// not see another member's private items.
|
||||
return this.guards.tripRead(params, ctx, (userId) => this.packing.listItems(num(params.tripId, 'tripId'), userId));
|
||||
}
|
||||
|
||||
@PluginMethod('packing.create', { permission: 'db:write:packing' })
|
||||
create(params: Record<string, unknown>, ctx: PluginRpcContext): unknown {
|
||||
const tripId = num(params.tripId, 'tripId');
|
||||
const actor = this.guards.requireActor(ctx, 'packing item');
|
||||
const parsed = packingCreateItemRequestSchema.safeParse(params.input);
|
||||
if (!parsed.success) throw new BadParams(`invalid packing item: ${parsed.error.issues[0]?.message ?? 'bad input'}`);
|
||||
this.guards.requireTripEdit(tripId, actor, PACKING_EDIT_ACTION);
|
||||
const item = this.packing.createItem(String(tripId), parsed.data as never, actor) as PrivacyItem;
|
||||
this.packing.emitToViewers(String(tripId), 'packing:created', { item }, item, undefined);
|
||||
return item;
|
||||
}
|
||||
|
||||
@PluginMethod('packing.update', { permission: 'db:write:packing' })
|
||||
update(params: Record<string, unknown>, ctx: PluginRpcContext): unknown {
|
||||
const tripId = num(params.tripId, 'tripId');
|
||||
const itemId = num(params.itemId, 'itemId');
|
||||
const actor = this.guards.requireActor(ctx, 'packing item');
|
||||
const parsed = packingUpdateItemRequestSchema.safeParse(params.input);
|
||||
if (!parsed.success) throw new BadParams(`invalid packing item: ${parsed.error.issues[0]?.message ?? 'bad input'}`);
|
||||
this.guards.requireTripEdit(tripId, actor, PACKING_EDIT_ACTION);
|
||||
// Read the privacy BEFORE the write, so a public/private toggle routes correctly.
|
||||
const before = this.packing.getItemPrivacy(tripId, itemId);
|
||||
const input = parsed.data as Record<string, unknown>;
|
||||
const updated = this.packing.updateItem(String(tripId), String(itemId), input as never, Object.keys(input), undefined, actor);
|
||||
if (!updated) throw new ForbiddenResource(`no packing item ${itemId} on trip ${tripId}`);
|
||||
if (isUpdateConflict(updated)) throw new BadParams('packing item was modified concurrently');
|
||||
this.packing.broadcastUpdate(String(tripId), itemId, updated as PrivacyItem, !!before?.is_private, undefined);
|
||||
return updated;
|
||||
}
|
||||
|
||||
@PluginMethod('packing.delete', { permission: 'db:write:packing' })
|
||||
delete(params: Record<string, unknown>, ctx: PluginRpcContext): unknown {
|
||||
const tripId = num(params.tripId, 'tripId');
|
||||
const itemId = num(params.itemId, 'itemId');
|
||||
const actor = this.guards.requireActor(ctx, 'packing item');
|
||||
this.guards.requireTripEdit(tripId, actor, PACKING_EDIT_ACTION);
|
||||
const deleted = this.packing.deleteItem(String(tripId), String(itemId)) as PrivacyItem | null;
|
||||
if (!deleted) throw new ForbiddenResource(`no packing item ${itemId} on trip ${tripId}`);
|
||||
this.packing.emitToViewers(String(tripId), 'packing:deleted', { itemId }, deleted, undefined);
|
||||
return { deleted: true };
|
||||
}
|
||||
|
||||
@PluginMethod('packing.listBags', { permission: 'db:write:packing' })
|
||||
listBags(params: Record<string, unknown>, ctx: PluginRpcContext): unknown[] {
|
||||
// Note the permission: the envelope really does gate this READ on the write
|
||||
// grant. The decorator makes the oddity visible instead of burying it.
|
||||
return this.guards.tripRead(params, ctx, () => this.packing.listBags(String(num(params.tripId, 'tripId'))) as unknown[]);
|
||||
}
|
||||
|
||||
@PluginMethod('packing.createBag', { permission: 'db:write:packing' })
|
||||
createBag(params: Record<string, unknown>, ctx: PluginRpcContext): unknown {
|
||||
const tripId = num(params.tripId, 'tripId');
|
||||
const actor = this.guards.requireActor(ctx, 'packing bag');
|
||||
const input = asPayload(params.input);
|
||||
if (typeof input.name !== 'string' || input.name.trim() === '') throw new BadParams('bag name is required');
|
||||
this.guards.requireTripEdit(tripId, actor, PACKING_EDIT_ACTION);
|
||||
const bag = this.packing.createBag(String(tripId), { name: input.name, color: typeof input.color === 'string' ? input.color : undefined });
|
||||
this.realtime.broadcast(tripId, 'packing:bag-created', { bag }, undefined);
|
||||
return bag;
|
||||
}
|
||||
|
||||
@PluginMethod('packing.updateBag', { permission: 'db:write:packing' })
|
||||
updateBag(params: Record<string, unknown>, ctx: PluginRpcContext): unknown {
|
||||
const tripId = num(params.tripId, 'tripId');
|
||||
const bagId = num(params.bagId, 'bagId');
|
||||
const actor = this.guards.requireActor(ctx, 'packing bag');
|
||||
this.guards.requireTripEdit(tripId, actor, PACKING_EDIT_ACTION);
|
||||
const input = asPayload(params.input);
|
||||
const bag = this.packing.updateBag(String(tripId), String(bagId), input as never, Object.keys(input));
|
||||
if (!bag) throw new ForbiddenResource(`no packing bag ${bagId} on trip ${tripId}`);
|
||||
this.realtime.broadcast(tripId, 'packing:bag-updated', { bag }, undefined);
|
||||
return bag;
|
||||
}
|
||||
|
||||
@PluginMethod('packing.deleteBag', { permission: 'db:write:packing' })
|
||||
deleteBag(params: Record<string, unknown>, ctx: PluginRpcContext): unknown {
|
||||
const tripId = num(params.tripId, 'tripId');
|
||||
const bagId = num(params.bagId, 'bagId');
|
||||
const actor = this.guards.requireActor(ctx, 'packing bag');
|
||||
this.guards.requireTripEdit(tripId, actor, PACKING_EDIT_ACTION);
|
||||
if (!this.packing.deleteBag(String(tripId), String(bagId))) {
|
||||
throw new ForbiddenResource(`no packing bag ${bagId} on trip ${tripId}`);
|
||||
}
|
||||
this.realtime.broadcast(tripId, 'packing:bag-deleted', { bagId }, undefined);
|
||||
return { deleted: true };
|
||||
}
|
||||
|
||||
@PluginMethod('packing.setBagMembers', { permission: 'db:write:packing' })
|
||||
setBagMembers(params: Record<string, unknown>, ctx: PluginRpcContext): unknown {
|
||||
const tripId = num(params.tripId, 'tripId');
|
||||
const bagId = num(params.bagId, 'bagId');
|
||||
const actor = this.guards.requireActor(ctx, 'packing bag');
|
||||
this.guards.requireTripEdit(tripId, actor, PACKING_EDIT_ACTION);
|
||||
// userIds sits on the params object itself, not under `input`.
|
||||
const raw = asPayload(params).userIds;
|
||||
const userIds = Array.isArray(raw) ? raw.filter((x): x is number => typeof x === 'number') : [];
|
||||
const members = this.packing.setBagMembers(String(tripId), String(bagId), userIds);
|
||||
if (!members) throw new ForbiddenResource(`no packing bag ${bagId} on trip ${tripId}`);
|
||||
this.realtime.broadcast(tripId, 'packing:bag-members-updated', { bagId, members }, undefined);
|
||||
return members;
|
||||
}
|
||||
}
|
||||
@@ -84,6 +84,49 @@ export class PackingService {
|
||||
return ids;
|
||||
}
|
||||
|
||||
/** Deliver an item event to exactly the people who can see it (#858): the whole
|
||||
* room for a Common item, or owner + recipients for a restricted one. */
|
||||
emitToViewers<E extends TrekWsTripEventName>(tripId: string, event: E, payload: TrekWsPayload<E>, item: PrivacyFields | null | undefined, socketId: string | undefined): void {
|
||||
const viewers = this.viewersOf(item);
|
||||
if (viewers === null) {
|
||||
this.broadcast(tripId, event, payload, socketId);
|
||||
} else {
|
||||
this.broadcastToViewers(tripId, event, payload, viewers, socketId);
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* The four public/private transitions after an update (#858). `wasPrivate` must be
|
||||
* read BEFORE the write: getting it wrong leaks a freshly-privatized item to the
|
||||
* whole room.
|
||||
*
|
||||
* - private -> private: owner-only update
|
||||
* - public -> private: drop it from the room, then re-add it for the owner
|
||||
* - private -> public: add it for the members who did not have it, then update
|
||||
* - public -> public: a plain update to everyone
|
||||
*
|
||||
* Both the REST controller and the plugin RPC handler call this. It used to exist
|
||||
* three times over (here, in the controller, and as a standalone copy inside the
|
||||
* plugin deps factory), with a comment asking for all of them to be kept in
|
||||
* lockstep by hand.
|
||||
*/
|
||||
broadcastUpdate(tripId: string, id: string | number, item: PrivacyFields, wasPrivate: boolean, socketId: string | undefined): void {
|
||||
const nowPrivate = !!item.is_private;
|
||||
if (nowPrivate) {
|
||||
if (wasPrivate) {
|
||||
this.broadcastItem(tripId, 'packing:updated', { item } as TrekWsPayload<'packing:updated'>, item, socketId);
|
||||
} else {
|
||||
this.broadcast(tripId, 'packing:deleted', { itemId: Number(id) }, socketId);
|
||||
this.broadcastItem(tripId, 'packing:created', { item } as TrekWsPayload<'packing:created'>, item, socketId);
|
||||
}
|
||||
} else {
|
||||
if (wasPrivate) {
|
||||
this.broadcast(tripId, 'packing:created', { item } as TrekWsPayload<'packing:created'>, socketId);
|
||||
}
|
||||
this.broadcast(tripId, 'packing:updated', { item } as TrekWsPayload<'packing:updated'>, socketId);
|
||||
}
|
||||
}
|
||||
|
||||
// ── Items ──────────────────────────────────────────────────────────────────
|
||||
|
||||
/**
|
||||
|
||||
@@ -1,9 +1,6 @@
|
||||
import { Injectable } from '@nestjs/common';
|
||||
import type { TrekWsPayload, TrekWsTripEventName } from '@trek/shared';
|
||||
import { readEnv } from '../../../app-config';
|
||||
import { RealtimeService } from '../../realtime/realtime.service';
|
||||
import { isUpdateConflict } from '../../common/conflictResult';
|
||||
import { getWeather } from '../../weather/weather.impl';
|
||||
import { BLOCKED_EXTENSIONS, filesDir } from '../../files/files.constants';
|
||||
import { TripMembershipService } from '../../trip-membership/trip-membership.service';
|
||||
import { NotificationsService } from '../../notifications/notifications.service';
|
||||
@@ -64,54 +61,10 @@ function mirrorJourneys(run: () => void): void {
|
||||
try { run(); } catch { /* non-fatal */ }
|
||||
}
|
||||
|
||||
// --- #858 packing privacy: viewer-scoped fan-out mirrored from
|
||||
// packing.controller.broadcastUpdate/emitToViewers and PackingService's
|
||||
// broadcastItem/viewersOf/broadcastToViewers. A private item's events reach ONLY
|
||||
// its owner (+ recipients for a shared item), never the whole trip room; passing the
|
||||
// wrong onlyUserId — or forgetting to drop a freshly-privatized item from the room —
|
||||
// leaks it. Kept as a local copy (rather than delegating to the injected
|
||||
// PackingService) so the factory's unit test can assert the fan-out through the
|
||||
// websocket mock while PackingService stays a plain data stub. Keep in lockstep
|
||||
// with packing.controller.ts + packing.service.ts. ---
|
||||
type PackingPrivacy = { is_private?: number; owner_id?: number | null; recipients?: { user_id: number }[] };
|
||||
|
||||
function packingViewersOf(item: PackingPrivacy | null | undefined): number[] | null {
|
||||
if (!item || !item.is_private) return null; // Common — visible to the whole room
|
||||
return [item.owner_id, ...(item.recipients || []).map((r) => r.user_id)].filter((x): x is number => x != null);
|
||||
}
|
||||
|
||||
/** CREATE/DELETE fan-out: whole room for a Common item, else owner + recipients only. */
|
||||
function emitPackingToViewers<E extends TrekWsTripEventName>(realtime: RealtimeService, tripId: number, event: E, payload: TrekWsPayload<E>, item: PackingPrivacy): void {
|
||||
const viewers = packingViewersOf(item);
|
||||
if (viewers === null) {
|
||||
realtime.broadcast(tripId, event, payload, undefined);
|
||||
return;
|
||||
}
|
||||
for (const uid of new Set(viewers)) if (uid != null) realtime.broadcast(tripId, event, payload, undefined, uid);
|
||||
}
|
||||
|
||||
/** An item event delivered owner-only when the item is private (else to the room). */
|
||||
function broadcastPackingItem<E extends TrekWsTripEventName>(realtime: RealtimeService, tripId: number, event: E, payload: TrekWsPayload<E>, item: PackingPrivacy): void {
|
||||
const onlyUserId = item?.is_private && item.owner_id != null ? item.owner_id : undefined;
|
||||
realtime.broadcast(tripId, event, payload, undefined, onlyUserId);
|
||||
}
|
||||
|
||||
/** The four public<->private transitions (packing.controller.broadcastUpdate). `wasPrivate`
|
||||
* is read BEFORE the write — getting this wrong LEAKS a freshly-privatized item. */
|
||||
function broadcastPackingUpdate(realtime: RealtimeService, tripId: number, itemId: number, item: PackingPrivacy, wasPrivate: boolean): void {
|
||||
const nowPrivate = !!item.is_private;
|
||||
if (nowPrivate) {
|
||||
if (wasPrivate) {
|
||||
broadcastPackingItem(realtime, tripId, 'packing:updated', { item }, item); // stays private -> owner-only
|
||||
} else {
|
||||
realtime.broadcast(tripId, 'packing:deleted', { itemId }, undefined); // public->private: drop from the room...
|
||||
broadcastPackingItem(realtime, tripId, 'packing:created', { item }, item); // ...then re-add for the owner
|
||||
}
|
||||
} else {
|
||||
if (wasPrivate) realtime.broadcast(tripId, 'packing:created', { item }, undefined); // private->public: add for members who lacked it
|
||||
realtime.broadcast(tripId, 'packing:updated', { item }, undefined);
|
||||
}
|
||||
}
|
||||
// The #858 privacy-scoped packing broadcasts used to be copied here, with a comment
|
||||
// asking for the copy to be kept in lockstep with packing.controller.ts and
|
||||
// packing.service.ts by hand. They now live once, in PackingService, and both the
|
||||
// REST controller and PackingRpc call them there.
|
||||
|
||||
// Quotas for plugin entity metadata (db:meta) — a cheap disk-DoS guard on the
|
||||
// shared trek.db volume. Generous for real use, small enough to bound abuse.
|
||||
@@ -243,7 +196,6 @@ export class PluginHostDepsFactory {
|
||||
},
|
||||
// --- Read scopes (packing/files). Membership is checked by the host (tripRead);
|
||||
// these just delegate to the same services the REST paths use. ---
|
||||
listPackingItems: (tripId, userId) => this.packing.listItems(tripId, userId),
|
||||
listTripFiles: (tripId) => this.files.listFiles(tripId, false),
|
||||
// A file's bytes as base64, size-capped BEFORE the read so a 500MB video can't
|
||||
// be pulled (~667MB base64) through the IPC pipe. 10MB matches the plugin upload
|
||||
@@ -788,53 +740,7 @@ export class PluginHostDepsFactory {
|
||||
// --- Packing (packing_edit). Reuses PackingService + replicates the #858
|
||||
// privacy-scoped broadcasts (create/delete via emitPackingToViewers, update via
|
||||
// the four-case broadcastPackingUpdate) so a private item never leaks room-wide. ---
|
||||
canEditPacking: (tripId, userId) => this.canEditTripAs('packing_edit', tripId, userId),
|
||||
createPackingItem: (tripId, input, actingUserId) => {
|
||||
const i = input as { name: string; category?: string; checked?: boolean; is_private?: boolean; visibility?: 'common' | 'personal' | 'shared'; recipient_ids?: number[] };
|
||||
const item = this.packing.createItem(String(tripId), i, actingUserId) as PackingPrivacy;
|
||||
emitPackingToViewers(this.realtime, tripId, 'packing:created', { item }, item);
|
||||
return item;
|
||||
},
|
||||
updatePackingItem: (tripId, itemId, input, actingUserId) => {
|
||||
// Privacy BEFORE the write, so a public<->private toggle routes correctly.
|
||||
const before = this.packing.getItemPrivacy(tripId, itemId);
|
||||
const updated = this.packing.updateItem(String(tripId), String(itemId), input as never, Object.keys(input), undefined, actingUserId);
|
||||
if (!updated) throw new ForbiddenResource(`no packing item ${itemId} on trip ${tripId}`);
|
||||
if (isUpdateConflict(updated)) throw new BadParams('packing item was modified concurrently');
|
||||
broadcastPackingUpdate(this.realtime, tripId, itemId, updated as PackingPrivacy, !!before?.is_private);
|
||||
return updated;
|
||||
},
|
||||
deletePackingItem: (tripId, itemId) => {
|
||||
const deleted = this.packing.deleteItem(String(tripId), String(itemId)) as PackingPrivacy | null;
|
||||
if (!deleted) throw new ForbiddenResource(`no packing item ${itemId} on trip ${tripId}`);
|
||||
emitPackingToViewers(this.realtime, tripId, 'packing:deleted', { itemId }, deleted);
|
||||
return { deleted: true };
|
||||
},
|
||||
// --- Packing bags (no privacy — plain room broadcasts). ---
|
||||
listPackingBags: (tripId) => this.packing.listBags(String(tripId)) as unknown[],
|
||||
createPackingBag: (tripId, input) => {
|
||||
const i = input as { name: string; color?: string };
|
||||
const bag = this.packing.createBag(String(tripId), { name: i.name, color: i.color });
|
||||
this.realtime.broadcast(tripId, 'packing:bag-created', { bag }, undefined);
|
||||
return bag;
|
||||
},
|
||||
updatePackingBag: (tripId, bagId, input) => {
|
||||
const bag = this.packing.updateBag(String(tripId), String(bagId), input as never, Object.keys(input));
|
||||
if (!bag) throw new ForbiddenResource(`no packing bag ${bagId} on trip ${tripId}`);
|
||||
this.realtime.broadcast(tripId, 'packing:bag-updated', { bag }, undefined);
|
||||
return bag;
|
||||
},
|
||||
deletePackingBag: (tripId, bagId) => {
|
||||
if (!this.packing.deleteBag(String(tripId), String(bagId))) throw new ForbiddenResource(`no packing bag ${bagId} on trip ${tripId}`);
|
||||
this.realtime.broadcast(tripId, 'packing:bag-deleted', { bagId }, undefined);
|
||||
return { deleted: true };
|
||||
},
|
||||
setPackingBagMembers: (tripId, bagId, userIds) => {
|
||||
const members = this.packing.setBagMembers(String(tripId), String(bagId), userIds);
|
||||
if (!members) throw new ForbiddenResource(`no packing bag ${bagId} on trip ${tripId}`);
|
||||
this.realtime.broadcast(tripId, 'packing:bag-members-updated', { bagId, members }, undefined);
|
||||
return members;
|
||||
},
|
||||
// --- Read-convenience: weather (host cache, tenant-free), categories (global), roster ---
|
||||
tripMembers: (tripId) =>
|
||||
this.db.prepare('SELECT u.id, u.username, u.display_name, u.avatar FROM trip_members tm JOIN users u ON u.id = tm.user_id WHERE tm.trip_id = ?').all(tripId) as unknown[],
|
||||
|
||||
@@ -91,7 +91,6 @@ export interface HostDeps {
|
||||
/** True if the acting user may create costs on the trip (the 'budget_edit' permission). */
|
||||
canEditCosts(tripId: number, userId: number): boolean;
|
||||
/** A trip's packing items visible to `userId` (#858 private-item filter), for `packing.list`. */
|
||||
listPackingItems(tripId: number, userId: number): unknown[];
|
||||
/** A trip's files (trash excluded), for `files.list`. */
|
||||
listTripFiles(tripId: number): unknown[];
|
||||
/** One trip file's bytes as base64 (size-capped), for `files.getContent`. Throws if it's not on the trip or exceeds the cap. */
|
||||
@@ -222,19 +221,10 @@ export interface HostDeps {
|
||||
/** Delete a reservation from a trip (same side effects); returns { deleted: true }. */
|
||||
deleteReservation(tripId: number, reservationId: number, actingUserId: number): unknown;
|
||||
// --- Packing (the 'packing_edit' permission; #858 privacy-scoped broadcasts) ---
|
||||
canEditPacking(tripId: number, userId: number): boolean;
|
||||
/** Create a packing item (owner = acting user); privacy-scoped packing:created broadcast; returns it. */
|
||||
createPackingItem(tripId: number, input: Record<string, unknown>, actingUserId: number): unknown;
|
||||
/** Update a packing item; four-case public<->private broadcast; throws if not on the trip. */
|
||||
updatePackingItem(tripId: number, itemId: number, input: Record<string, unknown>, actingUserId: number): unknown;
|
||||
/** Delete a packing item; owner+recipients-scoped packing:deleted broadcast; returns { deleted: true }. */
|
||||
deletePackingItem(tripId: number, itemId: number): unknown;
|
||||
// --- Packing bags (packing_edit; no privacy — broadcast to the whole room) ---
|
||||
listPackingBags(tripId: number): unknown[];
|
||||
createPackingBag(tripId: number, input: Record<string, unknown>): unknown;
|
||||
updatePackingBag(tripId: number, bagId: number, input: Record<string, unknown>): unknown;
|
||||
deletePackingBag(tripId: number, bagId: number): unknown;
|
||||
setPackingBagMembers(tripId: number, bagId: number, userIds: number[]): unknown;
|
||||
// --- Read-convenience: weather (tenant-free), categories (global), the trip roster ---
|
||||
tripMembers(tripId: number): unknown[];
|
||||
// --- Tags (the acting user's own; no trip) ---
|
||||
@@ -325,13 +315,6 @@ export class PluginRpcHost {
|
||||
// The trip's member roster (ids + display fields only), membership-checked.
|
||||
this.methods.set('trips.members', (p, uid) => this.tripRead(p, uid, () => deps.tripMembers(num(p.tripId, 'tripId'))));
|
||||
}
|
||||
if (has('db:read:packing')) {
|
||||
// Delegate to the packing service, scoped to the acting user so its #858 private-
|
||||
// item visibility filter applies (a plugin must not see other members' private items).
|
||||
this.methods.set('packing.list', (p, uid) =>
|
||||
this.tripRead(p, uid, (userId) => deps.listPackingItems(num(p.tripId, 'tripId'), userId)),
|
||||
);
|
||||
}
|
||||
if (has('db:read:files')) {
|
||||
// Trip files, trash excluded — same view the files tab shows.
|
||||
this.methods.set('files.list', (p, uid) =>
|
||||
@@ -823,69 +806,7 @@ export class PluginRpcHost {
|
||||
});
|
||||
}
|
||||
|
||||
if (has('db:write:packing')) {
|
||||
// Packing list write. Gated exactly like the packing REST path — trip access +
|
||||
// the 'packing_edit' permission for the HOST-bound acting user. The deps reuse
|
||||
// packingService and replicate the #858 privacy-scoped broadcasts 1:1, so a
|
||||
// private item is never leaked to the whole trip room.
|
||||
this.methods.set('packing.create', (p, uid) => {
|
||||
const tripId = num(p.tripId, 'tripId');
|
||||
const actor = this.requireActor(uid, 'packing item');
|
||||
const parsed = packingCreateItemRequestSchema.safeParse(p.input);
|
||||
if (!parsed.success) throw new BadParams(`invalid packing item: ${parsed.error.issues[0]?.message ?? 'bad input'}`);
|
||||
this.requireTripEdit(tripId, actor, deps.canEditPacking);
|
||||
return deps.createPackingItem(tripId, parsed.data as Record<string, unknown>, actor);
|
||||
});
|
||||
this.methods.set('packing.update', (p, uid) => {
|
||||
const tripId = num(p.tripId, 'tripId');
|
||||
const itemId = num(p.itemId, 'itemId');
|
||||
const actor = this.requireActor(uid, 'packing item');
|
||||
const parsed = packingUpdateItemRequestSchema.safeParse(p.input);
|
||||
if (!parsed.success) throw new BadParams(`invalid packing item: ${parsed.error.issues[0]?.message ?? 'bad input'}`);
|
||||
this.requireTripEdit(tripId, actor, deps.canEditPacking);
|
||||
return deps.updatePackingItem(tripId, itemId, parsed.data as Record<string, unknown>, actor);
|
||||
});
|
||||
this.methods.set('packing.delete', (p, uid) => {
|
||||
const tripId = num(p.tripId, 'tripId');
|
||||
const itemId = num(p.itemId, 'itemId');
|
||||
const actor = this.requireActor(uid, 'packing item');
|
||||
this.requireTripEdit(tripId, actor, deps.canEditPacking);
|
||||
return deps.deletePackingItem(tripId, itemId);
|
||||
});
|
||||
// Bags carry no privacy — a plain packing:bag-* broadcast to the whole room.
|
||||
this.methods.set('packing.listBags', (p, uid) => this.tripRead(p, uid, () => deps.listPackingBags(num(p.tripId, 'tripId'))));
|
||||
this.methods.set('packing.createBag', (p, uid) => {
|
||||
const tripId = num(p.tripId, 'tripId');
|
||||
const actor = this.requireActor(uid, 'packing bag');
|
||||
const input = asPayload(p.input);
|
||||
if (typeof input.name !== 'string' || input.name.trim() === '') throw new BadParams('bag name is required');
|
||||
this.requireTripEdit(tripId, actor, deps.canEditPacking);
|
||||
return deps.createPackingBag(tripId, input);
|
||||
});
|
||||
this.methods.set('packing.updateBag', (p, uid) => {
|
||||
const tripId = num(p.tripId, 'tripId');
|
||||
const bagId = num(p.bagId, 'bagId');
|
||||
const actor = this.requireActor(uid, 'packing bag');
|
||||
this.requireTripEdit(tripId, actor, deps.canEditPacking);
|
||||
return deps.updatePackingBag(tripId, bagId, asPayload(p.input));
|
||||
});
|
||||
this.methods.set('packing.deleteBag', (p, uid) => {
|
||||
const tripId = num(p.tripId, 'tripId');
|
||||
const bagId = num(p.bagId, 'bagId');
|
||||
const actor = this.requireActor(uid, 'packing bag');
|
||||
this.requireTripEdit(tripId, actor, deps.canEditPacking);
|
||||
return deps.deletePackingBag(tripId, bagId);
|
||||
});
|
||||
this.methods.set('packing.setBagMembers', (p, uid) => {
|
||||
const tripId = num(p.tripId, 'tripId');
|
||||
const bagId = num(p.bagId, 'bagId');
|
||||
const actor = this.requireActor(uid, 'packing bag');
|
||||
this.requireTripEdit(tripId, actor, deps.canEditPacking);
|
||||
const raw = asPayload(p).userIds;
|
||||
const userIds = Array.isArray(raw) ? raw.filter((x): x is number => typeof x === 'number') : [];
|
||||
return deps.setPackingBagMembers(tripId, bagId, userIds);
|
||||
});
|
||||
}
|
||||
// packing.* now lives in src/nest/packing/packing.rpc.ts.
|
||||
|
||||
// weather.get and categories.list now live in their own domains' *.rpc.ts.
|
||||
// tags.* now lives in src/nest/tags/tags.rpc.ts, bound through the registry below.
|
||||
|
||||
@@ -6,6 +6,7 @@ import { WeatherRpc } from '../../src/nest/weather/weather.rpc';
|
||||
import { ExchangeRatesRpc } from '../../src/nest/budget/exchange-rates.rpc';
|
||||
import { TodoRpc } from '../../src/nest/todo/todo.rpc';
|
||||
import { DayNotesRpc } from '../../src/nest/days/day-notes.rpc';
|
||||
import { PackingRpc } from '../../src/nest/packing/packing.rpc';
|
||||
|
||||
/**
|
||||
* The stubbed HostDeps every plugin router test builds on. It used to live inside
|
||||
@@ -186,5 +187,6 @@ export function allRpcControllers(): object[] {
|
||||
new ExchangeRatesRpc(anyService()),
|
||||
new TodoRpc(anyService(), anyService(), anyService()),
|
||||
new DayNotesRpc(anyService(), anyService(), anyService()),
|
||||
new PackingRpc(anyService(), anyService(), anyService()),
|
||||
];
|
||||
}
|
||||
|
||||
@@ -10,7 +10,7 @@ const trip = { id: 5, user_id: 1 };
|
||||
|
||||
/** Service mock with trip access granted + edit allowed by default. */
|
||||
function makeService(overrides: Partial<PackingService> = {}): PackingService {
|
||||
return {
|
||||
const base = {
|
||||
verifyTripAccess: vi.fn().mockReturnValue(trip),
|
||||
canEdit: vi.fn().mockReturnValue(true),
|
||||
broadcast: vi.fn(),
|
||||
@@ -23,6 +23,30 @@ function makeService(overrides: Partial<PackingService> = {}): PackingService {
|
||||
notifyTagged: vi.fn(),
|
||||
...overrides,
|
||||
} as unknown as PackingService;
|
||||
// emitToViewers and broadcastUpdate moved from the controller into the service, so
|
||||
// the mock reproduces their routing on top of whatever broadcast stubs a test
|
||||
// supplied. That keeps every #858 assertion below pointed at the same three
|
||||
// primitives it always was.
|
||||
const svc = base as unknown as PackingService & Record<string, (...a: never[]) => unknown>;
|
||||
svc.emitToViewers = ((tripId, event, payload, item, socketId) => {
|
||||
const viewers = svc.viewersOf(item);
|
||||
if (viewers === null) svc.broadcast(tripId, event, payload, socketId);
|
||||
else svc.broadcastToViewers(tripId, event, payload, viewers, socketId);
|
||||
}) as PackingService['emitToViewers'];
|
||||
svc.broadcastUpdate = ((tripId, id, item, wasPrivate, socketId) => {
|
||||
if (item.is_private) {
|
||||
if (wasPrivate) {
|
||||
svc.broadcastItem(tripId, 'packing:updated', { item }, item, socketId);
|
||||
} else {
|
||||
svc.broadcast(tripId, 'packing:deleted', { itemId: Number(id) }, socketId);
|
||||
svc.broadcastItem(tripId, 'packing:created', { item }, item, socketId);
|
||||
}
|
||||
} else {
|
||||
if (wasPrivate) svc.broadcast(tripId, 'packing:created', { item }, socketId);
|
||||
svc.broadcast(tripId, 'packing:updated', { item }, socketId);
|
||||
}
|
||||
}) as PackingService['broadcastUpdate'];
|
||||
return svc;
|
||||
}
|
||||
|
||||
function thrown(fn: () => unknown): { status: number; body: unknown } {
|
||||
|
||||
@@ -0,0 +1,203 @@
|
||||
/**
|
||||
* The packing plugin surface after it moved onto @PluginMethod.
|
||||
*
|
||||
* The four public/private transitions (#858) are the reason this file is long. Each
|
||||
* one is asserted separately, because getting any of them wrong leaks a private item
|
||||
* to the whole trip room, and because the logic now lives once in PackingService
|
||||
* instead of three times over.
|
||||
*/
|
||||
import { describe, it, expect, vi, beforeEach } from 'vitest';
|
||||
import { PluginRpcHost } from '../../../src/nest/plugins/host/rpc-host';
|
||||
import { createTestPluginRegistry } from '../../../src/nest/plugins/host/rpc-kit/testing';
|
||||
import { PluginGuards } from '../../../src/nest/plugins/host/plugin-guards.service';
|
||||
import { PackingRpc } from '../../../src/nest/packing/packing.rpc';
|
||||
import { PackingModule } from '../../../src/nest/packing/packing.module';
|
||||
import { PackingService } from '../../../src/nest/packing/packing.service';
|
||||
import type { RealtimeService } from '../../../src/nest/realtime/realtime.service';
|
||||
import type { DatabaseService } from '../../../src/nest/database/database.service';
|
||||
import type { PermissionsService } from '../../../src/nest/permissions/permissions.service';
|
||||
import type { AddonsService } from '../../../src/nest/addons/addons.service';
|
||||
import type { RpcRequest, RpcError } from '../../../src/nest/plugins/protocol/envelope';
|
||||
import { makeDeps } from '../../helpers/rpc-host-deps';
|
||||
|
||||
const req = (method: string, params: Record<string, unknown> = {}): RpcRequest => ({ k: 'req', id: 'x', method, params });
|
||||
|
||||
type Item = { id: number; name?: string; is_private?: number; owner_id?: number | null; recipients?: { user_id: number }[] };
|
||||
|
||||
/**
|
||||
* A REAL PackingService over a fake realtime, so the broadcast fan-out under test is
|
||||
* the production one; only the data methods are stubbed.
|
||||
*/
|
||||
function build(opts: { canEdit?: boolean; before?: Item | undefined; updated?: Item | null } = {}) {
|
||||
const realtime = { broadcast: vi.fn() } as unknown as RealtimeService & { broadcast: ReturnType<typeof vi.fn> };
|
||||
const packing = new PackingService({} as never, {} as never, realtime);
|
||||
const data = {
|
||||
listItems: vi.fn(() => [{ id: 70, name: 'Socks' }]),
|
||||
createItem: vi.fn((_t: string, i: Record<string, unknown>) => ({ id: 70, ...i })),
|
||||
getItemPrivacy: vi.fn(() => opts.before),
|
||||
updateItem: vi.fn(() => (opts.updated === undefined ? { id: 70, is_private: 0 } : opts.updated)),
|
||||
deleteItem: vi.fn((_t: string, id: string) => (id === '70' ? { id: 70, is_private: 0 } : null)),
|
||||
listBags: vi.fn(() => [{ id: 80, name: 'Backpack' }]),
|
||||
createBag: vi.fn((_t: string, b: Record<string, unknown>) => ({ id: 80, ...b })),
|
||||
updateBag: vi.fn((_t: string, id: string) => (id === '80' ? { id: 80 } : null)),
|
||||
deleteBag: vi.fn((_t: string, id: string) => id === '80'),
|
||||
setBagMembers: vi.fn((_t: string, id: string, ids: number[]) => (id === '80' ? { bagId: 80, members: ids } : null)),
|
||||
};
|
||||
Object.assign(packing, data);
|
||||
const guards = new PluginGuards(
|
||||
{
|
||||
canAccessTrip: vi.fn((tripId: number, userId: number) => (tripId === 1 && userId === 42 ? { id: 1, user_id: 42 } : undefined)),
|
||||
prepare: vi.fn(() => ({ get: () => ({ role: 'user' }) })),
|
||||
} as unknown as DatabaseService,
|
||||
{ checkPermission: vi.fn(() => opts.canEdit ?? true) } as unknown as PermissionsService,
|
||||
{ isAddonEnabled: vi.fn(() => true) } as unknown as AddonsService,
|
||||
);
|
||||
const rpc = new PackingRpc(packing, realtime, guards);
|
||||
const host = (...grants: string[]) => new PluginRpcHost('p', new Set(grants), makeDeps(), createTestPluginRegistry([rpc]));
|
||||
return { packing, data, realtime, rpc, host };
|
||||
}
|
||||
|
||||
/** Every (event, onlyUserId) pair the fan-out produced, in order. */
|
||||
const fanout = (realtime: { broadcast: ReturnType<typeof vi.fn> }) =>
|
||||
realtime.broadcast.mock.calls.map((c) => [c[1], c[4]]);
|
||||
|
||||
describe('PackingRpc through the router', () => {
|
||||
it('PACKING-RPC-001 packing.list is membership-checked and scoped to the acting user', async () => {
|
||||
const f = build();
|
||||
const host = f.host('db:read:packing');
|
||||
expect((await host.dispatch(req('packing.list', { tripId: 1 }), 42)).ok).toBe(true);
|
||||
// The user id is handed through so the #858 visibility filter applies.
|
||||
expect(f.data.listItems).toHaveBeenCalledWith(1, 42);
|
||||
expect(((await host.dispatch(req('packing.list', { tripId: 2 }), 42)) as RpcError).error.code).toBe('RESOURCE_FORBIDDEN');
|
||||
});
|
||||
|
||||
it('PACKING-RPC-002 create needs the write grant, the edit right and a bound user', async () => {
|
||||
const f = build();
|
||||
const host = f.host('db:write:packing');
|
||||
expect((await host.dispatch(req('packing.create', { tripId: 1, input: { name: 'Socks' } }), 42)).ok).toBe(true);
|
||||
expect((await host.dispatch(req('packing.create', { tripId: 1, input: { name: '' } }), 42)).ok).toBe(false);
|
||||
expect(((await host.dispatch(req('packing.create', { tripId: 2, input: { name: 'x' } }), 42)) as RpcError).error.code).toBe('RESOURCE_FORBIDDEN');
|
||||
const noUser = (await host.dispatch(req('packing.create', { tripId: 1, input: { name: 'x' } }), undefined)) as RpcError;
|
||||
expect(noUser.error.message).toBe('packing item writes require an authenticated user context');
|
||||
});
|
||||
|
||||
it('PACKING-RPC-003 db:read:packing does not unlock a write', async () => {
|
||||
const f = build();
|
||||
const res = (await f.host('db:read:packing').dispatch(req('packing.create', { tripId: 1, input: { name: 'x' } }), 42)) as RpcError;
|
||||
expect(res.error.code).toBe('PERMISSION_DENIED');
|
||||
});
|
||||
|
||||
it('PACKING-RPC-004 a missing item is RESOURCE_FORBIDDEN, naming it', async () => {
|
||||
const f = build({ updated: null });
|
||||
const res = (await f.host('db:write:packing').dispatch(req('packing.update', { tripId: 1, itemId: 404, input: { name: 'x' } }), 42)) as RpcError;
|
||||
expect(res.error.message).toBe('no packing item 404 on trip 1');
|
||||
});
|
||||
|
||||
it('PACKING-RPC-005 bags are gated on the write grant too, listBags included', async () => {
|
||||
const f = build();
|
||||
const host = f.host('db:write:packing');
|
||||
expect((await host.dispatch(req('packing.listBags', { tripId: 1 }), 42)).ok).toBe(true);
|
||||
expect((await host.dispatch(req('packing.createBag', { tripId: 1, input: { name: 'Bag' } }), 42)).ok).toBe(true);
|
||||
expect((await host.dispatch(req('packing.setBagMembers', { tripId: 1, bagId: 80, userIds: [5, 6] }), 42)).ok).toBe(true);
|
||||
expect(f.data.setBagMembers).toHaveBeenCalledWith('1', '80', [5, 6]);
|
||||
});
|
||||
|
||||
it('PACKING-RPC-006 a bag name is required, and a missing bag is named', async () => {
|
||||
const f = build();
|
||||
const host = f.host('db:write:packing');
|
||||
expect(((await host.dispatch(req('packing.createBag', { tripId: 1, input: { name: ' ' } }), 42)) as RpcError).error.message).toBe('bag name is required');
|
||||
expect(((await host.dispatch(req('packing.updateBag', { tripId: 1, bagId: 404, input: {} }), 42)) as RpcError).error.message).toBe('no packing bag 404 on trip 1');
|
||||
});
|
||||
|
||||
it('PACKING-RPC-007 non-numeric entries in userIds are dropped', async () => {
|
||||
const f = build();
|
||||
await f.host('db:write:packing').dispatch(req('packing.setBagMembers', { tripId: 1, bagId: 80, userIds: [5, 'six', null, 6] }), 42);
|
||||
expect(f.data.setBagMembers).toHaveBeenCalledWith('1', '80', [5, 6]);
|
||||
});
|
||||
|
||||
it('PACKING-RPC-008 the class is listed in its module providers', () => {
|
||||
expect(Reflect.getMetadata('providers', PackingModule) as unknown[]).toContain(PackingRpc);
|
||||
});
|
||||
});
|
||||
|
||||
describe('PackingRpc keeps private items off the room (#858)', () => {
|
||||
it('PACKING-RPC-009 a common item is created for the whole room', async () => {
|
||||
const f = build();
|
||||
await f.host('db:write:packing').dispatch(req('packing.create', { tripId: 1, input: { name: 'Socks' } }), 42);
|
||||
expect(fanout(f.realtime)).toEqual([['packing:created', undefined]]);
|
||||
});
|
||||
|
||||
it('PACKING-RPC-010 a private item is created only for its owner and recipients', async () => {
|
||||
const f = build();
|
||||
f.data.createItem.mockReturnValueOnce({ id: 70, is_private: 1, owner_id: 42, recipients: [{ user_id: 7 }] });
|
||||
await f.host('db:write:packing').dispatch(req('packing.create', { tripId: 1, input: { name: 'Gift', is_private: true } }), 42);
|
||||
expect(fanout(f.realtime)).toEqual([
|
||||
['packing:created', 42],
|
||||
['packing:created', 7],
|
||||
]);
|
||||
});
|
||||
|
||||
it('PACKING-RPC-011 private stays private: an owner-only update, never a room event', async () => {
|
||||
const f = build({ before: { id: 70, is_private: 1 }, updated: { id: 70, is_private: 1, owner_id: 42 } });
|
||||
await f.host('db:write:packing').dispatch(req('packing.update', { tripId: 1, itemId: 70, input: { name: 'x' } }), 42);
|
||||
expect(fanout(f.realtime)).toEqual([['packing:updated', 42]]);
|
||||
});
|
||||
|
||||
it('PACKING-RPC-012 public to private: dropped from the room FIRST, then re-added for the owner', async () => {
|
||||
const f = build({ before: { id: 70, is_private: 0 }, updated: { id: 70, is_private: 1, owner_id: 42 } });
|
||||
await f.host('db:write:packing').dispatch(req('packing.update', { tripId: 1, itemId: 70, input: { is_private: true } }), 42);
|
||||
// The order matters: a member who already had the item must lose it before the
|
||||
// owner gets the private copy.
|
||||
expect(fanout(f.realtime)).toEqual([
|
||||
['packing:deleted', undefined],
|
||||
['packing:created', 42],
|
||||
]);
|
||||
});
|
||||
|
||||
it('PACKING-RPC-013 private to public: created for the members who lacked it, then updated for all', async () => {
|
||||
const f = build({ before: { id: 70, is_private: 1 }, updated: { id: 70, is_private: 0 } });
|
||||
await f.host('db:write:packing').dispatch(req('packing.update', { tripId: 1, itemId: 70, input: { is_private: false } }), 42);
|
||||
expect(fanout(f.realtime)).toEqual([
|
||||
['packing:created', undefined],
|
||||
['packing:updated', undefined],
|
||||
]);
|
||||
});
|
||||
|
||||
it('PACKING-RPC-014 public stays public: one plain update to everyone', async () => {
|
||||
const f = build({ before: { id: 70, is_private: 0 }, updated: { id: 70, is_private: 0 } });
|
||||
await f.host('db:write:packing').dispatch(req('packing.update', { tripId: 1, itemId: 70, input: { name: 'x' } }), 42);
|
||||
expect(fanout(f.realtime)).toEqual([['packing:updated', undefined]]);
|
||||
});
|
||||
|
||||
it('PACKING-RPC-015 the privacy is read BEFORE the write, not after', async () => {
|
||||
const f = build({ before: { id: 70, is_private: 0 }, updated: { id: 70, is_private: 1, owner_id: 42 } });
|
||||
await f.host('db:write:packing').dispatch(req('packing.update', { tripId: 1, itemId: 70, input: { is_private: true } }), 42);
|
||||
// Reading it afterwards would report "already private" and skip the room delete,
|
||||
// leaving the item on every member's screen.
|
||||
expect(f.data.getItemPrivacy.mock.invocationCallOrder[0]).toBeLessThan(f.data.updateItem.mock.invocationCallOrder[0]);
|
||||
});
|
||||
|
||||
it('PACKING-RPC-016 deleting a private item tells only its viewers', async () => {
|
||||
const f = build();
|
||||
f.data.deleteItem.mockReturnValueOnce({ id: 70, is_private: 1, owner_id: 42, recipients: [{ user_id: 7 }] });
|
||||
await f.host('db:write:packing').dispatch(req('packing.delete', { tripId: 1, itemId: 70 }), 42);
|
||||
expect(fanout(f.realtime)).toEqual([
|
||||
['packing:deleted', 42],
|
||||
['packing:deleted', 7],
|
||||
]);
|
||||
});
|
||||
|
||||
it('PACKING-RPC-016b a stale-write conflict is BAD_PARAMS and broadcasts nothing', async () => {
|
||||
const f = build({ before: { id: 70, is_private: 0 }, updated: { conflict: true, server: { id: 70 } } as never });
|
||||
const res = (await f.host('db:write:packing').dispatch(req('packing.update', { tripId: 1, itemId: 70, input: { name: 'x' } }), 42)) as RpcError;
|
||||
expect(res.error.code).toBe('BAD_PARAMS');
|
||||
expect(res.error.message).toBe('packing item was modified concurrently');
|
||||
expect(f.realtime.broadcast).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('PACKING-RPC-017 a refused write broadcasts nothing at all', async () => {
|
||||
const f = build({ canEdit: false });
|
||||
await f.host('db:write:packing').dispatch(req('packing.create', { tripId: 1, input: { name: 'x' } }), 42);
|
||||
expect(f.realtime.broadcast).not.toHaveBeenCalled();
|
||||
});
|
||||
});
|
||||
@@ -585,13 +585,6 @@ describe('host-deps factory — planner write + metadata deps', () => {
|
||||
expect((await call(h, 'costs.delete', { tripId: 1, itemId: 404 })).error.code).toBe('RESOURCE_FORBIDDEN');
|
||||
});
|
||||
|
||||
it('packing/files read deps delegate to their services (trash excluded for files)', async () => {
|
||||
const h = host('db:read:packing', 'db:read:files');
|
||||
// acting user 5 is threaded to the packing service (#858 private-item filter); _uid proves it
|
||||
expect((await call(h, 'packing.list', { tripId: 1 })).result).toEqual([{ id: 1, trip_id: 1, name: 'Socks', _uid: 5 }]);
|
||||
expect((await call(h, 'files.list', { tripId: 1 })).result).toEqual([{ id: 2, trip_id: 1, trash: false }]);
|
||||
});
|
||||
|
||||
it('users.getById is scoped to people the acting user shares a trip with', async () => {
|
||||
const h = host('db:read:users');
|
||||
expect((await call(h, 'users.getById', { id: 6 }, 5)).ok).toBe(true); // 5 (owner) + 6 (member) share trip 1
|
||||
@@ -714,68 +707,9 @@ describe('host-deps factory — reservations, day notes, cross-trip + addon read
|
||||
});
|
||||
});
|
||||
|
||||
describe('host-deps factory — packing write with #858 privacy-scoped broadcasts', () => {
|
||||
const host = () => createRealRpcHost('pk', new Set(['db:write:packing']))
|
||||
// eslint-disable-next-line @typescript-eslint/no-explicit-any
|
||||
const call = async (method: string, params: Record<string, unknown>, uid = 5): Promise<any> =>
|
||||
host().dispatch({ k: 'req', id: 'x', method, params }, uid)
|
||||
// the onlyUserId (5th arg) of every broadcast for `event` since the last clear:
|
||||
// undefined = whole trip room, a number = that user's sockets only.
|
||||
const fanout = (event: string) => broadcast.mock.calls.filter((c) => c[1] === event).map((c) => c[4])
|
||||
beforeEach(() => { checkPermission.mockReset(); checkPermission.mockReturnValue(true); isAddonEnabled.mockReset(); isAddonEnabled.mockReturnValue(true); broadcast.mockClear() })
|
||||
afterAll(() => closePluginDataDb('pk'))
|
||||
|
||||
it('create: Common -> whole room; Personal -> owner-only; Shared -> owner + recipients', async () => {
|
||||
expect((await call('packing.create', { tripId: 1, input: { name: 'Common', visibility: 'common' } })).ok).toBe(true)
|
||||
expect(fanout('packing:created')).toEqual([undefined]) // whole room
|
||||
|
||||
broadcast.mockClear()
|
||||
expect((await call('packing.create', { tripId: 1, input: { name: 'Mine', visibility: 'personal' } })).ok).toBe(true)
|
||||
expect(fanout('packing:created')).toEqual([5]) // owner-only
|
||||
|
||||
broadcast.mockClear()
|
||||
expect((await call('packing.create', { tripId: 1, input: { name: 'Ours', visibility: 'shared', recipient_ids: [6] } })).ok).toBe(true)
|
||||
expect([...fanout('packing:created')].sort()).toEqual([5, 6]) // owner + recipient, never the room
|
||||
})
|
||||
|
||||
it('update: the four public<->private transitions route correctly (never leaks a privatized item)', async () => {
|
||||
await call('packing.update', { tripId: 1, itemId: 71, input: { is_private: true } }) // stays private (71 seeded private)
|
||||
expect(fanout('packing:updated')).toEqual([5])
|
||||
expect(fanout('packing:deleted')).toEqual([])
|
||||
expect(fanout('packing:created')).toEqual([])
|
||||
|
||||
broadcast.mockClear()
|
||||
await call('packing.update', { tripId: 1, itemId: 70, input: { is_private: true } }) // public -> private (70 seeded public)
|
||||
expect(fanout('packing:deleted')).toEqual([undefined]) // drop from the room FIRST (the anti-leak)
|
||||
expect(fanout('packing:created')).toEqual([5]) // then re-add owner-only
|
||||
|
||||
broadcast.mockClear()
|
||||
await call('packing.update', { tripId: 1, itemId: 71, input: { is_private: false } }) // private -> public
|
||||
expect(fanout('packing:created')).toEqual([undefined])
|
||||
expect(fanout('packing:updated')).toEqual([undefined])
|
||||
|
||||
broadcast.mockClear()
|
||||
await call('packing.update', { tripId: 1, itemId: 70, input: { is_private: false } }) // stays public
|
||||
expect(fanout('packing:updated')).toEqual([undefined])
|
||||
expect(fanout('packing:deleted')).toEqual([])
|
||||
})
|
||||
|
||||
it('update: a stale-write conflict is BAD_PARAMS and never broadcasts', async () => {
|
||||
const res = await call('packing.update', { tripId: 1, itemId: 99, input: { name: 'x' } })
|
||||
expect((res as { error: { code: string } }).error.code).toBe('BAD_PARAMS')
|
||||
expect(broadcast).not.toHaveBeenCalled()
|
||||
})
|
||||
|
||||
it('delete: a private item is owner-scoped; a missing one is RESOURCE_FORBIDDEN', async () => {
|
||||
await call('packing.delete', { tripId: 1, itemId: 71 })
|
||||
expect(fanout('packing:deleted')).toEqual([5]) // owner-only (recipients get no packing:deleted)
|
||||
broadcast.mockClear()
|
||||
await call('packing.delete', { tripId: 1, itemId: 70 })
|
||||
expect(fanout('packing:deleted')).toEqual([undefined]) // common -> room
|
||||
const missing = await call('packing.delete', { tripId: 1, itemId: 404 })
|
||||
expect((missing as { error: { code: string } }).error.code).toBe('RESOURCE_FORBIDDEN')
|
||||
})
|
||||
})
|
||||
// The packing write deps left this factory with the decorator migration. The #858
|
||||
// transitions are asserted in tests/unit/packing/packing.rpc.test.ts, against the
|
||||
// PackingService methods the REST controller uses too, so there is one copy now.
|
||||
|
||||
describe('host-deps factory — Wave 1 wiring (weather/categories/tags/todos/roster/bags)', () => {
|
||||
const host = (...perms: string[]) => createRealRpcHost('w1', new Set(perms))
|
||||
@@ -800,15 +734,6 @@ describe('host-deps factory — Wave 1 wiring (weather/categories/tags/todos/ros
|
||||
// todos.* left this factory with the decorator migration; the cases now run in
|
||||
// tests/unit/todo/todo.rpc.test.ts against TodoRpc.
|
||||
|
||||
it('packing bags list/create/update/delete/setMembers run the wiring', async () => {
|
||||
const h = host('db:write:packing')
|
||||
expect((await call(h, 'packing.listBags', { tripId: 1 }, 5)).ok).toBe(true)
|
||||
expect((await call(h, 'packing.createBag', { tripId: 1, input: { name: 'Bag' } }, 5)).ok).toBe(true)
|
||||
expect((await call(h, 'packing.updateBag', { tripId: 1, bagId: 80, input: { name: 'X' } }, 5)).ok).toBe(true)
|
||||
expect((await call(h, 'packing.setBagMembers', { tripId: 1, bagId: 80, userIds: [5] }, 5)).ok).toBe(true)
|
||||
expect((await call(h, 'packing.deleteBag', { tripId: 1, bagId: 80 }, 5)).ok).toBe(true)
|
||||
expect(((await call(h, 'packing.deleteBag', { tripId: 1, bagId: 404 }, 5)) as { error: { code: string } }).error.code).toBe('RESOURCE_FORBIDDEN')
|
||||
})
|
||||
})
|
||||
|
||||
describe('host-deps factory — Wave 2 wiring (atlas/vacay/journal/collections writes)', () => {
|
||||
|
||||
@@ -54,8 +54,8 @@ describe('plugin RPC coverage ledger', () => {
|
||||
// Update these two numbers in every rollout PR. They make the diff state the size
|
||||
// of the move out loud, which is the number a reviewer wants to see.
|
||||
const total = KNOWN_METHODS.length + UNCONDITIONAL_METHODS.length;
|
||||
expect(fromRegistry.size).toBe(15);
|
||||
expect(legacy.size).toBe(total - 15);
|
||||
expect(fromRegistry.size).toBe(24);
|
||||
expect(legacy.size).toBe(total - 24);
|
||||
});
|
||||
|
||||
it('RPCLEDGER-006 the migrated methods are exactly the ones this PR claims', () => {
|
||||
@@ -65,6 +65,15 @@ describe('plugin RPC coverage ledger', () => {
|
||||
'daynotes.delete',
|
||||
'daynotes.list',
|
||||
'daynotes.update',
|
||||
'packing.create',
|
||||
'packing.createBag',
|
||||
'packing.delete',
|
||||
'packing.deleteBag',
|
||||
'packing.list',
|
||||
'packing.listBags',
|
||||
'packing.setBagMembers',
|
||||
'packing.update',
|
||||
'packing.updateBag',
|
||||
'rates.get',
|
||||
'tags.create',
|
||||
'tags.delete',
|
||||
|
||||
@@ -10,6 +10,11 @@ import { describe, it, expect, vi } from 'vitest';
|
||||
import { PluginRpcHost } from '../../../src/nest/plugins/host/rpc-host';
|
||||
import type { RpcRequest, RpcResponse } from '../../../src/nest/plugins/protocol/envelope';
|
||||
import { makeDeps } from '../../helpers/rpc-host-deps';
|
||||
import { createTestPluginRegistry } from '../../../src/nest/plugins/host/rpc-kit/testing';
|
||||
import { PackingRpc } from '../../../src/nest/packing/packing.rpc';
|
||||
import type { PackingService } from '../../../src/nest/packing/packing.service';
|
||||
import type { PluginGuards } from '../../../src/nest/plugins/host/plugin-guards.service';
|
||||
import type { RealtimeService } from '../../../src/nest/realtime/realtime.service';
|
||||
|
||||
const req = (method: string, params: Record<string, unknown>): RpcRequest => ({ k: 'req', id: 'x', method, params });
|
||||
|
||||
@@ -60,11 +65,23 @@ describe('dispatch strips the supervisor _inv marker', () => {
|
||||
});
|
||||
|
||||
it('RPCINV-006 a handler reading the whole payload object no longer finds _inv in it', async () => {
|
||||
const deps = makeDeps();
|
||||
const host = new PluginRpcHost('p', new Set(['db:write:packing']), deps);
|
||||
// packing.setBagMembers is the one handler that reads the params object as a
|
||||
// whole (asPayload(p).userIds) rather than a named field off it.
|
||||
// whole (asPayload(p).userIds) rather than a named field off it, which makes it
|
||||
// the case where a stray _inv could actually change behaviour.
|
||||
const setBagMembers = vi.fn(() => ({ bagId: 80, members: [3] }));
|
||||
const packing = { setBagMembers } as unknown as PackingService;
|
||||
const guards = {
|
||||
requireActor: () => 42,
|
||||
requireTripEdit: () => undefined,
|
||||
} as unknown as PluginGuards;
|
||||
const realtime = { broadcast: vi.fn() } as unknown as RealtimeService;
|
||||
const host = new PluginRpcHost(
|
||||
'p',
|
||||
new Set(['db:write:packing']),
|
||||
makeDeps(),
|
||||
createTestPluginRegistry([new PackingRpc(packing, realtime, guards)]),
|
||||
);
|
||||
await host.dispatch(req('packing.setBagMembers', { tripId: 1, bagId: 80, userIds: [3], _inv: 'req-7' }), 42);
|
||||
expect(deps.setPackingBagMembers).toHaveBeenCalledWith(1, 80, [3]);
|
||||
expect(setBagMembers).toHaveBeenCalledWith('1', '80', [3]);
|
||||
});
|
||||
});
|
||||
|
||||
@@ -77,23 +77,6 @@ describe('PluginRpcHost — capability enforcement', () => {
|
||||
expect((allowed as RpcResponse).result).toEqual([{ id: 7, name: 'Place' }]);
|
||||
});
|
||||
|
||||
it('db:read:packing / db:read:files delegate to the service, membership-checked, and stay separate scopes', async () => {
|
||||
const packing = new PluginRpcHost('p', new Set(['db:read:packing']), deps);
|
||||
// no access to trip 2 → refused before the service is called
|
||||
expect(((await packing.dispatch(req('packing.list', { tripId: 2 }), 42)) as RpcError).error.code).toBe('RESOURCE_FORBIDDEN');
|
||||
expect(deps.listPackingItems).not.toHaveBeenCalled();
|
||||
const okP = await packing.dispatch(req('packing.list', { tripId: 1 }), 42);
|
||||
expect(ok(okP)).toBe(true);
|
||||
// the acting user is threaded through so packing's #858 private-item filter applies
|
||||
expect(deps.listPackingItems).toHaveBeenCalledWith(1, 42);
|
||||
// the packing scope does NOT unlock files
|
||||
expect(((await packing.dispatch(req('files.list', { tripId: 1 }), 42)) as RpcError).error.code).toBe('PERMISSION_DENIED');
|
||||
|
||||
const files = new PluginRpcHost('p', new Set(['db:read:files']), deps);
|
||||
const okF = await files.dispatch(req('files.list', { tripId: 1 }), 42);
|
||||
expect((okF as RpcResponse).result).toEqual([{ id: 2, trip_id: 1, filename: 'visa.pdf' }]);
|
||||
});
|
||||
|
||||
it('db:read:users returns only the public projection for a visible user', async () => {
|
||||
const host = new PluginRpcHost('p', new Set(['db:read:users']), deps);
|
||||
const res = await host.dispatch(req('users.getById', { id: 3 }), 42);
|
||||
@@ -334,26 +317,6 @@ describe('PluginRpcHost — capability enforcement', () => {
|
||||
expect((await host.dispatch(req('collections.listMine'), undefined)).ok).toBe(false);
|
||||
});
|
||||
|
||||
it('packing.create needs db:write:packing + packing_edit, membership-checked, name required', async () => {
|
||||
const host = new PluginRpcHost('p', new Set(['db:write:packing']), deps);
|
||||
const good = await host.dispatch(req('packing.create', { tripId: 1, input: { name: 'Socks' } }), 42);
|
||||
expect(ok(good)).toBe(true);
|
||||
expect(deps.createPackingItem).toHaveBeenCalledWith(1, expect.objectContaining({ name: 'Socks' }), 42);
|
||||
expect((await host.dispatch(req('packing.create', { tripId: 1, input: { name: '' } }), 42)).ok).toBe(false); // schema: name min 1
|
||||
expect(((await host.dispatch(req('packing.create', { tripId: 2, input: { name: 'x' } }), 42)) as RpcError).error.code).toBe('RESOURCE_FORBIDDEN');
|
||||
expect((await host.dispatch(req('packing.create', { tripId: 1, input: { name: 'x' } }), undefined)).ok).toBe(false); // no acting user
|
||||
});
|
||||
|
||||
it('packing.update/delete are gated the same way; nothing without the grant', async () => {
|
||||
const host = new PluginRpcHost('p', new Set(['db:write:packing']), deps);
|
||||
expect(ok(await host.dispatch(req('packing.update', { tripId: 1, itemId: 70, input: { checked: true } }), 42))).toBe(true);
|
||||
expect(deps.updatePackingItem).toHaveBeenCalledWith(1, 70, expect.objectContaining({ checked: true }), 42);
|
||||
expect(ok(await host.dispatch(req('packing.delete', { tripId: 1, itemId: 70 }), 42))).toBe(true);
|
||||
expect(deps.deletePackingItem).toHaveBeenCalledWith(1, 70);
|
||||
const noGrant = new PluginRpcHost('p', new Set(['db:read:packing']), deps);
|
||||
expect((await noGrant.dispatch(req('packing.create', { tripId: 1, input: { name: 'x' } }), 42)).ok).toBe(false);
|
||||
});
|
||||
|
||||
// weather.get, categories.list and rates.get moved to tests/unit/plugins/tenant-free.rpc.test.ts.
|
||||
|
||||
// The tags cases moved to tests/unit/tags/tags.rpc.test.ts with the handlers.
|
||||
@@ -364,14 +327,6 @@ describe('PluginRpcHost — capability enforcement', () => {
|
||||
expect(((await host.dispatch(req('trips.members', { tripId: 2 }), 42)) as RpcError).error.code).toBe('RESOURCE_FORBIDDEN');
|
||||
});
|
||||
|
||||
it('packing-bags writes need the grant + edit right + a bound user', async () => {
|
||||
const host = new PluginRpcHost('p', new Set(['db:write:packing']), deps);
|
||||
expect(ok(await host.dispatch(req('packing.createBag', { tripId: 1, input: { name: 'Bag' } }), 42))).toBe(true);
|
||||
expect(ok(await host.dispatch(req('packing.setBagMembers', { tripId: 1, bagId: 80, userIds: [5, 6] }), 42))).toBe(true);
|
||||
expect(deps.setPackingBagMembers).toHaveBeenCalledWith(1, 80, [5, 6]);
|
||||
expect((await host.dispatch(req('packing.createBag', { tripId: 1, input: { name: 'Bag' } }), undefined)).ok).toBe(false);
|
||||
});
|
||||
|
||||
it('atlas writes are uid-bound (code-validated, userless refused)', async () => {
|
||||
const host = new PluginRpcHost('p', new Set(['db:write:atlas']), deps);
|
||||
expect(ok(await host.dispatch(req('atlas.markCountry', { code: 'jp' }), 42))).toBe(true);
|
||||
|
||||
Reference in New Issue
Block a user