From 22d178d2f9f1435e2d64028b7920abe6373139c1 Mon Sep 17 00:00:00 2001 From: Maurice <61554723+mauriceboe@users.noreply.github.com> Date: Fri, 7 Aug 2026 18:42:53 +0200 Subject: [PATCH] refactor(server): one addon guard, one demo-mode condition (#1829) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two pieces of copy-paste in the guard layer. Three addon guards — collections, airtrail, journey — did the same thing and differed only in the addon id they read and the word leading the 404. They are one AddonGuard plus a @RequireAddon(addonId, label) decorator now, read through the Reflector. A handler-level decorator overrides the controller's, and a route without the decorator is not gated at all. The demo-mode upload block was copy-pasted into six handlers, each carrying its own copy of the message. All six call isDemoWriteBlocked now, against the same DEMO_WRITE_ERROR body. That one is deliberately NOT a guard, though all six sites are handlers and a guard is the obvious shape — and this PR did build one first. Guards run before the multipart parser, so throwing from one leaves the request body unread and Node resets the connection: the client gets ECONNRESET instead of the 403. PROFILE-015 caught it. The check has to sit inside the handler, after the interceptor has drained the upload, for the response to arrive at all. The function carries that reasoning so the next person does not rediscover it. Two smaller findings on the way there. The DemoWriteGuard attempt broke eight e2e suites that never touch demo mode: AuthModule listed it as a provider, and RuntimeEnvService was not resolvable inside a TestingModule built around one domain module. AppConfigModule is @Global, but @Global only applies to modules that are in the graph. The five modules whose controllers now inject RuntimeEnvService import it explicitly, the same way PermissionsModule is handled. And both new guards get a test asserting the decorators are actually applied. Once a check moves out of a handler, that handler's own tests stop noticing if the decorator is dropped — the assertions on __guards__ and the @RequireAddon metadata are what keeps the four upload endpoints and the four addon route groups gated, and gated on the right addon. --- server/src/nest/addons/addon.guard.ts | 33 +++++ server/src/nest/addons/addons.module.ts | 5 +- .../nest/addons/require-addon.decorator.ts | 24 ++++ server/src/nest/auth/auth.controller.ts | 10 +- server/src/nest/auth/auth.module.ts | 3 +- .../collections/collections-addon.guard.ts | 20 --- .../collections/collections.controller.ts | 21 ++-- .../nest/collections/collections.module.ts | 6 +- server/src/nest/common/demo-write.ts | 24 ++++ server/src/nest/files/files.controller.ts | 15 ++- server/src/nest/files/files.module.ts | 3 +- .../nest/integrations/airtrail-addon.guard.ts | 21 ---- .../airtrail-import.controller.ts | 7 +- .../nest/integrations/airtrail.controller.ts | 7 +- .../src/nest/integrations/airtrail.module.ts | 2 - .../src/nest/journey/journey-addon.guard.ts | 20 --- server/src/nest/journey/journey.controller.ts | 7 +- server/src/nest/journey/journey.module.ts | 3 +- server/src/nest/places/places.controller.ts | 15 ++- server/src/nest/places/places.module.ts | 3 +- server/src/nest/trips/trips.controller.ts | 10 +- server/src/nest/trips/trips.module.ts | 3 +- server/tests/unit/nest/addon-guard.test.ts | 108 ++++++++++++++++ .../tests/unit/nest/auth.controller.test.ts | 3 +- .../unit/nest/collections.controller.test.ts | 51 ++++---- server/tests/unit/nest/demo-write.test.ts | 62 ++++++++++ .../tests/unit/nest/files.controller.test.ts | 67 +++++----- .../unit/nest/journey.controller.test.ts | 8 -- .../tests/unit/nest/places.controller.test.ts | 115 +++++++++--------- .../tests/unit/nest/trips.controller.test.ts | 3 +- 30 files changed, 445 insertions(+), 234 deletions(-) create mode 100644 server/src/nest/addons/addon.guard.ts create mode 100644 server/src/nest/addons/require-addon.decorator.ts delete mode 100644 server/src/nest/collections/collections-addon.guard.ts create mode 100644 server/src/nest/common/demo-write.ts delete mode 100644 server/src/nest/integrations/airtrail-addon.guard.ts delete mode 100644 server/src/nest/journey/journey-addon.guard.ts create mode 100644 server/tests/unit/nest/addon-guard.test.ts create mode 100644 server/tests/unit/nest/demo-write.test.ts diff --git a/server/src/nest/addons/addon.guard.ts b/server/src/nest/addons/addon.guard.ts new file mode 100644 index 000000000..2dadbb787 --- /dev/null +++ b/server/src/nest/addons/addon.guard.ts @@ -0,0 +1,33 @@ +import { CanActivate, ExecutionContext, HttpException, Injectable } from '@nestjs/common'; +import { Reflector } from '@nestjs/core'; +import { AddonsService } from './addons.service'; +import { REQUIRE_ADDON, type RequireAddonMeta } from './require-addon.decorator'; + +/** + * Enforces @RequireAddon. Replaces the three hand-written addon guards + * (collections, airtrail, journey), which differed only in which addon id they + * read and which word led the 404 body. + * + * A handler-level @RequireAddon overrides the controller's, so a route group + * can carry one addon and a single endpoint another. Without the decorator the + * guard is a no-op — it never gates a route that did not ask for it. + */ +@Injectable() +export class AddonGuard implements CanActivate { + constructor( + private readonly addons: AddonsService, + private readonly reflector: Reflector, + ) {} + + canActivate(context: ExecutionContext): boolean { + const meta = this.reflector.getAllAndOverride(REQUIRE_ADDON, [ + context.getHandler(), + context.getClass(), + ]); + if (!meta) return true; + if (!this.addons.isAddonEnabled(meta.addonId)) { + throw new HttpException({ error: `${meta.label} addon is not enabled` }, 404); + } + return true; + } +} diff --git a/server/src/nest/addons/addons.module.ts b/server/src/nest/addons/addons.module.ts index 2a7bb1d5d..75fd6c6d3 100644 --- a/server/src/nest/addons/addons.module.ts +++ b/server/src/nest/addons/addons.module.ts @@ -1,6 +1,7 @@ import { Module } from '@nestjs/common'; import { AddonsController } from './addons.controller'; import { AddonsService } from './addons.service'; +import { AddonGuard } from './addon.guard'; /** * GET /api/addons — enabled add-ons + photo providers (was an inline handler in @@ -15,7 +16,7 @@ import { AddonsService } from './addons.service'; */ @Module({ controllers: [AddonsController], - providers: [AddonsService], - exports: [AddonsService], + providers: [AddonsService, AddonGuard], + exports: [AddonsService, AddonGuard], }) export class AddonsModule {} diff --git a/server/src/nest/addons/require-addon.decorator.ts b/server/src/nest/addons/require-addon.decorator.ts new file mode 100644 index 000000000..296828520 --- /dev/null +++ b/server/src/nest/addons/require-addon.decorator.ts @@ -0,0 +1,24 @@ +import { SetMetadata } from '@nestjs/common'; + +export const REQUIRE_ADDON = 'trek:require-addon'; + +export interface RequireAddonMeta { + addonId: string; + /** Leads the 404 body: `${label} addon is not enabled`. */ + label: string; +} + +/** + * Gate a controller (or a single handler) on a global addon toggle. When the + * admin has the addon off, the route answers 404 as if it did not exist. + * + * Pair it with AddonGuard, and list that guard BEFORE JwtAuthGuard: + * + * @UseGuards(AddonGuard, JwtAuthGuard) + * @RequireAddon(ADDON_IDS.JOURNEY, 'Journey') + * + * The order is the contract, not a preference — a disabled addon has to answer + * 404 to anonymous callers too, so the addon check has to run before the 401. + */ +export const RequireAddon = (addonId: string, label: string) => + SetMetadata(REQUIRE_ADDON, { addonId, label }); diff --git a/server/src/nest/auth/auth.controller.ts b/server/src/nest/auth/auth.controller.ts index e5b0eb432..c1b11012c 100644 --- a/server/src/nest/auth/auth.controller.ts +++ b/server/src/nest/auth/auth.controller.ts @@ -15,12 +15,13 @@ import { UseInterceptors, } from '@nestjs/common'; import { FileInterceptor } from '@nestjs/platform-express'; +import { isDemoWriteBlocked, DEMO_WRITE_ERROR } from '../common/demo-write'; +import { RuntimeEnvService } from '../app-config/runtime-env.service'; import { diskStorage } from 'multer'; import type { Request, Response } from 'express'; import path from 'path'; import fs from 'fs'; import { v4 as uuid } from 'uuid'; -import { readEnv } from '../../app-config'; import { AuthService } from './auth.service'; import { avatarDir } from './auth.helpers'; import { @@ -39,7 +40,6 @@ import { JwtAuthGuard } from './jwt-auth.guard'; import { CurrentUser } from './current-user.decorator'; import { getClientIp } from '../audit/client-ip'; import { AuditService } from '../audit/audit.service'; -import { isDemoEmail } from '../common/demo'; import type { User } from '../../types'; const WINDOW = 15 * 60 * 1000; @@ -72,7 +72,7 @@ const AVATAR_UPLOAD = { @Controller('api/auth') @UseGuards(JwtAuthGuard) export class AuthController { - constructor(private readonly auth: AuthService, private readonly rl: RateLimitService, private readonly audit: AuditService) {} + constructor(private readonly auth: AuthService, private readonly rl: RateLimitService, private readonly audit: AuditService, private readonly env: RuntimeEnvService) {} private limit(bucket: string, req: Request, max: number): void { if (!this.rl.check(bucket, req.ip || 'unknown', max, WINDOW, Date.now())) { @@ -145,8 +145,8 @@ export class AuthController { @HttpCode(200) @UseInterceptors(FileInterceptor('avatar', AVATAR_UPLOAD)) async avatar(@CurrentUser() user: User, @UploadedFile() file: Express.Multer.File | undefined) { - if (readEnv().demo.enabled && isDemoEmail(user.email)) { - throw new HttpException({ error: 'Uploads are disabled in demo mode. Self-host TREK for full functionality.' }, 403); + if (isDemoWriteBlocked(this.env, user.email)) { + throw new HttpException(DEMO_WRITE_ERROR, 403); } if (!file) { throw new HttpException({ error: 'No image uploaded' }, 400); diff --git a/server/src/nest/auth/auth.module.ts b/server/src/nest/auth/auth.module.ts index 8a82cdf37..5f756b25b 100644 --- a/server/src/nest/auth/auth.module.ts +++ b/server/src/nest/auth/auth.module.ts @@ -5,6 +5,7 @@ import { AuthController } from './auth.controller'; import { PasskeyController } from './passkey.controller'; import { AuthService } from './auth.service'; import { PasskeyService } from './passkey.service'; +import { AppConfigModule } from '../app-config/app-config.module'; import { AuditModule } from '../audit/audit.module'; import { PermissionsModule } from '../permissions/permissions.module'; import { AtlasModule } from '../atlas/atlas.module'; @@ -23,7 +24,7 @@ import { TripMembershipModule } from '../trip-membership/trip-membership.module' * everything outside the container goes through auth.bridge.ts. */ @Module({ - imports: [RateLimitModule, AuditModule, PermissionsModule, AtlasModule, TripMembershipModule], + imports: [RateLimitModule, AuditModule, PermissionsModule, AtlasModule, TripMembershipModule, AppConfigModule], controllers: [AuthPublicController, AuthController, PasskeyController], providers: [AuthService, PasskeyService], exports: [AuthService, PasskeyService], diff --git a/server/src/nest/collections/collections-addon.guard.ts b/server/src/nest/collections/collections-addon.guard.ts deleted file mode 100644 index 40de86956..000000000 --- a/server/src/nest/collections/collections-addon.guard.ts +++ /dev/null @@ -1,20 +0,0 @@ -import { CanActivate, HttpException, Injectable } from '@nestjs/common'; -import { AddonsService } from '../addons/addons.service'; -import { ADDON_IDS } from '../../addons'; - -/** - * Mirrors the addon mount gate: when the Collections addon is disabled the whole - * route group answers 404, regardless of auth. Declared before JwtAuthGuard so - * the addon check wins over the 401, exactly as the Journey addon guard does. - */ -@Injectable() -export class CollectionsAddonGuard implements CanActivate { - constructor(private readonly addons: AddonsService) {} - - canActivate(): boolean { - if (!this.addons.isAddonEnabled(ADDON_IDS.COLLECTIONS)) { - throw new HttpException({ error: 'Collections addon is not enabled' }, 404); - } - return true; - } -} diff --git a/server/src/nest/collections/collections.controller.ts b/server/src/nest/collections/collections.controller.ts index 02a704159..94c686778 100644 --- a/server/src/nest/collections/collections.controller.ts +++ b/server/src/nest/collections/collections.controller.ts @@ -16,20 +16,22 @@ import { UseInterceptors, } from '@nestjs/common'; import { FileInterceptor } from '@nestjs/platform-express'; +import { isDemoWriteBlocked, DEMO_WRITE_ERROR } from '../common/demo-write'; +import { RuntimeEnvService } from '../app-config/runtime-env.service'; import type { Request } from 'express'; import { diskStorage } from 'multer'; import path from 'path'; import fs from 'fs'; -import { readEnv } from '../../app-config'; import { v4 as uuidv4 } from 'uuid'; import type { User } from '../../types'; import { CollectionsService } from './collections.service'; -import { CollectionsAddonGuard } from './collections-addon.guard'; +import { AddonGuard } from '../addons/addon.guard'; +import { RequireAddon } from '../addons/require-addon.decorator'; +import { ADDON_IDS } from '../../addons'; import { JwtAuthGuard } from '../auth/jwt-auth.guard'; import { CurrentUser } from '../auth/current-user.decorator'; import { PLACE_IMAGE_UPLOAD } from '../common/place-image-upload'; import { placeImageUrl } from '../../services/placeImage'; -import { isDemoEmail } from '../common/demo'; import { CollectionCreateDto, CollectionUpdateDto, @@ -82,9 +84,10 @@ const COVER_UPLOAD = { * excluded from the WS broadcast. */ @Controller('api/addons/collections') -@UseGuards(CollectionsAddonGuard, JwtAuthGuard) +@UseGuards(AddonGuard, JwtAuthGuard) +@RequireAddon(ADDON_IDS.COLLECTIONS, 'Collections') export class CollectionsController { - constructor(private readonly collections: CollectionsService) {} + constructor(private readonly collections: CollectionsService, private readonly env: RuntimeEnvService) {} // ── Lists ───────────────────────────────────────────────────────────────── @Get() @@ -174,8 +177,8 @@ export class CollectionsController { @UploadedFile() file: Express.Multer.File | undefined, @Headers('x-socket-id') socketId?: string, ) { - if (readEnv().demo.enabled && isDemoEmail(user.email)) { - throw new HttpException({ error: 'Uploads are disabled in demo mode. Self-host TREK for full functionality.' }, 403); + if (isDemoWriteBlocked(this.env, user.email)) { + throw new HttpException(DEMO_WRITE_ERROR, 403); } if (!file) throw new HttpException({ error: 'No image uploaded' }, 400); return this.collections.setPlaceImage(user.id, Number(pid), placeImageUrl(file.filename), socketId); @@ -345,8 +348,8 @@ export class CollectionsController { @Post(':id/cover') @UseInterceptors(FileInterceptor('cover', COVER_UPLOAD)) uploadCover(@CurrentUser() user: User, @Param('id') id: string, @UploadedFile() file: Express.Multer.File | undefined, @Headers('x-socket-id') socketId?: string) { - if (readEnv().demo.enabled && isDemoEmail(user.email)) { - throw new HttpException({ error: 'Uploads are disabled in demo mode. Self-host TREK for full functionality.' }, 403); + if (isDemoWriteBlocked(this.env, user.email)) { + throw new HttpException(DEMO_WRITE_ERROR, 403); } if (!file) throw new HttpException({ error: 'No image uploaded' }, 400); const coverUrl = `/uploads/covers/${file.filename}`; diff --git a/server/src/nest/collections/collections.module.ts b/server/src/nest/collections/collections.module.ts index 3301e7038..65fb52efa 100644 --- a/server/src/nest/collections/collections.module.ts +++ b/server/src/nest/collections/collections.module.ts @@ -1,8 +1,8 @@ import { Module } from '@nestjs/common'; import { CollectionsController } from './collections.controller'; import { CollectionsService } from './collections.service'; +import { AppConfigModule } from '../app-config/app-config.module'; import { CollectionsMcp } from './collections.mcp'; -import { CollectionsAddonGuard } from './collections-addon.guard'; import { AddonsModule } from '../addons/addons.module'; import { PermissionsModule } from '../permissions/permissions.module'; import { AuthModule } from '../auth/auth.module'; @@ -11,9 +11,9 @@ import { AuthModule } from '../auth/auth.module'; * Exports CollectionsService for in-container consumers (PluginsModule's * RPC host deps factory). */ @Module({ - imports: [AddonsModule, PermissionsModule, AuthModule], + imports: [AddonsModule, PermissionsModule, AuthModule, AppConfigModule], controllers: [CollectionsController], - providers: [CollectionsService, CollectionsMcp, CollectionsAddonGuard], + providers: [CollectionsService, CollectionsMcp], exports: [CollectionsService], }) export class CollectionsModule {} diff --git a/server/src/nest/common/demo-write.ts b/server/src/nest/common/demo-write.ts new file mode 100644 index 000000000..73ee8224a --- /dev/null +++ b/server/src/nest/common/demo-write.ts @@ -0,0 +1,24 @@ +import { RuntimeEnvService } from '../app-config/runtime-env.service'; +import { isDemoEmail } from './demo'; + +/** The exact 403 body the six upload endpoints have always returned. */ +export const DEMO_WRITE_ERROR = { + error: 'Uploads are disabled in demo mode. Self-host TREK for full functionality.', +} as const; + +/** + * Whether this user's writes are blocked because the instance runs in demo mode. + * + * The condition was copy-pasted into six upload handlers, each with its own copy + * of the message. It lives here once now, and every one of them calls it. + * + * Deliberately a function and not a guard, though all six sites are handlers and + * a guard is the obvious shape. Guards run before the multipart parser, so + * throwing from one leaves the request body unread and Node resets the + * connection: the client sees ECONNRESET instead of the 403. PROFILE-015 catches + * exactly that. Checking inside the handler, after the interceptor has drained + * the upload, is what makes the response arrive. + */ +export function isDemoWriteBlocked(env: RuntimeEnvService, email: string | null | undefined): boolean { + return env.isDemoMode() && isDemoEmail(email); +} diff --git a/server/src/nest/files/files.controller.ts b/server/src/nest/files/files.controller.ts index c637d722d..da4b2fba6 100644 --- a/server/src/nest/files/files.controller.ts +++ b/server/src/nest/files/files.controller.ts @@ -16,11 +16,12 @@ import { UseInterceptors, } from '@nestjs/common'; import { FileInterceptor } from '@nestjs/platform-express'; +import { isDemoWriteBlocked, DEMO_WRITE_ERROR } from '../common/demo-write'; +import { RuntimeEnvService } from '../app-config/runtime-env.service'; import { diskStorage } from 'multer'; import path from 'path'; import fs from 'fs'; import { v4 as uuidv4 } from 'uuid'; -import { readEnv } from '../../app-config'; import type { User } from '../../types'; import { FilesService } from './files.service'; import { JwtAuthGuard } from '../auth/jwt-auth.guard'; @@ -28,7 +29,6 @@ import { CurrentUser } from '../auth/current-user.decorator'; import { MAX_FILE_SIZE, MAX_VIDEO_SIZE, BLOCKED_EXTENSIONS, filesDir, isVideoExtension } from './files.constants'; import { getAllowedExtensions } from './files.bridge'; import { FileUploadDto, FileUpdateDto, FileLinkDto } from './files.dto'; -import { isDemoEmail } from '../common/demo'; const UPLOAD = { storage: diskStorage({ @@ -69,7 +69,10 @@ const UPLOAD = { @Controller('api/trips/:tripId/files') @UseGuards(JwtAuthGuard) export class FilesController { - constructor(private readonly files: FilesService) {} + constructor( + private readonly files: FilesService, + private readonly env: RuntimeEnvService, + ) {} private requireTrip(tripId: string, user: User) { const trip = this.files.verifyTripAccess(tripId, user.id); @@ -109,8 +112,10 @@ export class FilesController { const cleanup = () => { if (file?.path) { try { fs.unlinkSync(file.path); } catch { /* best-effort */ } } }; try { const trip = this.requireTrip(tripId, user); - if (readEnv().demo.enabled && isDemoEmail(user.email)) { - throw new HttpException({ error: 'Uploads are disabled in demo mode. Self-host TREK for full functionality.' }, 403); + // Inline rather than DemoWriteGuard: requireTrip above answers 404 for a + // trip the caller cannot reach, and a guard would run before it. + if (isDemoWriteBlocked(this.env, user.email)) { + throw new HttpException(DEMO_WRITE_ERROR, 403); } if (!this.files.can('file_upload', trip, user)) { throw new HttpException({ error: 'No permission to upload files' }, 403); diff --git a/server/src/nest/files/files.module.ts b/server/src/nest/files/files.module.ts index c90e5fe13..a307bb3f7 100644 --- a/server/src/nest/files/files.module.ts +++ b/server/src/nest/files/files.module.ts @@ -3,9 +3,10 @@ import { FilesController } from './files.controller'; import { FilesDownloadController } from './files-download.controller'; import { FilesService } from './files.service'; import { PermissionsModule } from '../permissions/permissions.module'; +import { AppConfigModule } from '../app-config/app-config.module'; @Module({ - imports: [PermissionsModule], + imports: [PermissionsModule, AppConfigModule], controllers: [FilesController, FilesDownloadController], providers: [FilesService], exports: [FilesService], diff --git a/server/src/nest/integrations/airtrail-addon.guard.ts b/server/src/nest/integrations/airtrail-addon.guard.ts deleted file mode 100644 index c592813dc..000000000 --- a/server/src/nest/integrations/airtrail-addon.guard.ts +++ /dev/null @@ -1,21 +0,0 @@ -import { CanActivate, HttpException, Injectable } from '@nestjs/common'; -import { AddonsService } from '../addons/addons.service'; -import { ADDON_IDS } from '../../addons'; - -/** - * Gates the AirTrail integration routes on the global `airtrail` addon. When the - * admin has it disabled the whole group answers 404. Declared before the - * JwtAuthGuard so the addon check wins over the 401 (same ordering as the - * Journey addon gate). - */ -@Injectable() -export class AirtrailAddonGuard implements CanActivate { - constructor(private readonly addons: AddonsService) {} - - canActivate(): boolean { - if (!this.addons.isAddonEnabled(ADDON_IDS.AIRTRAIL)) { - throw new HttpException({ error: 'AirTrail addon is not enabled' }, 404); - } - return true; - } -} diff --git a/server/src/nest/integrations/airtrail-import.controller.ts b/server/src/nest/integrations/airtrail-import.controller.ts index 26e160a97..764b7d2fe 100644 --- a/server/src/nest/integrations/airtrail-import.controller.ts +++ b/server/src/nest/integrations/airtrail-import.controller.ts @@ -2,7 +2,9 @@ import { Body, Controller, Headers, HttpException, Param, Post, UseGuards } from import type { User } from '../../types'; import { JwtAuthGuard } from '../auth/jwt-auth.guard'; import { CurrentUser } from '../auth/current-user.decorator'; -import { AirtrailAddonGuard } from './airtrail-addon.guard'; +import { AddonGuard } from '../addons/addon.guard'; +import { RequireAddon } from '../addons/require-addon.decorator'; +import { ADDON_IDS } from '../../addons'; import { AirtrailImportDto } from './airtrail.dto'; import type { AirtrailImportResult } from '@trek/shared'; import { PermissionsService } from '../permissions/permissions.service'; @@ -15,7 +17,8 @@ import { importAirtrailFlights } from '../../services/airtrail/airtrailImport'; * flights are re-fetched server-side with the caller's own key. */ @Controller('api/trips/:tripId/reservations/import') -@UseGuards(AirtrailAddonGuard, JwtAuthGuard) +@UseGuards(AddonGuard, JwtAuthGuard) +@RequireAddon(ADDON_IDS.AIRTRAIL, 'AirTrail') export class AirtrailImportController { constructor( private readonly permissions: PermissionsService, diff --git a/server/src/nest/integrations/airtrail.controller.ts b/server/src/nest/integrations/airtrail.controller.ts index 7bcf72e78..a85e0de2b 100644 --- a/server/src/nest/integrations/airtrail.controller.ts +++ b/server/src/nest/integrations/airtrail.controller.ts @@ -3,7 +3,9 @@ import type { Request } from 'express'; import type { User } from '../../types'; import { JwtAuthGuard } from '../auth/jwt-auth.guard'; import { CurrentUser } from '../auth/current-user.decorator'; -import { AirtrailAddonGuard } from './airtrail-addon.guard'; +import { AddonGuard } from '../addons/addon.guard'; +import { RequireAddon } from '../addons/require-addon.decorator'; +import { ADDON_IDS } from '../../addons'; import { AirtrailSettingsDto } from './airtrail.dto'; import { getClientIp } from '../audit/client-ip'; import { @@ -24,7 +26,8 @@ import { runAirtrailSyncForUser } from '../../services/airtrail/airtrailSync'; * gated on the `airtrail` addon (404 when disabled). */ @Controller('api/integrations/airtrail') -@UseGuards(AirtrailAddonGuard, JwtAuthGuard) +@UseGuards(AddonGuard, JwtAuthGuard) +@RequireAddon(ADDON_IDS.AIRTRAIL, 'AirTrail') export class AirtrailController { @Get('settings') getSettings(@CurrentUser() user: User) { diff --git a/server/src/nest/integrations/airtrail.module.ts b/server/src/nest/integrations/airtrail.module.ts index c8e50a8b4..ec8999c5c 100644 --- a/server/src/nest/integrations/airtrail.module.ts +++ b/server/src/nest/integrations/airtrail.module.ts @@ -3,7 +3,6 @@ import { AirtrailController } from './airtrail.controller'; import { AirtrailImportController } from './airtrail-import.controller'; import { PermissionsModule } from '../permissions/permissions.module'; import { AddonsModule } from '../addons/addons.module'; -import { AirtrailAddonGuard } from './airtrail-addon.guard'; /** * AirTrail integration domain. The connection lives under @@ -14,6 +13,5 @@ import { AirtrailAddonGuard } from './airtrail-addon.guard'; @Module({ imports: [PermissionsModule, AddonsModule], controllers: [AirtrailController, AirtrailImportController], - providers: [AirtrailAddonGuard], }) export class AirtrailModule {} diff --git a/server/src/nest/journey/journey-addon.guard.ts b/server/src/nest/journey/journey-addon.guard.ts deleted file mode 100644 index 0e5c9ed3e..000000000 --- a/server/src/nest/journey/journey-addon.guard.ts +++ /dev/null @@ -1,20 +0,0 @@ -import { CanActivate, HttpException, Injectable } from '@nestjs/common'; -import { JourneyService } from './journey.service'; - -/** - * Mirrors the legacy `/api/journeys` mount gate: when the Journey addon is - * disabled the whole route group answers 404, regardless of auth. Declared - * before the JwtAuthGuard so the addon check wins over the 401, exactly as the - * Express middleware ordering did. - */ -@Injectable() -export class JourneyAddonGuard implements CanActivate { - constructor(private readonly journey: JourneyService) {} - - canActivate(): boolean { - if (!this.journey.journeyAddonEnabled()) { - throw new HttpException({ error: 'Journey addon is not enabled' }, 404); - } - return true; - } -} diff --git a/server/src/nest/journey/journey.controller.ts b/server/src/nest/journey/journey.controller.ts index c0c13ab72..062b2fee6 100644 --- a/server/src/nest/journey/journey.controller.ts +++ b/server/src/nest/journey/journey.controller.ts @@ -22,7 +22,9 @@ import fs from 'node:fs'; import crypto from 'node:crypto'; import type { User } from '../../types'; import { JourneyService } from './journey.service'; -import { JourneyAddonGuard } from './journey-addon.guard'; +import { AddonGuard } from '../addons/addon.guard'; +import { RequireAddon } from '../addons/require-addon.decorator'; +import { ADDON_IDS } from '../../addons'; import { JwtAuthGuard } from '../auth/jwt-auth.guard'; import { CurrentUser } from '../auth/current-user.decorator'; import { isVideoMime, isVideoExtension, MAX_VIDEO_SIZE } from '../files/files.constants'; @@ -99,7 +101,8 @@ const VIDEO_UPLOAD = { * are declared before /:id so they win over the param. */ @Controller('api/journeys') -@UseGuards(JourneyAddonGuard, JwtAuthGuard) +@UseGuards(AddonGuard, JwtAuthGuard) +@RequireAddon(ADDON_IDS.JOURNEY, 'Journey') export class JourneyController { constructor(private readonly journey: JourneyService) {} diff --git a/server/src/nest/journey/journey.module.ts b/server/src/nest/journey/journey.module.ts index 57aabbefd..6d23abb2d 100644 --- a/server/src/nest/journey/journey.module.ts +++ b/server/src/nest/journey/journey.module.ts @@ -2,12 +2,11 @@ import { Module } from '@nestjs/common'; import { JourneyController } from './journey.controller'; import { JourneyPublicController } from './journey-public.controller'; import { JourneyService } from './journey.service'; -import { JourneyAddonGuard } from './journey-addon.guard'; import { AddonsModule } from '../addons/addons.module'; @Module({ imports: [AddonsModule], controllers: [JourneyController, JourneyPublicController], - providers: [JourneyService, JourneyAddonGuard], + providers: [JourneyService], }) export class JourneyModule {} diff --git a/server/src/nest/places/places.controller.ts b/server/src/nest/places/places.controller.ts index 79761a493..ca5960f7e 100644 --- a/server/src/nest/places/places.controller.ts +++ b/server/src/nest/places/places.controller.ts @@ -15,9 +15,10 @@ import { UseInterceptors, } from '@nestjs/common'; import { FileInterceptor } from '@nestjs/platform-express'; +import { isDemoWriteBlocked, DEMO_WRITE_ERROR } from '../common/demo-write'; +import { RuntimeEnvService } from '../app-config/runtime-env.service'; import { memoryStorage } from 'multer'; import { hexColorSchema } from '@trek/shared'; -import { readEnv } from '../../app-config'; import type { User } from '../../types'; import { PlacesService } from './places.service'; import { isUpdateConflict } from '../common/conflictResult'; @@ -25,7 +26,6 @@ import { JwtAuthGuard } from '../auth/jwt-auth.guard'; import { CurrentUser } from '../auth/current-user.decorator'; import { PLACE_IMAGE_UPLOAD } from '../common/place-image-upload'; import { placeImageUrl } from '../../services/placeImage'; -import { isDemoEmail } from '../common/demo'; import { PlaceBulkDeleteDto, PlaceBulkUpdateDto, @@ -86,7 +86,10 @@ function parseBool(v: unknown, defaultVal: boolean): boolean { @Controller('api/trips/:tripId/places') @UseGuards(JwtAuthGuard) export class PlacesController { - constructor(private readonly places: PlacesService) {} + constructor( + private readonly places: PlacesService, + private readonly env: RuntimeEnvService, + ) {} private requireTrip(tripId: string, user: User) { const trip = this.places.verifyTripAccess(tripId, user.id); @@ -309,8 +312,10 @@ export class PlacesController { ) { const trip = this.requireTrip(tripId, user); this.requireEdit(trip, user); - if (readEnv().demo.enabled && isDemoEmail(user.email)) { - throw new HttpException({ error: 'Uploads are disabled in demo mode. Self-host TREK for full functionality.' }, 403); + // Inline rather than DemoWriteGuard: requireEdit above answers 404/403 for a + // trip the caller cannot reach, and a guard would run before it. + if (isDemoWriteBlocked(this.env, user.email)) { + throw new HttpException(DEMO_WRITE_ERROR, 403); } if (!file) { throw new HttpException({ error: 'No image uploaded' }, 400); diff --git a/server/src/nest/places/places.module.ts b/server/src/nest/places/places.module.ts index c0a72fb5b..ffbf86121 100644 --- a/server/src/nest/places/places.module.ts +++ b/server/src/nest/places/places.module.ts @@ -3,6 +3,7 @@ import { PlacesController } from './places.controller'; import { PlacesService } from './places.service'; import { PlacesMcp } from './places.mcp'; import { PermissionsModule } from '../permissions/permissions.module'; +import { AppConfigModule } from '../app-config/app-config.module'; import { QueryHelpersModule } from '../query-helpers/query-helpers.module'; import { MapsModule } from '../maps/maps.module'; import { AuthModule } from '../auth/auth.module'; @@ -16,7 +17,7 @@ import { AuthModule } from '../auth/auth.module'; * is no places.bridge.ts: nothing outside the container consumes this domain. */ @Module({ - imports: [PermissionsModule, QueryHelpersModule, MapsModule, AuthModule], + imports: [PermissionsModule, QueryHelpersModule, MapsModule, AuthModule, AppConfigModule], controllers: [PlacesController], providers: [PlacesService, PlacesMcp], exports: [PlacesService], diff --git a/server/src/nest/trips/trips.controller.ts b/server/src/nest/trips/trips.controller.ts index 51c2694c6..85f008bb2 100644 --- a/server/src/nest/trips/trips.controller.ts +++ b/server/src/nest/trips/trips.controller.ts @@ -17,11 +17,12 @@ import { UseInterceptors, } from '@nestjs/common'; import { FileInterceptor } from '@nestjs/platform-express'; +import { isDemoWriteBlocked, DEMO_WRITE_ERROR } from '../common/demo-write'; +import { RuntimeEnvService } from '../app-config/runtime-env.service'; import type { Request, Response } from 'express'; import { diskStorage } from 'multer'; import path from 'path'; import fs from 'fs'; -import { readEnv } from '../../app-config'; import { v4 as uuidv4 } from 'uuid'; import type { User } from '../../types'; import { TripsService } from './trips.service'; @@ -30,7 +31,6 @@ import { CurrentUser } from '../auth/current-user.decorator'; import { getClientIp } from '../audit/client-ip'; import { logInfo } from '../audit/audit-log.logger'; import { AuditService } from '../audit/audit.service'; -import { isDemoEmail } from '../common/demo'; import { NotFoundError, ValidationError } from './trips.service'; import { TripCreateDto, TripUpdateDto, TripCopyDto, TripAddMemberDto, TripTransferOwnershipDto, TripCreateGuestDto, TripRenameGuestDto } from './trips.dto'; import { saveUnsplashCover, isUnsplashCoverUrl } from '../../services/unsplashService'; @@ -69,7 +69,7 @@ const addDays = (d: Date, n: number) => { const r = new Date(d); r.setDate(r.get @Controller('api/trips') @UseGuards(JwtAuthGuard) export class TripsController { - constructor(private readonly trips: TripsService, private readonly audit: AuditService) {} + constructor(private readonly trips: TripsService, private readonly audit: AuditService, private readonly env: RuntimeEnvService) {} @Get() list(@CurrentUser() user: User, @Query('archived') archived?: string) { @@ -179,8 +179,8 @@ export class TripsController { @Post(':id/cover') @UseInterceptors(FileInterceptor('cover', COVER_UPLOAD)) cover(@CurrentUser() user: User, @Param('id') id: string, @UploadedFile() file: Express.Multer.File | undefined) { - if (readEnv().demo.enabled && isDemoEmail(user.email)) { - throw new HttpException({ error: 'Uploads are disabled in demo mode. Self-host TREK for full functionality.' }, 403); + if (isDemoWriteBlocked(this.env, user.email)) { + throw new HttpException(DEMO_WRITE_ERROR, 403); } const access = this.trips.canAccessTrip(id, user.id); if (!access?.user_id) { diff --git a/server/src/nest/trips/trips.module.ts b/server/src/nest/trips/trips.module.ts index 3867b5975..5c3d506c7 100644 --- a/server/src/nest/trips/trips.module.ts +++ b/server/src/nest/trips/trips.module.ts @@ -12,13 +12,14 @@ import { AuditModule } from '../audit/audit.module'; import { PlacesModule } from '../places/places.module'; import { TripsController } from './trips.controller'; import { TripsService } from './trips.service'; +import { AppConfigModule } from '../app-config/app-config.module'; import { TripsMcp } from './trips.mcp'; import { AuthModule } from '../auth/auth.module'; /** 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, PermissionsModule, AuditModule, BudgetModule, CollabModule, VacayModule, PlacesModule, AuthModule], + imports: [TodoModule, PackingModule, FilesModule, ReservationsModule, DaysModule, PermissionsModule, AuditModule, BudgetModule, CollabModule, VacayModule, PlacesModule, AuthModule, AppConfigModule], controllers: [TripsController], providers: [TripsService, TripsMcp], // Exported for FeedsModule (ICS feeds) and PluginsModule (RPC host injection). diff --git a/server/tests/unit/nest/addon-guard.test.ts b/server/tests/unit/nest/addon-guard.test.ts new file mode 100644 index 000000000..313dfa18e --- /dev/null +++ b/server/tests/unit/nest/addon-guard.test.ts @@ -0,0 +1,108 @@ +/** + * The addon route gate. + * + * Three hand-written guards (collections, airtrail, journey) did the same thing + * and differed only in the addon id they read and the word leading the 404. They + * are one guard plus a @RequireAddon decorator now. + * + * The last describe matters as much as the first: with the addon id in a + * decorator, nothing in a controller's own tests notices if the decorator is + * dropped or points at the wrong addon. These assertions are what keeps the + * three route groups gated, and gated on the right thing. + */ +import { describe, it, expect, vi } from 'vitest'; +import { HttpException } from '@nestjs/common'; +import type { ExecutionContext } from '@nestjs/common'; +import { Reflector } from '@nestjs/core'; + +import { AddonGuard } from '../../../src/nest/addons/addon.guard'; +import { RequireAddon, REQUIRE_ADDON, type RequireAddonMeta } from '../../../src/nest/addons/require-addon.decorator'; +import type { AddonsService } from '../../../src/nest/addons/addons.service'; +import { ADDON_IDS } from '../../../src/addons'; +import { JourneyController } from '../../../src/nest/journey/journey.controller'; +import { CollectionsController } from '../../../src/nest/collections/collections.controller'; +import { AirtrailController } from '../../../src/nest/integrations/airtrail.controller'; +import { AirtrailImportController } from '../../../src/nest/integrations/airtrail-import.controller'; + +/** A context whose handler/class carry the decorator under test. */ +function ctxFor(handler: object, cls: object = class {}): ExecutionContext { + return { + getHandler: () => handler, + getClass: () => cls, + } as unknown as ExecutionContext; +} + +function addons(enabled: boolean): AddonsService { + return { isAddonEnabled: vi.fn(() => enabled) } as unknown as AddonsService; +} + +function thrown(fn: () => unknown) { + try { + fn(); + return undefined; + } catch (err) { + return err instanceof HttpException ? { status: err.getStatus(), body: err.getResponse() } : err; + } +} + +/** Apply @RequireAddon to a throwaway class the way a controller would. */ +function decorated(addonId: string, label: string) { + class Gated {} + RequireAddon(addonId, label)(Gated); + return Gated; +} + +describe('AddonGuard', () => { + it('ADDON-GUARD-001: 404s with the decorator label when the addon is disabled', () => { + const cls = decorated(ADDON_IDS.JOURNEY, 'Journey'); + const guard = new AddonGuard(addons(false), new Reflector()); + + expect(thrown(() => guard.canActivate(ctxFor(() => {}, cls)))) + .toEqual({ status: 404, body: { error: 'Journey addon is not enabled' } }); + }); + + it('ADDON-GUARD-002: passes when the addon is enabled, asking for the declared id', () => { + const cls = decorated(ADDON_IDS.COLLECTIONS, 'Collections'); + const svc = addons(true); + const guard = new AddonGuard(svc, new Reflector()); + + expect(guard.canActivate(ctxFor(() => {}, cls))).toBe(true); + expect(svc.isAddonEnabled).toHaveBeenCalledWith(ADDON_IDS.COLLECTIONS); + }); + + it('ADDON-GUARD-003: is a no-op on a route that never asked for an addon', () => { + const svc = addons(false); + const guard = new AddonGuard(svc, new Reflector()); + + expect(guard.canActivate(ctxFor(() => {}, class {}))).toBe(true); + expect(svc.isAddonEnabled).not.toHaveBeenCalled(); + }); + + it('ADDON-GUARD-004: a handler-level decorator overrides the controller-level one', () => { + const cls = decorated(ADDON_IDS.JOURNEY, 'Journey'); + const handler = () => {}; + RequireAddon(ADDON_IDS.AIRTRAIL, 'AirTrail')(handler); + const svc = addons(false); + + expect(thrown(() => new AddonGuard(svc, new Reflector()).canActivate(ctxFor(handler, cls)))) + .toEqual({ status: 404, body: { error: 'AirTrail addon is not enabled' } }); + expect(svc.isAddonEnabled).toHaveBeenCalledWith(ADDON_IDS.AIRTRAIL); + }); +}); + +describe('the gated route groups still declare their addon', () => { + const metaOf = (cls: object): RequireAddonMeta | undefined => Reflect.getMetadata(REQUIRE_ADDON, cls); + const guardsOf = (cls: object): unknown[] => Reflect.getMetadata('__guards__', cls) ?? []; + + it.each([ + ['JourneyController', JourneyController, ADDON_IDS.JOURNEY, 'Journey'], + ['CollectionsController', CollectionsController, ADDON_IDS.COLLECTIONS, 'Collections'], + ['AirtrailController', AirtrailController, ADDON_IDS.AIRTRAIL, 'AirTrail'], + ['AirtrailImportController', AirtrailImportController, ADDON_IDS.AIRTRAIL, 'AirTrail'], + ])('ADDON-GUARD-005: %s gates on the right addon, with AddonGuard first', (_name, cls, addonId, label) => { + expect(metaOf(cls)).toEqual({ addonId, label }); + // Order is the contract: a disabled addon has to answer 404 to anonymous + // callers, so the addon check must run before the 401. + expect(guardsOf(cls)[0]).toBe(AddonGuard); + }); +}); diff --git a/server/tests/unit/nest/auth.controller.test.ts b/server/tests/unit/nest/auth.controller.test.ts index a78817865..26bf82c72 100644 --- a/server/tests/unit/nest/auth.controller.test.ts +++ b/server/tests/unit/nest/auth.controller.test.ts @@ -1,4 +1,5 @@ import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest'; +import { RuntimeEnvService } from '../../../src/nest/app-config/runtime-env.service'; import { HttpException } from '@nestjs/common'; import type { Request, Response } from 'express'; @@ -28,7 +29,7 @@ function rl(): RateLimitService { return new RateLimitService(); } const writeAudit = vi.fn(); const audit = { writeAudit } as unknown as AuditService; const apc = (a: AuthService, limiter: RateLimitService) => new AuthPublicController(a, limiter, audit); -const ac = (a: AuthService, limiter: RateLimitService) => new AuthController(a, limiter, audit); +const ac = (a: AuthService, limiter: RateLimitService) => new AuthController(a, limiter, audit, new RuntimeEnvService()); function thrown(fn: () => unknown): { status: number; body: unknown } { try { fn(); } catch (err) { diff --git a/server/tests/unit/nest/collections.controller.test.ts b/server/tests/unit/nest/collections.controller.test.ts index 51fb461b6..d264d051e 100644 --- a/server/tests/unit/nest/collections.controller.test.ts +++ b/server/tests/unit/nest/collections.controller.test.ts @@ -1,4 +1,5 @@ import { describe, it, expect, vi, afterEach } from 'vitest'; +import { RuntimeEnvService } from '../../../src/nest/app-config/runtime-env.service'; import { HttpException } from '@nestjs/common'; import { CollectionsController } from '../../../src/nest/collections/collections.controller'; import type { CollectionsService } from '../../../src/nest/collections/collections.service'; @@ -57,7 +58,7 @@ describe('CollectionsController', () => { it('lists / creates / reads / deletes lists (pass-throughs)', () => { const svc = makeService(); - const c = new CollectionsController(svc); + const c = new CollectionsController(svc, new RuntimeEnvService()); expect(c.list(user)).toEqual({ collections: [], incomingInvites: [] }); expect(c.create(user, { name: 'Italy' } as never)).toEqual({ id: 7 }); expect(c.get(user, '3')).toEqual({ collection: { id: 3 }, places: [] }); @@ -73,7 +74,7 @@ describe('CollectionsController', () => { // bad payload before the handler runs. it('reorders a valid array', () => { const svc = makeService(); - expect(new CollectionsController(svc).reorder(user, { orderedIds: [3, 1, 2] } as never)).toEqual({ success: true }); + expect(new CollectionsController(svc, new RuntimeEnvService()).reorder(user, { orderedIds: [3, 1, 2] } as never)).toEqual({ success: true }); expect(svc.reorderCollections).toHaveBeenCalledWith(1, [3, 1, 2]); }); }); @@ -81,7 +82,7 @@ describe('CollectionsController', () => { describe('places', () => { it('savePlace / updatePlace / setStatus / deletePlace forward the socket id', () => { const svc = makeService(); - const c = new CollectionsController(svc); + const c = new CollectionsController(svc, new RuntimeEnvService()); c.savePlace(user, { collection_id: 3, name: 'A' } as never, 'sid'); expect(svc.savePlace).toHaveBeenCalledWith(1, { collection_id: 3, name: 'A' }, 'sid'); c.updatePlace(user, '9', { name: 'B' } as never, 'sid'); @@ -100,7 +101,7 @@ describe('CollectionsController', () => { // bad payload before the handler runs. it('deleteMany deletes a valid list', () => { const svc = makeService(); - expect(new CollectionsController(svc).deleteMany(user, { ids: [1, 2] } as never, 'sid')).toEqual({ deleted: [1, 2] }); + expect(new CollectionsController(svc, new RuntimeEnvService()).deleteMany(user, { ids: [1, 2] } as never, 'sid')).toEqual({ deleted: [1, 2] }); expect(svc.deletePlacesMany).toHaveBeenCalledWith(1, [1, 2], 'sid'); }); }); @@ -108,12 +109,12 @@ describe('CollectionsController', () => { describe('membership lookup', () => { it('parses lat/lng when present', () => { const svc = makeService(); - new CollectionsController(svc).membership(user, 'gpid', undefined, 'Rome', '41.9', '12.5'); + new CollectionsController(svc, new RuntimeEnvService()).membership(user, 'gpid', undefined, 'Rome', '41.9', '12.5'); expect(svc.findMembership).toHaveBeenCalledWith(1, { google_place_id: 'gpid', google_ftid: undefined, name: 'Rome', lat: 41.9, lng: 12.5 }); }); it('leaves lat/lng undefined when absent', () => { const svc = makeService(); - new CollectionsController(svc).membership(user); + new CollectionsController(svc, new RuntimeEnvService()).membership(user); expect(svc.findMembership).toHaveBeenCalledWith(1, { google_place_id: undefined, google_ftid: undefined, name: undefined, lat: undefined, lng: undefined }); }); }); @@ -121,29 +122,29 @@ describe('CollectionsController', () => { describe('invites (owner-gated)', () => { it('403 when a non-owner invites', () => { const svc = makeService({ isOwner: vi.fn().mockReturnValue(false) }); - expect(thrown(() => new CollectionsController(svc).invite(user, { collection_id: 3, user_id: 2 } as never))) + expect(thrown(() => new CollectionsController(svc, new RuntimeEnvService()).invite(user, { collection_id: 3, user_id: 2 } as never))) .toEqual({ status: 403, body: { error: 'Only the owner can invite' } }); }); it('surfaces a sendInvite error with its status', () => { const svc = makeService({ sendInvite: vi.fn().mockReturnValue({ error: 'Already a member', status: 409 }) }); - expect(thrown(() => new CollectionsController(svc).invite(user, { collection_id: 3, user_id: 2 } as never))) + expect(thrown(() => new CollectionsController(svc, new RuntimeEnvService()).invite(user, { collection_id: 3, user_id: 2 } as never))) .toEqual({ status: 409, body: { error: 'Already a member' } }); }); it('invites successfully as owner', () => { const svc = makeService(); - expect(new CollectionsController(svc).invite(user, { collection_id: 3, user_id: 2 } as never)).toEqual({ success: true }); + expect(new CollectionsController(svc, new RuntimeEnvService()).invite(user, { collection_id: 3, user_id: 2 } as never)).toEqual({ success: true }); }); it('accept surfaces an error, else succeeds', () => { const bad = makeService({ acceptInvite: vi.fn().mockReturnValue({ error: 'Gone', status: 404 }) }); - expect(thrown(() => new CollectionsController(bad).acceptInvite(user, { collection_id: 3 } as never, 'sid'))) + expect(thrown(() => new CollectionsController(bad, new RuntimeEnvService()).acceptInvite(user, { collection_id: 3 } as never, 'sid'))) .toEqual({ status: 404, body: { error: 'Gone' } }); const svc = makeService(); - expect(new CollectionsController(svc).acceptInvite(user, { collection_id: 3 } as never, 'sid')).toEqual({ success: true }); + expect(new CollectionsController(svc, new RuntimeEnvService()).acceptInvite(user, { collection_id: 3 } as never, 'sid')).toEqual({ success: true }); expect(svc.acceptInvite).toHaveBeenCalledWith(1, 3, 'sid'); }); it('decline + leave forward the socket id', () => { const svc = makeService(); - const c = new CollectionsController(svc); + const c = new CollectionsController(svc, new RuntimeEnvService()); expect(c.declineInvite(user, { collection_id: 3 } as never, 'sid')).toEqual({ success: true }); expect(svc.declineInvite).toHaveBeenCalledWith(1, 3, 'sid'); expect(c.leave(user, { collection_id: 3 } as never, 'sid')).toEqual({ success: true }); @@ -151,27 +152,27 @@ describe('CollectionsController', () => { }); it('cancel is owner-gated', () => { const notOwner = makeService({ isOwner: vi.fn().mockReturnValue(false) }); - expect(thrown(() => new CollectionsController(notOwner).cancelInvite(user, { collection_id: 3, user_id: 2 } as never))) + expect(thrown(() => new CollectionsController(notOwner, new RuntimeEnvService()).cancelInvite(user, { collection_id: 3, user_id: 2 } as never))) .toEqual({ status: 403, body: { error: 'Only the owner can cancel invites' } }); const svc = makeService(); - expect(new CollectionsController(svc).cancelInvite(user, { collection_id: 3, user_id: 2 } as never)).toEqual({ success: true }); + expect(new CollectionsController(svc, new RuntimeEnvService()).cancelInvite(user, { collection_id: 3, user_id: 2 } as never)).toEqual({ success: true }); }); }); describe('members', () => { it('removeMember is owner-gated', () => { const notOwner = makeService({ isOwner: vi.fn().mockReturnValue(false) }); - expect(thrown(() => new CollectionsController(notOwner).removeMember(user, { collection_id: 3, user_id: 2 } as never))) + expect(thrown(() => new CollectionsController(notOwner, new RuntimeEnvService()).removeMember(user, { collection_id: 3, user_id: 2 } as never))) .toEqual({ status: 403, body: { error: 'Only the owner can remove members' } }); const svc = makeService(); - expect(new CollectionsController(svc).removeMember(user, { collection_id: 3, user_id: 2 } as never)).toEqual({ success: true }); + expect(new CollectionsController(svc, new RuntimeEnvService()).removeMember(user, { collection_id: 3, user_id: 2 } as never)).toEqual({ success: true }); expect(svc.removeMember).toHaveBeenCalledWith(1, 3, 2); }); it('availableUsers is owner-gated', () => { const notOwner = makeService({ isOwner: vi.fn().mockReturnValue(false) }); - expect(thrown(() => new CollectionsController(notOwner).availableUsers(user, '3'))) + expect(thrown(() => new CollectionsController(notOwner, new RuntimeEnvService()).availableUsers(user, '3'))) .toEqual({ status: 403, body: { error: 'Only the owner can manage members' } }); - expect(new CollectionsController(makeService()).availableUsers(user, '3')).toEqual({ users: [{ id: 2 }] }); + expect(new CollectionsController(makeService(), new RuntimeEnvService()).availableUsers(user, '3')).toEqual({ users: [{ id: 2 }] }); }); }); @@ -180,16 +181,16 @@ describe('CollectionsController', () => { it('403 in demo mode for a demo user', () => { process.env.DEMO_MODE = 'true'; const demo = { ...user, email: 'demo@trek.app' } as User; - expect(thrown(() => new CollectionsController(makeService()).uploadCover(demo, '3', file))) + expect(thrown(() => new CollectionsController(makeService(), new RuntimeEnvService()).uploadCover(demo, '3', file))) .toEqual({ status: 403, body: { error: 'Uploads are disabled in demo mode. Self-host TREK for full functionality.' } }); }); it('400 when no file was uploaded', () => { - expect(thrown(() => new CollectionsController(makeService()).uploadCover(user, '3', undefined))) + expect(thrown(() => new CollectionsController(makeService(), new RuntimeEnvService()).uploadCover(user, '3', undefined))) .toEqual({ status: 400, body: { error: 'No image uploaded' } }); }); it('stores the cover and forwards the socket id', () => { const svc = makeService(); - expect(new CollectionsController(svc).uploadCover(user, '3', file, 'sid')).toEqual({ id: 3 }); + expect(new CollectionsController(svc, new RuntimeEnvService()).uploadCover(user, '3', file, 'sid')).toEqual({ id: 3 }); expect(svc.setCollectionCover).toHaveBeenCalledWith(1, 3, '/uploads/covers/x.jpg', 'sid'); }); }); @@ -199,16 +200,16 @@ describe('CollectionsController', () => { it('403 in demo mode for a demo user', () => { process.env.DEMO_MODE = 'true'; const demo = { ...user, email: 'demo@trek.app' } as User; - expect(thrown(() => new CollectionsController(makeService()).uploadPlaceImage(demo, '9', file))) + expect(thrown(() => new CollectionsController(makeService(), new RuntimeEnvService()).uploadPlaceImage(demo, '9', file))) .toEqual({ status: 403, body: { error: 'Uploads are disabled in demo mode. Self-host TREK for full functionality.' } }); }); it('400 when no file was uploaded', () => { - expect(thrown(() => new CollectionsController(makeService()).uploadPlaceImage(user, '9', undefined))) + expect(thrown(() => new CollectionsController(makeService(), new RuntimeEnvService()).uploadPlaceImage(user, '9', undefined))) .toEqual({ status: 400, body: { error: 'No image uploaded' } }); }); it('stores the place image via /uploads/places and forwards the socket id', () => { const svc = makeService({ setPlaceImage: vi.fn().mockReturnValue({ id: 9 }) }); - expect(new CollectionsController(svc).uploadPlaceImage(user, '9', file, 'sid')).toEqual({ id: 9 }); + expect(new CollectionsController(svc, new RuntimeEnvService()).uploadPlaceImage(user, '9', file, 'sid')).toEqual({ id: 9 }); expect(svc.setPlaceImage).toHaveBeenCalledWith(1, 9, '/uploads/places/x.jpg', 'sid'); }); }); @@ -216,7 +217,7 @@ describe('CollectionsController', () => { describe('labels', () => { it('create / update / delete / assign / unassign forward user + socket id', () => { const svc = makeService(); - const c = new CollectionsController(svc); + const c = new CollectionsController(svc, new RuntimeEnvService()); c.createLabel(user, { collection_id: 3, name: 'Berlin', color: '#ff0000' } as never, 'sock'); expect(svc.createLabel).toHaveBeenCalledWith(1, 3, 'Berlin', '#ff0000', 'sock'); diff --git a/server/tests/unit/nest/demo-write.test.ts b/server/tests/unit/nest/demo-write.test.ts new file mode 100644 index 000000000..c7d387557 --- /dev/null +++ b/server/tests/unit/nest/demo-write.test.ts @@ -0,0 +1,62 @@ +/** + * The demo-mode upload block. + * + * The condition was copy-pasted into six upload handlers, each carrying its own + * copy of the 403 body. All six call isDemoWriteBlocked now. + * + * It is a function rather than a guard on purpose, and that is worth a test of + * its own: guards run before the multipart parser, so throwing from one leaves + * the request body unread and Node resets the connection — the client gets + * ECONNRESET instead of the 403. PROFILE-015 in the integration suite is what + * demonstrates it end-to-end. + */ +import { describe, it, expect, vi, afterEach } from 'vitest'; + +vi.mock('../../../src/nest/common/demo', () => ({ isDemoEmail: vi.fn(() => false) })); + +import { isDemoWriteBlocked, DEMO_WRITE_ERROR } from '../../../src/nest/common/demo-write'; +import { RuntimeEnvService } from '../../../src/nest/app-config/runtime-env.service'; +import { isDemoEmail } from '../../../src/nest/common/demo'; + +const env = new RuntimeEnvService(); + +afterEach(() => { + delete process.env.DEMO_MODE; + vi.mocked(isDemoEmail).mockReturnValue(false); +}); + +describe('isDemoWriteBlocked', () => { + it('DEMO-WRITE-001: blocks the demo account while the instance runs in demo mode', () => { + process.env.DEMO_MODE = 'true'; + vi.mocked(isDemoEmail).mockReturnValue(true); + + expect(isDemoWriteBlocked(env, 'demo@trek.app')).toBe(true); + }); + + it('DEMO-WRITE-002: lets a normal account through even while demo mode is on', () => { + process.env.DEMO_MODE = 'true'; + vi.mocked(isDemoEmail).mockReturnValue(false); + + expect(isDemoWriteBlocked(env, 'alice@example.com')).toBe(false); + }); + + it('DEMO-WRITE-003: lets the demo account through when the instance is not in demo mode', () => { + vi.mocked(isDemoEmail).mockReturnValue(true); + + expect(isDemoWriteBlocked(env, 'demo@trek.app')).toBe(false); + }); + + it('DEMO-WRITE-004: reads the env live, so a toggle takes effect without a restart', () => { + vi.mocked(isDemoEmail).mockReturnValue(true); + expect(isDemoWriteBlocked(env, 'demo@trek.app')).toBe(false); + + process.env.DEMO_MODE = 'true'; + expect(isDemoWriteBlocked(env, 'demo@trek.app')).toBe(true); + }); + + it('DEMO-WRITE-005: the 403 body is the one the six endpoints have always sent', () => { + expect(DEMO_WRITE_ERROR).toEqual({ + error: 'Uploads are disabled in demo mode. Self-host TREK for full functionality.', + }); + }); +}); diff --git a/server/tests/unit/nest/files.controller.test.ts b/server/tests/unit/nest/files.controller.test.ts index 32009764c..e1a73dea1 100644 --- a/server/tests/unit/nest/files.controller.test.ts +++ b/server/tests/unit/nest/files.controller.test.ts @@ -1,4 +1,5 @@ import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest'; +import { RuntimeEnvService } from '../../../src/nest/app-config/runtime-env.service'; import { HttpException } from '@nestjs/common'; import type { Request, Response } from 'express'; import os from 'os'; @@ -41,9 +42,9 @@ afterEach(() => { delete process.env.DEMO_MODE; }); describe('FilesController (parity with the legacy /api/trips/:tripId/files route)', () => { it('GET / 404 without access, else lists with the trash flag', () => { - expect(thrown(() => new FilesController(fsvc({ verifyTripAccess: vi.fn().mockReturnValue(undefined) })).list(user, '5'))).toEqual({ status: 404, body: { error: 'Trip not found' } }); + expect(thrown(() => new FilesController(fsvc({ verifyTripAccess: vi.fn().mockReturnValue(undefined) }), new RuntimeEnvService()).list(user, '5'))).toEqual({ status: 404, body: { error: 'Trip not found' } }); const listFiles = vi.fn().mockReturnValue([{ id: 1 }]); - expect(new FilesController(fsvc({ listFiles } as Partial)).list(user, '5', 'true')).toEqual({ files: [{ id: 1 }] }); + expect(new FilesController(fsvc({ listFiles } as Partial), new RuntimeEnvService()).list(user, '5', 'true')).toEqual({ files: [{ id: 1 }] }); expect(listFiles).toHaveBeenCalledWith('5', true); }); @@ -52,107 +53,107 @@ describe('FilesController (parity with the legacy /api/trips/:tripId/files route it('403 in demo mode for a demo email', () => { process.env.DEMO_MODE = 'true'; vi.mocked(isDemoEmail).mockReturnValue(true); - expect(thrown(() => new FilesController(fsvc()).upload(user, '5', file, {}))).toEqual({ status: 403, body: { error: 'Uploads are disabled in demo mode. Self-host TREK for full functionality.' } }); + expect(thrown(() => new FilesController(fsvc(), new RuntimeEnvService()).upload(user, '5', file, {}))).toEqual({ status: 403, body: { error: 'Uploads are disabled in demo mode. Self-host TREK for full functionality.' } }); }); it('403 without file_upload, 400 without a file, else creates + broadcasts', () => { - expect(thrown(() => new FilesController(fsvc({ can: vi.fn().mockReturnValue(false) })).upload(user, '5', file, {}))).toEqual({ status: 403, body: { error: 'No permission to upload files' } }); - expect(thrown(() => new FilesController(fsvc()).upload(user, '5', undefined, {}))).toEqual({ status: 400, body: { error: 'No file uploaded' } }); + expect(thrown(() => new FilesController(fsvc({ can: vi.fn().mockReturnValue(false) }), new RuntimeEnvService()).upload(user, '5', file, {}))).toEqual({ status: 403, body: { error: 'No permission to upload files' } }); + expect(thrown(() => new FilesController(fsvc(), new RuntimeEnvService()).upload(user, '5', undefined, {}))).toEqual({ status: 400, body: { error: 'No file uploaded' } }); const createFile = vi.fn().mockReturnValue({ id: 9 }); const broadcast = vi.fn(); const s = fsvc({ createFile, broadcast } as Partial); - expect(new FilesController(s).upload(user, '5', file, { description: 'd' }, 'sock')).toEqual({ file: { id: 9 } }); + expect(new FilesController(s, new RuntimeEnvService()).upload(user, '5', file, { description: 'd' }, 'sock')).toEqual({ file: { id: 9 } }); expect(createFile).toHaveBeenCalledWith('5', file, 1, { place_id: undefined, description: 'd', reservation_id: undefined }); expect(broadcast).toHaveBeenCalledWith('5', 'file:created', { file: { id: 9 } }, 'sock'); }); it('caps non-video by extension and accepts large video; cleans up on rejection (#823)', () => { const big = { filename: 'big.pdf', originalname: 'big.pdf', size: 60 * 1024 * 1024, path: '' } as Express.Multer.File; - expect(thrown(() => new FilesController(fsvc()).upload(user, '5', big, {}))).toEqual({ status: 400, body: { error: 'File is too large' } }); + expect(thrown(() => new FilesController(fsvc(), new RuntimeEnvService()).upload(user, '5', big, {}))).toEqual({ status: 400, body: { error: 'File is too large' } }); const createFile = vi.fn().mockReturnValue({ id: 9 }); const vid = { filename: 'v.mp4', originalname: 'clip.mp4', size: 200 * 1024 * 1024, path: '' } as Express.Multer.File; - expect(new FilesController(fsvc({ createFile, broadcast: vi.fn() } as Partial)).upload(user, '5', vid, {})).toEqual({ file: { id: 9 } }); + expect(new FilesController(fsvc({ createFile, broadcast: vi.fn() } as Partial), new RuntimeEnvService()).upload(user, '5', vid, {})).toEqual({ file: { id: 9 } }); // A rejected upload with a real path triggers the unlink cleanup branch // (the file doesn't exist, so the inner best-effort catch swallows it). const withPath = { filename: 'a.pdf', path: '/nonexistent/zzz.pdf' } as Express.Multer.File; - expect(thrown(() => new FilesController(fsvc({ can: vi.fn().mockReturnValue(false) })).upload(user, '5', withPath, {}))).toEqual({ status: 403, body: { error: 'No permission to upload files' } }); + expect(thrown(() => new FilesController(fsvc({ can: vi.fn().mockReturnValue(false) }), new RuntimeEnvService()).upload(user, '5', withPath, {}))).toEqual({ status: 403, body: { error: 'No permission to upload files' } }); }); }); it('PUT /:id 403 without file_edit, 404 unknown, else updates + broadcasts', () => { - expect(thrown(() => new FilesController(fsvc({ can: vi.fn().mockReturnValue(false) })).update(user, '5', '9', {}))).toEqual({ status: 403, body: { error: 'No permission to edit files' } }); - expect(thrown(() => new FilesController(fsvc({ getFileById: vi.fn().mockReturnValue(undefined) } as Partial)).update(user, '5', '9', {}))).toEqual({ status: 404, body: { error: 'File not found' } }); + expect(thrown(() => new FilesController(fsvc({ can: vi.fn().mockReturnValue(false) }), new RuntimeEnvService()).update(user, '5', '9', {}))).toEqual({ status: 403, body: { error: 'No permission to edit files' } }); + expect(thrown(() => new FilesController(fsvc({ getFileById: vi.fn().mockReturnValue(undefined) } as Partial), new RuntimeEnvService()).update(user, '5', '9', {}))).toEqual({ status: 404, body: { error: 'File not found' } }); const updateFile = vi.fn().mockReturnValue({ id: 9 }); const s = fsvc({ getFileById: vi.fn().mockReturnValue({ id: 9, description: 'x' }), updateFile, broadcast: vi.fn() } as Partial); - expect(new FilesController(s).update(user, '5', '9', { description: 'new' })).toEqual({ file: { id: 9 } }); + expect(new FilesController(s, new RuntimeEnvService()).update(user, '5', '9', { description: 'new' })).toEqual({ file: { id: 9 } }); }); it('PATCH /:id/star 403/404, else toggles', () => { - expect(thrown(() => new FilesController(fsvc({ can: vi.fn().mockReturnValue(false) })).star(user, '5', '9'))).toEqual({ status: 403, body: { error: 'No permission' } }); - expect(thrown(() => new FilesController(fsvc({ getFileById: vi.fn().mockReturnValue(undefined) } as Partial)).star(user, '5', '9'))).toEqual({ status: 404, body: { error: 'File not found' } }); + expect(thrown(() => new FilesController(fsvc({ can: vi.fn().mockReturnValue(false) }), new RuntimeEnvService()).star(user, '5', '9'))).toEqual({ status: 403, body: { error: 'No permission' } }); + expect(thrown(() => new FilesController(fsvc({ getFileById: vi.fn().mockReturnValue(undefined) } as Partial), new RuntimeEnvService()).star(user, '5', '9'))).toEqual({ status: 404, body: { error: 'File not found' } }); const toggleStarred = vi.fn().mockReturnValue({ id: 9, starred: 1 }); const s = fsvc({ getFileById: vi.fn().mockReturnValue({ id: 9, starred: 0 }), toggleStarred, broadcast: vi.fn() } as Partial); - expect(new FilesController(s).star(user, '5', '9')).toEqual({ file: { id: 9, starred: 1 } }); + expect(new FilesController(s, new RuntimeEnvService()).star(user, '5', '9')).toEqual({ file: { id: 9, starred: 1 } }); expect(toggleStarred).toHaveBeenCalledWith('9', 0); }); it('DELETE /:id soft-delete 403/404, else success', () => { - expect(thrown(() => new FilesController(fsvc({ can: vi.fn().mockReturnValue(false) })).remove(user, '5', '9'))).toEqual({ status: 403, body: { error: 'No permission to delete files' } }); - expect(thrown(() => new FilesController(fsvc({ getFileById: vi.fn().mockReturnValue(undefined) } as Partial)).remove(user, '5', '9'))).toEqual({ status: 404, body: { error: 'File not found' } }); + expect(thrown(() => new FilesController(fsvc({ can: vi.fn().mockReturnValue(false) }), new RuntimeEnvService()).remove(user, '5', '9'))).toEqual({ status: 403, body: { error: 'No permission to delete files' } }); + expect(thrown(() => new FilesController(fsvc({ getFileById: vi.fn().mockReturnValue(undefined) } as Partial), new RuntimeEnvService()).remove(user, '5', '9'))).toEqual({ status: 404, body: { error: 'File not found' } }); const softDeleteFile = vi.fn(); const broadcast = vi.fn(); const s = fsvc({ getFileById: vi.fn().mockReturnValue({ id: 9 }), softDeleteFile, broadcast } as Partial); - expect(new FilesController(s).remove(user, '5', '9', 'sock')).toEqual({ success: true }); + expect(new FilesController(s, new RuntimeEnvService()).remove(user, '5', '9', 'sock')).toEqual({ success: true }); expect(broadcast).toHaveBeenCalledWith('5', 'file:deleted', { fileId: 9 }, 'sock'); }); it('POST /:id/restore 404 not in trash, else restores', () => { - expect(thrown(() => new FilesController(fsvc({ getDeletedFile: vi.fn().mockReturnValue(undefined) } as Partial)).restore(user, '5', '9'))).toEqual({ status: 404, body: { error: 'File not found in trash' } }); + expect(thrown(() => new FilesController(fsvc({ getDeletedFile: vi.fn().mockReturnValue(undefined) } as Partial), new RuntimeEnvService()).restore(user, '5', '9'))).toEqual({ status: 404, body: { error: 'File not found in trash' } }); const restoreFile = vi.fn().mockReturnValue({ id: 9 }); const s = fsvc({ getDeletedFile: vi.fn().mockReturnValue({ id: 9 }), restoreFile, broadcast: vi.fn() } as Partial); - expect(new FilesController(s).restore(user, '5', '9')).toEqual({ file: { id: 9 } }); + expect(new FilesController(s, new RuntimeEnvService()).restore(user, '5', '9')).toEqual({ file: { id: 9 } }); }); it('DELETE /:id/permanent 404 not in trash, else deletes', async () => { - await expect(new FilesController(fsvc({ getDeletedFile: vi.fn().mockReturnValue(undefined) } as Partial)).permanent(user, '5', '9')).rejects.toBeInstanceOf(HttpException); + await expect(new FilesController(fsvc({ getDeletedFile: vi.fn().mockReturnValue(undefined) } as Partial), new RuntimeEnvService()).permanent(user, '5', '9')).rejects.toBeInstanceOf(HttpException); const permanentDeleteFile = vi.fn().mockResolvedValue(undefined); const s = fsvc({ getDeletedFile: vi.fn().mockReturnValue({ id: 9 }), permanentDeleteFile, broadcast: vi.fn() } as Partial); - expect(await new FilesController(s).permanent(user, '5', '9')).toEqual({ success: true }); + expect(await new FilesController(s, new RuntimeEnvService()).permanent(user, '5', '9')).toEqual({ success: true }); }); it('DELETE /trash/empty 403, else returns the count', async () => { - await expect(new FilesController(fsvc({ can: vi.fn().mockReturnValue(false) })).emptyTrash(user, '5')).rejects.toBeInstanceOf(HttpException); + await expect(new FilesController(fsvc({ can: vi.fn().mockReturnValue(false) }), new RuntimeEnvService()).emptyTrash(user, '5')).rejects.toBeInstanceOf(HttpException); const s = fsvc({ emptyTrash: vi.fn().mockResolvedValue(3) } as Partial); - expect(await new FilesController(s).emptyTrash(user, '5')).toEqual({ success: true, deleted: 3 }); + expect(await new FilesController(s, new RuntimeEnvService()).emptyTrash(user, '5')).toEqual({ success: true, deleted: 3 }); }); it('POST /:id/link 404 unknown file, else links', () => { - expect(thrown(() => new FilesController(fsvc({ getFileById: vi.fn().mockReturnValue(undefined) } as Partial)).link(user, '5', '9', {}))).toEqual({ status: 404, body: { error: 'File not found' } }); + expect(thrown(() => new FilesController(fsvc({ getFileById: vi.fn().mockReturnValue(undefined) } as Partial), new RuntimeEnvService()).link(user, '5', '9', {}))).toEqual({ status: 404, body: { error: 'File not found' } }); const createFileLink = vi.fn().mockReturnValue([{ id: 1 }]); const s = fsvc({ getFileById: vi.fn().mockReturnValue({ id: 9 }), createFileLink } as Partial); - expect(new FilesController(s).link(user, '5', '9', { reservation_id: 2 })).toEqual({ success: true, links: [{ id: 1 }] }); + expect(new FilesController(s, new RuntimeEnvService()).link(user, '5', '9', { reservation_id: 2 })).toEqual({ success: true, links: [{ id: 1 }] }); }); it('DELETE /:id/link/:linkId removes the link; GET /:id/links lists', () => { const deleteFileLink = vi.fn(); - expect(new FilesController(fsvc({ deleteFileLink } as Partial)).unlink(user, '5', '9', '3')).toEqual({ success: true }); + expect(new FilesController(fsvc({ deleteFileLink } as Partial), new RuntimeEnvService()).unlink(user, '5', '9', '3')).toEqual({ success: true }); expect(deleteFileLink).toHaveBeenCalledWith('3', '9'); const s = fsvc({ getFileLinks: vi.fn().mockReturnValue([{ id: 1 }]) } as Partial); - expect(new FilesController(s).links(user, '5', '9')).toEqual({ links: [{ id: 1 }] }); + expect(new FilesController(s, new RuntimeEnvService()).links(user, '5', '9')).toEqual({ links: [{ id: 1 }] }); }); it('the trash + link routes all reject without file_delete / file_edit', async () => { const denied = () => fsvc({ can: vi.fn().mockReturnValue(false) }); - await expect(new FilesController(denied()).permanent(user, '5', '9')).rejects.toMatchObject({ status: 403 }); - expect(thrown(() => new FilesController(denied()).restore(user, '5', '9'))).toEqual({ status: 403, body: { error: 'No permission' } }); - expect(thrown(() => new FilesController(denied()).link(user, '5', '9', {}))).toEqual({ status: 403, body: { error: 'No permission' } }); - expect(thrown(() => new FilesController(denied()).unlink(user, '5', '9', '3'))).toEqual({ status: 403, body: { error: 'No permission' } }); + await expect(new FilesController(denied(), new RuntimeEnvService()).permanent(user, '5', '9')).rejects.toMatchObject({ status: 403 }); + expect(thrown(() => new FilesController(denied(), new RuntimeEnvService()).restore(user, '5', '9'))).toEqual({ status: 403, body: { error: 'No permission' } }); + expect(thrown(() => new FilesController(denied(), new RuntimeEnvService()).link(user, '5', '9', {}))).toEqual({ status: 403, body: { error: 'No permission' } }); + expect(thrown(() => new FilesController(denied(), new RuntimeEnvService()).unlink(user, '5', '9', '3'))).toEqual({ status: 403, body: { error: 'No permission' } }); }); it('GET /:id/links 404 without trip access', () => { const s = fsvc({ verifyTripAccess: vi.fn().mockReturnValue(undefined) }); - expect(thrown(() => new FilesController(s).links(user, '5', '9'))).toEqual({ status: 404, body: { error: 'Trip not found' } }); + expect(thrown(() => new FilesController(s, new RuntimeEnvService()).links(user, '5', '9'))).toEqual({ status: 404, body: { error: 'Trip not found' } }); }); }); diff --git a/server/tests/unit/nest/journey.controller.test.ts b/server/tests/unit/nest/journey.controller.test.ts index 952e28adc..e892e6619 100644 --- a/server/tests/unit/nest/journey.controller.test.ts +++ b/server/tests/unit/nest/journey.controller.test.ts @@ -6,7 +6,6 @@ import fs from 'node:fs'; import { JourneyController } from '../../../src/nest/journey/journey.controller'; import { JourneyPublicController } from '../../../src/nest/journey/journey-public.controller'; -import { JourneyAddonGuard } from '../../../src/nest/journey/journey-addon.guard'; import type { JourneyService } from '../../../src/nest/journey/journey.service'; import type { User } from '../../../src/types'; @@ -35,13 +34,6 @@ async function thrownAsync(fn: () => Promise): Promise<{ status: number beforeEach(() => vi.clearAllMocks()); -describe('JourneyAddonGuard', () => { - it('404 when the addon is disabled, passes when enabled', () => { - expect(thrown(() => new JourneyAddonGuard(svc({ journeyAddonEnabled: vi.fn().mockReturnValue(false) })).canActivate())).toEqual({ status: 404, body: { error: 'Journey addon is not enabled' } }); - expect(new JourneyAddonGuard(svc()).canActivate()).toBe(true); - }); -}); - describe('JourneyController', () => { it('GET / lists; POST / 400 without title, else creates', () => { expect(new JourneyController(svc({ listJourneys: vi.fn().mockReturnValue([{ id: 1 }]) } as Partial)).list(user)).toEqual({ journeys: [{ id: 1 }] }); diff --git a/server/tests/unit/nest/places.controller.test.ts b/server/tests/unit/nest/places.controller.test.ts index 6bc073a4f..b539357d8 100644 --- a/server/tests/unit/nest/places.controller.test.ts +++ b/server/tests/unit/nest/places.controller.test.ts @@ -1,4 +1,5 @@ import { describe, it, expect, vi, beforeEach } from 'vitest'; +import { RuntimeEnvService } from '../../../src/nest/app-config/runtime-env.service'; import { HttpException } from '@nestjs/common'; import { PlacesController } from '../../../src/nest/places/places.controller'; import type { PlacesService } from '../../../src/nest/places/places.service'; @@ -39,16 +40,16 @@ beforeEach(() => vi.spyOn(console, 'error').mockImplementation(() => {})); describe('PlacesController (parity with the legacy /api/trips/:tripId/places route)', () => { it('GET / lists with filters; 404 when trip not accessible', () => { - expect(thrown(() => new PlacesController(svc({ verifyTripAccess: vi.fn().mockReturnValue(undefined) })).list(user, '5'))).toEqual({ status: 404, body: { error: 'Trip not found' } }); + expect(thrown(() => new PlacesController(svc({ verifyTripAccess: vi.fn().mockReturnValue(undefined) }), new RuntimeEnvService()).list(user, '5'))).toEqual({ status: 404, body: { error: 'Trip not found' } }); const list = vi.fn().mockReturnValue([{ id: 1 }]); - expect(new PlacesController(svc({ list } as Partial)).list(user, '5', 'beach', 'cat', 'tag')).toEqual({ places: [{ id: 1 }] }); + expect(new PlacesController(svc({ list } as Partial), new RuntimeEnvService()).list(user, '5', 'beach', 'cat', 'tag')).toEqual({ places: [{ id: 1 }] }); expect(list).toHaveBeenCalledWith('5', { search: 'beach', category: 'cat', tag: 'tag' }); }); describe('POST / (create)', () => { it('400 on an over-long name (length guard before permission)', () => { const canEdit = vi.fn().mockReturnValue(false); // would 403 if reached - expect(thrown(() => new PlacesController(svc({ canEdit })).create(user, '5', { name: 'x'.repeat(201) }))).toEqual({ + expect(thrown(() => new PlacesController(svc({ canEdit }), new RuntimeEnvService()).create(user, '5', { name: 'x'.repeat(201) }))).toEqual({ status: 400, body: { error: 'name must be 200 characters or less' }, }); expect(canEdit).not.toHaveBeenCalled(); @@ -58,10 +59,10 @@ describe('PlacesController (parity with the legacy /api/trips/:tripId/places rou // pins `name`, so the ZodValidationPipe rejects a nameless body before the // handler runs (see the e2e suite for the envelope it produces). it('403 without place_edit, then creates + hooks', () => { - expect(thrown(() => new PlacesController(svc({ canEdit: vi.fn().mockReturnValue(false) })).create(user, '5', { name: 'Spot' }))).toEqual({ status: 403, body: { error: 'No permission' } }); + expect(thrown(() => new PlacesController(svc({ canEdit: vi.fn().mockReturnValue(false) }), new RuntimeEnvService()).create(user, '5', { name: 'Spot' }))).toEqual({ status: 403, body: { error: 'No permission' } }); const create = vi.fn().mockReturnValue({ id: 9 }); const broadcast = vi.fn(); const onCreated = vi.fn(); const s = svc({ create, broadcast, onCreated } as Partial); - expect(new PlacesController(s).create(user, '5', { name: 'Spot' }, 'sock')).toEqual({ place: { id: 9 } }); + expect(new PlacesController(s, new RuntimeEnvService()).create(user, '5', { name: 'Spot' }, 'sock')).toEqual({ place: { id: 9 } }); expect(broadcast).toHaveBeenCalledWith('5', 'place:created', { place: { id: 9 } }, 'sock'); expect(onCreated).toHaveBeenCalledWith('5', 9); }); @@ -70,22 +71,22 @@ describe('PlacesController (parity with the legacy /api/trips/:tripId/places rou describe('POST /import/gpx', () => { const file = { buffer: Buffer.from('gpx'), originalname: 'r.gpx' } as Express.Multer.File; it('400 without a file', () => { - expect(thrown(() => new PlacesController(svc()).importGpx(user, '5', undefined, {}))).toEqual({ status: 400, body: { error: 'No file uploaded' } }); + expect(thrown(() => new PlacesController(svc(), new RuntimeEnvService()).importGpx(user, '5', undefined, {}))).toEqual({ status: 400, body: { error: 'No file uploaded' } }); }); it('400 when all import types are disabled', () => { - expect(thrown(() => new PlacesController(svc()).importGpx(user, '5', file, { importWaypoints: 'false', importRoutes: 'false', importTracks: 'false' }))).toEqual({ + expect(thrown(() => new PlacesController(svc(), new RuntimeEnvService()).importGpx(user, '5', file, { importWaypoints: 'false', importRoutes: 'false', importTracks: 'false' }))).toEqual({ status: 400, body: { error: 'No import types selected' }, }); }); it('400 when the GPX yields nothing', () => { - expect(thrown(() => new PlacesController(svc({ importGpx: vi.fn().mockReturnValue(null) } as Partial)).importGpx(user, '5', file, {}))).toEqual({ + expect(thrown(() => new PlacesController(svc({ importGpx: vi.fn().mockReturnValue(null) } as Partial), new RuntimeEnvService()).importGpx(user, '5', file, {}))).toEqual({ status: 400, body: { error: 'No matching places found in GPX file' }, }); }); it('imports and broadcasts per place', () => { const broadcast = vi.fn(); const s = svc({ importGpx: vi.fn().mockReturnValue({ places: [{ id: 1 }, { id: 2 }], count: 2, skipped: 0 }), broadcast } as Partial); - expect(new PlacesController(s).importGpx(user, '5', file, {}, 'sock')).toEqual({ places: [{ id: 1 }, { id: 2 }], count: 2, skipped: 0 }); + expect(new PlacesController(s, new RuntimeEnvService()).importGpx(user, '5', file, {}, 'sock')).toEqual({ places: [{ id: 1 }, { id: 2 }], count: 2, skipped: 0 }); expect(broadcast).toHaveBeenCalledTimes(2); }); }); @@ -93,23 +94,23 @@ describe('PlacesController (parity with the legacy /api/trips/:tripId/places rou describe('POST /import/map', () => { const file = { buffer: Buffer.from(''), originalname: 'm.kml' } as Express.Multer.File; it('400 without a file', async () => { - expect(await thrownAsync(() => new PlacesController(svc()).importMap(user, '5', undefined, {}))).toEqual({ status: 400, body: { error: 'No file uploaded' } }); + expect(await thrownAsync(() => new PlacesController(svc(), new RuntimeEnvService()).importMap(user, '5', undefined, {}))).toEqual({ status: 400, body: { error: 'No file uploaded' } }); }); it('403 without place_edit (permission runs before the file check)', async () => { const importMapFile = vi.fn(); const s = svc({ canEdit: vi.fn().mockReturnValue(false), importMapFile } as Partial); - expect(await thrownAsync(() => new PlacesController(s).importMap(user, '5', file, {}))).toEqual({ status: 403, body: { error: 'No permission' } }); + expect(await thrownAsync(() => new PlacesController(s, new RuntimeEnvService()).importMap(user, '5', file, {}))).toEqual({ status: 403, body: { error: 'No permission' } }); expect(importMapFile).not.toHaveBeenCalled(); }); it('400 when both import types are disabled', async () => { - expect(await thrownAsync(() => new PlacesController(svc()).importMap(user, '5', file, { importPoints: 'false', importPaths: 'false' }))).toEqual({ + expect(await thrownAsync(() => new PlacesController(svc(), new RuntimeEnvService()).importMap(user, '5', file, { importPoints: 'false', importPaths: 'false' }))).toEqual({ status: 400, body: { error: 'No import types selected' }, }); }); it('400 when the map file has no Placemarks (and carries the summary through)', async () => { const summary = { totalPlacemarks: 0 }; const s = svc({ importMapFile: vi.fn().mockResolvedValue({ places: [], summary }) } as Partial); - expect(await thrownAsync(() => new PlacesController(s).importMap(user, '5', file, {}))).toEqual({ + expect(await thrownAsync(() => new PlacesController(s, new RuntimeEnvService()).importMap(user, '5', file, {}))).toEqual({ status: 400, body: { error: 'No valid Placemarks found in map file', summary }, }); }); @@ -117,26 +118,26 @@ describe('PlacesController (parity with the legacy /api/trips/:tripId/places rou const broadcast = vi.fn(); const result = { places: [{ id: 1 }, { id: 2 }], summary: { totalPlacemarks: 2 }, count: 2 }; const s = svc({ importMapFile: vi.fn().mockResolvedValue(result), broadcast } as Partial); - expect(await new PlacesController(s).importMap(user, '5', file, {}, 'sock')).toEqual(result); + expect(await new PlacesController(s, new RuntimeEnvService()).importMap(user, '5', file, {}, 'sock')).toEqual(result); expect(broadcast).toHaveBeenCalledTimes(2); expect(broadcast).toHaveBeenCalledWith('5', 'place:created', { place: { id: 1 } }, 'sock'); }); it('passes a missing summary through (no zero-placemark guard) and still imports', async () => { const result = { places: [{ id: 7 }] }; const s = svc({ importMapFile: vi.fn().mockResolvedValue(result), broadcast: vi.fn() } as Partial); - expect(await new PlacesController(s).importMap(user, '5', file, {})).toEqual(result); + expect(await new PlacesController(s, new RuntimeEnvService()).importMap(user, '5', file, {})).toEqual(result); }); it('wraps a thrown Error from the service in a 400 with its message', async () => { const s = svc({ importMapFile: vi.fn().mockRejectedValue(new Error('bad kml')) } as Partial); - expect(await thrownAsync(() => new PlacesController(s).importMap(user, '5', file, {}))).toEqual({ status: 400, body: { error: 'bad kml' } }); + expect(await thrownAsync(() => new PlacesController(s, new RuntimeEnvService()).importMap(user, '5', file, {}))).toEqual({ status: 400, body: { error: 'bad kml' } }); }); it('falls back to a generic 400 message for a non-Error rejection', async () => { const s = svc({ importMapFile: vi.fn().mockRejectedValue('boom') } as Partial); - expect(await thrownAsync(() => new PlacesController(s).importMap(user, '5', file, {}))).toEqual({ status: 400, body: { error: 'Failed to import map file' } }); + expect(await thrownAsync(() => new PlacesController(s, new RuntimeEnvService()).importMap(user, '5', file, {}))).toEqual({ status: 400, body: { error: 'Failed to import map file' } }); }); it('re-throws an HttpException raised inside the try untouched', async () => { const s = svc({ importMapFile: vi.fn().mockRejectedValue(new HttpException({ error: 'teapot' }, 418)) } as Partial); - expect(await thrownAsync(() => new PlacesController(s).importMap(user, '5', file, {}))).toEqual({ status: 418, body: { error: 'teapot' } }); + expect(await thrownAsync(() => new PlacesController(s, new RuntimeEnvService()).importMap(user, '5', file, {}))).toEqual({ status: 418, body: { error: 'teapot' } }); }); }); @@ -146,29 +147,29 @@ describe('PlacesController (parity with the legacy /api/trips/:tripId/places rou // handler runs. it('maps a service { error, status } to the same response', async () => { const s = svc({ importGoogleList: vi.fn().mockResolvedValue({ error: 'List is empty', status: 400 }) } as Partial); - expect(await thrownAsync(() => new PlacesController(s).importGoogle(user, '5', { url: 'http://x' }))).toEqual({ status: 400, body: { error: 'List is empty' } }); + expect(await thrownAsync(() => new PlacesController(s, new RuntimeEnvService()).importGoogle(user, '5', { url: 'http://x' }))).toEqual({ status: 400, body: { error: 'List is empty' } }); }); it('imports a naver list and returns the count + listName', async () => { const s = svc({ importNaverList: vi.fn().mockResolvedValue({ places: [{ id: 1 }], listName: 'Trip', skipped: 2 }), broadcast: vi.fn() } as Partial); - expect(await new PlacesController(s).importNaver(user, '5', { url: 'http://x' })).toEqual({ places: [{ id: 1 }], count: 1, listName: 'Trip', skipped: 2 }); + expect(await new PlacesController(s, new RuntimeEnvService()).importNaver(user, '5', { url: 'http://x' })).toEqual({ places: [{ id: 1 }], count: 1, listName: 'Trip', skipped: 2 }); }); it('forwards the enrich flag + userId and broadcasts each imported place', async () => { const importGoogleList = vi.fn().mockResolvedValue({ places: [{ id: 1 }, { id: 2 }], listName: 'L', skipped: 0 }); const broadcast = vi.fn(); const s = svc({ importGoogleList, broadcast } as Partial); - expect(await new PlacesController(s).importGoogle(user, '5', { url: 'http://x', enrich: true }, 'sock')).toEqual({ places: [{ id: 1 }, { id: 2 }], count: 2, listName: 'L', skipped: 0 }); + expect(await new PlacesController(s, new RuntimeEnvService()).importGoogle(user, '5', { url: 'http://x', enrich: true }, 'sock')).toEqual({ places: [{ id: 1 }, { id: 2 }], count: 2, listName: 'L', skipped: 0 }); expect(importGoogleList).toHaveBeenCalledWith('5', 'http://x', { enrich: true, userId: 1 }); expect(broadcast).toHaveBeenCalledTimes(2); }); it('wraps a thrown Error in the provider-specific 400 (Google)', async () => { const s = svc({ importGoogleList: vi.fn().mockRejectedValue(new Error('network down')) } as Partial); - expect(await thrownAsync(() => new PlacesController(s).importGoogle(user, '5', { url: 'http://x' }))).toEqual({ + expect(await thrownAsync(() => new PlacesController(s, new RuntimeEnvService()).importGoogle(user, '5', { url: 'http://x' }))).toEqual({ status: 400, body: { error: 'Failed to import Google Maps list. Make sure the list is shared publicly.' }, }); }); it('wraps a non-Error rejection in the provider-specific 400 (Naver)', async () => { const s = svc({ importNaverList: vi.fn().mockRejectedValue('weird') } as Partial); - expect(await thrownAsync(() => new PlacesController(s).importNaver(user, '5', { url: 'http://x' }))).toEqual({ + expect(await thrownAsync(() => new PlacesController(s, new RuntimeEnvService()).importNaver(user, '5', { url: 'http://x' }))).toEqual({ status: 400, body: { error: 'Failed to import Naver Maps list. Make sure the list is shared publicly.' }, }); }); @@ -180,14 +181,14 @@ describe('PlacesController (parity with the legacy /api/trips/:tripId/places rou // element before the handler runs. it('returns empty for an empty list without touching the service', () => { const removeMany = vi.fn(); - expect(new PlacesController(svc({ removeMany } as Partial)).bulkDelete(user, '5', { ids: [] })).toEqual({ deleted: [], count: 0 }); + expect(new PlacesController(svc({ removeMany } as Partial), new RuntimeEnvService()).bulkDelete(user, '5', { ids: [] })).toEqual({ deleted: [], count: 0 }); expect(removeMany).not.toHaveBeenCalled(); }); it('deletes, fires hooks + broadcasts per deleted id', () => { const removeMany = vi.fn().mockReturnValue([1, 2]); const onDeleted = vi.fn(); const broadcast = vi.fn(); const scopedIds = vi.fn().mockReturnValue([1, 2]); const s = svc({ removeMany, onDeleted, broadcast, scopedIds } as Partial); - expect(new PlacesController(s).bulkDelete(user, '5', { ids: [1, 2] }, 'sock')).toEqual({ deleted: [1, 2], count: 2 }); + expect(new PlacesController(s, new RuntimeEnvService()).bulkDelete(user, '5', { ids: [1, 2] }, 'sock')).toEqual({ deleted: [1, 2], count: 2 }); expect(onDeleted).toHaveBeenCalledTimes(2); expect(broadcast).toHaveBeenCalledTimes(2); }); @@ -199,7 +200,7 @@ describe('PlacesController (parity with the legacy /api/trips/:tripId/places rou const scopedIds = vi.fn().mockReturnValue([1]); const onDeleted = vi.fn(); const s = svc({ removeMany, scopedIds, onDeleted, broadcast: vi.fn() } as Partial); - new PlacesController(s).bulkDelete(user, '5', { ids: [1, 99] }); + new PlacesController(s, new RuntimeEnvService()).bulkDelete(user, '5', { ids: [1, 99] }); expect(scopedIds).toHaveBeenCalledWith('5', [1, 99]); expect(onDeleted).toHaveBeenCalledTimes(1); expect(onDeleted).toHaveBeenCalledWith(1); @@ -209,24 +210,24 @@ describe('PlacesController (parity with the legacy /api/trips/:tripId/places rou describe('POST /bulk-update', () => { it('404 when trip not accessible, 403 without place_edit (before any write)', () => { - expect(thrown(() => new PlacesController(svc({ verifyTripAccess: vi.fn().mockReturnValue(undefined) })).bulkUpdate(user, '5', { ids: [1], category_id: 3 }))).toEqual({ status: 404, body: { error: 'Trip not found' } }); - expect(thrown(() => new PlacesController(svc({ canEdit: vi.fn().mockReturnValue(false) })).bulkUpdate(user, '5', { ids: [1], category_id: 3 }))).toEqual({ status: 403, body: { error: 'No permission' } }); + expect(thrown(() => new PlacesController(svc({ verifyTripAccess: vi.fn().mockReturnValue(undefined) }), new RuntimeEnvService()).bulkUpdate(user, '5', { ids: [1], category_id: 3 }))).toEqual({ status: 404, body: { error: 'Trip not found' } }); + expect(thrown(() => new PlacesController(svc({ canEdit: vi.fn().mockReturnValue(false) }), new RuntimeEnvService()).bulkUpdate(user, '5', { ids: [1], category_id: 3 }))).toEqual({ status: 403, body: { error: 'No permission' } }); }); // Same as bulk-delete: placeBulkUpdateRequestSchema types `ids`, so the // pipe owns that 400 now. `.min(1)` was deliberately left off the schema so // the empty-list short-circuit below stays reachable. it('400 when no patch field is present', () => { - expect(thrown(() => new PlacesController(svc()).bulkUpdate(user, '5', { ids: [1] }))).toEqual({ status: 400, body: { error: 'Provide at least one field to update' } }); + expect(thrown(() => new PlacesController(svc(), new RuntimeEnvService()).bulkUpdate(user, '5', { ids: [1] }))).toEqual({ status: 400, body: { error: 'Provide at least one field to update' } }); }); it('returns empty for an empty list without touching the service', () => { const updateMany = vi.fn(); - expect(new PlacesController(svc({ updateMany } as Partial)).bulkUpdate(user, '5', { ids: [] })).toEqual({ updated: [], count: 0 }); + expect(new PlacesController(svc({ updateMany } as Partial), new RuntimeEnvService()).bulkUpdate(user, '5', { ids: [] })).toEqual({ updated: [], count: 0 }); expect(updateMany).not.toHaveBeenCalled(); }); it('updates, fires hooks + broadcasts per updated place', () => { const updateMany = vi.fn().mockReturnValue([{ id: 1 }, { id: 2 }]); const onUpdated = vi.fn(); const broadcast = vi.fn(); const s = svc({ updateMany, onUpdated, broadcast } as Partial); - expect(new PlacesController(s).bulkUpdate(user, '5', { ids: [1, 2], category_id: 3 }, 'sock')).toEqual({ updated: [1, 2], count: 2 }); + expect(new PlacesController(s, new RuntimeEnvService()).bulkUpdate(user, '5', { ids: [1, 2], category_id: 3 }, 'sock')).toEqual({ updated: [1, 2], count: 2 }); expect(updateMany).toHaveBeenCalledWith('5', [1, 2], { category_id: 3 }); expect(onUpdated).toHaveBeenCalledTimes(2); expect(broadcast).toHaveBeenCalledWith('5', 'place:updated', { place: { id: 1 } }, 'sock'); @@ -234,22 +235,22 @@ describe('PlacesController (parity with the legacy /api/trips/:tripId/places rou it('passes category_id: null through to clear the category', () => { const updateMany = vi.fn().mockReturnValue([{ id: 1 }]); const s = svc({ updateMany } as Partial); - expect(new PlacesController(s).bulkUpdate(user, '5', { ids: [1], category_id: null })).toEqual({ updated: [1], count: 1 }); + expect(new PlacesController(s, new RuntimeEnvService()).bulkUpdate(user, '5', { ids: [1], category_id: null })).toEqual({ updated: [1], count: 1 }); expect(updateMany).toHaveBeenCalledWith('5', [1], { category_id: null }); }); }); it('GET /:id returns the place when found, 404 when missing', () => { - expect(thrown(() => new PlacesController(svc({ get: vi.fn().mockReturnValue(undefined) } as Partial)).get(user, '5', '9'))).toEqual({ status: 404, body: { error: 'Place not found' } }); + expect(thrown(() => new PlacesController(svc({ get: vi.fn().mockReturnValue(undefined) } as Partial), new RuntimeEnvService()).get(user, '5', '9'))).toEqual({ status: 404, body: { error: 'Place not found' } }); const s = svc({ get: vi.fn().mockReturnValue({ id: 9 }) } as Partial); - expect(new PlacesController(s).get(user, '5', '9')).toEqual({ place: { id: 9 } }); + expect(new PlacesController(s, new RuntimeEnvService()).get(user, '5', '9')).toEqual({ place: { id: 9 } }); }); it('PUT /:id 404 when missing, else updates + hooks', () => { - expect(thrown(() => new PlacesController(svc({ update: vi.fn().mockReturnValue(null) } as Partial)).update(user, '5', '9', { name: 'X' }))).toEqual({ status: 404, body: { error: 'Place not found' } }); + expect(thrown(() => new PlacesController(svc({ update: vi.fn().mockReturnValue(null) } as Partial), new RuntimeEnvService()).update(user, '5', '9', { name: 'X' }))).toEqual({ status: 404, body: { error: 'Place not found' } }); const update = vi.fn().mockReturnValue({ id: 9 }); const onUpdated = vi.fn(); const broadcast = vi.fn(); const s = svc({ update, onUpdated, broadcast } as Partial); - expect(new PlacesController(s).update(user, '5', '9', { name: 'X' }, 'sock')).toEqual({ place: { id: 9 } }); + expect(new PlacesController(s, new RuntimeEnvService()).update(user, '5', '9', { name: 'X' }, 'sock')).toEqual({ place: { id: 9 } }); expect(onUpdated).toHaveBeenCalledWith(9); }); @@ -257,10 +258,10 @@ describe('PlacesController (parity with the legacy /api/trips/:tripId/places rou it('400s on anything that is not a hex colour, before the permission check', () => { const canEdit = vi.fn().mockReturnValue(false); // would 403 if it got that far const err = { status: 400, body: { error: 'route_color must be a hex colour like #4f46e5' } }; - expect(thrown(() => new PlacesController(svc({ canEdit })).update(user, '5', '9', { route_color: 'red' }))).toEqual(err); - expect(thrown(() => new PlacesController(svc({ canEdit })).update(user, '5', '9', { route_color: '#12345' }))).toEqual(err); - expect(thrown(() => new PlacesController(svc({ canEdit })).update(user, '5', '9', { route_color: 123 }))).toEqual(err); - expect(thrown(() => new PlacesController(svc({ canEdit })).create(user, '5', { name: 'T', route_color: 'red' }))).toEqual(err); + expect(thrown(() => new PlacesController(svc({ canEdit }), new RuntimeEnvService()).update(user, '5', '9', { route_color: 'red' }))).toEqual(err); + expect(thrown(() => new PlacesController(svc({ canEdit }), new RuntimeEnvService()).update(user, '5', '9', { route_color: '#12345' }))).toEqual(err); + expect(thrown(() => new PlacesController(svc({ canEdit }), new RuntimeEnvService()).update(user, '5', '9', { route_color: 123 }))).toEqual(err); + expect(thrown(() => new PlacesController(svc({ canEdit }), new RuntimeEnvService()).create(user, '5', { name: 'T', route_color: 'red' }))).toEqual(err); expect(canEdit).not.toHaveBeenCalled(); }); @@ -268,19 +269,19 @@ describe('PlacesController (parity with the legacy /api/trips/:tripId/places rou const update = vi.fn().mockReturnValue({ id: 9, route_color: '#e11d48' }); const broadcast = vi.fn(); const s = svc({ update, broadcast } as Partial); - expect(new PlacesController(s).update(user, '5', '9', { route_color: '#e11d48' }, 'sock')) + expect(new PlacesController(s, new RuntimeEnvService()).update(user, '5', '9', { route_color: '#e11d48' }, 'sock')) .toEqual({ place: { id: 9, route_color: '#e11d48' } }); expect(update).toHaveBeenCalledWith('5', '9', expect.objectContaining({ route_color: '#e11d48' }), undefined); // The colour has to reach the other members too, not just the DB. expect(broadcast).toHaveBeenCalledWith('5', 'place:updated', { place: { id: 9, route_color: '#e11d48' } }, 'sock'); - new PlacesController(svc({ update } as Partial)).update(user, '5', '9', { route_color: null }); + new PlacesController(svc({ update } as Partial), new RuntimeEnvService()).update(user, '5', '9', { route_color: null }); expect(update).toHaveBeenLastCalledWith('5', '9', expect.objectContaining({ route_color: null }), undefined); }); it('accepts the short #abc form', () => { const update = vi.fn().mockReturnValue({ id: 9 }); - expect(new PlacesController(svc({ update } as Partial)).update(user, '5', '9', { route_color: '#abc' })).toEqual({ place: { id: 9 } }); + expect(new PlacesController(svc({ update } as Partial), new RuntimeEnvService()).update(user, '5', '9', { route_color: '#abc' })).toEqual({ place: { id: 9 } }); }); }); @@ -288,7 +289,7 @@ describe('PlacesController (parity with the legacy /api/trips/:tripId/places rou const update = vi.fn().mockReturnValue({ conflict: true, server: { id: 9, name: 'Theirs' } }); const onUpdated = vi.fn(); const broadcast = vi.fn(); const s = svc({ update, onUpdated, broadcast } as Partial); - expect(thrown(() => new PlacesController(s).update(user, '5', '9', { name: 'Mine' }, 'sock', '2026-01-01 00:00:00'))).toEqual({ + expect(thrown(() => new PlacesController(s, new RuntimeEnvService()).update(user, '5', '9', { name: 'Mine' }, 'sock', '2026-01-01 00:00:00'))).toEqual({ status: 409, body: { error: 'conflict', server: { id: 9, name: 'Theirs' } }, }); expect(update).toHaveBeenCalledWith('5', '9', expect.objectContaining({ name: 'Mine' }), '2026-01-01 00:00:00'); @@ -299,11 +300,11 @@ describe('PlacesController (parity with the legacy /api/trips/:tripId/places rou it('DELETE /:id fires the hook then 404 / success', () => { const onDeleted = vi.fn(); const remove = vi.fn().mockReturnValue(false); - expect(thrown(() => new PlacesController(svc({ remove, onDeleted } as Partial)).remove(user, '5', '9'))).toEqual({ status: 404, body: { error: 'Place not found' } }); + expect(thrown(() => new PlacesController(svc({ remove, onDeleted } as Partial), new RuntimeEnvService()).remove(user, '5', '9'))).toEqual({ status: 404, body: { error: 'Place not found' } }); expect(onDeleted).toHaveBeenCalledWith(9); expect(onDeleted.mock.invocationCallOrder[0]).toBeLessThan(remove.mock.invocationCallOrder[0]); const s = svc({ remove: vi.fn().mockReturnValue(true), broadcast: vi.fn() } as Partial); - expect(new PlacesController(s).remove(user, '5', '9')).toEqual({ success: true }); + expect(new PlacesController(s, new RuntimeEnvService()).remove(user, '5', '9')).toEqual({ success: true }); }); // #1745: a place on another trip must 404 without the hook ever running — @@ -312,45 +313,45 @@ describe('PlacesController (parity with the legacy /api/trips/:tripId/places rou it('DELETE /:id 404s a foreign place before the journey hook runs', () => { const onDeleted = vi.fn(); const remove = vi.fn(); const s = svc({ get: vi.fn().mockReturnValue(null), onDeleted, remove } as Partial); - expect(thrown(() => new PlacesController(s).remove(user, '5', '99'))).toEqual({ status: 404, body: { error: 'Place not found' } }); + expect(thrown(() => new PlacesController(s, new RuntimeEnvService()).remove(user, '5', '99'))).toEqual({ status: 404, body: { error: 'Place not found' } }); expect(onDeleted).not.toHaveBeenCalled(); expect(remove).not.toHaveBeenCalled(); }); it('GET /:id/image maps service error + returns photos', async () => { const s = svc({ searchImage: vi.fn().mockResolvedValue({ photos: [{ url: 'x' }] }) } as Partial); - expect(await new PlacesController(s).image(user, '5', '9')).toEqual({ photos: [{ url: 'x' }] }); + expect(await new PlacesController(s, new RuntimeEnvService()).image(user, '5', '9')).toEqual({ photos: [{ url: 'x' }] }); const e = svc({ searchImage: vi.fn().mockResolvedValue({ error: 'No key', status: 400 }) } as Partial); - expect(await thrownAsync(() => new PlacesController(e).image(user, '5', '9'))).toEqual({ status: 400, body: { error: 'No key' } }); + expect(await thrownAsync(() => new PlacesController(e, new RuntimeEnvService()).image(user, '5', '9'))).toEqual({ status: 400, body: { error: 'No key' } }); }); it('GET /:id/image turns an unexpected throw into a 500, but re-throws an HttpException as-is', async () => { const boom = svc({ searchImage: vi.fn().mockRejectedValue(new Error('Unsplash down')) } as Partial); - expect(await thrownAsync(() => new PlacesController(boom).image(user, '5', '9'))).toEqual({ status: 500, body: { error: 'Error searching for image' } }); + expect(await thrownAsync(() => new PlacesController(boom, new RuntimeEnvService()).image(user, '5', '9'))).toEqual({ status: 500, body: { error: 'Error searching for image' } }); const http = svc({ searchImage: vi.fn().mockRejectedValue(new HttpException({ error: 'rate limited' }, 429)) } as Partial); - expect(await thrownAsync(() => new PlacesController(http).image(user, '5', '9'))).toEqual({ status: 429, body: { error: 'rate limited' } }); + expect(await thrownAsync(() => new PlacesController(http, new RuntimeEnvService()).image(user, '5', '9'))).toEqual({ status: 429, body: { error: 'rate limited' } }); }); describe('POST /:id/image (custom place image #1136)', () => { const file = { filename: 'abc.jpg' } as Express.Multer.File; it('404 when the trip is not accessible, 403 without place_edit', () => { - expect(thrown(() => new PlacesController(svc({ verifyTripAccess: vi.fn().mockReturnValue(undefined) })).uploadImage(user, '5', '9', file))).toEqual({ status: 404, body: { error: 'Trip not found' } }); - expect(thrown(() => new PlacesController(svc({ canEdit: vi.fn().mockReturnValue(false) })).uploadImage(user, '5', '9', file))).toEqual({ status: 403, body: { error: 'No permission' } }); + expect(thrown(() => new PlacesController(svc({ verifyTripAccess: vi.fn().mockReturnValue(undefined) }), new RuntimeEnvService()).uploadImage(user, '5', '9', file))).toEqual({ status: 404, body: { error: 'Trip not found' } }); + expect(thrown(() => new PlacesController(svc({ canEdit: vi.fn().mockReturnValue(false) }), new RuntimeEnvService()).uploadImage(user, '5', '9', file))).toEqual({ status: 403, body: { error: 'No permission' } }); }); it('400 when no file was uploaded', () => { - expect(thrown(() => new PlacesController(svc()).uploadImage(user, '5', '9', undefined))).toEqual({ status: 400, body: { error: 'No image uploaded' } }); + expect(thrown(() => new PlacesController(svc(), new RuntimeEnvService()).uploadImage(user, '5', '9', undefined))).toEqual({ status: 400, body: { error: 'No image uploaded' } }); }); it('404 when the place is missing (service returns null)', () => { - expect(thrown(() => new PlacesController(svc({ update: vi.fn().mockReturnValue(null) } as Partial)).uploadImage(user, '5', '9', file))).toEqual({ status: 404, body: { error: 'Place not found' } }); + expect(thrown(() => new PlacesController(svc({ update: vi.fn().mockReturnValue(null) } as Partial), new RuntimeEnvService()).uploadImage(user, '5', '9', file))).toEqual({ status: 404, body: { error: 'Place not found' } }); }); it('stores the uploaded file as image_url, broadcasts + fires the update hook', () => { const update = vi.fn().mockReturnValue({ id: 9 }); const onUpdated = vi.fn(); const broadcast = vi.fn(); const s = svc({ update, onUpdated, broadcast } as Partial); - expect(new PlacesController(s).uploadImage(user, '5', '9', file, 'sock')).toEqual({ place: { id: 9 } }); + expect(new PlacesController(s, new RuntimeEnvService()).uploadImage(user, '5', '9', file, 'sock')).toEqual({ place: { id: 9 } }); expect(update).toHaveBeenCalledWith('5', '9', { image_url: '/uploads/places/abc.jpg' }); expect(broadcast).toHaveBeenCalledWith('5', 'place:updated', { place: { id: 9 } }, 'sock'); expect(onUpdated).toHaveBeenCalledWith(9); @@ -361,7 +362,7 @@ describe('PlacesController (parity with the legacy /api/trips/:tripId/places rou process.env.DEMO_MODE = 'true'; const demo = { ...user, email: 'demo@trek.app' } as User; try { - expect(thrown(() => new PlacesController(svc()).uploadImage(demo, '5', '9', file))).toEqual({ + expect(thrown(() => new PlacesController(svc(), new RuntimeEnvService()).uploadImage(demo, '5', '9', file))).toEqual({ status: 403, body: { error: 'Uploads are disabled in demo mode. Self-host TREK for full functionality.' }, }); } finally { diff --git a/server/tests/unit/nest/trips.controller.test.ts b/server/tests/unit/nest/trips.controller.test.ts index 916159dc0..7de402043 100644 --- a/server/tests/unit/nest/trips.controller.test.ts +++ b/server/tests/unit/nest/trips.controller.test.ts @@ -1,4 +1,5 @@ import { describe, it, expect, vi, beforeEach } from 'vitest'; +import { RuntimeEnvService } from '../../../src/nest/app-config/runtime-env.service'; import { HttpException } from '@nestjs/common'; import type { Request } from 'express'; @@ -26,7 +27,7 @@ const req = { headers: {} } as Request; // wrapper keeps the historical construction sites positional. const writeAudit = vi.fn(); const audit = { writeAudit } as unknown as AuditService; -const tc = (s: TripsService) => new TripsController(s, audit); +const tc = (s: TripsService) => new TripsController(s, audit, new RuntimeEnvService()); function svc(o: Partial = {}): TripsService { return {