diff --git a/server/src/nest/plugins/host/plugin-guards.service.ts b/server/src/nest/plugins/host/plugin-guards.service.ts new file mode 100644 index 000000000..601105463 --- /dev/null +++ b/server/src/nest/plugins/host/plugin-guards.service.ts @@ -0,0 +1,98 @@ +import { Injectable } from '@nestjs/common'; +import { DatabaseService } from '../../database/database.service'; +import { PermissionsService } from '../../permissions/permissions.service'; +import { AddonsService } from '../../addons/addons.service'; +import { BadParams, ForbiddenResource } from './rpc-errors'; +import { num } from './rpc-params'; +import type { PluginRpcContext } from './rpc-kit/types'; + +/** + * The resource gates every plugin RPC handler needs, lifted out of PluginRpcHost's + * private methods and PluginHostDepsFactory's canEditTripAs / requireAddon so that + * decorated *.rpc.ts handlers get them by injection instead of through a closure. + * + * Every message string is character-identical to what the legacy router produced. + * Do not "improve" one: rpc-host.test.ts asserts them, and shipped plugins read + * them. Note in particular that trip READS say "trip reads" while requireActor + * appends the word "writes", and that some domains throw their own inline variant + * rather than calling requireActor at all. + */ +@Injectable() +export class PluginGuards { + constructor( + private readonly db: DatabaseService, + private readonly permissions: PermissionsService, + private readonly addons: AddonsService, + ) {} + + /** + * Membership-check a trip read against the HOST-bound acting user. The acting user + * comes from the supervisor's invocation map, never from a plugin-supplied field, + * so a plugin cannot read another user's trips by naming their id. A userless + * context (a job, or onLoad) has no acting user and is refused. + */ + tripRead(params: Record, ctx: PluginRpcContext, read: (userId: number) => T): T { + const tripId = num(params.tripId, 'tripId'); + if (ctx.actingUserId === undefined) { + throw new ForbiddenResource('trip reads require an authenticated user context'); + } + if (!this.db.canAccessTrip(tripId, ctx.actingUserId)) { + throw new ForbiddenResource(`no access to trip ${tripId}`); + } + // The read runs only for a bound, membership-checked user, so hand the id through + // and per-user visibility filters (packing's private items) still apply. + return read(ctx.actingUserId); + } + + /** + * Every write needs a HOST-bound acting user. `noun` is the domain word, and the + * message appends "writes" to it, so pass 'tag', not 'tag writes'. + */ + requireActor(ctx: PluginRpcContext, noun: string): number { + if (ctx.actingUserId === undefined) { + throw new ForbiddenResource(`${noun} writes require an authenticated user context`); + } + return ctx.actingUserId; + } + + /** A write is allowed only if the acting user can access AND edit the trip. */ + requireTripEdit(tripId: number, userId: number, action: string): void { + if (!this.db.canAccessTrip(tripId, userId)) throw new ForbiddenResource(`no access to trip ${tripId}`); + if (!this.canEditAs(action, tripId, userId)) throw new ForbiddenResource(`no permission to edit trip ${tripId}`); + } + + /** + * The trip-access + role gate every planner write uses, mirroring the app's + * per-domain canEdit. Returns false and never throws, so the caller decides which + * message the refusal carries. + */ + canEditAs(action: string, tripId: number, userId: number): boolean { + const trip = this.db.canAccessTrip(tripId, userId); + if (!trip) return false; + const user = this.db.prepare('SELECT role FROM users WHERE id = ?').get(userId) as { role?: string } | undefined; + if (!user) return false; + return this.permissions.checkPermission(action, user.role ?? 'user', trip.user_id, userId, trip.user_id !== userId); + } + + /** + * A subsystem read is refused when its addon is off, matching the app, where a + * disabled addon means there is simply nothing to read. + */ + requireAddon(addonId: string, noun: string): void { + if (!this.addons.isAddonEnabled(addonId)) throw new ForbiddenResource(`the ${noun} addon is disabled`); + } + + /** + * The @trek/shared write schemas carry no string-length caps, so mirror the ones + * the REST controllers add. Without this a plugin could write a field the web app + * would reject with 400. + */ + capStrings(input: Record, limits: Record): void { + for (const [field, max] of Object.entries(limits)) { + const value = input[field]; + if (typeof value === 'string' && value.length > max) { + throw new BadParams(`${field} must be ${max} characters or fewer`); + } + } + } +} diff --git a/server/tests/unit/plugins/plugin-guards.test.ts b/server/tests/unit/plugins/plugin-guards.test.ts new file mode 100644 index 000000000..86082c45b --- /dev/null +++ b/server/tests/unit/plugins/plugin-guards.test.ts @@ -0,0 +1,166 @@ +/** + * PluginGuards holds the resource gates the decorated *.rpc.ts handlers will use from + * the first rollout PR onward. Its messages are copied character-for-character from + * the legacy router, because rpc-host.test.ts asserts them and shipped plugins read + * them, so these tests pin the exact strings rather than just the refusal. + */ +import { describe, it, expect, vi } from 'vitest'; +import { PluginGuards } from '../../../src/nest/plugins/host/plugin-guards.service'; +import { BadParams, ForbiddenResource } from '../../../src/nest/plugins/host/rpc-errors'; +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 { PluginRpcContext } from '../../../src/nest/plugins/host/rpc-kit/types'; + +const ctx = (actingUserId: number | undefined): PluginRpcContext => ({ + pluginId: 'p', + actingUserId, + data: {} as PluginRpcContext['data'], +}); + +/** Trip 1 belongs to user 42 and only user 42 may reach it, mirroring rpc-host.test.ts. */ +function build(overrides: { role?: string | undefined; allow?: boolean; addonOn?: boolean } = {}) { + const db = { + canAccessTrip: vi.fn((tripId: number, userId: number) => + tripId === 1 && userId === 42 ? { id: 1, user_id: 42 } : undefined, + ), + prepare: vi.fn(() => ({ + get: () => ('role' in overrides ? (overrides.role === undefined ? undefined : { role: overrides.role }) : { role: 'user' }), + })), + } as unknown as DatabaseService; + const permissions = { + checkPermission: vi.fn(() => overrides.allow ?? true), + } as unknown as PermissionsService; + const addons = { + isAddonEnabled: vi.fn(() => overrides.addonOn ?? true), + } as unknown as AddonsService; + return { guards: new PluginGuards(db, permissions, addons), db, permissions, addons }; +} + +describe('PluginGuards — tripRead', () => { + it('PGUARD-001 runs the read for a member and hands it the acting user', () => { + const { guards } = build(); + const read = vi.fn((userId: number) => ({ seenBy: userId })); + expect(guards.tripRead({ tripId: 1 }, ctx(42), read)).toEqual({ seenBy: 42 }); + expect(read).toHaveBeenCalledWith(42); + }); + + it('PGUARD-002 a userless context is refused before the read runs', () => { + const { guards } = build(); + const read = vi.fn(); + expect(() => guards.tripRead({ tripId: 1 }, ctx(undefined), read)).toThrow( + new ForbiddenResource('trip reads require an authenticated user context'), + ); + expect(read).not.toHaveBeenCalled(); + }); + + it('PGUARD-003 a non-member is refused, naming the trip', () => { + const { guards } = build(); + const read = vi.fn(); + expect(() => guards.tripRead({ tripId: 2 }, ctx(42), read)).toThrow( + new ForbiddenResource('no access to trip 2'), + ); + expect(read).not.toHaveBeenCalled(); + }); + + it('PGUARD-004 a missing tripId is BAD_PARAMS, not a refusal', () => { + const { guards } = build(); + expect(() => guards.tripRead({}, ctx(42), vi.fn())).toThrow(new BadParams('tripId must be a number')); + }); + + it('PGUARD-005 a numeric string tripId is accepted, as the wire contract allows', () => { + const { guards } = build(); + expect(guards.tripRead({ tripId: '1' }, ctx(42), (u) => u)).toBe(42); + }); +}); + +describe('PluginGuards — requireActor', () => { + it('PGUARD-006 returns the acting user', () => { + expect(build().guards.requireActor(ctx(42), 'tag')).toBe(42); + }); + + it('PGUARD-007 appends the word "writes" to the noun, verbatim as the router did', () => { + expect(() => build().guards.requireActor(ctx(undefined), 'tag')).toThrow( + new ForbiddenResource('tag writes require an authenticated user context'), + ); + }); +}); + +describe('PluginGuards — requireTripEdit and canEditAs', () => { + it('PGUARD-008 passes when the user may access and edit', () => { + const { guards, permissions } = build({ allow: true }); + expect(() => guards.requireTripEdit(1, 42, 'trip_edit')).not.toThrow(); + expect(permissions.checkPermission).toHaveBeenCalledWith('trip_edit', 'user', 42, 42, false); + }); + + it('PGUARD-009 no access wins over no permission, and names the trip', () => { + const { guards } = build({ allow: false }); + expect(() => guards.requireTripEdit(2, 42, 'trip_edit')).toThrow( + new ForbiddenResource('no access to trip 2'), + ); + }); + + it('PGUARD-010 access without the edit permission is a different message', () => { + const { guards } = build({ allow: false }); + expect(() => guards.requireTripEdit(1, 42, 'trip_edit')).toThrow( + new ForbiddenResource('no permission to edit trip 1'), + ); + }); + + it('PGUARD-011 canEditAs returns false rather than throwing when there is no access', () => { + expect(build().guards.canEditAs('trip_edit', 2, 42)).toBe(false); + }); + + it('PGUARD-012 canEditAs returns false when the user row is gone', () => { + expect(build({ role: undefined }).guards.canEditAs('trip_edit', 1, 42)).toBe(false); + }); + + it('PGUARD-013 a missing role column falls back to "user"', () => { + const { guards, permissions } = build({ role: null as unknown as string }); + guards.canEditAs('trip_edit', 1, 42); + expect(permissions.checkPermission).toHaveBeenCalledWith('trip_edit', 'user', 42, 42, false); + }); + + it('PGUARD-014 a non-owner member is flagged as shared', () => { + const db = { + canAccessTrip: vi.fn(() => ({ id: 1, user_id: 7 })), + prepare: vi.fn(() => ({ get: () => ({ role: 'user' }) })), + } as unknown as DatabaseService; + const permissions = { checkPermission: vi.fn(() => true) } as unknown as PermissionsService; + const guards = new PluginGuards(db, permissions, {} as AddonsService); + guards.canEditAs('trip_edit', 1, 42); + expect(permissions.checkPermission).toHaveBeenCalledWith('trip_edit', 'user', 7, 42, true); + }); +}); + +describe('PluginGuards — requireAddon', () => { + it('PGUARD-015 an enabled addon passes', () => { + expect(() => build({ addonOn: true }).guards.requireAddon('budget', 'costs')).not.toThrow(); + }); + + it('PGUARD-016 a disabled addon is refused with the noun in the message', () => { + expect(() => build({ addonOn: false }).guards.requireAddon('budget', 'costs')).toThrow( + new ForbiddenResource('the costs addon is disabled'), + ); + }); +}); + +describe('PluginGuards — capStrings', () => { + it('PGUARD-017 a field within the cap passes', () => { + expect(() => build().guards.capStrings({ name: 'ok' }, { name: 200 })).not.toThrow(); + }); + + it('PGUARD-018 an oversized field is BAD_PARAMS, naming field and cap', () => { + expect(() => build().guards.capStrings({ name: 'x'.repeat(201) }, { name: 200 })).toThrow( + new BadParams('name must be 200 characters or fewer'), + ); + }); + + it('PGUARD-019 a non-string value is left alone', () => { + expect(() => build().guards.capStrings({ name: 12345 }, { name: 2 })).not.toThrow(); + }); + + it('PGUARD-020 an absent field is left alone', () => { + expect(() => build().guards.capStrings({}, { name: 1 })).not.toThrow(); + }); +});