diff --git a/server/src/mcp/tools/_shared.ts b/server/src/mcp/tools/_shared.ts index 212c0366b..2f21ca276 100644 --- a/server/src/mcp/tools/_shared.ts +++ b/server/src/mcp/tools/_shared.ts @@ -1,6 +1,6 @@ import { errorResult } from '@trek/nest-mcp'; import { db } from '../../db/database'; -import { checkPermission } from '../../services/permissions'; +import { checkPermission } from '../../nest/permissions/permissions.bridge'; import { broadcast } from '../../websocket'; // Generic MCP result helpers and annotation presets live in @trek/nest-mcp diff --git a/server/src/nest/app.module.ts b/server/src/nest/app.module.ts index 6e2286bf5..8b4a8c9d9 100644 --- a/server/src/nest/app.module.ts +++ b/server/src/nest/app.module.ts @@ -43,6 +43,7 @@ import { OidcModule } from './oidc/oidc.module'; import { OauthModule } from './oauth/oauth.module'; import { AdminModule } from './admin/admin.module'; import { AddonsModule } from './addons/addons.module'; +import { PermissionsModule } from './permissions/permissions.module'; import { PluginsModule } from './plugins/plugins.module'; import { McpModule } from '@trek/nest-mcp'; import { trekMcpAccessPolicy } from '../mcp/nest-mcp-policy'; @@ -56,7 +57,7 @@ import { IdempotencyInterceptor } from './common/idempotency.interceptor'; * migrated. */ @Module({ - imports: [AppConfigModule, DatabaseModule, McpModule.forRoot({ accessPolicy: trekMcpAccessPolicy }), WeatherModule, HelpModule, AirportsModule, ConfigModule, SystemNoticesModule, MapsModule, CategoriesModule, TagsModule, NotificationsModule, AtlasModule, VacayModule, PackingModule, TodoModule, BudgetModule, ReservationsModule, DaysModule, AssignmentsModule, PlacesModule, TripsModule, CollabModule, FilesModule, PhotosModule, MemoriesModule, AirtrailModule, JourneyModule, CollectionsModule, ShareModule, TripInviteModule, TransitModule, FeedsModule, SettingsModule, BackupModule, AuthModule, OidcModule, OauthModule, AdminModule, AddonsModule, PluginsModule, BookingImportModule], + imports: [AppConfigModule, DatabaseModule, McpModule.forRoot({ accessPolicy: trekMcpAccessPolicy }), WeatherModule, HelpModule, AirportsModule, ConfigModule, SystemNoticesModule, MapsModule, CategoriesModule, TagsModule, NotificationsModule, AtlasModule, VacayModule, PackingModule, TodoModule, BudgetModule, ReservationsModule, DaysModule, AssignmentsModule, PlacesModule, TripsModule, CollabModule, FilesModule, PhotosModule, MemoriesModule, AirtrailModule, JourneyModule, CollectionsModule, ShareModule, TripInviteModule, TransitModule, FeedsModule, SettingsModule, BackupModule, AuthModule, OidcModule, OauthModule, AdminModule, AddonsModule, PermissionsModule, PluginsModule, BookingImportModule], controllers: [HealthController], providers: [ HealthService, diff --git a/server/src/nest/assignments/assignments.bridge.ts b/server/src/nest/assignments/assignments.bridge.ts index 273e2f21a..2c23df08a 100644 --- a/server/src/nest/assignments/assignments.bridge.ts +++ b/server/src/nest/assignments/assignments.bridge.ts @@ -1,6 +1,7 @@ import { db } from '../../db/database'; import { DatabaseService } from '../database/database.service'; import { AssignmentsService } from './assignments.service'; +import { PermissionsService } from '../permissions/permissions.service'; /** * Non-Nest entry point for the assignments domain — for code running OUTSIDE @@ -25,7 +26,7 @@ import { AssignmentsService } from './assignments.service'; */ let instance: AssignmentsService | undefined; function assignments(): AssignmentsService { - return (instance ??= new AssignmentsService(new DatabaseService(db))); + return (instance ??= new AssignmentsService(new DatabaseService(db), new PermissionsService(new DatabaseService(db)))); } export function createAssignment(dayId: string | number, placeId: string | number, notes: string | null) { diff --git a/server/src/nest/assignments/assignments.module.ts b/server/src/nest/assignments/assignments.module.ts index 69866ef64..604a75ba2 100644 --- a/server/src/nest/assignments/assignments.module.ts +++ b/server/src/nest/assignments/assignments.module.ts @@ -1,5 +1,6 @@ import { Module } from '@nestjs/common'; import { DaysModule } from '../days/days.module'; +import { PermissionsModule } from '../permissions/permissions.module'; import { DayAssignmentsController, AssignmentOpsController } from './assignments.controller'; import { AssignmentsService } from './assignments.service'; import { AssignmentsMcp } from './assignments.mcp'; @@ -12,7 +13,7 @@ import { AssignmentsMcp } from './assignments.mcp'; */ @Module({ // DaysModule: AssignmentsMcp injects DaysService for the target-day checks. - imports: [DaysModule], + imports: [DaysModule, PermissionsModule], controllers: [DayAssignmentsController, AssignmentOpsController], providers: [AssignmentsService, AssignmentsMcp], exports: [AssignmentsService], diff --git a/server/src/nest/assignments/assignments.service.ts b/server/src/nest/assignments/assignments.service.ts index 977190695..97435c6b3 100644 --- a/server/src/nest/assignments/assignments.service.ts +++ b/server/src/nest/assignments/assignments.service.ts @@ -1,7 +1,7 @@ import { Injectable } from '@nestjs/common'; import { broadcast } from '../../websocket'; import { DatabaseService } from '../database/database.service'; -import { checkPermission } from '../../services/permissions'; +import { PermissionsService } from '../permissions/permissions.service'; import { loadTagsByPlaceIds, loadParticipantsByAssignmentIds, formatAssignmentWithPlace } from '../../services/queryHelpers'; import type { AssignmentRow, DayAssignment, User } from '../../types'; import { reconcileTripSkeletons } from '../../services/journeyService'; @@ -23,14 +23,17 @@ type Trip = { user_id: number }; */ @Injectable() export class AssignmentsService { - constructor(private readonly dbs: DatabaseService) {} + constructor( + private readonly dbs: DatabaseService, + private readonly permissions: PermissionsService, + ) {} verifyTripAccess(tripId: string | number, userId: number) { return this.dbs.canAccessTrip(Number(tripId), userId) as Trip | null | undefined; } canEdit(trip: Trip, user: User): boolean { - return checkPermission('day_edit', user.role, trip.user_id, user.id, trip.user_id !== user.id); + return this.permissions.checkPermission('day_edit', user.role, trip.user_id, user.id, trip.user_id !== user.id); } broadcast(tripId: string, event: string, payload: Record, socketId: string | undefined): void { diff --git a/server/src/nest/booking-import/booking-import.module.ts b/server/src/nest/booking-import/booking-import.module.ts index 533f4c535..0e1746558 100644 --- a/server/src/nest/booking-import/booking-import.module.ts +++ b/server/src/nest/booking-import/booking-import.module.ts @@ -6,9 +6,10 @@ import { KitineraryExtractorService } from './kitinerary-extractor.service'; import { FeaturesController } from './features.controller'; import { LlmParseModule } from '../llm-parse/llm-parse.module'; import { ReservationsModule } from '../reservations/reservations.module'; +import { PermissionsModule } from '../permissions/permissions.module'; @Module({ - imports: [LlmParseModule, ReservationsModule], + imports: [LlmParseModule, ReservationsModule, PermissionsModule], controllers: [BookingImportController, FeaturesController], providers: [BookingImportService, KitineraryExtractorService, ImportJobsService], }) diff --git a/server/src/nest/booking-import/booking-import.service.ts b/server/src/nest/booking-import/booking-import.service.ts index d4aedd2cd..8ac95535b 100644 --- a/server/src/nest/booking-import/booking-import.service.ts +++ b/server/src/nest/booking-import/booking-import.service.ts @@ -1,6 +1,6 @@ import { Injectable, HttpException } from '@nestjs/common'; import { broadcast } from '../../websocket'; -import { checkPermission } from '../../services/permissions'; +import { PermissionsService } from '../permissions/permissions.service'; import { verifyTripAccess } from '../../services/tripAccess'; import { ReservationsService } from '../reservations/reservations.service'; import { createPlace } from '../../services/placeService'; @@ -24,6 +24,7 @@ export class BookingImportService { private readonly llmParse: LlmParseService, private readonly dbs: DatabaseService, private readonly reservations: ReservationsService, + private readonly permissions: PermissionsService, ) {} private get db() { @@ -56,7 +57,7 @@ export class BookingImportService { } canEdit(trip: NonNullable>, user: User): boolean { - return checkPermission('reservation_edit', user.role, trip.user_id, user.id, trip.user_id !== user.id); + return this.permissions.checkPermission('reservation_edit', user.role, trip.user_id, user.id, trip.user_id !== user.id); } /** diff --git a/server/src/nest/budget/budget.module.ts b/server/src/nest/budget/budget.module.ts index cca2eeec2..e0b31ad94 100644 --- a/server/src/nest/budget/budget.module.ts +++ b/server/src/nest/budget/budget.module.ts @@ -1,9 +1,11 @@ import { Module } from '@nestjs/common'; import { BudgetController } from './budget.controller'; import { BudgetService } from './budget.service'; +import { PermissionsModule } from '../permissions/permissions.module'; /** Budget domain (S4 — Phase 2 trip sub-domain). Registered in AppModule. */ @Module({ + imports: [PermissionsModule], controllers: [BudgetController], providers: [BudgetService], // For in-container consumers (PluginHostDepsFactory). diff --git a/server/src/nest/budget/budget.service.ts b/server/src/nest/budget/budget.service.ts index 560bd5d08..e7693c306 100644 --- a/server/src/nest/budget/budget.service.ts +++ b/server/src/nest/budget/budget.service.ts @@ -1,7 +1,7 @@ import { Injectable } from '@nestjs/common'; import { DatabaseService } from '../database/database.service'; import { broadcast } from '../../websocket'; -import { checkPermission } from '../../services/permissions'; +import { PermissionsService } from '../permissions/permissions.service'; import type { User } from '../../types'; import * as svc from '../../services/budgetService'; import { getRates } from '../../services/exchangeRateService'; @@ -15,7 +15,10 @@ type Trip = NonNullable>; */ @Injectable() export class BudgetService { - constructor(private readonly dbs: DatabaseService) {} + constructor( + private readonly dbs: DatabaseService, + private readonly permissions: PermissionsService, + ) {} private get db() { return this.dbs.connection; @@ -26,7 +29,7 @@ export class BudgetService { } canEdit(trip: Trip, user: User): boolean { - return checkPermission('budget_edit', user.role, trip.user_id, user.id, trip.user_id !== user.id); + return this.permissions.checkPermission('budget_edit', user.role, trip.user_id, user.id, trip.user_id !== user.id); } broadcast(tripId: string, event: string, payload: Record, socketId: string | undefined): void { diff --git a/server/src/nest/collab/collab.bridge.ts b/server/src/nest/collab/collab.bridge.ts index 534718cf3..6a09189a9 100644 --- a/server/src/nest/collab/collab.bridge.ts +++ b/server/src/nest/collab/collab.bridge.ts @@ -1,6 +1,7 @@ import { db } from '../../db/database'; import { DatabaseService } from '../database/database.service'; import { CollabService } from './collab.service'; +import { PermissionsService } from '../permissions/permissions.service'; /** * Non-Nest entry point for the collab domain — for code running OUTSIDE the @@ -16,7 +17,7 @@ import { CollabService } from './collab.service'; * Module-level construction is safe: `db` is the reinitialize-proof Proxy onto * the shared better-sqlite3 singleton. */ -const collab = new CollabService(new DatabaseService(db)); +const collab = new CollabService(new DatabaseService(db), new PermissionsService(new DatabaseService(db))); export function listNotes(tripId: string | number) { return collab.listNotes(tripId); diff --git a/server/src/nest/collab/collab.module.ts b/server/src/nest/collab/collab.module.ts index fbbc1ea2b..c7b3709a7 100644 --- a/server/src/nest/collab/collab.module.ts +++ b/server/src/nest/collab/collab.module.ts @@ -2,8 +2,10 @@ import { Module } from '@nestjs/common'; import { CollabController } from './collab.controller'; import { CollabService } from './collab.service'; import { CollabMcp } from './collab.mcp'; +import { PermissionsModule } from '../permissions/permissions.module'; @Module({ + imports: [PermissionsModule], controllers: [CollabController], providers: [CollabService, CollabMcp], // For in-container consumers (PluginHostDepsFactory). diff --git a/server/src/nest/collab/collab.service.ts b/server/src/nest/collab/collab.service.ts index 630ad2bec..dd7465682 100644 --- a/server/src/nest/collab/collab.service.ts +++ b/server/src/nest/collab/collab.service.ts @@ -3,7 +3,7 @@ import fs from 'fs'; import { Injectable } from '@nestjs/common'; import { DatabaseService } from '../database/database.service'; import { broadcast } from '../../websocket'; -import { checkPermission } from '../../services/permissions'; +import { PermissionsService } from '../permissions/permissions.service'; import { verifyTripAccess } from '../../services/tripAccess'; import { avatarUrl } from '../../services/avatarUrl'; import { checkSsrf, createPinnedDispatcher } from '../../utils/ssrfGuard'; @@ -62,18 +62,21 @@ export interface LinkPreviewResult { */ @Injectable() export class CollabService { - constructor(private readonly db: DatabaseService) {} + constructor( + private readonly db: DatabaseService, + private readonly permissions: PermissionsService, + ) {} verifyTripAccess(tripId: string | number, userId: number) { return verifyTripAccess(tripId, userId); } canEdit(trip: Trip, user: User): boolean { - return checkPermission('collab_edit', user.role, trip.user_id, user.id, trip.user_id !== user.id); + return this.permissions.checkPermission('collab_edit', user.role, trip.user_id, user.id, trip.user_id !== user.id); } canUploadFiles(trip: Trip, user: User): boolean { - return checkPermission('file_upload', user.role, trip.user_id, user.id, trip.user_id !== user.id); + return this.permissions.checkPermission('file_upload', user.role, trip.user_id, user.id, trip.user_id !== user.id); } broadcast(tripId: string, event: string, payload: Record, socketId: string | undefined): void { diff --git a/server/src/nest/days/day-notes.service.ts b/server/src/nest/days/day-notes.service.ts index a779c0089..9ebb978b8 100644 --- a/server/src/nest/days/day-notes.service.ts +++ b/server/src/nest/days/day-notes.service.ts @@ -1,6 +1,6 @@ import { Injectable } from '@nestjs/common'; import { broadcast } from '../../websocket'; -import { checkPermission } from '../../services/permissions'; +import { PermissionsService } from '../permissions/permissions.service'; import type { DayNote, User } from '../../types'; import { DatabaseService, type TripAccess } from '../database/database.service'; @@ -13,14 +13,17 @@ import { DatabaseService, type TripAccess } from '../database/database.service'; */ @Injectable() export class DayNotesService { - constructor(private readonly dbs: DatabaseService) {} + constructor( + private readonly dbs: DatabaseService, + private readonly permissions: PermissionsService, + ) {} verifyTripAccess(tripId: string | number, userId: number): TripAccess | undefined { return this.dbs.canAccessTrip(tripId, userId); } canEdit(trip: TripAccess, user: User): boolean { - return checkPermission('day_edit', user.role, trip.user_id, user.id, trip.user_id !== user.id); + return this.permissions.checkPermission('day_edit', user.role, trip.user_id, user.id, trip.user_id !== user.id); } broadcast(tripId: string, event: string, payload: Record, socketId: string | undefined): void { diff --git a/server/src/nest/days/days.bridge.ts b/server/src/nest/days/days.bridge.ts index 411b25dce..3040c4737 100644 --- a/server/src/nest/days/days.bridge.ts +++ b/server/src/nest/days/days.bridge.ts @@ -1,6 +1,7 @@ import { db } from '../../db/database'; import { DatabaseService } from '../database/database.service'; import { DaysService, addDays } from './days.service'; +import { PermissionsService } from '../permissions/permissions.service'; /** * Non-Nest entry point for the day domain — for code running OUTSIDE the @@ -16,7 +17,7 @@ import { DaysService, addDays } from './days.service'; * Module-level construction is safe: `db` is the reinitialize-proof Proxy onto * the shared better-sqlite3 singleton. */ -const days = new DaysService(new DatabaseService(db)); +const days = new DaysService(new DatabaseService(db), new PermissionsService(new DatabaseService(db))); export { addDays }; diff --git a/server/src/nest/days/days.module.ts b/server/src/nest/days/days.module.ts index 54ff9cf30..5fc4b4923 100644 --- a/server/src/nest/days/days.module.ts +++ b/server/src/nest/days/days.module.ts @@ -5,6 +5,7 @@ import { DaysMcp } from './days.mcp'; import { DayNotesController } from './day-notes.controller'; import { DayNotesService } from './day-notes.service'; import { DayNotesMcp } from './day-notes.mcp'; +import { PermissionsModule } from '../permissions/permissions.module'; /** * Days + day-notes domain (S6 — Phase 2 trip sub-domain). The single prefix @@ -16,6 +17,7 @@ import { DayNotesMcp } from './day-notes.mcp'; * controllers). */ @Module({ + imports: [PermissionsModule], controllers: [DaysController, DayNotesController], providers: [DaysService, DaysMcp, DayNotesService, DayNotesMcp], exports: [DaysService, DayNotesService], diff --git a/server/src/nest/days/days.service.ts b/server/src/nest/days/days.service.ts index 96eff9db2..44086f9e7 100644 --- a/server/src/nest/days/days.service.ts +++ b/server/src/nest/days/days.service.ts @@ -1,7 +1,7 @@ import { Injectable } from '@nestjs/common'; import { broadcast } from '../../websocket'; import { DatabaseService } from '../database/database.service'; -import { checkPermission } from '../../services/permissions'; +import { PermissionsService } from '../permissions/permissions.service'; import { loadTagsByPlaceIds, loadParticipantsByAssignmentIds, formatAssignmentWithPlace } from '../../services/queryHelpers'; import type { AssignmentRow, Day, DayNote, User } from '../../types'; @@ -84,14 +84,17 @@ export interface CreateAccommodationData { */ @Injectable() export class DaysService { - constructor(private readonly db: DatabaseService) {} + constructor( + private readonly db: DatabaseService, + private readonly permissions: PermissionsService, + ) {} verifyTripAccess(tripId: string | number, userId: number) { return this.db.canAccessTrip(Number(tripId), userId) as Trip | null | undefined; } canEdit(trip: Trip, user: User): boolean { - return checkPermission('day_edit', user.role, trip.user_id, user.id, trip.user_id !== user.id); + return this.permissions.checkPermission('day_edit', user.role, trip.user_id, user.id, trip.user_id !== user.id); } broadcast(tripId: string, event: string, payload: Record, socketId: string | undefined): void { diff --git a/server/src/nest/files/files.bridge.ts b/server/src/nest/files/files.bridge.ts index 18a79c83e..4c3ff5330 100644 --- a/server/src/nest/files/files.bridge.ts +++ b/server/src/nest/files/files.bridge.ts @@ -1,6 +1,7 @@ import { db } from '../../db/database'; import { DatabaseService } from '../database/database.service'; import { FilesService } from './files.service'; +import { PermissionsService } from '../permissions/permissions.service'; /** * Non-Nest entry point for the files domain. Unlike todo.bridge.ts this does @@ -14,7 +15,7 @@ import { FilesService } from './files.service'; * Module-level construction is safe: `db` is the reinitialize-proof Proxy onto * the shared better-sqlite3 singleton. */ -const files = new FilesService(new DatabaseService(db)); +const files = new FilesService(new DatabaseService(db), new PermissionsService(new DatabaseService(db))); export function getAllowedExtensions(): string { return files.getAllowedExtensions(); diff --git a/server/src/nest/files/files.module.ts b/server/src/nest/files/files.module.ts index a7b121dce..c90e5fe13 100644 --- a/server/src/nest/files/files.module.ts +++ b/server/src/nest/files/files.module.ts @@ -2,8 +2,10 @@ import { Module } from '@nestjs/common'; import { FilesController } from './files.controller'; import { FilesDownloadController } from './files-download.controller'; import { FilesService } from './files.service'; +import { PermissionsModule } from '../permissions/permissions.module'; @Module({ + imports: [PermissionsModule], controllers: [FilesController, FilesDownloadController], providers: [FilesService], exports: [FilesService], diff --git a/server/src/nest/files/files.service.ts b/server/src/nest/files/files.service.ts index b032e4dec..89cd6403c 100644 --- a/server/src/nest/files/files.service.ts +++ b/server/src/nest/files/files.service.ts @@ -3,7 +3,7 @@ import path from 'path'; import fs from 'fs'; import type { Request } from 'express'; import { broadcast } from '../../websocket'; -import { checkPermission } from '../../services/permissions'; +import { PermissionsService } from '../permissions/permissions.service'; import { avatarUrl } from '../../services/avatarUrl'; import { consumeEphemeralToken } from '../../services/ephemeralTokens'; import { verifyJwtAndLoadUser } from '../../middleware/auth'; @@ -54,14 +54,17 @@ export interface FileLink { */ @Injectable() export class FilesService { - constructor(private readonly db: DatabaseService) {} + constructor( + private readonly db: DatabaseService, + private readonly permissions: PermissionsService, + ) {} verifyTripAccess(tripId: string | number, userId: number) { return this.db.canAccessTrip(tripId, userId); } can(action: FilePermission, trip: Trip, user: User): boolean { - return checkPermission(action, user.role, trip.user_id, user.id, trip.user_id !== user.id); + return this.permissions.checkPermission(action, user.role, trip.user_id, user.id, trip.user_id !== user.id); } broadcast(tripId: string, event: string, payload: Record, socketId: string | undefined): void { diff --git a/server/src/nest/integrations/airtrail-import.controller.ts b/server/src/nest/integrations/airtrail-import.controller.ts index a0ded35e2..1607f81aa 100644 --- a/server/src/nest/integrations/airtrail-import.controller.ts +++ b/server/src/nest/integrations/airtrail-import.controller.ts @@ -6,7 +6,7 @@ import { AirtrailAddonGuard } from './airtrail-addon.guard'; import { AirtrailImportDto } from './airtrail.dto'; import type { AirtrailImportResult } from '@trek/shared'; import { verifyTripAccess } from '../../services/tripAccess'; -import { checkPermission } from '../../services/permissions'; +import { PermissionsService } from '../permissions/permissions.service'; import { importAirtrailFlights } from '../../services/airtrail/airtrailImport'; /** @@ -17,10 +17,12 @@ import { importAirtrailFlights } from '../../services/airtrail/airtrailImport'; @Controller('api/trips/:tripId/reservations/import') @UseGuards(AirtrailAddonGuard, JwtAuthGuard) export class AirtrailImportController { + constructor(private readonly permissions: PermissionsService) {} + private requireEdit(tripId: string, user: User): void { const trip = verifyTripAccess(tripId, user.id); if (!trip) throw new HttpException({ error: 'Trip not found' }, 404); - if (!checkPermission('reservation_edit', user.role, trip.user_id, user.id, trip.user_id !== user.id)) { + if (!this.permissions.checkPermission('reservation_edit', user.role, trip.user_id, user.id, trip.user_id !== user.id)) { throw new HttpException({ error: 'No permission' }, 403); } } diff --git a/server/src/nest/integrations/airtrail.module.ts b/server/src/nest/integrations/airtrail.module.ts index 0feec084d..ec4545158 100644 --- a/server/src/nest/integrations/airtrail.module.ts +++ b/server/src/nest/integrations/airtrail.module.ts @@ -1,6 +1,7 @@ import { Module } from '@nestjs/common'; import { AirtrailController } from './airtrail.controller'; import { AirtrailImportController } from './airtrail-import.controller'; +import { PermissionsModule } from '../permissions/permissions.module'; /** * AirTrail integration domain. The connection lives under @@ -9,6 +10,7 @@ import { AirtrailImportController } from './airtrail-import.controller'; * services/airtrail/* (plain functions over better-sqlite3). */ @Module({ + imports: [PermissionsModule], controllers: [AirtrailController, AirtrailImportController], }) export class AirtrailModule {} diff --git a/server/src/nest/packing/packing.bridge.ts b/server/src/nest/packing/packing.bridge.ts index 2ef664f51..52aaf7d63 100644 --- a/server/src/nest/packing/packing.bridge.ts +++ b/server/src/nest/packing/packing.bridge.ts @@ -1,6 +1,7 @@ import { db } from '../../db/database'; import { DatabaseService } from '../database/database.service'; import { PackingService } from './packing.service'; +import { PermissionsService } from '../permissions/permissions.service'; /** * Non-Nest entry point for the packing domain — for code running OUTSIDE the @@ -16,7 +17,7 @@ import { PackingService } from './packing.service'; * Module-level construction is safe: `db` is the reinitialize-proof Proxy onto * the shared better-sqlite3 singleton. */ -const packing = new PackingService(new DatabaseService(db)); +const packing = new PackingService(new DatabaseService(db), new PermissionsService(new DatabaseService(db))); export function listItems(tripId: string | number, userId?: number) { return packing.listItems(tripId, userId); diff --git a/server/src/nest/packing/packing.module.ts b/server/src/nest/packing/packing.module.ts index 1b2ae3471..6659de87b 100644 --- a/server/src/nest/packing/packing.module.ts +++ b/server/src/nest/packing/packing.module.ts @@ -2,11 +2,13 @@ import { Module } from '@nestjs/common'; import { PackingController } from './packing.controller'; import { PackingMcp } from './packing.mcp'; import { PackingService } from './packing.service'; +import { PermissionsModule } from '../permissions/permissions.module'; /** Packing domain (S2 — Phase 2 trip sub-domain). Registered in AppModule. * Exports PackingService for in-container consumers (TripsService bundle, * PluginHostDepsFactory). */ @Module({ + imports: [PermissionsModule], controllers: [PackingController], providers: [PackingService, PackingMcp], exports: [PackingService], diff --git a/server/src/nest/packing/packing.service.ts b/server/src/nest/packing/packing.service.ts index 7de9c2c88..214fb531d 100644 --- a/server/src/nest/packing/packing.service.ts +++ b/server/src/nest/packing/packing.service.ts @@ -1,6 +1,6 @@ import { Injectable } from '@nestjs/common'; import { broadcast } from '../../websocket'; -import { checkPermission } from '../../services/permissions'; +import { PermissionsService } from '../permissions/permissions.service'; import { verifyTripAccess } from '../../services/tripAccess'; import { avatarUrl } from '../../services/avatarUrl'; import type { UpdateConflict } from '../../services/conflictResult'; @@ -39,7 +39,10 @@ const BAG_COLORS = ['#6366f1', '#ec4899', '#f97316', '#10b981', '#06b6d4', '#8b5 */ @Injectable() export class PackingService { - constructor(private readonly db: DatabaseService) {} + constructor( + private readonly db: DatabaseService, + private readonly permissions: PermissionsService, + ) {} verifyTripAccess(tripId: string | number, userId: number) { return verifyTripAccess(tripId, userId); @@ -47,7 +50,7 @@ export class PackingService { /** Mirrors the inline checkPermission('packing_edit', ...) the legacy route runs. */ canEdit(trip: Trip, user: User): boolean { - return checkPermission('packing_edit', user.role, trip.user_id, user.id, trip.user_id !== user.id); + return this.permissions.checkPermission('packing_edit', user.role, trip.user_id, user.id, trip.user_id !== user.id); } broadcast(tripId: string, event: string, payload: Record, socketId: string | undefined): void { diff --git a/server/src/nest/permissions/permissions.bridge.ts b/server/src/nest/permissions/permissions.bridge.ts new file mode 100644 index 000000000..a07887939 --- /dev/null +++ b/server/src/nest/permissions/permissions.bridge.ts @@ -0,0 +1,50 @@ +import { db } from '../../db/database'; +import { DatabaseService } from '../database/database.service'; +import { PermissionsService } from './permissions.service'; +import type { PermissionLevel } from './permissions.service'; + +/** + * Non-Nest entry point for the permissions domain — for code running OUTSIDE + * the Nest container (the legacy MCP _shared.ts hasTripPermission helper and + * the legacy adminService/authService/backupService/collectionsService). + * Exports the legacy services/permissions names 1:1 so repointing a consumer + * is an import-path-only diff. Inside the container, inject PermissionsService + * instead. Delete this file when the last legacy consumer migrates. + * + * The permissions cache is module-scoped in permissions.service.ts, so this + * instance and the container singleton share one cache — invalidating through + * either flushes both (backupService restores depend on this). + * + * Module-level construction is safe: `db` is the reinitialize-proof Proxy onto + * the shared better-sqlite3 singleton. + */ +const permissions = new PermissionsService(new DatabaseService(db)); + +export { PERMISSION_ACTIONS } from './permissions.service'; +export type { PermissionLevel, PermissionAction } from './permissions.service'; + +export function invalidatePermissionsCache(): void { + permissions.invalidatePermissionsCache(); +} + +export function getPermissionLevel(actionKey: string): PermissionLevel { + return permissions.getPermissionLevel(actionKey); +} + +export function getAllPermissions(): Record { + return permissions.getAllPermissions(); +} + +export function savePermissions(settings: Record): { skipped: string[] } { + return permissions.savePermissions(settings); +} + +export function checkPermission( + actionKey: string, + userRole: string, + tripUserId: number | null, + userId: number, + isMember: boolean +): boolean { + return permissions.checkPermission(actionKey, userRole, tripUserId, userId, isMember); +} diff --git a/server/src/nest/permissions/permissions.module.ts b/server/src/nest/permissions/permissions.module.ts new file mode 100644 index 000000000..49e9f4046 --- /dev/null +++ b/server/src/nest/permissions/permissions.module.ts @@ -0,0 +1,13 @@ +import { Module } from '@nestjs/common'; +import { PermissionsService } from './permissions.service'; + +/** Cross-cutting permissions domain (Wave 2). No controller/MCP surface of its + * own — the admin HTTP surface stays with AdminModule. Exports + * PermissionsService for the 17 in-container consumers; deliberately NOT + * @Global so e2e TestingModules resolve it transitively through each + * consumer's explicit import. Registered in AppModule. */ +@Module({ + providers: [PermissionsService], + exports: [PermissionsService], +}) +export class PermissionsModule {} diff --git a/server/src/nest/permissions/permissions.service.ts b/server/src/nest/permissions/permissions.service.ts new file mode 100644 index 000000000..f59756af2 --- /dev/null +++ b/server/src/nest/permissions/permissions.service.ts @@ -0,0 +1,161 @@ +import { Injectable } from '@nestjs/common'; +import { DatabaseService } from '../database/database.service'; + +/** + * 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'; + +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])); + +// In-memory cache, invalidated on save. Deliberately MODULE-scoped, not +// instance state: two instances exist at runtime (the container singleton and +// the permissions.bridge instance for out-of-container consumers), and legacy +// callers — backupService invalidating after a restore, integration tests — +// must flush the same cache the request path reads. Both instances wrap the +// same shared connection, so a single cache is also the correct data shape. +let cache: Map | null = null; + +@Injectable() +export class PermissionsService { + constructor(private readonly dbs: DatabaseService) {} + + private loadPermissions(): Map { + if (cache) return cache; + cache = new Map(); + try { + const rows = this.dbs.all<{ key: string; value: string }>( + "SELECT key, value FROM app_settings WHERE key LIKE 'perm_%'" + ); + for (const row of rows) { + const actionKey = row.key.replace('perm_', ''); + if (ACTIONS_MAP.has(actionKey)) { + cache.set(actionKey, row.value as PermissionLevel); + } + } + } catch { /* table might not exist yet during init */ } + return cache; + } + + invalidatePermissionsCache(): void { + cache = null; + } + + getPermissionLevel(actionKey: string): PermissionLevel { + const perms = this.loadPermissions(); + const stored = perms.get(actionKey); + if (stored) return stored; + const action = ACTIONS_MAP.get(actionKey); + return action?.defaultLevel ?? 'trip_owner'; + } + + getAllPermissions(): Record { + const perms = this.loadPermissions(); + const result: Record = {}; + for (const action of PERMISSION_ACTIONS) { + result[action.key] = perms.get(action.key) ?? action.defaultLevel; + } + return result; + } + + savePermissions(settings: Record): { skipped: string[] } { + const skipped: string[] = []; + const upsert = this.dbs.prepare('INSERT OR REPLACE INTO app_settings (key, value) VALUES (?, ?)'); + this.dbs.transaction(() => { + for (const [actionKey, level] of Object.entries(settings)) { + const action = ACTIONS_MAP.get(actionKey); + if (!action || !action.allowedLevels.includes(level as PermissionLevel)) { + skipped.push(actionKey); + continue; + } + upsert.run(`perm_${actionKey}`, level); + } + }); + this.invalidatePermissionsCache(); + return { skipped }; + } + + /** + * Check if a user passes the permission check for a given action. + * + * @param actionKey - The permission action key + * @param userRole - 'admin' | 'user' + * @param tripUserId - The trip owner's user ID (null for non-trip actions like trip_create) + * @param userId - The requesting user's ID + * @param isMember - Whether the user is a trip member (not owner) + */ + checkPermission( + actionKey: string, + userRole: string, + tripUserId: number | null, + userId: number, + isMember: boolean + ): boolean { + // Admins always pass + if (userRole === 'admin') return true; + + const required = 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; + } + } +} diff --git a/server/src/nest/places/places.module.ts b/server/src/nest/places/places.module.ts index 60b84e1fe..143ab16cd 100644 --- a/server/src/nest/places/places.module.ts +++ b/server/src/nest/places/places.module.ts @@ -1,9 +1,11 @@ import { Module } from '@nestjs/common'; import { PlacesController } from './places.controller'; import { PlacesService } from './places.service'; +import { PermissionsModule } from '../permissions/permissions.module'; /** Places domain (S8 — Phase 2 trip sub-domain). Depends on L4 Categories + L5 Tags. */ @Module({ + imports: [PermissionsModule], controllers: [PlacesController], providers: [PlacesService], }) diff --git a/server/src/nest/places/places.service.ts b/server/src/nest/places/places.service.ts index d57e08636..e8f1fdb38 100644 --- a/server/src/nest/places/places.service.ts +++ b/server/src/nest/places/places.service.ts @@ -1,7 +1,7 @@ import { Injectable } from '@nestjs/common'; import { broadcast } from '../../websocket'; import { DatabaseService } from '../database/database.service'; -import { checkPermission } from '../../services/permissions'; +import { PermissionsService } from '../permissions/permissions.service'; import type { User } from '../../types'; import * as svc from '../../services/placeService'; import { onPlaceCreated, onPlaceUpdated, onPlaceDeleted } from '../../services/journeyService'; @@ -16,14 +16,17 @@ type Trip = { user_id: number }; */ @Injectable() export class PlacesService { - constructor(private readonly dbs: DatabaseService) {} + constructor( + private readonly dbs: DatabaseService, + private readonly permissions: PermissionsService, + ) {} verifyTripAccess(tripId: string, userId: number) { return this.dbs.canAccessTrip(Number(tripId), userId) as Trip | null | undefined; } canEdit(trip: Trip, user: User): boolean { - return checkPermission('place_edit', user.role, trip.user_id, user.id, trip.user_id !== user.id); + return this.permissions.checkPermission('place_edit', user.role, trip.user_id, user.id, trip.user_id !== user.id); } broadcast(tripId: string, event: string, payload: Record, socketId: string | undefined): void { diff --git a/server/src/nest/plugins/host/plugin-host-deps.factory.ts b/server/src/nest/plugins/host/plugin-host-deps.factory.ts index be2ba2269..abb3b3c53 100644 --- a/server/src/nest/plugins/host/plugin-host-deps.factory.ts +++ b/server/src/nest/plugins/host/plugin-host-deps.factory.ts @@ -14,7 +14,7 @@ import { PluginOAuthService } from '../plugin-oauth.service'; import fsMod from 'node:fs'; import pathMod from 'node:path'; import { randomUUID } from 'node:crypto'; -import { checkPermission } from '../../../services/permissions'; +import { PermissionsService } from '../../permissions/permissions.service'; import { listTrips, updateTrip, createTrip, removeMember as removeTripMemberSvc, NotFoundError, ValidationError } from '../../../services/tripService'; import { createPlace, updatePlace, deletePlace } from '../../../services/placeService'; import { isAddonEnabled } from '../../../services/adminService'; @@ -155,6 +155,7 @@ export class PluginHostDepsFactory { private readonly collab: CollabService, private readonly vacay: VacayService, private readonly days: DaysService, + private readonly permissions: PermissionsService, ) {} /** @@ -168,7 +169,7 @@ export class PluginHostDepsFactory { if (!trip) return false; const u = this.db.prepare('SELECT role FROM users WHERE id = ?').get(userId) as { role?: string } | undefined; if (!u) return false; - return checkPermission(action, u.role ?? 'user', trip.user_id, userId, trip.user_id !== userId); + return this.permissions.checkPermission(action, u.role ?? 'user', trip.user_id, userId, trip.user_id !== userId); } // The booking notification the REST controller sends after a create/update/delete @@ -217,7 +218,7 @@ export class PluginHostDepsFactory { if (!trip) return false; const u = this.db.prepare('SELECT role FROM users WHERE id = ?').get(userId) as { role?: string } | undefined; if (!u) return false; - return checkPermission('budget_edit', u.role ?? 'user', trip.user_id, userId, trip.user_id !== userId); + return this.permissions.checkPermission('budget_edit', u.role ?? 'user', trip.user_id, userId, trip.user_id !== userId); }, // --- Read scopes (packing/files). Membership is checked by the host (tripRead); // these just delegate to the same services the REST paths use. --- @@ -521,7 +522,7 @@ export class PluginHostDepsFactory { // is only visible to its owner, who refetches (same as the REST POST). --- canCreateTrip: (userId) => { const u = this.db.prepare('SELECT role FROM users WHERE id = ?').get(userId) as { role?: string } | undefined; - return checkPermission('trip_create', u?.role ?? 'user', null, userId, false); + return this.permissions.checkPermission('trip_create', u?.role ?? 'user', null, userId, false); }, createTripForUser: (userId, input) => { try { diff --git a/server/src/nest/plugins/plugins.module.ts b/server/src/nest/plugins/plugins.module.ts index 60b35665b..6d838b661 100644 --- a/server/src/nest/plugins/plugins.module.ts +++ b/server/src/nest/plugins/plugins.module.ts @@ -36,6 +36,7 @@ import { LlmParseModule } from '../llm-parse/llm-parse.module'; import { FilesModule } from '../files/files.module'; import { CollabModule } from '../collab/collab.module'; import { VacayModule } from '../vacay/vacay.module'; +import { PermissionsModule } from '../permissions/permissions.module'; /** * Plugin system (#plugins). M0 read side + M2 isolated runtime + M3 frontend: @@ -47,7 +48,7 @@ import { VacayModule } from '../vacay/vacay.module'; @Module({ // The DI-native domain services the plugin host wiring injects // (PluginHostDepsFactory); DatabaseModule is @Global, so not listed. - imports: [TagsModule, CategoriesModule, BudgetModule, ReservationsModule, TodoModule, PackingModule, DaysModule, AssignmentsModule, LlmParseModule, FilesModule, CollabModule, VacayModule], + imports: [TagsModule, CategoriesModule, BudgetModule, ReservationsModule, TodoModule, PackingModule, DaysModule, AssignmentsModule, LlmParseModule, FilesModule, CollabModule, VacayModule, PermissionsModule], controllers: [PluginsController, PluginsFeedController, PluginsProxyController, PluginFrameController, PlaceDetailsController, TripWarningsController, ViewContributionsController, TripCardContributionsController, PluginPhotosController, PluginCalendarController, MapMarkersController, MapLayersController, PluginRoutesController, DayScheduleController, PdfSectionsController, AtlasLayersController, JournalEntryRowsController, PluginUserSettingsController, PluginOAuthController, PluginActivityController], providers: [PluginsService, PluginRuntimeService, PluginRegistryService, PluginOAuthService, PluginHostDepsFactory], // Exported so the admin addon-toggle handler can cascade-disable plugins whose diff --git a/server/src/nest/reservations/accommodations.service.ts b/server/src/nest/reservations/accommodations.service.ts index 23ce6f393..ff09b5b02 100644 --- a/server/src/nest/reservations/accommodations.service.ts +++ b/server/src/nest/reservations/accommodations.service.ts @@ -1,7 +1,7 @@ import { Injectable } from '@nestjs/common'; import { broadcast } from '../../websocket'; import { DatabaseService } from '../database/database.service'; -import { checkPermission } from '../../services/permissions'; +import { PermissionsService } from '../permissions/permissions.service'; import type { User } from '../../types'; import { DaysService } from '../days/days.service'; @@ -18,6 +18,7 @@ export class AccommodationsService { constructor( private readonly dbs: DatabaseService, private readonly days: DaysService, + private readonly permissions: PermissionsService, ) {} /** Mirrors the requireTripAccess middleware (owner or member), returning the trip. */ @@ -26,7 +27,7 @@ export class AccommodationsService { } canEdit(trip: Trip, user: User): boolean { - return checkPermission('day_edit', user.role, trip.user_id, user.id, trip.user_id !== user.id); + return this.permissions.checkPermission('day_edit', user.role, trip.user_id, user.id, trip.user_id !== user.id); } broadcast(tripId: string, event: string, payload: Record, socketId: string | undefined): void { diff --git a/server/src/nest/reservations/reservations.bridge.ts b/server/src/nest/reservations/reservations.bridge.ts index 7357cd8e0..b902fb600 100644 --- a/server/src/nest/reservations/reservations.bridge.ts +++ b/server/src/nest/reservations/reservations.bridge.ts @@ -1,6 +1,7 @@ import { db } from '../../db/database'; import { DatabaseService } from '../database/database.service'; import { ReservationsService } from './reservations.service'; +import { PermissionsService } from '../permissions/permissions.service'; export type { EndpointInput, ReservationEndpoint, ReservationTraveler } from './reservations.service'; @@ -18,7 +19,7 @@ export type { EndpointInput, ReservationEndpoint, ReservationTraveler } from './ * Module-level construction is safe: `db` is the reinitialize-proof Proxy onto * the shared better-sqlite3 singleton. */ -const reservations = new ReservationsService(new DatabaseService(db)); +const reservations = new ReservationsService(new DatabaseService(db), new PermissionsService(new DatabaseService(db))); export function listReservations(tripId: string | number) { return reservations.list(tripId); diff --git a/server/src/nest/reservations/reservations.module.ts b/server/src/nest/reservations/reservations.module.ts index 8814b8f00..10f728eeb 100644 --- a/server/src/nest/reservations/reservations.module.ts +++ b/server/src/nest/reservations/reservations.module.ts @@ -1,5 +1,6 @@ import { Module } from '@nestjs/common'; import { DaysModule } from '../days/days.module'; +import { PermissionsModule } from '../permissions/permissions.module'; import { ReservationsController } from './reservations.controller'; import { ReservationsService } from './reservations.service'; import { ReservationsMcp } from './reservations.mcp'; @@ -15,7 +16,7 @@ import { UpcomingReservationsController } from './upcoming-reservations.controll */ @Module({ // DaysModule: AccommodationsService + ReservationsMcp inject DaysService. - imports: [DaysModule], + imports: [DaysModule, PermissionsModule], controllers: [ReservationsController, AccommodationsController, UpcomingReservationsController], providers: [ReservationsService, AccommodationsService, ReservationsMcp], // For in-container consumers (PluginHostDepsFactory, TripsService, BookingImportService). diff --git a/server/src/nest/reservations/reservations.service.ts b/server/src/nest/reservations/reservations.service.ts index d85ee8cca..7591fb792 100644 --- a/server/src/nest/reservations/reservations.service.ts +++ b/server/src/nest/reservations/reservations.service.ts @@ -1,7 +1,7 @@ import { Injectable } from '@nestjs/common'; import { DatabaseService } from '../database/database.service'; import { broadcast } from '../../websocket'; -import { checkPermission } from '../../services/permissions'; +import { PermissionsService } from '../permissions/permissions.service'; import { verifyTripAccess } from '../../services/tripAccess'; import { avatarUrl } from '../../services/avatarUrl'; import type { Reservation, User } from '../../types'; @@ -129,14 +129,17 @@ type AccommodationTimesMeta = { */ @Injectable() export class ReservationsService { - constructor(private readonly db: DatabaseService) {} + constructor( + private readonly db: DatabaseService, + private readonly permissions: PermissionsService, + ) {} verifyTripAccess(tripId: string | number, userId: number) { return verifyTripAccess(tripId, userId); } canEdit(trip: Trip, user: User): boolean { - return checkPermission('reservation_edit', user.role, trip.user_id, user.id, trip.user_id !== user.id); + return this.permissions.checkPermission('reservation_edit', user.role, trip.user_id, user.id, trip.user_id !== user.id); } broadcast(tripId: string, event: string, payload: Record, socketId: string | undefined): void { diff --git a/server/src/nest/share/share.bridge.ts b/server/src/nest/share/share.bridge.ts index 0c4b2e4f1..538bb94aa 100644 --- a/server/src/nest/share/share.bridge.ts +++ b/server/src/nest/share/share.bridge.ts @@ -1,6 +1,7 @@ import { db } from '../../db/database'; import { DatabaseService } from '../database/database.service'; import { ShareService } from './share.service'; +import { PermissionsService } from '../permissions/permissions.service'; import type { SharePermissions } from './share.service'; import { SettingsService } from '../settings/settings.service'; @@ -17,7 +18,7 @@ import { SettingsService } from '../settings/settings.service'; * Module-level construction is safe: `db` is the reinitialize-proof Proxy onto * the shared better-sqlite3 singleton. */ -const share = new ShareService(new DatabaseService(db), new SettingsService(new DatabaseService(db))); +const share = new ShareService(new DatabaseService(db), new SettingsService(new DatabaseService(db)), new PermissionsService(new DatabaseService(db))); export function createOrUpdateShareLink(tripId: string, createdBy: number, permissions: SharePermissions) { return share.createOrUpdate(tripId, createdBy, permissions); diff --git a/server/src/nest/share/share.module.ts b/server/src/nest/share/share.module.ts index f1a47dc64..38d6cedb7 100644 --- a/server/src/nest/share/share.module.ts +++ b/server/src/nest/share/share.module.ts @@ -2,9 +2,10 @@ import { Module } from '@nestjs/common'; import { TripShareController, SharedController } from './share.controller'; import { ShareService } from './share.service'; import { SettingsModule } from '../settings/settings.module'; +import { PermissionsModule } from '../permissions/permissions.module'; @Module({ - imports: [SettingsModule], + imports: [SettingsModule, PermissionsModule], controllers: [TripShareController, SharedController], providers: [ShareService], }) diff --git a/server/src/nest/share/share.service.ts b/server/src/nest/share/share.service.ts index 126810136..fa5a92d05 100644 --- a/server/src/nest/share/share.service.ts +++ b/server/src/nest/share/share.service.ts @@ -2,7 +2,7 @@ import { Injectable } from '@nestjs/common'; import crypto from 'crypto'; import { DatabaseService } from '../database/database.service'; import type { TripAccess } from '../database/database.service'; -import { checkPermission } from '../../services/permissions'; +import { PermissionsService } from '../permissions/permissions.service'; import { loadTagsByPlaceIds } from '../../services/queryHelpers'; import { serveFilePath } from '../../services/placePhotoCache'; import { SettingsService } from '../settings/settings.service'; @@ -55,6 +55,7 @@ export class ShareService { constructor( private readonly dbs: DatabaseService, private readonly settings: SettingsService, + private readonly permissions: PermissionsService, ) {} verifyTripAccess(tripId: string, userId: number) { @@ -62,7 +63,7 @@ export class ShareService { } canManage(trip: Trip, user: User): boolean { - return checkPermission('share_manage', user.role, trip.user_id, user.id, trip.user_id !== user.id); + return this.permissions.checkPermission('share_manage', user.role, trip.user_id, user.id, trip.user_id !== user.id); } /** diff --git a/server/src/nest/todo/todo.bridge.ts b/server/src/nest/todo/todo.bridge.ts index 63da985c0..f38653188 100644 --- a/server/src/nest/todo/todo.bridge.ts +++ b/server/src/nest/todo/todo.bridge.ts @@ -1,6 +1,7 @@ import { db } from '../../db/database'; import { DatabaseService } from '../database/database.service'; import { TodoService } from './todo.service'; +import { PermissionsService } from '../permissions/permissions.service'; /** * Non-Nest entry point for the todo domain — for code running OUTSIDE the @@ -15,7 +16,7 @@ import { TodoService } from './todo.service'; * Module-level construction is safe: `db` is the reinitialize-proof Proxy onto * the shared better-sqlite3 singleton. */ -const todos = new TodoService(new DatabaseService(db)); +const todos = new TodoService(new DatabaseService(db), new PermissionsService(new DatabaseService(db))); export function listItems(tripId: string | number) { return todos.listItems(tripId); diff --git a/server/src/nest/todo/todo.module.ts b/server/src/nest/todo/todo.module.ts index 21c23353c..be7d76f57 100644 --- a/server/src/nest/todo/todo.module.ts +++ b/server/src/nest/todo/todo.module.ts @@ -2,10 +2,12 @@ import { Module } from '@nestjs/common'; import { TodoController } from './todo.controller'; import { TodoMcp } from './todo.mcp'; import { TodoService } from './todo.service'; +import { PermissionsModule } from '../permissions/permissions.module'; /** To-do domain (S3 — Phase 2 trip sub-domain). Registered in AppModule. * Exports TodoService for in-container consumers (TripsService bundle). */ @Module({ + imports: [PermissionsModule], controllers: [TodoController], providers: [TodoService, TodoMcp], exports: [TodoService], diff --git a/server/src/nest/todo/todo.service.ts b/server/src/nest/todo/todo.service.ts index 05357a762..0985c5b1d 100644 --- a/server/src/nest/todo/todo.service.ts +++ b/server/src/nest/todo/todo.service.ts @@ -1,6 +1,6 @@ import { Injectable } from '@nestjs/common'; import { broadcast } from '../../websocket'; -import { checkPermission } from '../../services/permissions'; +import { PermissionsService } from '../permissions/permissions.service'; import { verifyTripAccess } from '../../services/tripAccess'; import type { User } from '../../types'; import { DatabaseService } from '../database/database.service'; @@ -18,14 +18,17 @@ type Trip = NonNullable>; */ @Injectable() export class TodoService { - constructor(private readonly db: DatabaseService) {} + constructor( + private readonly db: DatabaseService, + private readonly permissions: PermissionsService, + ) {} verifyTripAccess(tripId: string | number, userId: number) { return verifyTripAccess(tripId, userId); } canEdit(trip: Trip, user: User): boolean { - return checkPermission('packing_edit', user.role, trip.user_id, user.id, trip.user_id !== user.id); + return this.permissions.checkPermission('packing_edit', user.role, trip.user_id, user.id, trip.user_id !== user.id); } broadcast(tripId: string, event: string, payload: Record, socketId: string | undefined): void { diff --git a/server/src/nest/trip-invite/trip-invite.module.ts b/server/src/nest/trip-invite/trip-invite.module.ts index 285e1bacf..fcc55bce2 100644 --- a/server/src/nest/trip-invite/trip-invite.module.ts +++ b/server/src/nest/trip-invite/trip-invite.module.ts @@ -2,8 +2,10 @@ import { Module } from '@nestjs/common'; import { TripInviteLinkController, TripInviteController } from './trip-invite.controller'; import { TripInviteService } from './trip-invite.service'; import { RateLimitService } from '../auth/rate-limit.service'; +import { PermissionsModule } from '../permissions/permissions.module'; @Module({ + imports: [PermissionsModule], controllers: [TripInviteLinkController, TripInviteController], providers: [TripInviteService, RateLimitService], }) diff --git a/server/src/nest/trip-invite/trip-invite.service.ts b/server/src/nest/trip-invite/trip-invite.service.ts index 7c13244c6..547804453 100644 --- a/server/src/nest/trip-invite/trip-invite.service.ts +++ b/server/src/nest/trip-invite/trip-invite.service.ts @@ -2,7 +2,7 @@ import { Injectable } from '@nestjs/common'; import crypto from 'crypto'; import { DatabaseService } from '../database/database.service'; import type { TripAccess } from '../database/database.service'; -import { checkPermission } from '../../services/permissions'; +import { PermissionsService } from '../permissions/permissions.service'; import { joinTripAsMember } from '../../services/tripMembership'; import type { User } from '../../types'; @@ -28,14 +28,17 @@ export interface TripInviteInfo { */ @Injectable() export class TripInviteService { - constructor(private readonly dbs: DatabaseService) {} + constructor( + private readonly dbs: DatabaseService, + private readonly permissions: PermissionsService, + ) {} verifyTripAccess(tripId: string, userId: number) { return this.dbs.canAccessTrip(tripId, userId); } canManage(trip: Trip, user: User): boolean { - return checkPermission('share_manage', user.role, trip.user_id, user.id, trip.user_id !== user.id); + return this.permissions.checkPermission('share_manage', user.role, trip.user_id, user.id, trip.user_id !== user.id); } /** The current invite link for a trip, or null if none exists. */ diff --git a/server/src/nest/trips/trips.module.ts b/server/src/nest/trips/trips.module.ts index 0d1da19b0..da1c2da59 100644 --- a/server/src/nest/trips/trips.module.ts +++ b/server/src/nest/trips/trips.module.ts @@ -4,13 +4,14 @@ import { FilesModule } from '../files/files.module'; import { PackingModule } from '../packing/packing.module'; import { ReservationsModule } from '../reservations/reservations.module'; import { TodoModule } from '../todo/todo.module'; +import { PermissionsModule } from '../permissions/permissions.module'; import { TripsController } from './trips.controller'; import { TripsService } from './trips.service'; /** Trips aggregate root (C1 — Phase 3). Uses exact strangler prefixes so it does * not capture the nested sub-domain mounts (collab, files, ...). */ @Module({ - imports: [TodoModule, PackingModule, FilesModule, ReservationsModule, DaysModule], + imports: [TodoModule, PackingModule, FilesModule, ReservationsModule, DaysModule, PermissionsModule], controllers: [TripsController], providers: [TripsService], }) diff --git a/server/src/nest/trips/trips.service.ts b/server/src/nest/trips/trips.service.ts index 24ad68853..5fb720889 100644 --- a/server/src/nest/trips/trips.service.ts +++ b/server/src/nest/trips/trips.service.ts @@ -1,7 +1,7 @@ import { Injectable } from '@nestjs/common'; import { DatabaseService } from '../database/database.service'; import { broadcast } from '../../websocket'; -import { checkPermission } from '../../services/permissions'; +import { PermissionsService } from '../permissions/permissions.service'; import type { User } from '../../types'; import * as tripSvc from '../../services/tripService'; import { listPlaces } from '../../services/placeService'; @@ -29,6 +29,7 @@ export class TripsService { private readonly files: FilesService, private readonly reservations: ReservationsService, private readonly days: DaysService, + private readonly permissions: PermissionsService, ) {} private get db() { @@ -40,7 +41,7 @@ export class TripsService { } can(action: string, role: string, ownerId: number | null, userId: number, isMember: boolean): boolean { - return checkPermission(action, role, ownerId, userId, isMember); + return this.permissions.checkPermission(action, role, ownerId, userId, isMember); } broadcast(tripId: string, event: string, payload: Record, socketId: string | undefined): void { diff --git a/server/src/services/adminService.ts b/server/src/services/adminService.ts index 92b16807a..d1801cd8b 100644 --- a/server/src/services/adminService.ts +++ b/server/src/services/adminService.ts @@ -15,7 +15,7 @@ import { prepareLlmAddonConfigForWrite, maskLlmAddonConfig } from './llmConfig'; import { getPhotoProviderConfig } from './memories/helpersService'; import { send as sendNotification } from './notificationService'; import { validatePassword } from './passwordPolicy'; -import { getAllPermissions, savePermissions as savePerms, PERMISSION_ACTIONS } from './permissions'; +import { getAllPermissions, savePermissions as savePerms, PERMISSION_ACTIONS } from '../nest/permissions/permissions.bridge'; import { deleteUserCompletely } from './userCleanupService'; import bcrypt from 'bcryptjs'; diff --git a/server/src/services/authService.ts b/server/src/services/authService.ts index 10a2e79d4..9289e7306 100644 --- a/server/src/services/authService.ts +++ b/server/src/services/authService.ts @@ -11,7 +11,7 @@ import { db } from '../db/database'; import { JWT_SECRET, SESSION_DURATION_SECONDS, SESSION_DURATION_REMEMBER_SECONDS } from '../config'; import { validatePassword } from './passwordPolicy'; import { encryptMfaSecret, decryptMfaSecret } from './mfaCrypto'; -import { getAllPermissions } from './permissions'; +import { getAllPermissions } from '../nest/permissions/permissions.bridge'; import { decrypt_api_key, maybe_encrypt_api_key, encrypt_api_key } from './apiKeyCrypto'; import { createEphemeralToken } from './ephemeralTokens'; // Import from sessionManager directly, NOT the ../mcp barrel: the barrel pulls diff --git a/server/src/services/backupService.ts b/server/src/services/backupService.ts index e94791e50..6c0c66db6 100644 --- a/server/src/services/backupService.ts +++ b/server/src/services/backupService.ts @@ -6,7 +6,7 @@ import fs from 'fs'; import Database from 'better-sqlite3'; import { db, closeDb, reinitialize } from '../db/database'; import * as scheduler from '../scheduler'; -import { invalidatePermissionsCache } from './permissions'; +import { invalidatePermissionsCache } from '../nest/permissions/permissions.bridge'; import { pluginsCodeRoot, pluginsDataRoot } from '../nest/plugins/paths'; import { stageExtractedPluginTrees, applyStagedRestoreNow } from '../nest/plugins/plugin-backup'; import { snapshotAllPluginDataDbs } from '../nest/plugins/host/plugin-data.service'; diff --git a/server/src/services/collectionsService.ts b/server/src/services/collectionsService.ts index 3eab85a24..326e7841d 100644 --- a/server/src/services/collectionsService.ts +++ b/server/src/services/collectionsService.ts @@ -2,7 +2,7 @@ import fs from 'fs'; import path from 'path'; import { db, canAccessTrip } from '../db/database'; import { broadcastToUser } from '../websocket'; -import { checkPermission } from './permissions'; +import { checkPermission } from '../nest/permissions/permissions.bridge'; import { reclaimPlaceImage } from './placeImage'; import type { Collection, diff --git a/server/src/services/permissions.ts b/server/src/services/permissions.ts deleted file mode 100644 index 2a0fdddd8..000000000 --- a/server/src/services/permissions.ts +++ /dev/null @@ -1,149 +0,0 @@ -import { db } from '../db/database'; - -/** - * 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'; - -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])); - -// In-memory cache, invalidated on save -let cache: Map | null = null; - -function loadPermissions(): Map { - if (cache) return cache; - cache = new Map(); - try { - const rows = db.prepare("SELECT key, value FROM app_settings WHERE key LIKE 'perm_%'").all() as { key: string; value: string }[]; - for (const row of rows) { - const actionKey = row.key.replace('perm_', ''); - if (ACTIONS_MAP.has(actionKey)) { - cache.set(actionKey, row.value as PermissionLevel); - } - } - } catch { /* table might not exist yet during init */ } - return cache; -} - -export function invalidatePermissionsCache(): void { - cache = null; -} - -export function getPermissionLevel(actionKey: string): PermissionLevel { - const perms = loadPermissions(); - const stored = perms.get(actionKey); - if (stored) return stored; - const action = ACTIONS_MAP.get(actionKey); - return action?.defaultLevel ?? 'trip_owner'; -} - -export function getAllPermissions(): Record { - const perms = loadPermissions(); - const result: Record = {}; - for (const action of PERMISSION_ACTIONS) { - result[action.key] = perms.get(action.key) ?? action.defaultLevel; - } - return result; -} - -export function savePermissions(settings: Record): { skipped: string[] } { - const skipped: string[] = []; - const upsert = db.prepare("INSERT OR REPLACE INTO app_settings (key, value) VALUES (?, ?)"); - const txn = db.transaction(() => { - for (const [actionKey, level] of Object.entries(settings)) { - const action = ACTIONS_MAP.get(actionKey); - if (!action || !action.allowedLevels.includes(level as PermissionLevel)) { - skipped.push(actionKey); - continue; - } - upsert.run(`perm_${actionKey}`, level); - } - }); - txn(); - invalidatePermissionsCache(); - return { skipped }; -} - -/** - * Check if a user passes the permission check for a given action. - * - * @param actionKey - The permission action key - * @param userRole - 'admin' | 'user' - * @param tripUserId - The trip owner's user ID (null for non-trip actions like trip_create) - * @param userId - The requesting user's ID - * @param isMember - Whether the user is a trip member (not owner) - */ -export function checkPermission( - actionKey: string, - userRole: string, - tripUserId: number | null, - userId: number, - isMember: boolean -): boolean { - // Admins always pass - if (userRole === 'admin') return true; - - const required = 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; - } -} diff --git a/server/tests/e2e/assignments.e2e.test.ts b/server/tests/e2e/assignments.e2e.test.ts index d725c2207..5061c4a84 100644 --- a/server/tests/e2e/assignments.e2e.test.ts +++ b/server/tests/e2e/assignments.e2e.test.ts @@ -4,7 +4,7 @@ * (DI-injected, no service mock); journeyService, the permission check, * canAccessTrip and the WebSocket broadcast stay mocked. */ -import { describe, it, expect, beforeAll, afterAll, beforeEach, vi } from 'vitest'; +import { describe, it, expect, beforeAll, afterAll, beforeEach, vi, type MockInstance } from 'vitest'; import request from 'supertest'; import cookieParser from 'cookie-parser'; import type { Server } from 'http'; @@ -49,8 +49,11 @@ vi.mock('../../src/websocket', () => ({ broadcast: vi.fn() })); const { reconcileTripSkeletons } = vi.hoisted(() => ({ reconcileTripSkeletons: vi.fn() })); vi.mock('../../src/services/journeyService', () => ({ reconcileTripSkeletons })); -const { checkPermission } = vi.hoisted(() => ({ checkPermission: vi.fn() })); -vi.mock('../../src/services/permissions', () => ({ checkPermission })); +import { PermissionsService } from '../../src/nest/permissions/permissions.service'; + +// Since the permissions DI migration, the check is a spy on the container's +// PermissionsService singleton (created in beforeAll, after build()). +let checkPermission: MockInstance; import { AssignmentsModule } from '../../src/nest/assignments/assignments.module'; import { TrekExceptionFilter } from '../../src/nest/common/trek-exception.filter'; @@ -76,6 +79,7 @@ describe('Assignments e2e (real auth guard + temp SQLite)', () => { db.prepare('INSERT INTO days (id, trip_id) VALUES (3, 5), (4, 5)').run(); db.prepare('INSERT INTO places (id, trip_id, name) VALUES (2, 5, ?)').run('Louvre'); app = await build(); + checkPermission = vi.spyOn(app.get(PermissionsService), 'checkPermission'); server = app.getHttpServer(); }); diff --git a/server/tests/e2e/budget.e2e.test.ts b/server/tests/e2e/budget.e2e.test.ts index 393b65f27..6651e4d26 100644 --- a/server/tests/e2e/budget.e2e.test.ts +++ b/server/tests/e2e/budget.e2e.test.ts @@ -3,7 +3,7 @@ * through the real JwtAuthGuard against a temp SQLite db. budgetService, the * permission check and the WebSocket broadcast are mocked. */ -import { describe, it, expect, beforeAll, afterAll, beforeEach, vi } from 'vitest'; +import { describe, it, expect, beforeAll, afterAll, beforeEach, vi, type MockInstance } from 'vitest'; import request from 'supertest'; import cookieParser from 'cookie-parser'; import type { Server } from 'http'; @@ -24,8 +24,11 @@ const { db } = vi.hoisted(() => { vi.mock('../../src/db/database', () => ({ db, closeDb: () => {}, reinitialize: () => {} })); vi.mock('../../src/websocket', () => ({ broadcast: vi.fn() })); -const { checkPermission } = vi.hoisted(() => ({ checkPermission: vi.fn() })); -vi.mock('../../src/services/permissions', () => ({ checkPermission })); +import { PermissionsService } from '../../src/nest/permissions/permissions.service'; + +// Since the permissions DI migration, the check is a spy on the container's +// PermissionsService singleton (created in beforeAll, after build()). +let checkPermission: MockInstance; const { svc } = vi.hoisted(() => ({ svc: { @@ -56,6 +59,7 @@ describe('Budget e2e (real auth guard + temp SQLite)', () => { beforeAll(async () => { seedUser(db as never, { id: 1 }); app = await build(); + checkPermission = vi.spyOn(app.get(PermissionsService), 'checkPermission'); server = app.getHttpServer(); svc.listBudgetItems.mockReturnValue([{ id: 1, name: 'Hotel' }]); svc.createBudgetItem.mockReturnValue({ id: 9, name: 'Hotel' }); diff --git a/server/tests/e2e/collab.e2e.test.ts b/server/tests/e2e/collab.e2e.test.ts index 79b7f3965..2dc10e73f 100644 --- a/server/tests/e2e/collab.e2e.test.ts +++ b/server/tests/e2e/collab.e2e.test.ts @@ -6,7 +6,7 @@ * on auth, trip-access 404, permission 403, the create-201 status codes, the * vote/react 200 overrides and the persisted rows. */ -import { describe, it, expect, beforeAll, afterAll, beforeEach, vi } from 'vitest'; +import { describe, it, expect, beforeAll, afterAll, beforeEach, vi, type MockInstance } from 'vitest'; import request from 'supertest'; import cookieParser from 'cookie-parser'; import type { Server } from 'http'; @@ -59,8 +59,11 @@ vi.mock('../../src/db/database', () => ({ vi.mock('../../src/websocket', () => ({ broadcast: vi.fn() })); vi.mock('../../src/services/notificationService', () => ({ send: vi.fn().mockResolvedValue(undefined) })); -const { checkPermission } = vi.hoisted(() => ({ checkPermission: vi.fn() })); -vi.mock('../../src/services/permissions', () => ({ checkPermission })); +import { PermissionsService } from '../../src/nest/permissions/permissions.service'; + +// Since the permissions DI migration, the check is a spy on the container's +// PermissionsService singleton (created in beforeAll, after build()). +let checkPermission: MockInstance; import { CollabModule } from '../../src/nest/collab/collab.module'; import { TrekExceptionFilter } from '../../src/nest/common/trek-exception.filter'; @@ -81,6 +84,7 @@ describe('Collab e2e (real auth guard + temp SQLite)', () => { beforeAll(async () => { seedUser(db as never, { id: 1 }); app = await build(); + checkPermission = vi.spyOn(app.get(PermissionsService), 'checkPermission'); server = app.getHttpServer(); }); diff --git a/server/tests/e2e/days.e2e.test.ts b/server/tests/e2e/days.e2e.test.ts index e8ebe60a9..fb4b3c994 100644 --- a/server/tests/e2e/days.e2e.test.ts +++ b/server/tests/e2e/days.e2e.test.ts @@ -6,7 +6,7 @@ * canAccessTrip over the temp db. Only the permission check and the WebSocket * broadcast stay mocked. */ -import { describe, it, expect, beforeAll, afterAll, beforeEach, vi } from 'vitest'; +import { describe, it, expect, beforeAll, afterAll, beforeEach, vi, type MockInstance } from 'vitest'; import request from 'supertest'; import cookieParser from 'cookie-parser'; import type { Server } from 'http'; @@ -73,8 +73,11 @@ vi.mock('../../src/db/database', () => ({ })); vi.mock('../../src/websocket', () => ({ broadcast: vi.fn() })); -const { checkPermission } = vi.hoisted(() => ({ checkPermission: vi.fn() })); -vi.mock('../../src/services/permissions', () => ({ checkPermission })); +import { PermissionsService } from '../../src/nest/permissions/permissions.service'; + +// Since the permissions DI migration, the check is a spy on the container's +// PermissionsService singleton (created in beforeAll, after build()). +let checkPermission: MockInstance; import { DaysModule } from '../../src/nest/days/days.module'; import { TrekExceptionFilter } from '../../src/nest/common/trek-exception.filter'; @@ -99,6 +102,7 @@ describe('Days + day-notes e2e (real auth guard + temp SQLite, real day SQL)', ( db.prepare('INSERT INTO trips (id, user_id, title) VALUES (5, 1, ?)').run('Trip'); db.prepare('INSERT INTO days (id, trip_id, day_number) VALUES (3, 5, 1)').run(); app = await build(); + checkPermission = vi.spyOn(app.get(PermissionsService), 'checkPermission'); server = app.getHttpServer(); }); diff --git a/server/tests/e2e/files.e2e.test.ts b/server/tests/e2e/files.e2e.test.ts index 614193b13..c5d91b34d 100644 --- a/server/tests/e2e/files.e2e.test.ts +++ b/server/tests/e2e/files.e2e.test.ts @@ -7,7 +7,7 @@ * unguarded download's own token auth), trip-access 404, permission 403, the * photo id/access guards and status codes. */ -import { describe, it, expect, beforeAll, afterAll, beforeEach, vi } from 'vitest'; +import { describe, it, expect, beforeAll, afterAll, beforeEach, vi, type MockInstance } from 'vitest'; import request from 'supertest'; import cookieParser from 'cookie-parser'; import type { Server } from 'http'; @@ -47,8 +47,11 @@ vi.mock('../../src/db/database', () => ({ vi.mock('../../src/websocket', () => ({ broadcast: vi.fn() })); vi.mock('../../src/services/demo', () => ({ isDemoEmail: vi.fn(() => false) })); -const { checkPermission } = vi.hoisted(() => ({ checkPermission: vi.fn() })); -vi.mock('../../src/services/permissions', () => ({ checkPermission })); +import { PermissionsService } from '../../src/nest/permissions/permissions.service'; + +// Since the permissions DI migration, the check is a spy on the container's +// PermissionsService singleton (created in beforeAll, after build()). +let checkPermission: MockInstance; const { photoSvc, helperSvc } = vi.hoisted(() => ({ photoSvc: { streamPhoto: vi.fn(), getPhotoInfo: vi.fn(), resolveTrekPhoto: vi.fn() }, @@ -81,6 +84,7 @@ describe('Files + photos e2e (real auth guard + temp SQLite)', () => { db.prepare("INSERT INTO trip_files (id, trip_id, filename, original_name, uploaded_by) VALUES (1, 5, 'stored-a.pdf', 'a.pdf', 1)").run(); db.prepare("INSERT INTO trip_files (id, trip_id, filename, original_name, uploaded_by, starred) VALUES (9, 5, 'stored-b.pdf', 'b.pdf', 1, 0)").run(); app = await build(); + checkPermission = vi.spyOn(app.get(PermissionsService), 'checkPermission'); server = app.getHttpServer(); }); diff --git a/server/tests/e2e/packing.e2e.test.ts b/server/tests/e2e/packing.e2e.test.ts index ec6346301..80d1cb580 100644 --- a/server/tests/e2e/packing.e2e.test.ts +++ b/server/tests/e2e/packing.e2e.test.ts @@ -6,7 +6,7 @@ * real-SQL canAccessTrip over the temp db. Only the permission check, the * WebSocket broadcast and the notification sender stay mocked. */ -import { describe, it, expect, beforeAll, afterAll, beforeEach, vi } from 'vitest'; +import { describe, it, expect, beforeAll, afterAll, beforeEach, vi, type MockInstance } from 'vitest'; import request from 'supertest'; import cookieParser from 'cookie-parser'; import type { Server } from 'http'; @@ -112,8 +112,11 @@ vi.mock('../../src/db/database', () => ({ vi.mock('../../src/websocket', () => ({ broadcast: vi.fn() })); vi.mock('../../src/services/notificationService', () => ({ send: vi.fn().mockResolvedValue(undefined) })); -const { checkPermission } = vi.hoisted(() => ({ checkPermission: vi.fn() })); -vi.mock('../../src/services/permissions', () => ({ checkPermission })); +import { PermissionsService } from '../../src/nest/permissions/permissions.service'; + +// Since the permissions DI migration, the check is a spy on the container's +// PermissionsService singleton (created in beforeAll, after build()). +let checkPermission: MockInstance; import { PackingModule } from '../../src/nest/packing/packing.module'; import { DatabaseModule } from '../../src/nest/database/database.module'; @@ -149,6 +152,7 @@ describe('Packing e2e (real auth guard + real SQL over temp SQLite)', () => { seedUser(db as never, { id: 2, email: 'stranger@example.test' }); seedUser(db as never, { id: 3, email: 'admin@example.test', role: 'admin' }); app = await build(); + checkPermission = vi.spyOn(app.get(PermissionsService), 'checkPermission'); server = app.getHttpServer(); }); diff --git a/server/tests/e2e/places.e2e.test.ts b/server/tests/e2e/places.e2e.test.ts index 2ce10f28a..f6a6fe18c 100644 --- a/server/tests/e2e/places.e2e.test.ts +++ b/server/tests/e2e/places.e2e.test.ts @@ -4,7 +4,7 @@ * journeyService, the permission check, canAccessTrip and the WebSocket * broadcast are mocked. */ -import { describe, it, expect, beforeAll, afterAll, beforeEach, vi } from 'vitest'; +import { describe, it, expect, beforeAll, afterAll, beforeEach, vi, type MockInstance } from 'vitest'; import request from 'supertest'; import cookieParser from 'cookie-parser'; import type { Server } from 'http'; @@ -29,8 +29,11 @@ vi.mock('../../src/db/database', () => ({ vi.mock('../../src/websocket', () => ({ broadcast: vi.fn() })); vi.mock('../../src/services/journeyService', () => ({ onPlaceCreated: vi.fn(), onPlaceUpdated: vi.fn(), onPlaceDeleted: vi.fn() })); -const { checkPermission } = vi.hoisted(() => ({ checkPermission: vi.fn() })); -vi.mock('../../src/services/permissions', () => ({ checkPermission })); +import { PermissionsService } from '../../src/nest/permissions/permissions.service'; + +// Since the permissions DI migration, the check is a spy on the container's +// PermissionsService singleton (created in beforeAll, after build()). +let checkPermission: MockInstance; const { pl } = vi.hoisted(() => ({ pl: { @@ -64,6 +67,7 @@ describe('Places e2e (real auth guard + temp SQLite)', () => { beforeAll(async () => { seedUser(db as never, { id: 1 }); app = await build(); + checkPermission = vi.spyOn(app.get(PermissionsService), 'checkPermission'); server = app.getHttpServer(); pl.listPlaces.mockReturnValue([{ id: 1, name: 'Spot' }]); pl.createPlace.mockReturnValue({ id: 9, name: 'Spot' }); diff --git a/server/tests/e2e/reservations.e2e.test.ts b/server/tests/e2e/reservations.e2e.test.ts index fa5f4af8a..62dfa73ad 100644 --- a/server/tests/e2e/reservations.e2e.test.ts +++ b/server/tests/e2e/reservations.e2e.test.ts @@ -16,7 +16,7 @@ import { Test } from '@nestjs/testing'; import cookieParser from 'cookie-parser'; import type { Server } from 'http'; import request from 'supertest'; -import { describe, it, expect, beforeAll, afterAll, beforeEach, vi } from 'vitest'; +import { describe, it, expect, beforeAll, afterAll, beforeEach, vi, type MockInstance } from 'vitest'; const { db } = vi.hoisted(() => { // eslint-disable-next-line @typescript-eslint/no-require-imports @@ -39,8 +39,11 @@ vi.mock('../../src/websocket', () => ({ broadcast: vi.fn() })); const { notificationSend } = vi.hoisted(() => ({ notificationSend: vi.fn().mockResolvedValue(undefined) })); vi.mock('../../src/services/notificationService', () => ({ send: notificationSend })); -const { checkPermission } = vi.hoisted(() => ({ checkPermission: vi.fn() })); -vi.mock('../../src/services/permissions', () => ({ checkPermission })); +import { PermissionsService } from '../../src/nest/permissions/permissions.service'; + +// Since the permissions DI migration, the check is a spy on the container's +// PermissionsService singleton (created in beforeAll, after build()). +let checkPermission: MockInstance; const { budget } = vi.hoisted(() => ({ budget: { @@ -80,6 +83,7 @@ describe('Reservations + accommodations e2e (real auth guard + temp SQLite, real ).run(); tripId = Number(db.prepare("INSERT INTO trips (user_id, title) VALUES (1, 'E2E Trip')").run().lastInsertRowid); app = await build(); + checkPermission = vi.spyOn(app.get(PermissionsService), 'checkPermission'); server = app.getHttpServer(); }); diff --git a/server/tests/e2e/share.e2e.test.ts b/server/tests/e2e/share.e2e.test.ts index 98f7571db..a9eea5b60 100644 --- a/server/tests/e2e/share.e2e.test.ts +++ b/server/tests/e2e/share.e2e.test.ts @@ -8,7 +8,7 @@ * auth, trip-access 404, permission 403, the create-201-vs-update-200 split * and the unguarded public read. */ -import { describe, it, expect, beforeAll, afterAll, beforeEach, vi } from 'vitest'; +import { describe, it, expect, beforeAll, afterAll, beforeEach, vi, type MockInstance } from 'vitest'; import request from 'supertest'; import cookieParser from 'cookie-parser'; import type { Server } from 'http'; @@ -39,8 +39,11 @@ vi.mock('../../src/db/database', () => ({ reinitialize: () => {}, })); -const { checkPermission } = vi.hoisted(() => ({ checkPermission: vi.fn() })); -vi.mock('../../src/services/permissions', () => ({ checkPermission })); +import { PermissionsService } from '../../src/nest/permissions/permissions.service'; + +// Since the permissions DI migration, the check is a spy on the container's +// PermissionsService singleton (created in beforeAll, after build()). +let checkPermission: MockInstance; const { serveFilePath } = vi.hoisted(() => ({ serveFilePath: vi.fn() })); vi.mock('../../src/services/placePhotoCache', () => ({ serveFilePath })); @@ -80,6 +83,7 @@ describe('Share-link e2e (real auth guard + real SQL over temp SQLite)', () => { "INSERT INTO users (id, username, email, password_hash, role) VALUES (1, 'e2e-user', 'e2e@example.test', 'x', 'user')", ).run(); app = await build(); + checkPermission = vi.spyOn(app.get(PermissionsService), 'checkPermission'); server = app.getHttpServer(); }); diff --git a/server/tests/e2e/todo.e2e.test.ts b/server/tests/e2e/todo.e2e.test.ts index cac521725..2910a1309 100644 --- a/server/tests/e2e/todo.e2e.test.ts +++ b/server/tests/e2e/todo.e2e.test.ts @@ -6,7 +6,7 @@ * over the temp db. Only the permission check and the WebSocket broadcast stay * mocked. */ -import { describe, it, expect, beforeAll, afterAll, beforeEach, vi } from 'vitest'; +import { describe, it, expect, beforeAll, afterAll, beforeEach, vi, type MockInstance } from 'vitest'; import request from 'supertest'; import cookieParser from 'cookie-parser'; import type { Server } from 'http'; @@ -63,8 +63,11 @@ vi.mock('../../src/db/database', () => ({ })); vi.mock('../../src/websocket', () => ({ broadcast: vi.fn() })); -const { checkPermission } = vi.hoisted(() => ({ checkPermission: vi.fn() })); -vi.mock('../../src/services/permissions', () => ({ checkPermission })); +import { PermissionsService } from '../../src/nest/permissions/permissions.service'; + +// Since the permissions DI migration, the check is a spy on the container's +// PermissionsService singleton (created in beforeAll, after build()). +let checkPermission: MockInstance; import { TodoModule } from '../../src/nest/todo/todo.module'; import { DatabaseModule } from '../../src/nest/database/database.module'; @@ -99,6 +102,7 @@ describe('To-do e2e (real auth guard + real SQL over temp SQLite)', () => { seedUser(db as never, { id: 1 }); seedUser(db as never, { id: 2, email: 'stranger@example.test' }); app = await build(); + checkPermission = vi.spyOn(app.get(PermissionsService), 'checkPermission'); server = app.getHttpServer(); }); diff --git a/server/tests/e2e/trip-invite.e2e.test.ts b/server/tests/e2e/trip-invite.e2e.test.ts index 499287f13..0fb0a46c8 100644 --- a/server/tests/e2e/trip-invite.e2e.test.ts +++ b/server/tests/e2e/trip-invite.e2e.test.ts @@ -6,7 +6,7 @@ * audit log are mocked. Focuses on auth (401), trip-access 404, the * share_manage 403, the login-required join, and invalid-token 404s (#1143). */ -import { describe, it, expect, beforeAll, afterAll, beforeEach, vi } from 'vitest'; +import { describe, it, expect, beforeAll, afterAll, beforeEach, vi, type MockInstance } from 'vitest'; import request from 'supertest'; import cookieParser from 'cookie-parser'; import type { Server } from 'http'; @@ -39,8 +39,11 @@ vi.mock('../../src/db/database', () => ({ db, canAccessTrip, isOwner: vi.fn(() => true), getPlaceWithTags: vi.fn(), closeDb: () => {}, reinitialize: () => {}, })); -const { checkPermission } = vi.hoisted(() => ({ checkPermission: vi.fn() })); -vi.mock('../../src/services/permissions', () => ({ checkPermission })); +import { PermissionsService } from '../../src/nest/permissions/permissions.service'; + +// Since the permissions DI migration, the check is a spy on the container's +// PermissionsService singleton (created in beforeAll, after build()). +let checkPermission: MockInstance; const { joinTripAsMember } = vi.hoisted(() => ({ joinTripAsMember: vi.fn() })); vi.mock('../../src/services/tripMembership', () => ({ joinTripAsMember })); @@ -77,6 +80,7 @@ describe('Trip invite-link e2e (real auth guard + temp SQLite)', () => { seedUser(db as never, { id: 1 }); seedUser(db as never, { id: 2, username: 'e2e-user-2', email: 'e2e-2@example.test' }); app = await build(); + checkPermission = vi.spyOn(app.get(PermissionsService), 'checkPermission'); server = app.getHttpServer(); }); diff --git a/server/tests/e2e/trips.e2e.test.ts b/server/tests/e2e/trips.e2e.test.ts index 60d4f2704..7c5a4466e 100644 --- a/server/tests/e2e/trips.e2e.test.ts +++ b/server/tests/e2e/trips.e2e.test.ts @@ -4,7 +4,7 @@ * list-services, auditLog, demo, the permission check, canAccessTrip and the * WebSocket broadcast are mocked. */ -import { describe, it, expect, beforeAll, afterAll, beforeEach, vi } from 'vitest'; +import { describe, it, expect, beforeAll, afterAll, beforeEach, vi, type MockInstance } from 'vitest'; import request from 'supertest'; import cookieParser from 'cookie-parser'; import type { Server } from 'http'; @@ -78,8 +78,11 @@ vi.mock('../../src/services/notificationService', () => ({ send: vi.fn().mockRes vi.mock('../../src/services/auditLog', () => ({ writeAudit: vi.fn(), getClientIp: vi.fn(() => '1.2.3.4'), logInfo: vi.fn(), logError: vi.fn() })); vi.mock('../../src/services/demo', () => ({ isDemoEmail: vi.fn(() => false) })); -const { checkPermission } = vi.hoisted(() => ({ checkPermission: vi.fn() })); -vi.mock('../../src/services/permissions', () => ({ checkPermission })); +import { PermissionsService } from '../../src/nest/permissions/permissions.service'; + +// Since the permissions DI migration, the check is a spy on the container's +// PermissionsService singleton (created in beforeAll, after build()). +let checkPermission: MockInstance; const { tripSvc } = vi.hoisted(() => ({ tripSvc: { @@ -114,6 +117,7 @@ describe('Trips e2e (real auth guard + temp SQLite)', () => { beforeAll(async () => { seedUser(db as never, { id: 1 }); app = await build(); + checkPermission = vi.spyOn(app.get(PermissionsService), 'checkPermission'); server = app.getHttpServer(); tripSvc.listTrips.mockReturnValue([{ id: 1, title: 'T' }]); tripSvc.createTrip.mockReturnValue({ trip: { id: 9 }, tripId: 9, reminderDays: 0 }); diff --git a/server/tests/helpers/mcp-test-controllers.ts b/server/tests/helpers/mcp-test-controllers.ts index e0b53f179..c76d4c440 100644 --- a/server/tests/helpers/mcp-test-controllers.ts +++ b/server/tests/helpers/mcp-test-controllers.ts @@ -14,6 +14,7 @@ import { DaysMcp } from '../../src/nest/days/days.mcp'; import { DaysService } from '../../src/nest/days/days.service'; import { PackingMcp } from '../../src/nest/packing/packing.mcp'; import { PackingService } from '../../src/nest/packing/packing.service'; +import { PermissionsService } from '../../src/nest/permissions/permissions.service'; import { ReservationsMcp } from '../../src/nest/reservations/reservations.mcp'; import { ReservationsService } from '../../src/nest/reservations/reservations.service'; import { TagsMcp } from '../../src/nest/tags/tags.mcp'; @@ -32,18 +33,19 @@ import { VacayService } from '../../src/nest/vacay/vacay.service'; */ export function createMcpTestRegistry(): McpRegistry { const dbService = new DatabaseService(db); - const daysService = new DaysService(dbService); + const permissionsService = new PermissionsService(dbService); + const daysService = new DaysService(dbService, permissionsService); return createTestRegistry( [ new TagsMcp(new TagsService(dbService)), new CategoriesMcp(new CategoriesService(dbService)), - new TodoMcp(new TodoService(dbService)), - new PackingMcp(new PackingService(dbService)), - new ReservationsMcp(new ReservationsService(dbService), daysService), - new DayNotesMcp(new DayNotesService(dbService)), + new TodoMcp(new TodoService(dbService, permissionsService)), + new PackingMcp(new PackingService(dbService, permissionsService)), + new ReservationsMcp(new ReservationsService(dbService, permissionsService), daysService), + new DayNotesMcp(new DayNotesService(dbService, permissionsService)), new DaysMcp(daysService, dbService), - new AssignmentsMcp(new AssignmentsService(dbService), daysService), - new CollabMcp(new CollabService(dbService)), + new AssignmentsMcp(new AssignmentsService(dbService, permissionsService), daysService), + new CollabMcp(new CollabService(dbService, permissionsService)), new VacayMcp(new VacayService(dbService)), ], { accessPolicy: trekMcpAccessPolicy }, diff --git a/server/tests/helpers/plugin-host.ts b/server/tests/helpers/plugin-host.ts index 3635efdf2..7d8443915 100644 --- a/server/tests/helpers/plugin-host.ts +++ b/server/tests/helpers/plugin-host.ts @@ -17,6 +17,7 @@ import { SettingsService } from '../../src/nest/settings/settings.service'; import { FilesService } from '../../src/nest/files/files.service'; import { CollabService } from '../../src/nest/collab/collab.service'; import { VacayService } from '../../src/nest/vacay/vacay.service'; +import { PermissionsService } from '../../src/nest/permissions/permissions.service'; /** * Hand-wired counterpart of the PluginsModule DI graph for no-Nest tests @@ -25,22 +26,24 @@ import { VacayService } from '../../src/nest/vacay/vacay.service'; * the container. */ export function createHostDepsFactory(dbs: DatabaseService): PluginHostDepsFactory { + const permissions = new PermissionsService(dbs); return new PluginHostDepsFactory( - new BudgetService(dbs), - new ReservationsService(dbs), + new BudgetService(dbs, permissions), + new ReservationsService(dbs, permissions), new TagsService(dbs), new CategoriesService(dbs), - new TodoService(dbs), - new PackingService(dbs), + new TodoService(dbs, permissions), + new PackingService(dbs, permissions), new PluginOAuthService(dbs), - new DayNotesService(dbs), - new AssignmentsService(dbs), + new DayNotesService(dbs, permissions), + new AssignmentsService(dbs, permissions), new LlmConfigResolver(new SettingsService(dbs), dbs), dbs, - new FilesService(dbs), - new CollabService(dbs), + new FilesService(dbs, permissions), + new CollabService(dbs, permissions), new VacayService(dbs), - new DaysService(dbs), + new DaysService(dbs, permissions), + permissions, ); } diff --git a/server/tests/integration/budget.test.ts b/server/tests/integration/budget.test.ts index c1a88cf6a..126d48402 100644 --- a/server/tests/integration/budget.test.ts +++ b/server/tests/integration/budget.test.ts @@ -416,7 +416,7 @@ describe('Reorder budget items', () => { // Restrict budget_edit to trip_owner only testDb.prepare("INSERT OR REPLACE INTO app_settings (key, value) VALUES ('perm_budget_edit', 'trip_owner')").run(); - const { invalidatePermissionsCache } = await import('../../src/services/permissions'); + const { invalidatePermissionsCache } = await import('../../src/nest/permissions/permissions.bridge'); invalidatePermissionsCache(); const res = await request(app) @@ -519,7 +519,7 @@ describe('Budget edit permission enforcement', () => { const trip = createTrip(testDb, owner.id); addTripMember(testDb, trip.id, member.id); - const { invalidatePermissionsCache } = await import('../../src/services/permissions'); + const { invalidatePermissionsCache } = await import('../../src/nest/permissions/permissions.bridge'); testDb.prepare("INSERT OR REPLACE INTO app_settings (key, value) VALUES ('perm_budget_edit', 'trip_owner')").run(); invalidatePermissionsCache(); @@ -540,7 +540,7 @@ describe('Budget edit permission enforcement', () => { addTripMember(testDb, trip.id, member.id); createBudgetItem(testDb, trip.id, { name: 'Item', category: 'Transport' }); - const { invalidatePermissionsCache } = await import('../../src/services/permissions'); + const { invalidatePermissionsCache } = await import('../../src/nest/permissions/permissions.bridge'); testDb.prepare("INSERT OR REPLACE INTO app_settings (key, value) VALUES ('perm_budget_edit', 'trip_owner')").run(); invalidatePermissionsCache(); diff --git a/server/tests/integration/dayReorder.test.ts b/server/tests/integration/dayReorder.test.ts index f9f5e0449..fedcd459c 100644 --- a/server/tests/integration/dayReorder.test.ts +++ b/server/tests/integration/dayReorder.test.ts @@ -29,9 +29,10 @@ import { runMigrations } from '../../src/db/migrations'; import { resetTestDb } from '../helpers/test-db'; import { createUser, createTrip, createPlace, createDay, createDayAssignment, createReservation, createDayAccommodation } from '../helpers/factories'; import { DatabaseService } from '../../src/nest/database/database.service'; +import { PermissionsService } from '../../src/nest/permissions/permissions.service'; import { DaysService, DayReorderError } from '../../src/nest/days/days.service'; -const svc = new DaysService(new DatabaseService(testDb)); +const svc = new DaysService(new DatabaseService(testDb), new PermissionsService(new DatabaseService(testDb))); const reorderDays = (tripId: number, orderedIds: number[]) => svc.reorder(tripId, orderedIds); const insertDay = (tripId: number, position?: number) => svc.insert(tripId, position); diff --git a/server/tests/integration/journey.test.ts b/server/tests/integration/journey.test.ts index 4cbfc65ca..26128e9be 100644 --- a/server/tests/integration/journey.test.ts +++ b/server/tests/integration/journey.test.ts @@ -73,7 +73,7 @@ import { addJourneyContributor, } from '../helpers/factories'; import { authCookie } from '../helpers/auth'; -import { invalidatePermissionsCache } from '../../src/services/permissions'; +import { invalidatePermissionsCache } from '../../src/nest/permissions/permissions.bridge'; let nestApp: INestApplication; let app: Application; diff --git a/server/tests/integration/places.test.ts b/server/tests/integration/places.test.ts index 835715b1b..86450b1ae 100644 --- a/server/tests/integration/places.test.ts +++ b/server/tests/integration/places.test.ts @@ -64,7 +64,7 @@ import { resetTestDb, resetRateLimits } from '../helpers/test-db'; import { createUser, createAdmin, createTrip, createPlace, addTripMember } from '../helpers/factories'; import { authCookie } from '../helpers/auth'; import * as placeService from '../../src/services/placeService'; -import { invalidatePermissionsCache } from '../../src/services/permissions'; +import { invalidatePermissionsCache } from '../../src/nest/permissions/permissions.bridge'; let nestApp: INestApplication; let app: Application; diff --git a/server/tests/integration/todo.test.ts b/server/tests/integration/todo.test.ts index efbcbde29..168bb2449 100644 --- a/server/tests/integration/todo.test.ts +++ b/server/tests/integration/todo.test.ts @@ -44,7 +44,7 @@ import { runMigrations } from '../../src/db/migrations'; import { resetTestDb, resetRateLimits } from '../helpers/test-db'; import { createUser, createTrip, addTripMember } from '../helpers/factories'; import { authCookie } from '../helpers/auth'; -import { invalidatePermissionsCache } from '../../src/services/permissions'; +import { invalidatePermissionsCache } from '../../src/nest/permissions/permissions.bridge'; let nestApp: INestApplication; let app: Application; diff --git a/server/tests/integration/trips.test.ts b/server/tests/integration/trips.test.ts index 3bc442b13..0535e2eb1 100644 --- a/server/tests/integration/trips.test.ts +++ b/server/tests/integration/trips.test.ts @@ -57,7 +57,7 @@ import { runMigrations } from '../../src/db/migrations'; import { resetTestDb, resetRateLimits } from '../helpers/test-db'; import { createUser, createAdmin, createTrip, addTripMember, createPlace, createReservation, createTag, createDayAccommodation, createBudgetItem, createPackingItem, createDayNote, createDayAssignment } from '../helpers/factories'; import { authCookie } from '../helpers/auth'; -import { invalidatePermissionsCache } from '../../src/services/permissions'; +import { invalidatePermissionsCache } from '../../src/nest/permissions/permissions.bridge'; let nestApp: INestApplication; let app: Application; diff --git a/server/tests/unit/mcp/tools-transit.test.ts b/server/tests/unit/mcp/tools-transit.test.ts index 7ce8c7700..9c6984f54 100644 --- a/server/tests/unit/mcp/tools-transit.test.ts +++ b/server/tests/unit/mcp/tools-transit.test.ts @@ -1,6 +1,6 @@ import { runMigrations } from '../../../src/db/migrations'; import { createTables } from '../../../src/db/schema'; -import { invalidatePermissionsCache, savePermissions } from '../../../src/services/permissions'; +import { invalidatePermissionsCache, savePermissions } from '../../../src/nest/permissions/permissions.bridge'; import { addTripMember, createDay, createTrip, createUser } from '../../helpers/factories'; import { createMcpHarness, parseToolResult, type McpHarness } from '../../helpers/mcp-harness'; import { resetTestDb } from '../../helpers/test-db'; diff --git a/server/tests/unit/nest/assignments.service.test.ts b/server/tests/unit/nest/assignments.service.test.ts index 1227a1c79..e5aacd5d3 100644 --- a/server/tests/unit/nest/assignments.service.test.ts +++ b/server/tests/unit/nest/assignments.service.test.ts @@ -49,10 +49,11 @@ import { runMigrations } from '../../../src/db/migrations'; import { resetTestDb } from '../../helpers/test-db'; import { createUser, createTrip, addTripMember, createDay, createPlace, createDayAssignment, createTag } from '../../helpers/factories'; import { DatabaseService } from '../../../src/nest/database/database.service'; +import { PermissionsService } from '../../../src/nest/permissions/permissions.service'; import { AssignmentsService } from '../../../src/nest/assignments/assignments.service'; import { createAssignment, dayExists, placeExists, getAssignmentForTrip } from '../../../src/nest/assignments/assignments.bridge'; -const svc = new AssignmentsService(new DatabaseService(testDb)); +const svc = new AssignmentsService(new DatabaseService(testDb), new PermissionsService(new DatabaseService(testDb))); beforeAll(() => { createTables(testDb); diff --git a/server/tests/unit/nest/booking-import/booking-import.service.test.ts b/server/tests/unit/nest/booking-import/booking-import.service.test.ts index 879a5dd66..2c3157c71 100644 --- a/server/tests/unit/nest/booking-import/booking-import.service.test.ts +++ b/server/tests/unit/nest/booking-import/booking-import.service.test.ts @@ -7,7 +7,7 @@ vi.mock('../../../../src/db/database', () => ({ db: { prepare: vi.fn() }, closeD import { db as dbConn } from '../../../../src/db/database'; import { DatabaseService } from '../../../../src/nest/database/database.service'; vi.mock('../../../../src/websocket', () => ({ broadcast: vi.fn() })); -vi.mock('../../../../src/services/permissions', () => ({ checkPermission: vi.fn(() => true) })); +const permissionsStub = { checkPermission: vi.fn(() => true) }; vi.mock('../../../../src/services/tripAccess', () => ({ verifyTripAccess: vi.fn() })); vi.mock('../../../../src/services/placeService', () => ({ createPlace: vi.fn() })); vi.mock('../../../../src/services/mapsService', () => ({ searchNominatim: vi.fn() })); @@ -21,7 +21,7 @@ function make(opts: { kit?: boolean; ai?: boolean; extract?: any; parse?: any }) const extractor = { isAvailable: () => opts.kit ?? false, extract: vi.fn(opts.extract ?? (async () => [])) }; const llmParse = { isAvailable: () => opts.ai ?? false, parse: vi.fn(opts.parse ?? (async () => ({ kiItems: [], warnings: [] }))) }; const reservations = { create: vi.fn() }; - return { svc: new BookingImportService(extractor as any, llmParse as any, new DatabaseService(dbConn), reservations as never), extractor, llmParse, reservations }; + return { svc: new BookingImportService(extractor as any, llmParse as any, new DatabaseService(dbConn), reservations as never, permissionsStub as never), extractor, llmParse, reservations }; } beforeEach(() => vi.clearAllMocks()); diff --git a/server/tests/unit/nest/budget.service.test.ts b/server/tests/unit/nest/budget.service.test.ts index d83d22ad2..79e7f9ba7 100644 --- a/server/tests/unit/nest/budget.service.test.ts +++ b/server/tests/unit/nest/budget.service.test.ts @@ -8,12 +8,13 @@ const { dbMock } = vi.hoisted(() => { vi.mock('../../../src/db/database', () => ({ db: dbMock, closeDb: () => {}, reinitialize: () => {} })); import { db as dbConn } from '../../../src/db/database'; import { DatabaseService } from '../../../src/nest/database/database.service'; +import type { PermissionsService } from '../../../src/nest/permissions/permissions.service'; const { broadcast } = vi.hoisted(() => ({ broadcast: vi.fn() })); vi.mock('../../../src/websocket', () => ({ broadcast })); -const { checkPermission } = vi.hoisted(() => ({ checkPermission: vi.fn(() => true) })); -vi.mock('../../../src/services/permissions', () => ({ checkPermission })); +const checkPermission = vi.fn(() => true); +const permissionsStub = { checkPermission } as unknown as PermissionsService; const { getRates } = vi.hoisted(() => ({ getRates: vi.fn() })); vi.mock('../../../src/services/exchangeRateService', () => ({ getRates })); @@ -44,7 +45,7 @@ vi.mock('../../../src/services/budgetService', () => budget); import { BudgetService } from '../../../src/nest/budget/budget.service'; function svc() { - return new BudgetService(new DatabaseService(dbConn)); + return new BudgetService(new DatabaseService(dbConn), permissionsStub); } beforeEach(() => { diff --git a/server/tests/unit/nest/collab.service.test.ts b/server/tests/unit/nest/collab.service.test.ts index ee04d79d9..a00e2d1e2 100644 --- a/server/tests/unit/nest/collab.service.test.ts +++ b/server/tests/unit/nest/collab.service.test.ts @@ -59,10 +59,11 @@ import { resetTestDb } from '../../helpers/test-db'; import { createUser, createTrip } from '../../helpers/factories'; import { avatarUrl } from '../../../src/services/avatarUrl'; import { DatabaseService } from '../../../src/nest/database/database.service'; +import { PermissionsService } from '../../../src/nest/permissions/permissions.service'; import { CollabService } from '../../../src/nest/collab/collab.service'; import { listNotes as bridgeListNotes, listPolls as bridgeListPolls, countMessages as bridgeCountMessages } from '../../../src/nest/collab/collab.bridge'; -const svc = new CollabService(new DatabaseService(testDb)); +const svc = new CollabService(new DatabaseService(testDb), new PermissionsService(new DatabaseService(testDb))); beforeAll(() => { createTables(testDb); @@ -440,7 +441,7 @@ describe('hardening', () => { it('COLLAB-SVC-034: votePoll switch is atomic — prior vote survives a failed INSERT', () => { const { user1, trip } = setup(); const dbs = new DatabaseService(testDb); - const failing = new CollabService(dbs); + const failing = new CollabService(dbs, new PermissionsService(dbs)); const poll = failing.createPoll(trip.id, user1.id, { question: 'Q?', options: ['A', 'B'] }); failing.votePoll(trip.id, poll!.id, user1.id, 0); @@ -461,7 +462,7 @@ describe('hardening', () => { it('COLLAB-SVC-035: deleteNote is atomic — trip_files rows survive a failed note DELETE', () => { const { user1, trip } = setup(); const dbs = new DatabaseService(testDb); - const failing = new CollabService(dbs); + const failing = new CollabService(dbs, new PermissionsService(dbs)); const note = failing.createNote(trip.id, user1.id, { title: 'With file' }); testDb.prepare('INSERT INTO trip_files (trip_id, note_id, filename, original_name) VALUES (?, ?, ?, ?)') .run(trip.id, note.id, 'files/a.pdf', 'a.pdf'); diff --git a/server/tests/unit/nest/day-notes.service.test.ts b/server/tests/unit/nest/day-notes.service.test.ts index bf9a3f01c..2e5ca5784 100644 --- a/server/tests/unit/nest/day-notes.service.test.ts +++ b/server/tests/unit/nest/day-notes.service.test.ts @@ -47,10 +47,11 @@ import { runMigrations } from '../../../src/db/migrations'; import { resetTestDb } from '../../helpers/test-db'; import { createUser, createTrip, createDay, addTripMember } from '../../helpers/factories'; import { DatabaseService } from '../../../src/nest/database/database.service'; +import { PermissionsService } from '../../../src/nest/permissions/permissions.service'; import { DayNotesService } from '../../../src/nest/days/day-notes.service'; import type { DayNote } from '../../../src/types'; -const svc = new DayNotesService(new DatabaseService(testDb)); +const svc = new DayNotesService(new DatabaseService(testDb), new PermissionsService(new DatabaseService(testDb))); beforeAll(() => { createTables(testDb); diff --git a/server/tests/unit/nest/days.service.test.ts b/server/tests/unit/nest/days.service.test.ts index 847c7b669..5520c8e09 100644 --- a/server/tests/unit/nest/days.service.test.ts +++ b/server/tests/unit/nest/days.service.test.ts @@ -50,6 +50,7 @@ import { runMigrations } from '../../../src/db/migrations'; import { resetTestDb } from '../../helpers/test-db'; import { createUser, createTrip, createDay, createPlace, createDayAssignment, createDayAccommodation } from '../../helpers/factories'; import { DatabaseService } from '../../../src/nest/database/database.service'; +import { PermissionsService } from '../../../src/nest/permissions/permissions.service'; import { DaysService, addDays } from '../../../src/nest/days/days.service'; import { getDay as bridgeGetDay, @@ -59,7 +60,7 @@ import { resyncAccommodationDays as bridgeResyncAccommodationDays, } from '../../../src/nest/days/days.bridge'; -const svc = new DaysService(new DatabaseService(testDb)); +const svc = new DaysService(new DatabaseService(testDb), new PermissionsService(new DatabaseService(testDb))); beforeAll(() => { createTables(testDb); diff --git a/server/tests/unit/nest/files.service.test.ts b/server/tests/unit/nest/files.service.test.ts index f4cfaf8ae..1c6551f6b 100644 --- a/server/tests/unit/nest/files.service.test.ts +++ b/server/tests/unit/nest/files.service.test.ts @@ -45,8 +45,8 @@ vi.mock('../../../src/config', () => ({ })); vi.mock('../../../src/websocket', () => ({ broadcast: vi.fn() })); -const { checkPermission } = vi.hoisted(() => ({ checkPermission: vi.fn(() => true) })); -vi.mock('../../../src/services/permissions', () => ({ checkPermission })); +const checkPermission = vi.fn(() => true); +const permissionsStub = { checkPermission } as unknown as PermissionsService; const { verifyJwtAndLoadUser } = vi.hoisted(() => ({ verifyJwtAndLoadUser: vi.fn() })); vi.mock('../../../src/middleware/auth', () => ({ verifyJwtAndLoadUser })); @@ -60,6 +60,7 @@ import { runMigrations } from '../../../src/db/migrations'; import { resetTestDb } from '../../helpers/test-db'; import { createUser, createTrip, addTripMember, createPlace, createReservation, createDay, createDayAssignment, setAppSetting } from '../../helpers/factories'; import { DatabaseService } from '../../../src/nest/database/database.service'; +import type { PermissionsService } from '../../../src/nest/permissions/permissions.service'; import { FilesService } from '../../../src/nest/files/files.service'; import { getAllowedExtensions as bridgeGetAllowedExtensions } from '../../../src/nest/files/files.bridge'; import { @@ -73,7 +74,7 @@ import { } from '../../../src/nest/files/files.constants'; import type { TripFile, User } from '../../../src/types'; -const svc = new FilesService(new DatabaseService(testDb)); +const svc = new FilesService(new DatabaseService(testDb), permissionsStub); beforeAll(() => { createTables(testDb); @@ -181,7 +182,7 @@ describe('getAllowedExtensions', () => { }); it('FILE-SVC-009: returns the default when the query throws (no app_settings table)', () => { - const bareSvc = new FilesService(new DatabaseService(bareDb)); + const bareSvc = new FilesService(new DatabaseService(bareDb), permissionsStub); expect(bareSvc.getAllowedExtensions()).toBe(DEFAULT_ALLOWED_EXTENSIONS); }); }); diff --git a/server/tests/unit/nest/packing.service.test.ts b/server/tests/unit/nest/packing.service.test.ts index 91c528dc6..c1ec3df2e 100644 --- a/server/tests/unit/nest/packing.service.test.ts +++ b/server/tests/unit/nest/packing.service.test.ts @@ -45,8 +45,8 @@ vi.mock('../../../src/config', () => ({ const { broadcastMock } = vi.hoisted(() => ({ broadcastMock: vi.fn() })); vi.mock('../../../src/websocket', () => ({ broadcast: broadcastMock })); -const { checkPermission } = vi.hoisted(() => ({ checkPermission: vi.fn(() => true) })); -vi.mock('../../../src/services/permissions', () => ({ checkPermission })); +const checkPermission = vi.fn(() => true); +const permissionsStub = { checkPermission } as unknown as PermissionsService; const { send } = vi.hoisted(() => ({ send: vi.fn(() => Promise.resolve()) })); vi.mock('../../../src/services/notificationService', () => ({ send })); @@ -56,10 +56,11 @@ import { runMigrations } from '../../../src/db/migrations'; import { resetTestDb } from '../../helpers/test-db'; import { createUser, createTrip } from '../../helpers/factories'; import { DatabaseService } from '../../../src/nest/database/database.service'; +import type { PermissionsService } from '../../../src/nest/permissions/permissions.service'; import { PackingService } from '../../../src/nest/packing/packing.service'; import { listItems as bridgeListItems } from '../../../src/nest/packing/packing.bridge'; -const svc = new PackingService(new DatabaseService(testDb)); +const svc = new PackingService(new DatabaseService(testDb), permissionsStub); // ── Lifecycle ───────────────────────────────────────────────────────────────── diff --git a/server/tests/unit/nest/permissions.service.test.ts b/server/tests/unit/nest/permissions.service.test.ts new file mode 100644 index 000000000..1d0934cd7 --- /dev/null +++ b/server/tests/unit/nest/permissions.service.test.ts @@ -0,0 +1,222 @@ +/** + * Unit tests for the DI-native PermissionsService — PERM-SVC-001 through + * PERM-SVC-020 (001–013 moved 1:1 from the legacy + * tests/unit/services/permissions.test.ts, which had no case IDs — the IDs are + * introduced with the move; 014–016 pin the cache semantics the real DB now + * makes testable; 017–020 pin the permissions.bridge delegation and the + * module-scoped cache shared across the DI and bridge instances). Uses a real + * in-memory SQLite DB so the app_settings SQL is exercised faithfully. + */ +import { describe, it, expect, vi, beforeAll, beforeEach, afterAll } from 'vitest'; + +// ── DB setup ────────────────────────────────────────────────────────────────── + +const { testDb, dbMock } = vi.hoisted(() => { + const Database = require('better-sqlite3'); + const db = new Database(':memory:'); + db.exec('PRAGMA journal_mode = WAL'); + db.exec('PRAGMA foreign_keys = ON'); + db.exec('PRAGMA busy_timeout = 5000'); + const mock = { + db, + closeDb: () => {}, + reinitialize: () => {}, + }; + return { testDb: db, dbMock: mock }; +}); + +vi.mock('../../../src/db/database', () => dbMock); + +import { createTables } from '../../../src/db/schema'; +import { runMigrations } from '../../../src/db/migrations'; +import { DatabaseService } from '../../../src/nest/database/database.service'; +import { PermissionsService, PERMISSION_ACTIONS } from '../../../src/nest/permissions/permissions.service'; +import { + checkPermission as bridgeCheckPermission, + getPermissionLevel as bridgeGetPermissionLevel, + getAllPermissions as bridgeGetAllPermissions, + savePermissions as bridgeSavePermissions, + invalidatePermissionsCache as bridgeInvalidatePermissionsCache, +} from '../../../src/nest/permissions/permissions.bridge'; + +const svc = new PermissionsService(new DatabaseService(testDb)); + +beforeAll(() => { + createTables(testDb); + runMigrations(testDb); +}); + +beforeEach(() => { + testDb.prepare("DELETE FROM app_settings WHERE key LIKE 'perm_%'").run(); + svc.invalidatePermissionsCache(); +}); + +afterAll(() => { + testDb.close(); +}); + +// ── checkPermission ─────────────────────────────────────────────────────────── + +describe('checkPermission — admin bypass', () => { + it('PERM-SVC-001: admin always passes regardless of permission level', () => { + for (const action of PERMISSION_ACTIONS) { + expect(svc.checkPermission(action.key, 'admin', 1, 1, false)).toBe(true); + expect(svc.checkPermission(action.key, 'admin', 99, 1, false)).toBe(true); + } + }); +}); + +describe('checkPermission — everybody level', () => { + it('PERM-SVC-002: trip_create (everybody) allows any authenticated user', () => { + expect(svc.checkPermission('trip_create', 'user', null, 42, false)).toBe(true); + }); +}); + +describe('checkPermission — trip_owner level', () => { + const ownerId = 10; + const memberId = 20; + + it('PERM-SVC-003: trip owner passes trip_owner check', () => { + expect(svc.checkPermission('trip_delete', 'user', ownerId, ownerId, false)).toBe(true); + }); + + it('PERM-SVC-004: member fails trip_owner check', () => { + expect(svc.checkPermission('trip_delete', 'user', ownerId, memberId, true)).toBe(false); + }); + + it('PERM-SVC-005: non-member non-owner fails trip_owner check', () => { + expect(svc.checkPermission('trip_delete', 'user', ownerId, memberId, false)).toBe(false); + }); +}); + +describe('checkPermission — trip_member level', () => { + const ownerId = 10; + const memberId = 20; + const outsiderId = 30; + + it('PERM-SVC-006: trip owner passes trip_member check', () => { + expect(svc.checkPermission('day_edit', 'user', ownerId, ownerId, false)).toBe(true); + }); + + it('PERM-SVC-007: trip member passes trip_member check', () => { + expect(svc.checkPermission('day_edit', 'user', ownerId, memberId, true)).toBe(true); + }); + + it('PERM-SVC-008: outsider fails trip_member check', () => { + expect(svc.checkPermission('day_edit', 'user', ownerId, outsiderId, false)).toBe(false); + }); +}); + +// ── getPermissionLevel ──────────────────────────────────────────────────────── + +describe('getPermissionLevel — defaults', () => { + it('PERM-SVC-009: returns default level for known actions (no DB overrides)', () => { + const defaults: Record = { + trip_create: 'everybody', + trip_delete: 'trip_owner', + day_edit: 'trip_member', + budget_edit: 'trip_member', + }; + for (const [key, expected] of Object.entries(defaults)) { + expect(svc.getPermissionLevel(key)).toBe(expected); + } + }); + + it('PERM-SVC-010: returns trip_owner for unknown action key', () => { + expect(svc.getPermissionLevel('nonexistent_action')).toBe('trip_owner'); + }); +}); + +// ── savePermissions ─────────────────────────────────────────────────────────── + +describe('savePermissions — invalid input is silently skipped', () => { + it('PERM-SVC-011: returns skipped array containing invalid action key, writes no row', () => { + const result = svc.savePermissions({ nonexistent_action: 'trip_member' }); + expect(result.skipped).toContain('nonexistent_action'); + const rows = testDb.prepare("SELECT key FROM app_settings WHERE key LIKE 'perm_%'").all(); + expect(rows).toEqual([]); + }); + + it('PERM-SVC-012: returns skipped array when level is not in allowedLevels for the action', () => { + // trip_delete only allows ['admin', 'trip_owner'], so 'trip_member' is invalid + const result = svc.savePermissions({ trip_delete: 'trip_member' }); + expect(result.skipped).toContain('trip_delete'); + const rows = testDb.prepare("SELECT key FROM app_settings WHERE key LIKE 'perm_%'").all(); + expect(rows).toEqual([]); + }); +}); + +describe('checkPermission — default case', () => { + it('PERM-SVC-013: returns false when permission level is an unrecognized value', () => { + testDb.prepare('INSERT OR REPLACE INTO app_settings (key, value) VALUES (?, ?)').run('perm_trip_edit', 'unknown_level'); + svc.invalidatePermissionsCache(); + expect(svc.checkPermission('trip_edit', 'user', 10, 10, false)).toBe(false); + }); +}); + +// ── cache semantics (real DB) ───────────────────────────────────────────────── + +describe('stored overrides + cache', () => { + it('PERM-SVC-014: stored perm_ row overrides the default after invalidation', () => { + testDb.prepare('INSERT OR REPLACE INTO app_settings (key, value) VALUES (?, ?)').run('perm_trip_edit', 'trip_member'); + svc.invalidatePermissionsCache(); + expect(svc.getPermissionLevel('trip_edit')).toBe('trip_member'); + // A plain member now passes what defaults to a trip_owner-only action. + expect(svc.checkPermission('trip_edit', 'user', 10, 20, true)).toBe(true); + }); + + it('PERM-SVC-015: savePermissions persists the row and self-invalidates the cache', () => { + // Prime the cache with the defaults first. + expect(svc.getPermissionLevel('trip_edit')).toBe('trip_owner'); + const result = svc.savePermissions({ trip_edit: 'trip_member' }); + expect(result.skipped).toEqual([]); + const row = testDb.prepare('SELECT value FROM app_settings WHERE key = ?').get('perm_trip_edit') as { value: string }; + expect(row.value).toBe('trip_member'); + // No manual invalidation — savePermissions did it. + expect(svc.getPermissionLevel('trip_edit')).toBe('trip_member'); + }); + + it('PERM-SVC-016: the cache memoizes until invalidated', () => { + expect(svc.getPermissionLevel('trip_edit')).toBe('trip_owner'); + // Raw SQL write bypasses savePermissions' self-invalidation → stale value served. + testDb.prepare('INSERT OR REPLACE INTO app_settings (key, value) VALUES (?, ?)').run('perm_trip_edit', 'trip_member'); + expect(svc.getPermissionLevel('trip_edit')).toBe('trip_owner'); + svc.invalidatePermissionsCache(); + expect(svc.getPermissionLevel('trip_edit')).toBe('trip_member'); + }); +}); + +// ── permissions.bridge delegation ───────────────────────────────────────────── + +describe('permissions.bridge delegation', () => { + it('PERM-SVC-017: checkPermission delegates through the bridge instance', () => { + expect(bridgeCheckPermission('trip_create', 'user', null, 42, false)).toBe(true); + expect(bridgeCheckPermission('trip_delete', 'user', 10, 20, true)).toBe(false); + }); + + it('PERM-SVC-018: savePermissions via the bridge writes the row and reports skips', () => { + const result = bridgeSavePermissions({ trip_edit: 'trip_member', bogus: 'trip_member' }); + expect(result.skipped).toEqual(['bogus']); + const row = testDb.prepare('SELECT value FROM app_settings WHERE key = ?').get('perm_trip_edit') as { value: string }; + expect(row.value).toBe('trip_member'); + }); + + it('PERM-SVC-019: getPermissionLevel/getAllPermissions delegate through the bridge', () => { + expect(bridgeGetPermissionLevel('trip_create')).toBe('everybody'); + const all = bridgeGetAllPermissions(); + expect(all.trip_delete).toBe('trip_owner'); + expect(Object.keys(all)).toHaveLength(PERMISSION_ACTIONS.length); + }); + + it('PERM-SVC-020: the cache is module-scoped — shared by the DI and bridge instances', () => { + // Save through the DI instance; the bridge instance sees it immediately. + svc.savePermissions({ trip_edit: 'trip_member' }); + expect(bridgeGetPermissionLevel('trip_edit')).toBe('trip_member'); + // Raw SQL write, then invalidate through the BRIDGE — the DI instance + // must serve the fresh value (backup-restore relies on exactly this). + testDb.prepare('INSERT OR REPLACE INTO app_settings (key, value) VALUES (?, ?)').run('perm_trip_edit', 'trip_owner'); + expect(svc.getPermissionLevel('trip_edit')).toBe('trip_member'); // still cached + bridgeInvalidatePermissionsCache(); + expect(svc.getPermissionLevel('trip_edit')).toBe('trip_owner'); + }); +}); diff --git a/server/tests/unit/nest/reservations.service.test.ts b/server/tests/unit/nest/reservations.service.test.ts index fe26386c6..327dc3bfc 100644 --- a/server/tests/unit/nest/reservations.service.test.ts +++ b/server/tests/unit/nest/reservations.service.test.ts @@ -34,8 +34,8 @@ vi.mock('../../../src/db/database', () => dbMock); const { broadcast } = vi.hoisted(() => ({ broadcast: vi.fn() })); vi.mock('../../../src/websocket', () => ({ broadcast })); -const { checkPermission } = vi.hoisted(() => ({ checkPermission: vi.fn(() => true) })); -vi.mock('../../../src/services/permissions', () => ({ checkPermission })); +const checkPermission = vi.fn(() => true); +const permissionsStub = { checkPermission } as unknown as PermissionsService; const { budget } = vi.hoisted(() => ({ budget: { createBudgetItem: vi.fn(), updateBudgetItem: vi.fn(), deleteBudgetItem: vi.fn(), linkBudgetItemToReservation: vi.fn() }, @@ -50,10 +50,11 @@ import { runMigrations } from '../../../src/db/migrations'; import { resetTestDb } from '../../helpers/test-db'; import { createUser, createTrip, createReservation, createBudgetItem, createPlace, createDay, createDayAccommodation, addTripMember } from '../../helpers/factories'; import { DatabaseService } from '../../../src/nest/database/database.service'; +import type { PermissionsService } from '../../../src/nest/permissions/permissions.service'; import { ReservationsService } from '../../../src/nest/reservations/reservations.service'; import * as bridge from '../../../src/nest/reservations/reservations.bridge'; -const svc = new ReservationsService(new DatabaseService(testDb)); +const svc = new ReservationsService(new DatabaseService(testDb), permissionsStub); beforeAll(() => { createTables(testDb); runMigrations(testDb); }); beforeEach(() => { diff --git a/server/tests/unit/nest/share.service.test.ts b/server/tests/unit/nest/share.service.test.ts index 18a4a5482..945c5af66 100644 --- a/server/tests/unit/nest/share.service.test.ts +++ b/server/tests/unit/nest/share.service.test.ts @@ -39,8 +39,8 @@ vi.mock('../../../src/config', () => ({ updateJwtSecret: () => {}, })); -const { checkPermission } = vi.hoisted(() => ({ checkPermission: vi.fn() })); -vi.mock('../../../src/services/permissions', () => ({ checkPermission })); +const checkPermission = vi.fn(); +const permissionsStub = { checkPermission } as unknown as PermissionsService; const { serveFilePath } = vi.hoisted(() => ({ serveFilePath: vi.fn() })); vi.mock('../../../src/services/placePhotoCache', () => ({ serveFilePath })); @@ -52,12 +52,13 @@ import { createUser, createTrip, addTripMember, createDay, createPlace, createDayAssignment, } from '../../helpers/factories'; import { DatabaseService } from '../../../src/nest/database/database.service'; +import type { PermissionsService } from '../../../src/nest/permissions/permissions.service'; import { ShareService } from '../../../src/nest/share/share.service'; import { SettingsService } from '../../../src/nest/settings/settings.service'; import { createOrUpdateShareLink, getShareLink, deleteShareLink } from '../../../src/nest/share/share.bridge'; import type { User } from '../../../src/types'; -const svc = new ShareService(new DatabaseService(testDb), new SettingsService(new DatabaseService(testDb))); +const svc = new ShareService(new DatabaseService(testDb), new SettingsService(new DatabaseService(testDb)), permissionsStub); beforeAll(() => { createTables(testDb); diff --git a/server/tests/unit/nest/todo.service.test.ts b/server/tests/unit/nest/todo.service.test.ts index 88d899334..7b22dceb7 100644 --- a/server/tests/unit/nest/todo.service.test.ts +++ b/server/tests/unit/nest/todo.service.test.ts @@ -44,10 +44,11 @@ import { runMigrations } from '../../../src/db/migrations'; import { resetTestDb } from '../../helpers/test-db'; import { createUser, createTrip, addTripMember } from '../../helpers/factories'; import { DatabaseService } from '../../../src/nest/database/database.service'; +import { PermissionsService } from '../../../src/nest/permissions/permissions.service'; import { TodoService } from '../../../src/nest/todo/todo.service'; import { listItems, createItem, updateItem, deleteItem } from '../../../src/nest/todo/todo.bridge'; -const svc = new TodoService(new DatabaseService(testDb)); +const svc = new TodoService(new DatabaseService(testDb), new PermissionsService(new DatabaseService(testDb))); beforeAll(() => { createTables(testDb); diff --git a/server/tests/unit/nest/trip-invite.service.test.ts b/server/tests/unit/nest/trip-invite.service.test.ts index 59672edac..9bdbe2a69 100644 --- a/server/tests/unit/nest/trip-invite.service.test.ts +++ b/server/tests/unit/nest/trip-invite.service.test.ts @@ -37,9 +37,10 @@ import { runMigrations } from '../../../src/db/migrations'; import { resetTestDb } from '../../helpers/test-db'; import { createUser, createTrip } from '../../helpers/factories'; import { DatabaseService } from '../../../src/nest/database/database.service'; +import { PermissionsService } from '../../../src/nest/permissions/permissions.service'; import { TripInviteService } from '../../../src/nest/trip-invite/trip-invite.service'; -const svc = new TripInviteService(new DatabaseService(testDb)); +const svc = new TripInviteService(new DatabaseService(testDb), new PermissionsService(new DatabaseService(testDb))); beforeAll(() => { createTables(testDb); runMigrations(testDb); }); beforeEach(() => resetTestDb(testDb)); diff --git a/server/tests/unit/nest/trips.service.test.ts b/server/tests/unit/nest/trips.service.test.ts index 89b2d2883..820414f82 100644 --- a/server/tests/unit/nest/trips.service.test.ts +++ b/server/tests/unit/nest/trips.service.test.ts @@ -8,11 +8,12 @@ const { canAccessTrip } = vi.hoisted(() => ({ canAccessTrip: vi.fn(() => ({ user vi.mock('../../../src/db/database', () => ({ db: dbMock, canAccessTrip, closeDb: () => {}, reinitialize: () => {} })); import { db as dbConn } from '../../../src/db/database'; import { DatabaseService } from '../../../src/nest/database/database.service'; +import type { PermissionsService } from '../../../src/nest/permissions/permissions.service'; const { broadcast } = vi.hoisted(() => ({ broadcast: vi.fn() })); vi.mock('../../../src/websocket', () => ({ broadcast })); -const { checkPermission } = vi.hoisted(() => ({ checkPermission: vi.fn(() => true) })); -vi.mock('../../../src/services/permissions', () => ({ checkPermission })); +const checkPermission = vi.fn(() => true); +const permissionsStub = { checkPermission } as unknown as PermissionsService; const { tripSvc } = vi.hoisted(() => ({ tripSvc: { @@ -44,7 +45,7 @@ const packingStub = { listItems: packingListItems } as unknown as PackingService const filesStub = { listFiles: () => [] } as unknown as FilesService; const reservationsStub = { list: () => [] } as unknown as ReservationsService; const daysStub = { list: () => ({ days: [1] }), listAccommodations: () => [] } as unknown as DaysService; -function svc() { return new TripsService(new DatabaseService(dbConn), todoStub, packingStub, filesStub, reservationsStub, daysStub); } +function svc() { return new TripsService(new DatabaseService(dbConn), todoStub, packingStub, filesStub, reservationsStub, daysStub, permissionsStub); } beforeEach(() => vi.clearAllMocks()); describe('TripsService (wrapper delegation + bundle/copy/notify helpers)', () => { diff --git a/server/tests/unit/plugins/plugin-host-deps.factory.test.ts b/server/tests/unit/plugins/plugin-host-deps.factory.test.ts index e46628d32..eae25510b 100644 --- a/server/tests/unit/plugins/plugin-host-deps.factory.test.ts +++ b/server/tests/unit/plugins/plugin-host-deps.factory.test.ts @@ -4,9 +4,9 @@ * per-plugin data db is cached, a granted db:own call works through the wired * host, and trip broadcasts are force-namespaced to plugin:{id}:{event}. * DI-native domains (budget/reservations/tags/categories/todo/packing/ - * day-notes/days/assignments/oauth/llm-config/files/collab/vacay) are - * constructor-injected stubs; legacy services/* domains stay path-mocked until - * their own DI migration lands. + * day-notes/days/assignments/oauth/llm-config/files/collab/vacay/permissions) + * are constructor-injected stubs; legacy services/* domains stay path-mocked + * until their own DI migration lands. */ import { describe, it, expect, beforeAll, beforeEach, afterAll, vi } from 'vitest'; import fs from 'node:fs'; @@ -57,10 +57,11 @@ const budgetStub = { remove(id: string, _tid: string) { return id !== '404'; }, } as unknown as BudgetService; -// Edit permission — flip per test to exercise the gates. +// Edit permission — flip per test to exercise the gates (constructor-injected +// stub since the permissions DI migration). // eslint-disable-next-line @typescript-eslint/no-explicit-any -const { checkPermission } = vi.hoisted(() => ({ checkPermission: vi.fn((..._a: any[]) => true as boolean) })); -vi.mock('../../../src/services/permissions', () => ({ checkPermission })); +const checkPermission = vi.fn((..._a: any[]) => true as boolean); +const permissionsStub = { checkPermission } as unknown as PermissionsService; // The core write services are delegated to; mock them so the host deps' // wiring + error branches run without the full core schema. The error classes must @@ -320,12 +321,13 @@ import type { LlmConfigResolver } from '../../../src/nest/llm-parse/llm-config.r import type { FilesService } from '../../../src/nest/files/files.service'; import type { CollabService } from '../../../src/nest/collab/collab.service'; import type { VacayService } from '../../../src/nest/vacay/vacay.service'; +import type { PermissionsService } from '../../../src/nest/permissions/permissions.service'; import { DatabaseService } from '../../../src/nest/database/database.service'; // The factory under test, wired exactly like PluginsModule does — but with the // DI-native domain services replaced by the stubs above. The shim keeps the // ~45 historical call sites unchanged and supplies a default no-op router. -const factory = new PluginHostDepsFactory(budgetStub, reservationsStub, tagsStub, categoriesStub, todoStub, packingStub, oauthStub, dayNotesStub, assignmentsStub, llmConfigStub, new DatabaseService(mockDb), filesStub, collabStub, vacayStub, daysStub); +const factory = new PluginHostDepsFactory(budgetStub, reservationsStub, tagsStub, categoriesStub, todoStub, packingStub, oauthStub, dayNotesStub, assignmentsStub, llmConfigStub, new DatabaseService(mockDb), filesStub, collabStub, vacayStub, daysStub, permissionsStub); const stubRouter: PluginCallRouter = { callPlugin: async () => undefined, emitPluginEvent: () => {} }; const createRealRpcHost = (id: string, granted: ReadonlySet, router: PluginCallRouter = stubRouter) => factory.create(id, granted, router); diff --git a/server/tests/unit/services/authService.test.ts b/server/tests/unit/services/authService.test.ts index 0e10a5677..ec456d3c5 100644 --- a/server/tests/unit/services/authService.test.ts +++ b/server/tests/unit/services/authService.test.ts @@ -11,7 +11,7 @@ vi.mock('../../../src/services/apiKeyCrypto', () => ({ maybe_encrypt_api_key: vi.fn((v) => v), encrypt_api_key: vi.fn((v) => v), })); -vi.mock('../../../src/services/permissions', () => ({ getAllPermissions: vi.fn(() => ({})), checkPermission: vi.fn() })); +vi.mock('../../../src/nest/permissions/permissions.bridge', () => ({ getAllPermissions: vi.fn(() => ({})), checkPermission: vi.fn() })); vi.mock('../../../src/services/ephemeralTokens', () => ({ createEphemeralToken: vi.fn() })); vi.mock('../../../src/mcp', () => ({ revokeUserSessions: vi.fn() })); vi.mock('../../../src/scheduler', () => ({ startTripReminders: vi.fn(), buildCronExpression: vi.fn() })); diff --git a/server/tests/unit/services/authServiceDb.test.ts b/server/tests/unit/services/authServiceDb.test.ts index 3af694c7c..104aca89a 100644 --- a/server/tests/unit/services/authServiceDb.test.ts +++ b/server/tests/unit/services/authServiceDb.test.ts @@ -49,7 +49,7 @@ vi.mock('../../../src/services/apiKeyCrypto', () => ({ mask_stored_api_key: vi.fn((v: string | null | undefined) => (v ? '••••••••' : null)), encrypt_api_key: vi.fn((v) => v), })); -vi.mock('../../../src/services/permissions', () => ({ +vi.mock('../../../src/nest/permissions/permissions.bridge', () => ({ getAllPermissions: vi.fn(() => ({})), checkPermission: vi.fn(), })); diff --git a/server/tests/unit/services/conflictUpdate.test.ts b/server/tests/unit/services/conflictUpdate.test.ts index 48da8120a..a5fa81ee2 100644 --- a/server/tests/unit/services/conflictUpdate.test.ts +++ b/server/tests/unit/services/conflictUpdate.test.ts @@ -42,8 +42,9 @@ import { updatePlace, createPlace } from '../../../src/services/placeService'; import { isUpdateConflict } from '../../../src/services/conflictResult'; import { DatabaseService } from '../../../src/nest/database/database.service'; import { PackingService } from '../../../src/nest/packing/packing.service'; +import { PermissionsService } from '../../../src/nest/permissions/permissions.service'; -const packing = new PackingService(new DatabaseService(testDb)); +const packing = new PackingService(new DatabaseService(testDb), new PermissionsService(new DatabaseService(testDb))); beforeAll(() => { createTables(testDb); diff --git a/server/tests/unit/services/permissions.test.ts b/server/tests/unit/services/permissions.test.ts deleted file mode 100644 index eaa533823..000000000 --- a/server/tests/unit/services/permissions.test.ts +++ /dev/null @@ -1,114 +0,0 @@ -import { describe, it, expect, vi } from 'vitest'; - -// Mutable rows array so individual tests can inject DB rows -const dbRows: { key: string; value: string }[] = []; - -// Mock database — permissions module queries app_settings at runtime -vi.mock('../../../src/db/database', () => ({ - db: { - prepare: () => ({ - all: () => dbRows, // no custom permissions → fall back to defaults - run: vi.fn(), - get: vi.fn(), - }), - transaction: (fn: () => void) => fn, - }, -})); - -import { checkPermission, getPermissionLevel, savePermissions, invalidatePermissionsCache, PERMISSION_ACTIONS } from '../../../src/services/permissions'; - -describe('permissions', () => { - describe('checkPermission — admin bypass', () => { - it('admin always passes regardless of permission level', () => { - for (const action of PERMISSION_ACTIONS) { - expect(checkPermission(action.key, 'admin', 1, 1, false)).toBe(true); - expect(checkPermission(action.key, 'admin', 99, 1, false)).toBe(true); - } - }); - }); - - describe('checkPermission — everybody level', () => { - it('trip_create (everybody) allows any authenticated user', () => { - expect(checkPermission('trip_create', 'user', null, 42, false)).toBe(true); - }); - }); - - describe('checkPermission — trip_owner level', () => { - const ownerId = 10; - const memberId = 20; - - it('trip owner passes trip_owner check', () => { - expect(checkPermission('trip_delete', 'user', ownerId, ownerId, false)).toBe(true); - }); - - it('member fails trip_owner check', () => { - expect(checkPermission('trip_delete', 'user', ownerId, memberId, true)).toBe(false); - }); - - it('non-member non-owner fails trip_owner check', () => { - expect(checkPermission('trip_delete', 'user', ownerId, memberId, false)).toBe(false); - }); - }); - - describe('checkPermission — trip_member level', () => { - const ownerId = 10; - const memberId = 20; - const outsiderId = 30; - - it('trip owner passes trip_member check', () => { - expect(checkPermission('day_edit', 'user', ownerId, ownerId, false)).toBe(true); - }); - - it('trip member passes trip_member check', () => { - expect(checkPermission('day_edit', 'user', ownerId, memberId, true)).toBe(true); - }); - - it('outsider fails trip_member check', () => { - expect(checkPermission('day_edit', 'user', ownerId, outsiderId, false)).toBe(false); - }); - }); - - describe('getPermissionLevel — defaults', () => { - it('returns default level for known actions (no DB overrides)', () => { - const defaults: Record = { - trip_create: 'everybody', - trip_delete: 'trip_owner', - day_edit: 'trip_member', - budget_edit: 'trip_member', - }; - for (const [key, expected] of Object.entries(defaults)) { - expect(getPermissionLevel(key)).toBe(expected); - } - }); - - it('returns trip_owner for unknown action key', () => { - expect(getPermissionLevel('nonexistent_action')).toBe('trip_owner'); - }); - }); - - describe('savePermissions — invalid action key is skipped', () => { - it('returns skipped array containing invalid action key', () => { - const result = savePermissions({ nonexistent_action: 'trip_member' }); - expect(result.skipped).toContain('nonexistent_action'); - }); - - it('returns skipped array when level is not in allowedLevels for the action', () => { - // trip_delete only allows ['admin', 'trip_owner'], so 'trip_member' is invalid - const result = savePermissions({ trip_delete: 'trip_member' }); - expect(result.skipped).toContain('trip_delete'); - }); - }); - - describe('checkPermission — default case', () => { - it('returns false when permission level is an unrecognized value', () => { - // Inject a DB row with an unknown level for trip_edit, then invalidate cache - dbRows.push({ key: 'perm_trip_edit', value: 'unknown_level' }); - invalidatePermissionsCache(); - const result = checkPermission('trip_edit', 'user', 10, 10, false); - // Clean up for subsequent tests - dbRows.length = 0; - invalidatePermissionsCache(); - expect(result).toBe(false); - }); - }); -}); diff --git a/server/tests/unit/services/tripService.test.ts b/server/tests/unit/services/tripService.test.ts index 0bbf148ff..66cab408a 100644 --- a/server/tests/unit/services/tripService.test.ts +++ b/server/tests/unit/services/tripService.test.ts @@ -37,9 +37,10 @@ import { createUser, createTrip, createReservation, createPlace, createDay, crea import { exportICS, generateDays, deleteOldCover, updateTrip, transferOwnership, createGuest, renameGuest, deleteGuest, listMembers, addMember } from '../../../src/services/tripService'; import { DatabaseService } from '../../../src/nest/database/database.service'; import { DaysService } from '../../../src/nest/days/days.service'; +import { PermissionsService } from '../../../src/nest/permissions/permissions.service'; import fs from 'fs'; -const daysSvc = new DaysService(new DatabaseService(testDb)); +const daysSvc = new DaysService(new DatabaseService(testDb), new PermissionsService(new DatabaseService(testDb))); const createAccommodation = daysSvc.createAccommodation.bind(daysSvc); beforeAll(() => {