diff --git a/server/src/nest/permissions/permissions.service.ts b/server/src/nest/permissions/permissions.service.ts index 02d640c1b..47e95ae92 100644 --- a/server/src/nest/permissions/permissions.service.ts +++ b/server/src/nest/permissions/permissions.service.ts @@ -5,63 +5,16 @@ import { UnitOfWork } from '../database/unit-of-work'; import { logError } from '../audit/audit-log.logger'; import { AppSettings } from '../../db/entities/AppSettings.entity'; import type { AppSettingsRepository } from '../../db/repositories/AppSettings.repository'; +import { PERMISSION_ACTIONS, evaluatePermission, type PermissionAction, type PermissionLevel } from '@trek/shared'; import { PermissionsCacheStore, permissionsCacheSlot } from './permissions-cache'; -/** - * Permission levels (hierarchical, higher includes lower): - * admin > trip_owner > trip_member > everybody - * - * "everybody" means any authenticated user with trip access. - * For trip_create, "everybody" means any authenticated user (no trip context). - */ -export type PermissionLevel = 'admin' | 'trip_owner' | 'trip_member' | 'everybody'; +// The catalog (keys, levels, defaults) and the decision rule live in +// @trek/shared, so the admin screen and the client read the same table the +// server enforces. Re-exported here for the server code that imported them. +export { PERMISSION_ACTIONS }; +export type { PermissionAction, PermissionLevel }; -export interface PermissionAction { - key: string; - defaultLevel: PermissionLevel; - allowedLevels: PermissionLevel[]; -} - -// All configurable actions with their defaults matching upstream behavior -export const PERMISSION_ACTIONS: PermissionAction[] = [ - // Trip management - { key: 'trip_create', defaultLevel: 'everybody', allowedLevels: ['admin', 'everybody'] }, - { key: 'trip_edit', defaultLevel: 'trip_owner', allowedLevels: ['trip_owner', 'trip_member'] }, - { key: 'trip_delete', defaultLevel: 'trip_owner', allowedLevels: ['admin', 'trip_owner'] }, - { key: 'trip_archive', defaultLevel: 'trip_owner', allowedLevels: ['trip_owner', 'trip_member'] }, - { key: 'trip_cover_upload', defaultLevel: 'trip_owner', allowedLevels: ['trip_owner', 'trip_member'] }, - - // Member management - { key: 'member_manage', defaultLevel: 'trip_owner', allowedLevels: ['admin', 'trip_owner', 'trip_member'] }, - - // Files - { key: 'file_upload', defaultLevel: 'trip_member', allowedLevels: ['admin', 'trip_owner', 'trip_member'] }, - { key: 'file_edit', defaultLevel: 'trip_member', allowedLevels: ['trip_owner', 'trip_member'] }, - { key: 'file_delete', defaultLevel: 'trip_member', allowedLevels: ['trip_owner', 'trip_member'] }, - - // Places - { key: 'place_edit', defaultLevel: 'trip_member', allowedLevels: ['trip_owner', 'trip_member'] }, - - // Budget - { key: 'budget_edit', defaultLevel: 'trip_member', allowedLevels: ['trip_owner', 'trip_member'] }, - - // Packing - { key: 'packing_edit', defaultLevel: 'trip_member', allowedLevels: ['trip_owner', 'trip_member'] }, - - // Reservations - { key: 'reservation_edit', defaultLevel: 'trip_member', allowedLevels: ['trip_owner', 'trip_member'] }, - - // Day notes & schedule - { key: 'day_edit', defaultLevel: 'trip_member', allowedLevels: ['trip_owner', 'trip_member'] }, - - // Collaboration (notes, polls, messages) - { key: 'collab_edit', defaultLevel: 'trip_member', allowedLevels: ['trip_owner', 'trip_member'] }, - - // Share link management - { key: 'share_manage', defaultLevel: 'trip_owner', allowedLevels: ['trip_owner', 'trip_member'] }, -]; - -const ACTIONS_MAP = new Map(PERMISSION_ACTIONS.map(a => [a.key, a])); +const ACTIONS_MAP = new Map(PERMISSION_ACTIONS.map(a => [a.key, a])); // The cache is a PermissionsCacheStore (./permissions-cache), injected so a // store shared between processes can be plugged in later. A hand-built @@ -192,22 +145,11 @@ export class PermissionsService { userId: number, isMember: boolean ): Promise { - // Admins always pass + // Admins always pass, without reading the configured level. if (userRole === 'admin') return true; const required = await this.getPermissionLevel(actionKey); - - switch (required) { - case 'admin': - return false; // already checked above - case 'trip_owner': - return tripUserId !== null && tripUserId === userId; - case 'trip_member': - return (tripUserId !== null && tripUserId === userId) || isMember; - case 'everybody': - return true; - default: - return false; - } + const isOwner = tripUserId !== null && tripUserId === userId; + return evaluatePermission(required, { isAdmin: false, isOwner, isMember }); } } diff --git a/server/src/nest/permissions/trip-access.guard.ts b/server/src/nest/permissions/trip-access.guard.ts index 9ecc1f2da..7fac8dbd3 100644 --- a/server/src/nest/permissions/trip-access.guard.ts +++ b/server/src/nest/permissions/trip-access.guard.ts @@ -2,6 +2,7 @@ import { CanActivate, ExecutionContext, HttpException, Injectable, SetMetadata } import { Reflector } from '@nestjs/core'; import { EntityManager } from '@mikro-orm/core'; import type { Request } from 'express'; +import type { PermissionKey } from '@trek/shared'; import { Trips } from '../../db/entities/Trips.entity'; import type { TripAccess } from '../../db/repositories/Trips.repository'; import { PermissionsService } from './permissions.service'; @@ -16,11 +17,12 @@ export const TRIP_PERMISSION_KEY = 'trekTripPermission'; /** * Requires a permission on the trip the route is scoped to, on top of access. * - * The action string is the same one the domain services pass to - * `PermissionsService.checkPermission` ('day_edit', 'budget_edit', …), so a route - * and its MCP counterpart cannot drift apart on which right they demand. + * The action is a key of the shared catalog (`PermissionKey` from `@trek/shared`), + * the same one the domain services pass to `PermissionsService.checkPermission` + * ('day_edit', 'budget_edit', …), so a route and its MCP counterpart cannot drift + * apart on which right they demand, and a misspelt key fails to compile. */ -export const RequirePermission = (action: string) => SetMetadata(TRIP_PERMISSION_KEY, action); +export const RequirePermission = (action: PermissionKey) => SetMetadata(TRIP_PERMISSION_KEY, action); type TripRequest = Request & { user?: User; [TRIP_REQUEST_KEY]?: TripAccess }; diff --git a/server/tests/unit/nest/trip-access.guard.test.ts b/server/tests/unit/nest/trip-access.guard.test.ts index 0f04a0e14..3ba2ef8a5 100644 --- a/server/tests/unit/nest/trip-access.guard.test.ts +++ b/server/tests/unit/nest/trip-access.guard.test.ts @@ -7,7 +7,8 @@ * no business knowing), and a mutation additionally needs its own permission. They * moved out of days.controller.test.ts with the check itself. */ -import { describe, it, expect, vi } from 'vitest'; +import { describe, it, expect, expectTypeOf, vi } from 'vitest'; +import type { PermissionKey } from '@trek/shared'; import { HttpException } from '@nestjs/common'; import { ROUTE_ARGS_METADATA } from '@nestjs/common/constants'; import { Reflector } from '@nestjs/core'; @@ -189,3 +190,11 @@ describe('@Trip() param decorator', () => { expect(() => tripFactory()(undefined, ctx({}))).toThrow(/without @UseGuards\(TripAccessGuard\)/); }); }); + +describe('RequirePermission', () => { + it('takes a key of the shared permission catalog, so a misspelt action does not compile', () => { + expectTypeOf(RequirePermission).parameter(0).toEqualTypeOf(); + expectTypeOf<'day_edit'>().toMatchTypeOf(); + expectTypeOf<'day_edits'>().not.toMatchTypeOf(); + }); +}); diff --git a/shared/CLAUDE.md b/shared/CLAUDE.md index 9b537df4a..727b1928b 100644 --- a/shared/CLAUDE.md +++ b/shared/CLAUDE.md @@ -64,3 +64,7 @@ The parity rule governs **existing** routes; it is not a license to mint new deb ## Money (`src/money/money.ts`) `toMinor`, `sumMinor`, `splitEqualShares` and `currencyDecimals` are the budget arithmetic both sides must agree on to the cent: amounts are netted in whole hundredths, equal splits hand the leftover hundredths out by rotation from `itemId % n`, and the shares always sum back to the total. The server's settlement uses these directly; `money.spec.ts` holds the share table that used to be duplicated between the server and client tests. The client still carries its own `splitEqualShares` (in euros, `CostsPanel.helpers.ts`) and `currencyDecimals` (`utils/formatters.ts`); folding them onto these is client work, not a second implementation to copy. + +## Permissions (`src/permissions/permissions.ts`) + +`PERMISSION_ACTIONS` is the catalog of configurable trip permissions (key, default level, the levels an admin may pick), `PermissionKey` the union of its keys and `evaluatePermission(level, { isAdmin, isOwner, isMember })` the one decision rule. The server's `PermissionsService` and `@RequirePermission(action: PermissionKey)` are built on them, so a new action starts here and a misspelt key fails to compile. The client still evaluates in its own `useCanDo` (with "not configured = allow"); moving it onto `evaluatePermission` is client work. diff --git a/shared/src/index.ts b/shared/src/index.ts index b1ffe584b..874693380 100644 --- a/shared/src/index.ts +++ b/shared/src/index.ts @@ -111,3 +111,6 @@ export * from './vacay/school-holiday-catalog.schema'; // Money arithmetic in whole hundredths (sums, equal splits, currency precision). export * from './money/money'; + +// Trip permission catalog (keys, levels, defaults) and the rule that decides a check. +export * from './permissions/permissions'; diff --git a/shared/src/permissions/permissions.spec.ts b/shared/src/permissions/permissions.spec.ts new file mode 100644 index 000000000..73c7a35f7 --- /dev/null +++ b/shared/src/permissions/permissions.spec.ts @@ -0,0 +1,60 @@ +import { + PERMISSION_ACTIONS, + PERMISSION_LEVELS, + evaluatePermission, + isPermissionKey, + type PermissionLevel, + type PermissionSubject, +} from './permissions'; + +import { describe, expect, it } from 'vitest'; + +const stranger: PermissionSubject = { isAdmin: false, isOwner: false, isMember: false }; +const member: PermissionSubject = { isAdmin: false, isOwner: false, isMember: true }; +const owner: PermissionSubject = { isAdmin: false, isOwner: true, isMember: false }; +const admin: PermissionSubject = { isAdmin: true, isOwner: false, isMember: false }; + +describe('PERMISSION_ACTIONS', () => { + it('lists every key once', () => { + const keys = PERMISSION_ACTIONS.map((a) => a.key); + expect(new Set(keys).size).toBe(keys.length); + }); + + it('allows the default level of every action', () => { + for (const action of PERMISSION_ACTIONS) { + expect(action.allowedLevels as readonly PermissionLevel[]).toContain(action.defaultLevel); + } + }); + + it('only uses known levels', () => { + for (const action of PERMISSION_ACTIONS) { + for (const level of action.allowedLevels) expect(PERMISSION_LEVELS).toContain(level); + } + }); +}); + +describe('isPermissionKey', () => { + it('knows the catalog and nothing else', () => { + expect(isPermissionKey('day_edit')).toBe(true); + expect(isPermissionKey('share_manage')).toBe(true); + expect(isPermissionKey('nonexistent_action')).toBe(false); + expect(isPermissionKey('')).toBe(false); + }); +}); + +describe('evaluatePermission', () => { + it.each([ + ['admin', [false, false, false, true]], + ['trip_owner', [false, false, true, true]], + ['trip_member', [false, true, true, true]], + ['everybody', [true, true, true, true]], + ] as const)('%s: stranger, member, owner, admin', (level, expected) => { + expect([stranger, member, owner, admin].map((s) => evaluatePermission(level, s))).toEqual(expected); + }); + + it('refuses a level it does not know, unless the caller is an admin', () => { + const bogus = 'root' as unknown as PermissionLevel; + expect(evaluatePermission(bogus, owner)).toBe(false); + expect(evaluatePermission(bogus, admin)).toBe(true); + }); +}); diff --git a/shared/src/permissions/permissions.ts b/shared/src/permissions/permissions.ts new file mode 100644 index 000000000..edf833ab1 --- /dev/null +++ b/shared/src/permissions/permissions.ts @@ -0,0 +1,100 @@ +/** + * The catalog of configurable trip permissions and the one rule that decides + * them. The server enforces it (`PermissionsService`, `@RequirePermission`); + * the admin screen and the client's `can()` read the same keys and levels. + * + * Levels are hierarchical, higher includes lower: + * admin > trip_owner > trip_member > everybody + * + * "everybody" means any authenticated user with trip access; for + * `trip_create`, which has no trip, any authenticated user. + */ +export const PERMISSION_LEVELS = ['admin', 'trip_owner', 'trip_member', 'everybody'] as const; + +export type PermissionLevel = (typeof PERMISSION_LEVELS)[number]; + +export interface PermissionAction { + readonly key: string; + readonly defaultLevel: PermissionLevel; + readonly allowedLevels: readonly PermissionLevel[]; +} + +/** Every configurable action with its default and the levels an admin may pick. */ +export const PERMISSION_ACTIONS = [ + // Trip management + { key: 'trip_create', defaultLevel: 'everybody', allowedLevels: ['admin', 'everybody'] }, + { key: 'trip_edit', defaultLevel: 'trip_owner', allowedLevels: ['trip_owner', 'trip_member'] }, + { key: 'trip_delete', defaultLevel: 'trip_owner', allowedLevels: ['admin', 'trip_owner'] }, + { key: 'trip_archive', defaultLevel: 'trip_owner', allowedLevels: ['trip_owner', 'trip_member'] }, + { key: 'trip_cover_upload', defaultLevel: 'trip_owner', allowedLevels: ['trip_owner', 'trip_member'] }, + + // Member management + { key: 'member_manage', defaultLevel: 'trip_owner', allowedLevels: ['admin', 'trip_owner', 'trip_member'] }, + + // Files + { key: 'file_upload', defaultLevel: 'trip_member', allowedLevels: ['admin', 'trip_owner', 'trip_member'] }, + { key: 'file_edit', defaultLevel: 'trip_member', allowedLevels: ['trip_owner', 'trip_member'] }, + { key: 'file_delete', defaultLevel: 'trip_member', allowedLevels: ['trip_owner', 'trip_member'] }, + + // Places + { key: 'place_edit', defaultLevel: 'trip_member', allowedLevels: ['trip_owner', 'trip_member'] }, + + // Budget + { key: 'budget_edit', defaultLevel: 'trip_member', allowedLevels: ['trip_owner', 'trip_member'] }, + + // Packing + { key: 'packing_edit', defaultLevel: 'trip_member', allowedLevels: ['trip_owner', 'trip_member'] }, + + // Reservations + { key: 'reservation_edit', defaultLevel: 'trip_member', allowedLevels: ['trip_owner', 'trip_member'] }, + + // Day notes & schedule + { key: 'day_edit', defaultLevel: 'trip_member', allowedLevels: ['trip_owner', 'trip_member'] }, + + // Collaboration (notes, polls, messages) + { key: 'collab_edit', defaultLevel: 'trip_member', allowedLevels: ['trip_owner', 'trip_member'] }, + + // Share link management + { key: 'share_manage', defaultLevel: 'trip_owner', allowedLevels: ['trip_owner', 'trip_member'] }, +] as const satisfies readonly PermissionAction[]; + +/** The name of one configurable action, `'day_edit'`, `'budget_edit'`, … */ +export type PermissionKey = (typeof PERMISSION_ACTIONS)[number]['key']; + +const PERMISSION_KEYS: ReadonlySet = new Set(PERMISSION_ACTIONS.map((a) => a.key)); + +/** Whether a string names a configurable action. */ +export function isPermissionKey(value: string): value is PermissionKey { + return PERMISSION_KEYS.has(value); +} + +/** Who is asking, relative to the trip the action is on. */ +export interface PermissionSubject { + /** An instance admin passes every check. */ + isAdmin: boolean; + /** Owns the trip. Always false for an action without a trip. */ + isOwner: boolean; + /** A member of the trip who is not its owner. */ + isMember: boolean; +} + +/** + * Whether `subject` passes an action whose configured level is `level`. An + * admin always passes; an `admin` level otherwise refuses everyone; an + * unrecognised level refuses (fail closed). + */ +export function evaluatePermission(level: PermissionLevel, subject: PermissionSubject): boolean { + if (subject.isAdmin) return true; + switch (level) { + case 'admin': + return false; + case 'trip_owner': + return subject.isOwner; + case 'trip_member': + return subject.isOwner || subject.isMember; + case 'everybody': + return true; + default: + return false; + } +}