diff --git a/server/CLAUDE.md b/server/CLAUDE.md index dbf891b4c..a78b35a23 100644 --- a/server/CLAUDE.md +++ b/server/CLAUDE.md @@ -97,7 +97,7 @@ SQLite is the engine today and Postgres is the planned second one. Code under `s - **Logging**: `src/nest/audit/audit-log.logger.ts` (`logInfo`/`logWarn`/`logError`/`logDebug`) prints each line to the console and queues it for `data/logs/trek.log`, which `log-file.ts` appends in batches off the event loop and rotates at 10 MB over five files; `index.ts` flushes what is queued on exit. Never write the log file synchronously from a request path. - **Request correlation** (`src/nest/common/request-correlation.ts`): every HTTP request runs under an id (a well-formed incoming `X-Request-Id`, else a fresh UUID) that the response carries in `X-Request-Id`, and every log line written inside it is tagged `[http ]`. The exception filter leaves a 5xx on `res.locals` so the access log writes the request line and the stack as one entry. The other entry points (MCP tool/resource/prompt calls via the registry's `around` seam, WebSocket messages in `TrekWsAdapter`, plugin RPC dispatches in the supervisor, cron ticks in `CronRegistrarService`) each run under their own id through `traceEntry()` (`nest/audit/entry-trace.logger.ts`), which logs one line per call: debug on success, debug as `skipped` when the call declines its work on purpose (a cron tick whose lease another process holds), warn or error with the reason on failure. A new entry point goes through `traceEntry()` too. - **Process safety nets** (`src/index.ts`, `src/shutdown.ts`): `unhandledRejection` and `uncaughtException` are logged to trek.log and end in the same orderly shutdown as SIGTERM, with exit code 1; a second fatal error during it exits at once. A shutdown first marks `ReadinessService` (`nest/health/`) as draining, so `/api/health/ready` answers 503 for the whole drain. The HTTP server keeps idle keep-alive connections for `HTTP_KEEP_ALIVE_TIMEOUT_MS` (65 s, above the usual proxy idle timeout) with the headers timeout one second above it (`applyProxyTimeouts` in `bootstrap.ts`). -- **Process-local state sits behind injectable ports**, each with an in-memory implementation that keeps today's behaviour: `RateLimitStore` (`nest/common/rate-limit.store.ts`, provided by `RateLimitModule`), `PendingCodeStore` (`nest/oauth/oauth.pending-codes.ts`), `OidcFlowStore` (`nest/oidc/oidc-flow.store.ts`), `PermissionsCacheStore` (`nest/permissions/permissions-cache.ts`) and `RoomRegistry` (`nest/realtime/ws-state.ts`). The owning module provides the implementation, and a hand-built consumer falls back to the same in-memory one, so a store shared between processes replaces a provider rather than the consumers. New process-local state follows the same shape; TREK still runs as one process. +- **Process-local state sits behind injectable ports**, each with an in-memory implementation that keeps today's behaviour: `RateLimitStore` (`nest/common/rate-limit.store.ts`, provided by `RateLimitModule`), `PendingCodeStore` (`nest/oauth/oauth.pending-codes.ts`), `OidcFlowStore` (`nest/oidc/oidc-flow.store.ts`), `PermissionsCacheStore` (`nest/permissions/permissions-cache.ts`) and `RoomRegistry` (`nest/realtime/ws-state.ts`). The owning module provides the implementation. Every port a shared store could back returns promises (a store in the database or on the network is async); `RoomRegistry` stays synchronous because membership holds live sockets, and several processes would need a fan-out in front of `broadcast()` instead. The process-wide ports (`PendingCodeStore`, `PermissionsCacheStore`, `RoomRegistry`) also have readers outside the container (the pending-code sweep, the backup restore's cache flush, the broadcast functions, hand-built consumers), so their module installs the store the container resolved in a `ProcessStoreSlot` (`nest/common/process-store-slot.ts`) those readers use: overriding the provider changes every reader. New process-local state follows the same shape; TREK still runs as one process. - **`src/middleware/globalMiddleware.ts`** is the only Express middleware left (helmet/CSP, CORS, HSTS, forced-HTTPS, logging, cookies). ## Tests diff --git a/server/src/nest/atlas/public-stats.controller.ts b/server/src/nest/atlas/public-stats.controller.ts index 96b1024aa..ca2d6e6e9 100644 --- a/server/src/nest/atlas/public-stats.controller.ts +++ b/server/src/nest/atlas/public-stats.controller.ts @@ -47,7 +47,7 @@ export class PublicStatsController { @Get('stats') async stats(@Req() req: Request): Promise { - enforcePublicApiRateLimit(this.rl, req); + await enforcePublicApiRateLimit(this.rl, req); requireScope(req, 'stats'); const userId = requireUserId(req); diff --git a/server/src/nest/auth/auth-public.controller.ts b/server/src/nest/auth/auth-public.controller.ts index 0093a02f1..84b416118 100644 --- a/server/src/nest/auth/auth-public.controller.ts +++ b/server/src/nest/auth/auth-public.controller.ts @@ -29,8 +29,8 @@ const delay = (ms: number) => new Promise((r) => setTimeout(r, ms)); export class AuthPublicController { constructor(private readonly auth: AuthService, private readonly rl: RateLimitService, private readonly audit: AuditService) {} - private limit(bucket: string, req: Request, max: number): void { - if (!this.rl.check(bucket, req.ip || 'unknown', max, WINDOW, Date.now())) { + private async limit(bucket: string, req: Request, max: number): Promise { + if (!(await this.rl.check(bucket, req.ip || 'unknown', max, WINDOW, Date.now()))) { throw new HttpException({ error: 'Too many attempts. Please try again later.' }, 429); } } @@ -57,7 +57,7 @@ export class AuthPublicController { @Get('invite/:token') @Public('the invite token IS the credential') async invite(@Param('token') token: string, @Req() req: Request) { - this.limit('login', req, 10); + await this.limit('login', req, 10); const result = await this.auth.validateInviteToken(token); if (result.error) { throw new HttpException({ error: result.error }, result.status!); @@ -69,7 +69,7 @@ export class AuthPublicController { @Public('creating the account that would carry the session') @HttpCode(201) async register(@Body() body: RegisterDto, @Req() req: Request, @Res({ passthrough: true }) res: Response) { - this.limit('login', req, 10); + await this.limit('login', req, 10); const result = await this.auth.registerUser(body); if (result.error) { throw new HttpException({ error: result.error }, result.status!); @@ -83,7 +83,7 @@ export class AuthPublicController { @Public('the login itself') @HttpCode(200) async login(@Body() body: LoginDto, @Req() req: Request, @Res({ passthrough: true }) res: Response) { - this.limit('login', req, 10); + await this.limit('login', req, 10); const started = Date.now(); const result = await this.auth.loginUser(body); if (result.auditAction) { @@ -111,7 +111,7 @@ export class AuthPublicController { @Public('reached by somebody who cannot log in') @HttpCode(200) async forgotPassword(@Body() body: ForgotPasswordDto, @Req() req: Request) { - this.limit('forgot', req, 3); + await this.limit('forgot', req, 3); const started = Date.now(); const rawEmail = typeof body?.email === 'string' ? body.email : ''; const ip = getClientIp(req); @@ -141,7 +141,7 @@ export class AuthPublicController { async resetPassword(@Body() body: ResetPasswordDto, @Req() req: Request) { // Per-IP brute-force guard, parity with the legacy resetLimiter (5 / 15 min on // a dedicated bucket) — without it reset tokens could be guessed unthrottled. - this.limit('reset', req, 5); + await this.limit('reset', req, 5); const ip = getClientIp(req); const result = await this.auth.resetPassword(body); if (result.error) { @@ -159,7 +159,7 @@ export class AuthPublicController { @Public('second factor of a login that has no session yet') @HttpCode(200) async verifyMfaLogin(@Body() body: MfaVerifyLoginDto, @Req() req: Request, @Res({ passthrough: true }) res: Response) { - this.limit('mfa', req, 5); + await this.limit('mfa', req, 5); const result = await this.auth.verifyMfaLogin(body); if (result.error) { throw new HttpException({ error: result.error }, result.status!); diff --git a/server/src/nest/auth/auth.controller.ts b/server/src/nest/auth/auth.controller.ts index 6a182a5aa..f7c26c620 100644 --- a/server/src/nest/auth/auth.controller.ts +++ b/server/src/nest/auth/auth.controller.ts @@ -75,8 +75,8 @@ export const AVATAR_FILE_FILTER: Options['fileFilter'] = (_req, file, cb) => { export class AuthController { constructor(private readonly auth: AuthService, private readonly profile: UserProfileService, private readonly tokens: TokenService, private readonly rl: RateLimitService, private readonly audit: AuditService, private readonly env: RuntimeEnvService, private readonly storage: StorageService) {} - private limit(bucket: string, req: Request, max: number): void { - if (!this.rl.check(bucket, req.ip || 'unknown', max, WINDOW, Date.now())) { + private async limit(bucket: string, req: Request, max: number): Promise { + if (!(await this.rl.check(bucket, req.ip || 'unknown', max, WINDOW, Date.now()))) { throw new HttpException({ error: 'Too many attempts. Please try again later.' }, 429); } } @@ -90,8 +90,8 @@ export class AuthController { * (login, register, forgot-password) keep the IP key — there is no account * to charge yet. */ - private limitUser(bucket: string, userId: number, max: number): void { - if (!this.rl.check(bucket, String(userId), max, WINDOW, Date.now())) { + private async limitUser(bucket: string, userId: number, max: number): Promise { + if (!(await this.rl.check(bucket, String(userId), max, WINDOW, Date.now()))) { throw new HttpException({ error: 'Too many attempts. Please try again later.' }, 429); } } @@ -108,7 +108,7 @@ export class AuthController { @Put('me/password') async changePassword(@CurrentUser() user: User, @Body() body: ChangePasswordDto, @Req() req: Request, @Res({ passthrough: true }) res: Response) { - this.limit('login', req, 5); + await this.limit('login', req, 5); // Carry the session's remember choice into the re-issued token/cookie so a // "remember me" login survives a password change (#1927). Bearer callers // have no cookie → undefined → the historical default duration. @@ -275,7 +275,7 @@ export class AuthController { @MfaExempt('completing setup is the way out of the policy') @HttpCode(200) async mfaEnable(@CurrentUser() user: User, @Body() body: MfaEnableDto, @Req() req: Request) { - this.limit('mfa', req, 5); + await this.limit('mfa', req, 5); const result = await this.auth.enableMfa(user.id, body.code); if (result.error) { throw new HttpException({ error: result.error }, result.status!); @@ -287,7 +287,7 @@ export class AuthController { @Post('mfa/disable') @HttpCode(200) async mfaDisable(@CurrentUser() user: User, @Body() body: MfaDisableDto, @Req() req: Request) { - this.limit('login', req, 5); + await this.limit('login', req, 5); const result = await this.auth.disableMfa(user.id, user.email, body); if (result.error) { throw new HttpException({ error: result.error }, result.status!); @@ -305,7 +305,7 @@ export class AuthController { @Post('mcp-tokens') @HttpCode(201) async createMcpToken(@CurrentUser() user: User, @Body() body: McpTokenCreateDto, @Req() req: Request) { - this.limit('login', req, 5); + await this.limit('login', req, 5); const result = await this.tokens.createMcpToken(user.id, body.name); if (result.error) { throw new HttpException({ error: result.error }, result.status!); @@ -342,7 +342,7 @@ export class AuthController { @Post('api-tokens') @HttpCode(201) async createApiToken(@CurrentUser() user: User, @Body() body: ApiTokenCreateDto, @Req() req: Request) { - this.limit('login', req, 5); + await this.limit('login', req, 5); // No `scopes` means the key reads everything, which is what every key minted // before this field existed does. Narrowing stays opt-in so the change // cannot break an integration that is already running. @@ -370,7 +370,7 @@ export class AuthController { // above any real client, which mints one token per socket connect, but it // stops a single account from filling the process-wide ephemeral store and // 503-ing every other user's ws and download tokens. - this.limitUser('ws_token', user.id, 120); + await this.limitUser('ws_token', user.id, 120); const result = await this.tokens.createWsToken(user.id); if (result.error) { throw new HttpException({ error: result.error }, result.status!); @@ -380,8 +380,8 @@ export class AuthController { @Post('resource-token') @HttpCode(200) - resourceToken(@CurrentUser() user: User, @Body() body: ResourceTokenDto) { - this.limitUser('resource_token', user.id, 120); + async resourceToken(@CurrentUser() user: User, @Body() body: ResourceTokenDto) { + await this.limitUser('resource_token', user.id, 120); const token = this.tokens.createResourceToken(user.id, body.purpose); if (!token) { throw new HttpException({ error: 'Service unavailable' }, 503); diff --git a/server/src/nest/auth/passkey.controller.ts b/server/src/nest/auth/passkey.controller.ts index ce7d3a4cf..8f0e50b90 100644 --- a/server/src/nest/auth/passkey.controller.ts +++ b/server/src/nest/auth/passkey.controller.ts @@ -36,8 +36,8 @@ export class PasskeyController { private readonly passkeys: PasskeyService, ) {} - private limit(bucket: string, req: Request, max: number): void { - if (!this.rl.check(bucket, req.ip || 'unknown', max, WINDOW, Date.now())) { + private async limit(bucket: string, req: Request, max: number): Promise { + if (!(await this.rl.check(bucket, req.ip || 'unknown', max, WINDOW, Date.now()))) { throw new HttpException({ error: 'Too many attempts. Please try again later.' }, 429); } } @@ -48,7 +48,7 @@ export class PasskeyController { @HttpCode(200) @UseGuards(PasskeyEnabledGuard, JwtAuthGuard) async registerOptions(@CurrentUser() user: User, @Body() body: PasskeyRegisterOptionsDto, @Req() req: Request) { - this.limit('mfa', req, 5); + await this.limit('mfa', req, 5); // The Origin header is only a pre-ceremony sanity input (see PasskeyService) // — the RP ID itself is never derived from request headers. const result = await this.passkeys.passkeyRegisterOptions(user.id, body?.password, req.headers.origin); @@ -73,7 +73,7 @@ export class PasskeyController { @HttpCode(200) @UseGuards(PasskeyEnabledGuard) async loginOptions(@Req() req: Request) { - this.limit('login', req, 10); + await this.limit('login', req, 10); const result = await this.passkeys.passkeyLoginOptions(req.headers.origin); if (result.error) throw new HttpException({ error: result.error }, result.status!); return result.options; @@ -84,7 +84,7 @@ export class PasskeyController { @HttpCode(200) @UseGuards(PasskeyEnabledGuard) async loginVerify(@Body() body: PasskeyLoginVerifyDto, @Req() req: Request, @Res({ passthrough: true }) res: Response) { - this.limit('login', req, 10); + await this.limit('login', req, 10); const started = Date.now(); const result = await this.passkeys.passkeyLoginVerify(body); if (result.auditAction) { @@ -119,7 +119,7 @@ export class PasskeyController { @Delete('credentials/:id') @UseGuards(JwtAuthGuard) async remove(@CurrentUser() user: User, @Param('id') id: string, @Body() body: PasskeyDeleteDto, @Req() req: Request) { - this.limit('login', req, 5); + await this.limit('login', req, 5); const result = await this.passkeys.deletePasskey(user.id, id, body?.password); if (result.error) throw new HttpException({ error: result.error }, result.status!); await this.audit.writeAudit({ userId: user.id, action: 'user.passkey_delete', resource: String(id), ip: getClientIp(req) }); diff --git a/server/src/nest/backup/backup.impl.ts b/server/src/nest/backup/backup.impl.ts index 071422c50..5e58dcfff 100644 --- a/server/src/nest/backup/backup.impl.ts +++ b/server/src/nest/backup/backup.impl.ts @@ -447,9 +447,13 @@ export async function restoreFromZip({ storage, database }: BackupDeps, zipPath: // still holds the pre-restore state. Any request using a cached // permission would decide against the wrong grants until the // next restart. Dropping the cache forces a fresh read. - // D6: no repository read happens on this path today (invalidation is a - // plain function, not a DB call), so no withRequestContext is owed here. - invalidatePermissionsCache(); + // D6: no repository read happens on this path today. The flush goes to + // the store PermissionsModule installed (permissionsCacheSlot), and the + // in-memory one makes no DB call, so no withRequestContext is owed here + // yet. A store that reads the database, or the domain phase that gives + // this restore path a repository read (Plan 3's admin/backup cluster), + // must wrap it then; see task-2-review.md's non-HTTP caller table. + await invalidatePermissionsCache(); } if (!reinitFailed) { @@ -517,7 +521,7 @@ export async function restoreFromZip({ storage, database }: BackupDeps, zipPath: // we leave the process in after a failed restore. // D6: same no-repository-read note as the other invalidatePermissionsCache() // call above — nothing to wrap yet. - try { invalidatePermissionsCache(); } catch { /* best-effort */ } + try { await invalidatePermissionsCache(); } catch { /* best-effort */ } throw err; } } diff --git a/server/src/nest/collab/collab.service.ts b/server/src/nest/collab/collab.service.ts index 7c3652552..e27c88931 100644 --- a/server/src/nest/collab/collab.service.ts +++ b/server/src/nest/collab/collab.service.ts @@ -646,18 +646,20 @@ export class CollabService { // entry yet, since the first has not answered. Joining the running fetch keeps // that a single outbound request instead of twenty. const running = this.inFlight.get(url); - if (running !== undefined) return { ...(await running), url }; - - // Charged per outbound fetch rather than per request, which is what the - // budget is actually protecting. Without a user there is no one to charge — - // no caller passes that today, and the fetch stays behind the SSRF guard. - if (userId !== undefined && !this.rateLimit.check('collab_link_preview', String(userId), PREVIEW_FETCHES_PER_MINUTE, 60_000, Date.now())) { - return { ...fallback, rateLimited: true }; + if (running !== undefined) { + const shared = await running; + // The asker this call joined was out of budget, which says nothing about + // this caller's: ask again, first in line or behind the next asker. + if (shared.rateLimited) return this.linkPreview(url, userId); + return { ...shared, url }; } // Memoised in-flight fetch, not a missing await: the promise is stored so the - // concurrent askers above can join it, and this frame awaits it below. - const task = this.fetchPreview(url, fallback); + // concurrent askers above can join it, and this frame awaits it below. It is + // stored before the budget check, which awaits its store, so an asker that + // arrives meanwhile joins this one instead of being charged for a fetch it + // does not make. + const task = this.chargeAndFetch(url, fallback, userId); this.inFlight.set(url, task); try { return await task; @@ -666,6 +668,18 @@ export class CollabService { } } + /** + * Charged per outbound fetch rather than per request, which is what the + * budget is actually protecting. Without a user there is no one to charge: + * no caller passes that today, and the fetch stays behind the SSRF guard. + */ + private async chargeAndFetch(url: string, fallback: LinkPreviewResult, userId?: number): Promise { + if (userId !== undefined && !(await this.rateLimit.check('collab_link_preview', String(userId), PREVIEW_FETCHES_PER_MINUTE, 60_000, Date.now()))) { + return { ...fallback, rateLimited: true }; + } + return this.fetchPreview(url, fallback); + } + /** The outbound half of linkPreview, past the cache and the budget. */ private async fetchPreview(url: string, fallback: LinkPreviewResult): Promise { const ssrf = await checkSsrf(url, true); diff --git a/server/src/nest/common/process-store-slot.ts b/server/src/nest/common/process-store-slot.ts new file mode 100644 index 000000000..a57d21f13 --- /dev/null +++ b/server/src/nest/common/process-store-slot.ts @@ -0,0 +1,41 @@ +/** + * The store a piece of process-wide state is kept in right now. + * + * A store port (the permissions cache, the OAuth pending codes, the socket + * rooms) is provided by its module, so a store shared between processes can + * be swapped in there. Some readers cannot take it through a constructor: the + * backup restore is plain functions, the broadcasts are free functions every + * domain calls, and the no-Nest test harnesses build consumers by hand. They + * read the slot instead, and the owning module installs whatever the + * container resolved for the port, so swapping the provider changes every + * reader, not only the injected ones. + * + * Starts on the in-memory default, which is also what the module provides + * unless it is overridden, so a process that never builds the module (a + * hand-built test) sees what it saw before. + */ +export class ProcessStoreSlot { + private current: T; + + constructor(private readonly fallback: T) { + this.current = fallback; + } + + /** The store every out-of-container reader uses now. */ + get(): T { + return this.current; + } + + /** Called by the owning module with the store the container resolved. */ + install(store: T): void { + this.current = store; + } + + /** + * Called when that module is torn down: back to the default, unless a later + * app has installed its own store since. + */ + release(store: T): void { + if (this.current === store) this.current = this.fallback; + } +} diff --git a/server/src/nest/common/rate-limit.service.ts b/server/src/nest/common/rate-limit.service.ts index 4a879cc13..5989d8d30 100644 --- a/server/src/nest/common/rate-limit.service.ts +++ b/server/src/nest/common/rate-limit.service.ts @@ -2,7 +2,7 @@ import { Injectable } from '@nestjs/common'; import { InMemoryRateLimitStore, RateLimitStore } from './rate-limit.store'; /** - * Per-IP rate limiting for the auth-adjacent routes. `check` returns false + * Per-IP rate limiting for the auth-adjacent routes. `check` resolves false * once a key exceeds `max` within `windowMs` (the caller answers 429). * * The counting lives in an injected RateLimitStore. RateLimitModule provides @@ -13,13 +13,13 @@ import { InMemoryRateLimitStore, RateLimitStore } from './rate-limit.store'; export class RateLimitService { constructor(private readonly store: RateLimitStore = new InMemoryRateLimitStore()) {} - /** Returns true when the request is allowed, false when it should be rejected (429). */ - check(bucket: string, key: string, max: number, windowMs: number, now: number): boolean { + /** Resolves true when the request is allowed, false when it should be rejected (429). */ + check(bucket: string, key: string, max: number, windowMs: number, now: number): Promise { return this.store.hit(bucket, key, max, windowMs, now); } /** Test helper: clear a bucket (mirrors the legacy exported maps used for resets). */ - reset(bucket?: string): void { - this.store.reset(bucket); + reset(bucket?: string): Promise { + return this.store.reset(bucket); } } diff --git a/server/src/nest/common/rate-limit.store.ts b/server/src/nest/common/rate-limit.store.ts index d70643581..e6669d81d 100644 --- a/server/src/nest/common/rate-limit.store.ts +++ b/server/src/nest/common/rate-limit.store.ts @@ -5,7 +5,8 @@ * as one process; with several, each would count on its own and a client * could spread its attempts over them. A store shared between processes * replaces InMemoryRateLimitStore in RateLimitModule without touching the - * service or its callers. + * service or its callers. Both methods return a promise, because such a + * store lives in the database or on the network. */ export abstract class RateLimitStore { /** @@ -13,9 +14,9 @@ export abstract class RateLimitStore { * already used `max` attempts in the current `windowMs` window (the caller * answers 429), true otherwise. */ - abstract hit(bucket: string, key: string, max: number, windowMs: number, now: number): boolean; + abstract hit(bucket: string, key: string, max: number, windowMs: number, now: number): Promise; /** Clear one bucket, or every bucket. */ - abstract reset(bucket?: string): void; + abstract reset(bucket?: string): Promise; } interface Attempt { @@ -40,7 +41,26 @@ export class InMemoryRateLimitStore extends RateLimitStore { /** Last sweep per bucket, so a busy bucket doesn't keep a quiet one from being cleaned. */ private readonly lastSweep = new Map(); - hit(bucket: string, key: string, max: number, windowMs: number, now: number): boolean { + hit(bucket: string, key: string, max: number, windowMs: number, now: number): Promise { + return Promise.resolve(this.count(bucket, key, max, windowMs, now)); + } + + reset(bucket?: string): Promise { + if (bucket) this.buckets.get(bucket)?.clear(); + else { + this.buckets.clear(); + this.lastSweep.clear(); + } + return Promise.resolve(); + } + + /** How many keys a bucket holds right now; the housekeeping is only visible here. */ + size(bucket: string): number { + return this.buckets.get(bucket)?.size ?? 0; + } + + /** The window check and the count, in one synchronous step so two hits cannot interleave. */ + private count(bucket: string, key: string, max: number, windowMs: number, now: number): boolean { const store = this.store(bucket); this.sweep(bucket, store, windowMs, now); const record = store.get(key); @@ -55,19 +75,6 @@ export class InMemoryRateLimitStore extends RateLimitStore { return true; } - reset(bucket?: string): void { - if (bucket) this.buckets.get(bucket)?.clear(); - else { - this.buckets.clear(); - this.lastSweep.clear(); - } - } - - /** How many keys a bucket holds right now; the housekeeping is only visible here. */ - size(bucket: string): number { - return this.buckets.get(bucket)?.size ?? 0; - } - private store(bucket: string): Map { let s = this.buckets.get(bucket); if (!s) { diff --git a/server/src/nest/oauth/oauth-api.controller.ts b/server/src/nest/oauth/oauth-api.controller.ts index 5ac8ccfcb..bf5ff5a81 100644 --- a/server/src/nest/oauth/oauth-api.controller.ts +++ b/server/src/nest/oauth/oauth-api.controller.ts @@ -34,7 +34,7 @@ export class OauthApiController { @Get('authorize/validate') @UseGuards(OptionalJwtGuard) async validate(@Req() req: Request, @Query() params: Partial, @Res({ passthrough: true }) res: Response) { - if (!this.rl.check('oauth_validate', req.ip || 'unknown', 30, MIN, Date.now())) { + if (!(await this.rl.check('oauth_validate', req.ip || 'unknown', 30, MIN, Date.now()))) { throw new HttpException({ error: 'too_many_requests', error_description: 'Too many attempts. Please try again later.' }, 429); } if (!(await this.oauth.mcpEnabled())) { @@ -100,7 +100,7 @@ export class OauthApiController { } const scopes = validation.scopes!; await this.oauth.saveConsent(body.client_id, user.id, scopes, ip); - const code = this.oauth.createAuthCode({ + const code = await this.oauth.createAuthCode({ clientId: body.client_id, userId: user.id, redirectUri: body.redirect_uri, diff --git a/server/src/nest/oauth/oauth-public.controller.ts b/server/src/nest/oauth/oauth-public.controller.ts index ccd325537..5a4c6b5b3 100644 --- a/server/src/nest/oauth/oauth-public.controller.ts +++ b/server/src/nest/oauth/oauth-public.controller.ts @@ -37,7 +37,7 @@ export class OauthPublicController { if (!(await this.oauth.mcpEnabled())) { res.status(404).end(); return; } const body: Record = typeof req.body === 'object' && req.body ? req.body : {}; - if (!this.rl.check('oauth_token', `${req.ip}|${body.client_id ?? ''}`, 30, MIN, Date.now())) { + if (!(await this.rl.check('oauth_token', `${req.ip}|${body.client_id ?? ''}`, 30, MIN, Date.now()))) { res.status(429).json({ error: 'too_many_requests', error_description: 'Too many attempts. Please try again later.' }); return; } @@ -58,7 +58,7 @@ export class OauthPublicController { res.status(400).json({ error: 'invalid_request', error_description: 'code, redirect_uri, and code_verifier are required' }); return; } - const pending = this.oauth.consumeAuthCode(code); + const pending = await this.oauth.consumeAuthCode(code); const invalidGrant = async (reason: string, userId: number | null) => { await this.audit.writeAudit({ userId, action: 'oauth.token.grant_failed', details: { client_id, reason }, ip }); res.status(400).json({ error: 'invalid_grant', error_description: 'Authorization grant is invalid.' }); @@ -155,7 +155,7 @@ export class OauthPublicController { @Post('revoke') async revoke(@Req() req: Request, @Res() res: Response): Promise { if (!(await this.oauth.mcpEnabled())) { res.status(404).end(); return; } - if (!this.rl.check('oauth_revoke', req.ip || 'unknown', 10, MIN, Date.now())) { + if (!(await this.rl.check('oauth_revoke', req.ip || 'unknown', 10, MIN, Date.now()))) { res.status(429).json({ error: 'too_many_requests', error_description: 'Too many attempts. Please try again later.' }); return; } diff --git a/server/src/nest/oauth/oauth-sdk.provider.ts b/server/src/nest/oauth/oauth-sdk.provider.ts index bfb66ac75..0475c35c4 100644 --- a/server/src/nest/oauth/oauth-sdk.provider.ts +++ b/server/src/nest/oauth/oauth-sdk.provider.ts @@ -162,7 +162,7 @@ export class TrekOAuthProvider implements OAuthServerProvider { redirectUri?: string, resource?: URL, ): Promise { - const pending = this.oauth.consumeAuthCode(code); + const pending = await this.oauth.consumeAuthCode(code); if (!pending || pending.clientId !== client.client_id) throw new Error('Authorization grant is invalid.'); diff --git a/server/src/nest/oauth/oauth.module.ts b/server/src/nest/oauth/oauth.module.ts index 35552a1d6..8a7354ce6 100644 --- a/server/src/nest/oauth/oauth.module.ts +++ b/server/src/nest/oauth/oauth.module.ts @@ -1,5 +1,5 @@ import { RateLimitModule } from '../common/rate-limit.module'; -import { Module } from '@nestjs/common'; +import { Module, type OnModuleDestroy } from '@nestjs/common'; import type { MiddlewareConsumer, NestModule } from '@nestjs/common'; import { MikroOrmModule } from '@mikro-orm/nestjs'; import { authorizationHandler } from '@modelcontextprotocol/sdk/server/auth/handlers/authorize'; @@ -18,7 +18,7 @@ import { OauthClients } from '../../db/entities/OauthClients.entity'; import { OauthTokens } from '../../db/entities/OauthTokens.entity'; import { OauthConsents } from '../../db/entities/OauthConsents.entity'; import { Users } from '../../db/entities/Users.entity'; -import { PendingCodeStore, processPendingCodes } from './oauth.pending-codes'; +import { PendingCodeStore, pendingCodesSlot, processPendingCodes } from './oauth.pending-codes'; /** * OAuth 2.1 server (MCP). Public token/userinfo/revoke endpoints + the SPA's @@ -40,7 +40,10 @@ import { PendingCodeStore, processPendingCodes } from './oauth.pending-codes'; * Pending authorization codes live behind the PendingCodeStore port * (oauth.pending-codes.ts), provided here as the process-wide in-memory * instance: the consent controller (container singleton) writes them, the - * SDK exchange path reads them back through the same injected singleton. + * SDK exchange path reads them back through the same injected singleton. The + * constructor installs whichever store the container resolved in + * pendingCodesSlot, so the sweep and a hand-built OauthService follow a + * swapped provider too. * * Exports OauthService for AdminController (admin OAuth-session panel) and the * MCP transport's token verification. @@ -67,12 +70,19 @@ import { PendingCodeStore, processPendingCodes } from './oauth.pending-codes'; ], exports: [OauthService], }) -export class OauthModule implements NestModule { +export class OauthModule implements NestModule, OnModuleDestroy { constructor( private readonly addons: AddonsService, private readonly provider: TrekOAuthProvider, private readonly clients: TrekClientsStore, - ) {} + private readonly pendingCodes: PendingCodeStore, + ) { + pendingCodesSlot.install(pendingCodes); + } + + onModuleDestroy(): void { + pendingCodesSlot.release(this.pendingCodes); + } configure(consumer: MiddlewareConsumer): void { const mcpAddonGate = createMcpAddonGate(this.addons); diff --git a/server/src/nest/oauth/oauth.pending-codes.ts b/server/src/nest/oauth/oauth.pending-codes.ts index 05c0add2c..2fe82cffc 100644 --- a/server/src/nest/oauth/oauth.pending-codes.ts +++ b/server/src/nest/oauth/oauth.pending-codes.ts @@ -1,10 +1,14 @@ +import { ProcessStoreSlot } from '../common/process-store-slot'; +import { logError } from '../audit/audit-log.logger'; + /** * The short-lived authorization-code store. * * An injectable port, PendingCodeStore, with the in-memory implementation * below. One instance is process-wide, with an import-time sweep on purpose * (legacy parity, the atlas-geo interval precedent): OauthModule provides - * that instance and a hand-built OauthService defaults to it. Sharing is not + * that instance and installs whatever the container resolved in + * pendingCodesSlot, which the sweep and a hand-built OauthService read. Sharing is not * a stylistic choice: the consent controller writes a code through the container OauthService and the SDK exchange path * (oauth-sdk.provider.ts) reads it back — through the same injected singleton * in production, but the integration harness and any second OauthService @@ -34,58 +38,64 @@ export const AUTH_CODE_TTL_MS = 2 * 60 * 1000; // 2 minutes * Where pending authorization codes are kept between the consent and the * token exchange. In memory today, which ties an authorization to the process * that issued it; a store shared between processes replaces the process - * instance in OauthModule without touching OauthService. + * instance in OauthModule without touching OauthService. Every method returns + * a promise, because such a store lives in the database or on the network. */ export abstract class PendingCodeStore { /** Keep `entry` under `code`; false when the store is at capacity (the caller answers with a null code). */ - abstract put(code: string, entry: PendingCode): boolean; + abstract put(code: string, entry: PendingCode): Promise; /** Single use: the entry is removed even when it turns out to be expired, and an expired one is null. */ - abstract take(code: string): PendingCode | null; + abstract take(code: string): Promise; /** Drop everything past its TTL. */ - abstract sweep(now?: number): void; + abstract sweep(now?: number): Promise; } /** The current behaviour: one capped map in this process's memory. */ export class InMemoryPendingCodeStore extends PendingCodeStore { private readonly codes = new Map(); - put(code: string, entry: PendingCode): boolean { - if (this.codes.size >= MAX_PENDING_CODES) return false; + put(code: string, entry: PendingCode): Promise { + if (this.codes.size >= MAX_PENDING_CODES) return Promise.resolve(false); this.codes.set(code, entry); - return true; + return Promise.resolve(true); } - take(code: string): PendingCode | null { + take(code: string): Promise { const entry = this.codes.get(code); - if (!entry) return null; + if (!entry) return Promise.resolve(null); this.codes.delete(code); - if (Date.now() > entry.expiresAt) return null; - return entry; + if (Date.now() > entry.expiresAt) return Promise.resolve(null); + return Promise.resolve(entry); } - sweep(now = Date.now()): void { + sweep(now = Date.now()): Promise { for (const [key, entry] of this.codes) { if (now > entry.expiresAt) this.codes.delete(key); } + return Promise.resolve(); } } -/** The one store this process shares: the consent route writes it, the SDK exchange reads it. */ +/** The in-memory store this process starts with, and what OauthModule provides unless overridden. */ export const processPendingCodes: PendingCodeStore = new InMemoryPendingCodeStore(); +/** + * The store the readers outside the container use: the import-time sweep and + * a hand-built OauthService. OauthModule installs the one the container + * resolved, so swapping the provider moves them too. + */ +export const pendingCodesSlot = new ProcessStoreSlot(processPendingCodes); + /** Drop everything past its TTL. Named so the interval body is reachable from a test. */ -export function sweepPendingCodes(now = Date.now()): void { - processPendingCodes.sweep(now); +export function sweepPendingCodes(now = Date.now()): Promise { + return pendingCodesSlot.get().sweep(now); } -setInterval(() => sweepPendingCodes(), 60_000).unref(); - -/** Returns false when the store is at capacity — the caller answers with a null code. */ -export function putPendingCode(code: string, entry: PendingCode): boolean { - return processPendingCodes.put(code, entry); +/** The interval body: a sweep that fails is logged, never thrown into the timer. */ +export function runPendingCodeSweep(now = Date.now()): Promise { + return sweepPendingCodes(now).catch((err: unknown) => { + logError(`OAuth pending-code sweep failed: ${err instanceof Error ? err.message : String(err)}`); + }); } -/** Single-use: the entry is removed even when it turns out to be expired. */ -export function takePendingCode(code: string): PendingCode | null { - return processPendingCodes.take(code); -} +setInterval(() => void runPendingCodeSweep(), 60_000).unref(); diff --git a/server/src/nest/oauth/oauth.service.ts b/server/src/nest/oauth/oauth.service.ts index 930bd3e10..b9d8b4db2 100644 --- a/server/src/nest/oauth/oauth.service.ts +++ b/server/src/nest/oauth/oauth.service.ts @@ -35,7 +35,7 @@ import { redirectUriMatches, timingSafeEqualHex, } from './oauth.helpers'; -import { AUTH_CODE_TTL_MS, PendingCodeStore, processPendingCodes, type PendingCode } from './oauth.pending-codes'; +import { AUTH_CODE_TTL_MS, PendingCodeStore, pendingCodesSlot, type PendingCode } from './oauth.pending-codes'; export type { PendingCode } from './oauth.pending-codes'; @@ -89,7 +89,7 @@ export class OauthService { @InjectRepository(OauthConsents) private readonly consents: OauthConsentsRepository, private readonly addons: AddonsService, private readonly audit: AuditService, - private readonly pendingCodes: PendingCodeStore = processPendingCodes, + private readonly pendingCodes: PendingCodeStore = pendingCodesSlot.get(), ) {} async mcpEnabled(): Promise { return this.addons.isAddonEnabled(ADDON_IDS.MCP); } @@ -231,7 +231,7 @@ export class OauthService { // Auth code (in-memory, 2-minute TTL) // ------------------------------------------------------------------------- - createAuthCode(params: { + async createAuthCode(params: { clientId: string; userId: number; redirectUri: string; @@ -239,13 +239,13 @@ export class OauthService { resource: string | null; codeChallenge: string; codeChallengeMethod: 'S256'; - }): string | null { + }): Promise { const rawCode = randomBytes(32).toString('hex'); - const stored = this.pendingCodes.put(rawCode, { ...params, expiresAt: Date.now() + AUTH_CODE_TTL_MS }); + const stored = await this.pendingCodes.put(rawCode, { ...params, expiresAt: Date.now() + AUTH_CODE_TTL_MS }); return stored ? rawCode : null; } - consumeAuthCode(code: string): PendingCode | null { + consumeAuthCode(code: string): Promise { return this.pendingCodes.take(code); } diff --git a/server/src/nest/oidc/oidc-flow.store.ts b/server/src/nest/oidc/oidc-flow.store.ts index b31874595..ca61afae7 100644 --- a/server/src/nest/oidc/oidc-flow.store.ts +++ b/server/src/nest/oidc/oidc-flow.store.ts @@ -29,19 +29,20 @@ export interface OidcAuthCode { * In memory today, which ties a login to the process that started it; with * several processes behind one address the callback can land on another one. * A store shared between processes replaces InMemoryOidcFlowStore in - * OidcModule without touching OidcService. + * OidcModule without touching OidcService. Every method returns a promise, + * because such a store lives in the database or on the network. */ export abstract class OidcFlowStore { - abstract putState(state: string, entry: OidcPendingState): void; + abstract putState(state: string, entry: OidcPendingState): Promise; /** The entry, removed in the same step; null when unknown. */ - abstract takeState(state: string): OidcPendingState | null; - abstract putCode(code: string, entry: OidcAuthCode): void; + abstract takeState(state: string): Promise; + abstract putCode(code: string, entry: OidcAuthCode): Promise; /** The entry, removed in the same step, expired or not (the caller decides); null when unknown. */ - abstract takeCode(code: string): OidcAuthCode | null; + abstract takeCode(code: string): Promise; /** Drop states created more than `ttlMs` before `now`. */ - abstract sweepStates(now: number, ttlMs: number): void; + abstract sweepStates(now: number, ttlMs: number): Promise; /** Drop codes created more than `ttlMs` before `now`. */ - abstract sweepCodes(now: number, ttlMs: number): void; + abstract sweepCodes(now: number, ttlMs: number): Promise; } /** The current behaviour: two maps in this process's memory. */ @@ -49,37 +50,41 @@ export class InMemoryOidcFlowStore extends OidcFlowStore { private readonly states = new Map(); private readonly codes = new Map(); - putState(state: string, entry: OidcPendingState): void { + putState(state: string, entry: OidcPendingState): Promise { this.states.set(state, entry); + return Promise.resolve(); } - takeState(state: string): OidcPendingState | null { + takeState(state: string): Promise { const entry = this.states.get(state); - if (!entry) return null; + if (!entry) return Promise.resolve(null); this.states.delete(state); - return entry; + return Promise.resolve(entry); } - putCode(code: string, entry: OidcAuthCode): void { + putCode(code: string, entry: OidcAuthCode): Promise { this.codes.set(code, entry); + return Promise.resolve(); } - takeCode(code: string): OidcAuthCode | null { + takeCode(code: string): Promise { const entry = this.codes.get(code); - if (!entry) return null; + if (!entry) return Promise.resolve(null); this.codes.delete(code); - return entry; + return Promise.resolve(entry); } - sweepStates(now: number, ttlMs: number): void { + sweepStates(now: number, ttlMs: number): Promise { for (const [state, entry] of this.states) { if (now - entry.createdAt > ttlMs) this.states.delete(state); } + return Promise.resolve(); } - sweepCodes(now: number, ttlMs: number): void { + sweepCodes(now: number, ttlMs: number): Promise { for (const [code, entry] of this.codes) { if (now - entry.created > ttlMs) this.codes.delete(code); } + return Promise.resolve(); } } diff --git a/server/src/nest/oidc/oidc.controller.ts b/server/src/nest/oidc/oidc.controller.ts index c35bc8768..0c8b5c921 100644 --- a/server/src/nest/oidc/oidc.controller.ts +++ b/server/src/nest/oidc/oidc.controller.ts @@ -69,7 +69,7 @@ export class OidcController { const query = oidcLoginQuerySchema.safeParse(req.query); const inviteToken = query.success ? query.data.invite : (req.query.invite as string | undefined); const remember = query.success && query.data.remember !== undefined ? query.data.remember === '1' : undefined; - const { state, codeChallenge } = this.oidc.createState(redirectUri, inviteToken, remember); + const { state, codeChallenge } = await this.oidc.createState(redirectUri, inviteToken, remember); // Bind the state to THIS browser. The callback requires a matching cookie, // so an attacker-initiated login (whose callback URL carries a valid state // from the shared server map) cannot be replayed in a victim's browser to @@ -117,7 +117,7 @@ export class OidcController { // Require the callback to come from the browser that started the flow. if (!boundState || boundState !== state) return f('/login?oidc_error=invalid_state'); - const pending = this.oidc.consumeState(state); + const pending = await this.oidc.consumeState(state); if (!pending) return f('/login?oidc_error=invalid_state'); const config = await this.oidc.getOidcConfig(); @@ -203,7 +203,7 @@ export class OidcController { // "absent", not `false`, or the sliding renewal would later downgrade the // default persistent cookie to a browser-session one (remember-me, #1927). const jwtToken = await this.oidc.generateToken(result.user, pending.remember); - const { code: authCode, binding } = this.oidc.createAuthCode(jwtToken, pending.remember); + const { code: authCode, binding } = await this.oidc.createAuthCode(jwtToken, pending.remember); // Bind the code to THIS browser, the way the state cookie binds the callback. // The code rides home in a URL, so it is readable from history, from a // referrer and from anything that logs URLs; without a second half nobody @@ -219,7 +219,7 @@ export class OidcController { } @Get('exchange') - exchange(@Query('code') code: string | undefined, @Req() req: Request, @Res() res: Response): void { + async exchange(@Query('code') code: string | undefined, @Req() req: Request, @Res() res: Response): Promise { // The binding cookie is single-use like the state cookie: one redemption // attempt per callback, whatever its outcome. const binding = (req.cookies as Record | undefined)?.[OIDC_EXCHANGE_COOKIE]; @@ -229,7 +229,7 @@ export class OidcController { res.status(400).json({ error: 'Code required' }); return; } - const result = this.oidc.consumeAuthCode(code, binding); + const result = await this.oidc.consumeAuthCode(code, binding); if ('error' in result) { res.status(400).json({ error: result.error }); return; diff --git a/server/src/nest/oidc/oidc.service.ts b/server/src/nest/oidc/oidc.service.ts index 618335bf8..4f8089f41 100644 --- a/server/src/nest/oidc/oidc.service.ts +++ b/server/src/nest/oidc/oidc.service.ts @@ -14,6 +14,7 @@ import { TripMembershipService } from '../trip-membership/trip-membership.servic import { setAuthCookie, RememberOption } from '../common/cookie'; import { AuthService } from '../auth/auth.service'; import { UnitOfWork } from '../database/unit-of-work'; +import { logError } from '../audit/audit-log.logger'; import { safeFetchAdminConfigured } from '../../utils/ssrfGuard'; import { Users } from '../../db/entities/Users.entity'; import type { UsersRepository, UserRow } from '../../db/repositories/Users.repository'; @@ -21,7 +22,7 @@ import { InviteTokens } from '../../db/entities/InviteTokens.entity'; import type { InviteTokensRepository, InviteTokenRow } from '../../db/repositories/InviteTokens.repository'; import { AppSettings } from '../../db/entities/AppSettings.entity'; import type { AppSettingsRepository } from '../../db/repositories/AppSettings.repository'; -import { InMemoryOidcFlowStore, OidcFlowStore } from './oidc-flow.store'; +import { InMemoryOidcFlowStore, OidcFlowStore, type OidcPendingState } from './oidc-flow.store'; // --------------------------------------------------------------------------- // Types @@ -247,8 +248,14 @@ export class OidcService implements OnModuleDestroy { @InjectRepository(AppSettings) private readonly appSettings: AppSettingsRepository, private readonly flows: OidcFlowStore = new InMemoryOidcFlowStore(), ) { - this.stateSweeper = setInterval(() => this.flows.sweepStates(Date.now(), STATE_TTL), STATE_CLEANUP); - this.codeSweeper = setInterval(() => this.flows.sweepCodes(Date.now(), AUTH_CODE_TTL), AUTH_CODE_CLEANUP); + const sweepFailed = (err: unknown) => + logError(`OIDC flow sweep failed: ${err instanceof Error ? err.message : String(err)}`); + this.stateSweeper = setInterval(() => { + this.flows.sweepStates(Date.now(), STATE_TTL).catch(sweepFailed); + }, STATE_CLEANUP); + this.codeSweeper = setInterval(() => { + this.flows.sweepCodes(Date.now(), AUTH_CODE_TTL).catch(sweepFailed); + }, AUTH_CODE_CLEANUP); } onModuleDestroy(): void { @@ -265,15 +272,19 @@ export class OidcService implements OnModuleDestroy { // Creates the login state and a matching PKCE pair. The verifier stays server // side (in the flow store); the S256 challenge goes to the provider so PKCE- // required setups (e.g. Pocket ID with PKCE = required) work. - createState(redirectUri: string, inviteToken?: string, remember?: boolean): { state: string; codeChallenge: string } { + async createState( + redirectUri: string, + inviteToken?: string, + remember?: boolean, + ): Promise<{ state: string; codeChallenge: string }> { const state = crypto.randomBytes(32).toString('hex'); const codeVerifier = base64url(crypto.randomBytes(32)); const codeChallenge = base64url(crypto.createHash('sha256').update(codeVerifier).digest()); - this.flows.putState(state, { createdAt: Date.now(), redirectUri, inviteToken, codeVerifier, remember }); + await this.flows.putState(state, { createdAt: Date.now(), redirectUri, inviteToken, codeVerifier, remember }); return { state, codeChallenge }; } - consumeState(state: string) { + consumeState(state: string): Promise { return this.flows.takeState(state); } @@ -284,18 +295,21 @@ export class OidcService implements OnModuleDestroy { * cookie, so redeeming the code takes both halves and only the browser that * completed the provider handshake has both. */ - createAuthCode(token: string, remember?: boolean): { code: string; binding: string } { + async createAuthCode(token: string, remember?: boolean): Promise<{ code: string; binding: string }> { const authCode: string = uuidv4(); const binding = crypto.randomBytes(32).toString('base64url'); - this.flows.putCode(authCode, { token, created: Date.now(), remember, bindingHash: sha256Hex(binding) }); + await this.flows.putCode(authCode, { token, created: Date.now(), remember, bindingHash: sha256Hex(binding) }); return { code: authCode, binding }; } - consumeAuthCode(code: string, binding?: string): { token: string; remember?: boolean } | { error: string } { + async consumeAuthCode( + code: string, + binding?: string, + ): Promise<{ token: string; remember?: boolean } | { error: string }> { // Single use, burnt on every outcome: a code seen by someone else must not // survive their attempt for a second guess, and the browser that owns it can // simply log in again. - const entry = this.flows.takeCode(code); + const entry = await this.flows.takeCode(code); if (!entry) return { error: 'Invalid or expired code' }; if (Date.now() - entry.created > AUTH_CODE_TTL) return { error: 'Code expired' }; // Same wording as the unknown-code case on purpose — whoever presents a code diff --git a/server/src/nest/permissions/permissions-cache.ts b/server/src/nest/permissions/permissions-cache.ts index 9d09f52bc..cc90638ea 100644 --- a/server/src/nest/permissions/permissions-cache.ts +++ b/server/src/nest/permissions/permissions-cache.ts @@ -1,60 +1,69 @@ +import { ProcessStoreSlot } from '../common/process-store-slot'; import type { PermissionLevel } from './permissions.service'; /** * Where the loaded permission levels are kept between requests. * * An injectable port (PermissionsService takes it, PermissionsModule provides - * it) so a store shared between processes can replace the in-memory one - * without touching the service: today a permission change on one process is - * only seen by that process, which is right while TREK runs as one. + * it). Every method returns a promise, because a store shared between + * processes lives in the database or on the network and repositories are + * async; the in-memory one below simply resolves at once. Today a permission + * change on one process is only seen by that process, which is right while + * TREK runs as one. */ export abstract class PermissionsCacheStore { /** The cached levels, or null when nothing is cached. */ - abstract get(): Map | null; + abstract get(): Promise | null>; /** Install a completed read and return it. */ - abstract set(next: Map): Map; + abstract set(next: Map): Promise>; /** Drop the cache, so the next reader loads the stored levels again. */ - abstract invalidate(): void; + abstract invalidate(): Promise; } /** The current behaviour: one map in this process's memory. */ export class InMemoryPermissionsCacheStore extends PermissionsCacheStore { private cache: Map | null = null; - get(): Map | null { - return this.cache; + get(): Promise | null> { + return Promise.resolve(this.cache); } - set(next: Map): Map { + set(next: Map): Promise> { this.cache = next; - return next; + return Promise.resolve(next); } - invalidate(): void { + invalidate(): Promise { this.cache = null; + return Promise.resolve(); } } /** - * The one store this process shares, in its own home (the - * oauth.pending-codes.ts precedent) so every path that must see the same - * cache does: PermissionsModule provides this instance to the container's - * PermissionsService, a hand-built PermissionsService (the no-Nest test - * harnesses) defaults to it, and the plain backup restore path - * (backup.impl.ts), which is free functions by design, flushes it through - * invalidatePermissionsCache() below after a restore rewrites app_settings. + * The in-memory store this process starts with. PermissionsModule provides + * this instance unless the provider is overridden, so the container's + * PermissionsService and every hand-built one (the no-Nest test harnesses) + * share one cache, as they did when it was a module-level map. */ export const processPermissionsCache: PermissionsCacheStore = new InMemoryPermissionsCacheStore(); -export function getPermissionsCache(): Map | null { - return processPermissionsCache.get(); +/** + * The store the readers outside the container use: PermissionsModule installs + * the one the container resolved, so the plain backup restore path + * (backup.impl.ts), which is free functions by design, flushes the same cache + * the request path reads even when the provider is swapped. + */ +export const permissionsCacheSlot = new ProcessStoreSlot(processPermissionsCache); + +export function getPermissionsCache(): Promise | null> { + return permissionsCacheSlot.get().get(); } /** Install a fresh cache map and return it (the loader fills it in place). */ -export function setPermissionsCache(next: Map): Map { - return processPermissionsCache.set(next); +export function setPermissionsCache(next: Map): Promise> { + return permissionsCacheSlot.get().set(next); } -export function invalidatePermissionsCache(): void { - processPermissionsCache.invalidate(); +export function invalidatePermissionsCache(): Promise { + return permissionsCacheSlot.get().invalidate(); } diff --git a/server/src/nest/permissions/permissions.module.ts b/server/src/nest/permissions/permissions.module.ts index 535dd369f..6c8a5a673 100644 --- a/server/src/nest/permissions/permissions.module.ts +++ b/server/src/nest/permissions/permissions.module.ts @@ -1,10 +1,10 @@ -import { Module } from '@nestjs/common'; +import { Module, type OnModuleDestroy } from '@nestjs/common'; import { MikroOrmModule } from '@mikro-orm/nestjs'; import { PermissionsService } from './permissions.service'; import { TripAccessGuard } from './trip-access.guard'; import { TripOwnerGuard } from './trip-owner.guard'; import { AppSettings } from '../../db/entities/AppSettings.entity'; -import { PermissionsCacheStore, processPermissionsCache } from './permissions-cache'; +import { PermissionsCacheStore, permissionsCacheSlot, processPermissionsCache } from './permissions-cache'; /** Cross-cutting permissions domain (Wave 2). No controller/MCP surface of its * own — the admin HTTP surface stays with AdminModule. Exports @@ -16,8 +16,9 @@ import { PermissionsCacheStore, processPermissionsCache } from './permissions-ca * canAccessTrip (Plan 3c), unaffected by this. */ @Module({ imports: [MikroOrmModule.forFeature([AppSettings])], - // The cache store is the process-wide in-memory one, the same instance the - // backup restore flushes; a shared store replaces it here. + // The cache store is the process-wide in-memory one; a shared store replaces + // it here, and the constructor below hands whichever one the container + // resolved to the readers outside it (the backup restore's flush). providers: [ PermissionsService, TripAccessGuard, @@ -26,4 +27,12 @@ import { PermissionsCacheStore, processPermissionsCache } from './permissions-ca ], exports: [PermissionsService, TripAccessGuard, TripOwnerGuard], }) -export class PermissionsModule {} +export class PermissionsModule implements OnModuleDestroy { + constructor(private readonly cacheStore: PermissionsCacheStore) { + permissionsCacheSlot.install(cacheStore); + } + + onModuleDestroy(): void { + permissionsCacheSlot.release(this.cacheStore); + } +} diff --git a/server/src/nest/permissions/permissions.service.ts b/server/src/nest/permissions/permissions.service.ts index 943821c06..02d640c1b 100644 --- a/server/src/nest/permissions/permissions.service.ts +++ b/server/src/nest/permissions/permissions.service.ts @@ -5,7 +5,7 @@ import { UnitOfWork } from '../database/unit-of-work'; import { logError } from '../audit/audit-log.logger'; import { AppSettings } from '../../db/entities/AppSettings.entity'; import type { AppSettingsRepository } from '../../db/repositories/AppSettings.repository'; -import { PermissionsCacheStore, processPermissionsCache } from './permissions-cache'; +import { PermissionsCacheStore, permissionsCacheSlot } from './permissions-cache'; /** * Permission levels (hierarchical, higher includes lower): @@ -64,10 +64,9 @@ export const PERMISSION_ACTIONS: PermissionAction[] = [ const ACTIONS_MAP = new Map(PERMISSION_ACTIONS.map(a => [a.key, a])); // The cache is a PermissionsCacheStore (./permissions-cache), injected so a -// store shared between processes can be plugged in later. PermissionsModule -// provides the process-wide instance, which a hand-built service defaults to -// as well: the backup restore path (backup.impl.ts) is plain functions, no -// DI, and must flush the same cache the request path reads. +// store shared between processes can be plugged in later. A hand-built +// service takes the store installed in permissionsCacheSlot, the one the +// backup restore path (backup.impl.ts, plain functions, no DI) flushes too. @Injectable() @@ -75,11 +74,11 @@ export class PermissionsService { constructor( @InjectRepository(AppSettings) private readonly appSettings: AppSettingsRepository, private readonly uow: UnitOfWork, - private readonly cacheStore: PermissionsCacheStore = processPermissionsCache, + private readonly cacheStore: PermissionsCacheStore = permissionsCacheSlot.get(), ) {} private async loadPermissions(): Promise> { - const cached = this.cacheStore.get(); + const cached = await this.cacheStore.get(); if (cached) return cached; const cache = new Map(); try { @@ -134,8 +133,8 @@ export class PermissionsService { return this.cacheStore.set(cache); } - invalidatePermissionsCache(): void { - this.cacheStore.invalidate(); + invalidatePermissionsCache(): Promise { + return this.cacheStore.invalidate(); } async getPermissionLevel(actionKey: string): Promise { @@ -173,7 +172,7 @@ export class PermissionsService { await this.appSettings.setValue(`perm_${actionKey}`, level); } }); - this.invalidatePermissionsCache(); + await this.invalidatePermissionsCache(); return { skipped }; } diff --git a/server/src/nest/place-enrichment/place-enrichment.controller.ts b/server/src/nest/place-enrichment/place-enrichment.controller.ts index aeb39e30b..bf54266df 100644 --- a/server/src/nest/place-enrichment/place-enrichment.controller.ts +++ b/server/src/nest/place-enrichment/place-enrichment.controller.ts @@ -41,7 +41,7 @@ export class PlaceEnrichmentController { ): Promise { // Keyed by user, not by IP: the cost lands on the key of whoever is signed // in, and a household behind one address should not share a budget. - if (!this.rl.check('place_enrichment', String(user.id), MAX_PER_MINUTE, MINUTE, Date.now())) { + if (!(await this.rl.check('place_enrichment', String(user.id), MAX_PER_MINUTE, MINUTE, Date.now()))) { throw new HttpException({ error: 'Too many requests' }, 429); } diff --git a/server/src/nest/public-api/public-api-request.ts b/server/src/nest/public-api/public-api-request.ts index dbed53680..949080a6d 100644 --- a/server/src/nest/public-api/public-api-request.ts +++ b/server/src/nest/public-api/public-api-request.ts @@ -26,9 +26,9 @@ export const PUBLIC_API_RATE_MAX_PER_MINUTE = 120; * Falls back to a constant key when a request somehow carries no user, so the * limiter can never end up with one shared bucket for every anonymous caller. */ -export function enforcePublicApiRateLimit(rl: RateLimitService, req: Request): void { +export async function enforcePublicApiRateLimit(rl: RateLimitService, req: Request): Promise { const key = `user:${req.user?.id ?? 'unknown'}`; - if (!rl.check('public-api', key, PUBLIC_API_RATE_MAX_PER_MINUTE, PUBLIC_API_RATE_WINDOW_MS, Date.now())) { + if (!(await rl.check('public-api', key, PUBLIC_API_RATE_MAX_PER_MINUTE, PUBLIC_API_RATE_WINDOW_MS, Date.now()))) { throw new HttpException({ error: 'Too many requests. Please slow down.' }, 429); } } diff --git a/server/src/nest/public-api/public-api.controller.ts b/server/src/nest/public-api/public-api.controller.ts index 5843f2257..c17057cdd 100644 --- a/server/src/nest/public-api/public-api.controller.ts +++ b/server/src/nest/public-api/public-api.controller.ts @@ -46,14 +46,14 @@ export class PublicApiController { private readonly rl: RateLimitService, ) {} - private limit(req: Request): void { - enforcePublicApiRateLimit(this.rl, req); + private limit(req: Request): Promise { + return enforcePublicApiRateLimit(this.rl, req); } /** Every trip the token's owner can reach, without itineraries. */ @Get('trips') async listTrips(@Req() req: Request): Promise { - this.limit(req); + await this.limit(req); requireScope(req, 'trips'); return { trips: await this.api.listTrips(requireUserId(req)) }; } @@ -67,7 +67,7 @@ export class PublicApiController { */ @Get('bucket-list') async listBucketList(@Req() req: Request): Promise { - this.limit(req); + await this.limit(req); requireScope(req, 'bucket-list'); return { items: await this.api.listBucketList(requireUserId(req)) }; } @@ -85,7 +85,7 @@ export class PublicApiController { @Param('id') id: string, @Query('include') include?: string, ): Promise { - this.limit(req); + await this.limit(req); requireScope(req, 'trips'); const tripId = parseTripId(id); diff --git a/server/src/nest/realtime/realtime-gateway.module.ts b/server/src/nest/realtime/realtime-gateway.module.ts index 97d4da9bf..637925471 100644 --- a/server/src/nest/realtime/realtime-gateway.module.ts +++ b/server/src/nest/realtime/realtime-gateway.module.ts @@ -1,4 +1,4 @@ -import { Module } from '@nestjs/common'; +import { Module, type OnModuleDestroy } from '@nestjs/common'; import { MikroOrmModule } from '@mikro-orm/nestjs'; import { RealtimeGateway } from './realtime.gateway'; import { EphemeralTokenModule } from '../auth/ephemeral-token.module'; @@ -6,7 +6,7 @@ import { JourneyDomainModule } from '../journey/journey-domain.module'; import { AppSettings } from '../../db/entities/AppSettings.entity'; import { Users } from '../../db/entities/Users.entity'; import { Trips } from '../../db/entities/Trips.entity'; -import { processRooms, RoomRegistry } from './ws-state'; +import { processRooms, RoomRegistry, roomsSlot } from './ws-state'; /** * The transport, kept out of RealtimeModule on purpose. @@ -30,8 +30,17 @@ import { processRooms, RoomRegistry } from './ws-state'; // Task 2 — handleJoin's own canAccessTrip delegate, now TripsRepository // directly. imports: [EphemeralTokenModule, JourneyDomainModule, MikroOrmModule.forFeature([Users, AppSettings, Trips])], - // The room registry is the process-wide in-memory one the broadcast - // functions read too; see RoomRegistry in ws-state.ts. + // The room registry is the process-wide in-memory one; the constructor + // below hands whichever one the container resolved to the broadcast + // functions, so the gateway and the broadcasts never use two registries. providers: [RealtimeGateway, { provide: RoomRegistry, useValue: processRooms }], }) -export class RealtimeGatewayModule {} +export class RealtimeGatewayModule implements OnModuleDestroy { + constructor(private readonly rooms: RoomRegistry) { + roomsSlot.install(rooms); + } + + onModuleDestroy(): void { + roomsSlot.release(this.rooms); + } +} diff --git a/server/src/nest/realtime/realtime.gateway.ts b/server/src/nest/realtime/realtime.gateway.ts index db4728f7f..a2ffe5cb8 100644 --- a/server/src/nest/realtime/realtime.gateway.ts +++ b/server/src/nest/realtime/realtime.gateway.ts @@ -23,9 +23,9 @@ import { logError } from '../audit/audit-log.logger'; import { bookPeers, broadcastToBook, - processRooms, registerSocket, RoomRegistry, + roomsSlot, socketIdOf, userOf, type TrekWebSocket, @@ -45,9 +45,10 @@ const HEARTBEAT_INTERVAL = 30_000; * than an interval nobody stops. * * What did NOT move is the socket registry. It stays one process-wide - * instance in ws-state.ts (`processRooms`, injected here as the RoomRegistry - * port) because out-of-container code (the no-Nest test harnesses, the - * vi.mock'd src/websocket seam) must see the same rooms; see the note there. + * instance in ws-state.ts (the RoomRegistry port, installed in `roomsSlot` by + * RealtimeGatewayModule) because out-of-container code (the no-Nest test + * harnesses, the vi.mock'd src/websocket seam) must see the same rooms; see + * the note there. * * The wire protocol is unchanged, down to the frame names. TrekWsAdapter maps * `{ type }` onto @SubscribeMessage, because the stock adapter dispatches on @@ -74,9 +75,10 @@ export class RealtimeGateway private readonly journeys: JourneyDomainService, @InjectRepository(Users) private readonly users: UsersRepository, @InjectRepository(AppSettings) private readonly appSettings: AppSettingsRepository, - // Room membership, behind its port. The process-wide registry by default: - // the broadcast functions in ws-state.ts read the same instance. - private readonly rooms: RoomRegistry = processRooms, + // Room membership, behind its port. The container's provider, which + // RealtimeGatewayModule also installs for the broadcast functions in + // ws-state.ts; a hand-built gateway takes the installed one. + private readonly rooms: RoomRegistry = roomsSlot.get(), ) {} afterInit(server: WebSocketServer): void { diff --git a/server/src/nest/realtime/ws-state.ts b/server/src/nest/realtime/ws-state.ts index ff934d89c..375987b29 100644 --- a/server/src/nest/realtime/ws-state.ts +++ b/server/src/nest/realtime/ws-state.ts @@ -1,12 +1,15 @@ import { WebSocketServer, WebSocket } from 'ws'; import { emitPluginEvent, pluginEventMeta } from '../../plugin-event-sink'; import { User } from '../../types'; +import { ProcessStoreSlot } from '../common/process-store-slot'; /** * The socket registry: rooms, per-socket identity, and the three fan-out * primitives. * - * MODULE state, not provider state, and that is load-bearing. The no-Nest test + * MODULE state, not provider state, and that is load-bearing: the rooms live + * in the registry `roomsSlot` holds, which the container's provider replaces + * when RealtimeGatewayModule is built and nothing else does. The no-Nest test * harnesses hand-build RealtimeService instances (mcp-test-controllers.ts and * ~60 unit suites), and the 115 vi.mock('src/websocket') seams assert on these * exact exports. If the rooms lived on a provider instance, an out-of-container @@ -31,12 +34,18 @@ export interface TrekWebSocket extends WebSocket { * contributor's sockets would send a pointer moving at ten frames a second to * someone reading the journey on their phone. * - * An injectable port: RealtimeGatewayModule provides `processRooms` and the - * gateway takes it, while the broadcast functions below read the same - * instance. Membership holds live sockets, so it is local to the process by - * nature; running several processes needs a fan-out between them (each - * delivering to its own sockets) rather than a different place to keep these - * sets. + * An injectable port: RealtimeGatewayModule provides it (`processRooms` + * unless overridden) and installs whatever the container resolved in + * `roomsSlot`, which the broadcast functions below read, so the gateway that + * joins sockets and the broadcasts that deliver to them always use one + * registry. + * + * Unlike the other process-state ports this one stays synchronous on purpose. + * Membership holds live sockets, which cannot be kept anywhere but the process + * that owns the connection, and `broadcast()` is a synchronous call every + * domain makes after a write. Running several processes needs a fan-out + * between them (each delivering to its own sockets), which is a port in front + * of `broadcast()`, not a different place to keep these sets. */ export abstract class RoomRegistry { /** Start tracking a new connection's rooms. */ @@ -119,9 +128,12 @@ export class InMemoryRoomRegistry extends RoomRegistry { } } -/** The one registry this process has; see RoomRegistry for who reads it. */ +/** The in-memory registry this process starts with, and what RealtimeGatewayModule provides unless overridden. */ export const processRooms: RoomRegistry = new InMemoryRoomRegistry(); +/** The registry the free functions below use; RealtimeGatewayModule installs the one the container resolved. */ +export const roomsSlot = new ProcessStoreSlot(processRooms); + const socketUser = new WeakMap(); const socketId = new WeakMap(); @@ -150,7 +162,7 @@ export function registerSocket(ws: TrekWebSocket, user: User): number { const sid = nextSocketId++; socketId.set(ws, sid); socketUser.set(ws, user); - processRooms.register(ws); + roomsSlot.get().register(ws); return sid; } @@ -159,15 +171,15 @@ export function userOf(ws: TrekWebSocket): User | undefined { } export function joinRoom(ws: TrekWebSocket, tripId: number): void { - processRooms.join(ws, tripId); + roomsSlot.get().join(ws, tripId); } export function leaveRoom(ws: TrekWebSocket, tripId: number): void { - processRooms.leave(ws, tripId); + roomsSlot.get().leave(ws, tripId); } export function leaveAllRooms(ws: TrekWebSocket): void { - processRooms.leaveAll(ws); + roomsSlot.get().leaveAll(ws); } // ── Studio books ────────────────────────────────────────────────────────── @@ -180,16 +192,16 @@ export interface BookPeer { } export function joinBook(ws: TrekWebSocket, journeyId: number): void { - processRooms.joinBook(ws, journeyId); + roomsSlot.get().joinBook(ws, journeyId); } export function leaveBook(ws: TrekWebSocket, journeyId: number): void { - processRooms.leaveBook(ws, journeyId); + roomsSlot.get().leaveBook(ws, journeyId); } /** Every book this socket had open — called when the connection goes. */ export function leaveAllBooks(ws: TrekWebSocket): number[] { - return processRooms.leaveAllBooks(ws); + return roomsSlot.get().leaveAllBooks(ws); } /** @@ -199,7 +211,7 @@ export function leaveAllBooks(ws: TrekWebSocket): number[] { * pointers, and a list keyed by user could not say which one moved. */ export function bookPeers(journeyId: number): BookPeer[] { - const room = processRooms.bookMembers(journeyId); + const room = roomsSlot.get().bookMembers(journeyId); if (!room) return []; const peers: BookPeer[] = []; for (const ws of room) { @@ -224,7 +236,7 @@ export function broadcastToBook( payload: Record, excludeSid?: number, ): void { - const room = processRooms.bookMembers(journeyId); + const room = roomsSlot.get().bookMembers(journeyId); if (!room || room.size === 0) return; for (const ws of room) { if (ws.readyState !== 1) continue; @@ -259,7 +271,7 @@ export function broadcast( // and with no ws server at all, and skipping plugin:* re-broadcasts so a // plugin's own events can't loop back. if (!eventType.startsWith('plugin:')) emitPluginEvent(tripId, eventType, pluginEventMeta(eventType, payload)); - const room = processRooms.members(tripId); + const room = roomsSlot.get().members(tripId); if (!room || room.size === 0) return; const excludeNum = excludeSid ? Number(excludeSid) : null; diff --git a/server/src/nest/transit/transit.controller.ts b/server/src/nest/transit/transit.controller.ts index 255507fd0..981ce71ba 100644 --- a/server/src/nest/transit/transit.controller.ts +++ b/server/src/nest/transit/transit.controller.ts @@ -28,8 +28,8 @@ export class TransitController { private readonly transit: TransitService, ) {} - private limit(bucket: string, req: Request, max: number): void { - if (!this.rl.check(bucket, req.ip || 'unknown', max, RL_WINDOW, Date.now())) { + private async limit(bucket: string, req: Request, max: number): Promise { + if (!(await this.rl.check(bucket, req.ip || 'unknown', max, RL_WINDOW, Date.now()))) { throw new HttpException({ error: 'Too many requests. Please try again later.' }, 429); } } @@ -48,7 +48,7 @@ export class TransitController { @CurrentUser() user: User, @Req() req: Request, ) { - this.limit('transit_geocode', req, 300); + await this.limit('transit_geocode', req, 300); try { return await this.transit.geocode(q || '', lang, near, user.id); } catch (err) { this.rethrow(err); } @@ -66,7 +66,7 @@ export class TransitController { @CurrentUser() user: User, @Req() req: Request, ) { - this.limit('transit_plan', req, 60); + await this.limit('transit_plan', req, 60); try { return await this.transit.plan({ from: from || '', diff --git a/server/src/nest/transit/transit.mcp.ts b/server/src/nest/transit/transit.mcp.ts index 0465d236d..28b7aca3d 100644 --- a/server/src/nest/transit/transit.mcp.ts +++ b/server/src/nest/transit/transit.mcp.ts @@ -25,9 +25,6 @@ import { SCHEDULED_TRANSIT_MODES, type TransitItinerary } from './transit.helper import { TransitService } from './transit.service'; const TRANSIT_RATE_WINDOW = 15 * 60 * 1000; -// Deliberately its own instance, separate from the REST controller's: the MCP -// buckets are keyed by userId (mcp_transit_*), the REST ones by req.ip. -const transitRateLimiter = new RateLimitService(); const transitModes = z.enum(['TRANSIT', ...SCHEDULED_TRANSIT_MODES]); @@ -40,8 +37,8 @@ function errorResult(err: unknown, fallback: string) { }; } -function rateLimit(userId: number, bucket: string, max: number) { - if (transitRateLimiter.check(bucket, String(userId), max, TRANSIT_RATE_WINDOW, Date.now())) return null; +async function rateLimit(limiter: RateLimitService, userId: number, bucket: string, max: number) { + if (await limiter.check(bucket, String(userId), max, TRANSIT_RATE_WINDOW, Date.now())) return null; return { content: [{ type: 'text' as const, text: 'Too many transit requests. Please try again later.' }], isError: true, @@ -74,6 +71,11 @@ export class TransitMcp { @InjectRepository(Trips) private readonly trips: TripsRepository, private readonly auth: AuthService, private readonly guards: McpToolGuardsService, + // The limiter RateLimitModule provides, so its store is the one every + // other caller counts in. The MCP buckets (mcp_transit_*) are keyed by + // user and the REST ones (transit_*) by address, so they never share a + // count. A hand-built instance (the MCP test harness) gets its own. + private readonly rl: RateLimitService = new RateLimitService(), ) {} @Tool({ @@ -95,7 +97,7 @@ export class TransitMcp { { query, language, near }: { query: string; language?: string; near?: { lat: number; lng: number } }, ctx: McpContext, ) { - const limited = rateLimit(ctx.userId, 'mcp_transit_geocode', 300); + const limited = await rateLimit(this.rl, ctx.userId, 'mcp_transit_geocode', 300); if (limited) return limited; try { return ok(await this.transit.geocode(query, language, near ? `${near.lat},${near.lng}` : undefined, ctx.userId)); @@ -134,7 +136,7 @@ export class TransitMcp { }, ctx: McpContext, ) { - const limited = rateLimit(ctx.userId, 'mcp_transit_plan', 60); + const limited = await rateLimit(this.rl, ctx.userId, 'mcp_transit_plan', 60); if (limited) return limited; try { const result = await this.transit.plan({ diff --git a/server/src/nest/trip-invite/trip-invite.controller.ts b/server/src/nest/trip-invite/trip-invite.controller.ts index 3bb098a41..2a7b8e801 100644 --- a/server/src/nest/trip-invite/trip-invite.controller.ts +++ b/server/src/nest/trip-invite/trip-invite.controller.ts @@ -75,15 +75,15 @@ export class TripInviteLinkController { export class TripInviteController { constructor(private readonly invites: TripInviteService, private readonly rl: RateLimitService, private readonly audit: AuditService) {} - private limit(req: Request, max: number): void { - if (!this.rl.check('trip_invite', req.ip || 'unknown', max, RL_WINDOW, Date.now())) { + private async limit(req: Request, max: number): Promise { + if (!(await this.rl.check('trip_invite', req.ip || 'unknown', max, RL_WINDOW, Date.now()))) { throw new HttpException({ error: 'Too many attempts. Please try again later.' }, 429); } } @Get(':token') async preview(@Param('token') token: string, @Req() req: Request) { - this.limit(req, 30); + await this.limit(req, 30); const resolved = await this.invites.resolve(token); if (!resolved) throw new HttpException({ error: 'Invalid or expired invite link' }, 404); return { trip_id: resolved.trip_id, title: resolved.title }; @@ -92,7 +92,7 @@ export class TripInviteController { @Post(':token/accept') @HttpCode(200) async accept(@CurrentUser() user: User, @Param('token') token: string, @Req() req: Request) { - this.limit(req, 20); + await this.limit(req, 20); const resolved = await this.invites.resolve(token); if (!resolved) throw new HttpException({ error: 'Invalid or expired invite link' }, 404); const result = await this.invites.join(resolved.trip_id, user.id); diff --git a/server/tests/e2e/auth.e2e.test.ts b/server/tests/e2e/auth.e2e.test.ts index e505d15ee..8347ec347 100644 --- a/server/tests/e2e/auth.e2e.test.ts +++ b/server/tests/e2e/auth.e2e.test.ts @@ -201,7 +201,7 @@ describe('Auth e2e (real auth guard + real service + real cookie service + temp }, 10000); it('PUT /me/password preserves the remember choice on the re-issued cookie (#1927)', async () => { - resetRateLimits(app); // earlier login cases share the same per-ip 'login' bucket + await resetRateLimits(app); // earlier login cases share the same per-ip 'login' bucket const seeded = createUser(db as never, { username: 'pw-remember', email: 'pw-remember@example.test' }); const login = await request(server) .post('/api/auth/login') @@ -378,7 +378,7 @@ describe('Auth e2e (real auth guard + real service + real cookie service + temp }); it('login/options gets the same gate on the same derivation chain', async () => { - resetRateLimits(app); // shares the per-ip 'login' bucket with the login cases above + await resetRateLimits(app); // shares the per-ip 'login' bucket with the login cases above const res = await request(server) .post('/api/auth/passkey/login/options') .set('Origin', 'https://trip.example.org') diff --git a/server/tests/e2e/collab.e2e.test.ts b/server/tests/e2e/collab.e2e.test.ts index d1c50ae37..b2364d0cf 100644 --- a/server/tests/e2e/collab.e2e.test.ts +++ b/server/tests/e2e/collab.e2e.test.ts @@ -233,7 +233,7 @@ describe('Collab e2e (real auth guard + temp SQLite)', () => { expect(last).toBe(429); // The counters live on the container singleton, so a spent budget would // follow this user into every test declared after it. - app.get(RateLimitService).reset('collab_link_preview'); + await app.get(RateLimitService).reset('collab_link_preview'); }); it('400 on link-preview without a url', async () => { diff --git a/server/tests/e2e/oidc.e2e.test.ts b/server/tests/e2e/oidc.e2e.test.ts index 1f8af781f..7b244bdc7 100644 --- a/server/tests/e2e/oidc.e2e.test.ts +++ b/server/tests/e2e/oidc.e2e.test.ts @@ -74,8 +74,8 @@ describe('OIDC e2e (real cookie service)', () => { const oidc = app.get(OidcService); vi.spyOn(oidc, 'getOidcConfig').mockResolvedValue({ issuer: 'https://idp', clientId: 'c', clientSecret: 's', displayName: 'SSO', discoveryUrl: null }); vi.spyOn(oidc, 'discover').mockResolvedValue({ authorization_endpoint: 'https://idp/auth', userinfo_endpoint: 'https://idp/ui', issuer: 'https://idp' } as never); - vi.spyOn(oidc, 'createState').mockReturnValue({ state: 'st', codeChallenge: 'cc' }); - consumeAuthCode = vi.spyOn(oidc, 'consumeAuthCode').mockReturnValue({ token: 'jwt.value' }); + vi.spyOn(oidc, 'createState').mockResolvedValue({ state: 'st', codeChallenge: 'cc' }); + consumeAuthCode = vi.spyOn(oidc, 'consumeAuthCode').mockResolvedValue({ token: 'jwt.value' }); }); beforeEach(() => { toggles.oidc_login = true; }); @@ -104,7 +104,7 @@ describe('OIDC e2e (real cookie service)', () => { }); it('GET /exchange sets the httpOnly trek_session cookie + returns the token', async () => { - consumeAuthCode.mockReturnValue({ token: 'jwt.value' }); + consumeAuthCode.mockResolvedValue({ token: 'jwt.value' }); const res = await request(server).get('/api/auth/oidc/exchange').query({ code: 'good' }); expect(res.status).toBe(200); expect(res.body).toEqual({ token: 'jwt.value' }); @@ -113,7 +113,7 @@ describe('OIDC e2e (real cookie service)', () => { }); it('GET /exchange with a remembered code sets a persistent Max-Age cookie (#1927)', async () => { - consumeAuthCode.mockReturnValue({ token: 'jwt.value', remember: true }); + consumeAuthCode.mockResolvedValue({ token: 'jwt.value', remember: true }); const res = await request(server).get('/api/auth/oidc/exchange').query({ code: 'good' }); expect(res.status).toBe(200); const cookie = (res.headers['set-cookie'] as unknown as string[]).find((c) => c.startsWith('trek_session='))!; @@ -121,7 +121,7 @@ describe('OIDC e2e (real cookie service)', () => { }); it('GET /exchange with remember=false sets a browser-session cookie (no Max-Age) (#1927)', async () => { - consumeAuthCode.mockReturnValue({ token: 'jwt.value', remember: false }); + consumeAuthCode.mockResolvedValue({ token: 'jwt.value', remember: false }); const res = await request(server).get('/api/auth/oidc/exchange').query({ code: 'good' }); expect(res.status).toBe(200); const cookie = (res.headers['set-cookie'] as unknown as string[]).find((c) => c.startsWith('trek_session='))!; diff --git a/server/tests/e2e/place-enrichment.e2e.test.ts b/server/tests/e2e/place-enrichment.e2e.test.ts index 70b8f01ec..545dfd133 100644 --- a/server/tests/e2e/place-enrichment.e2e.test.ts +++ b/server/tests/e2e/place-enrichment.e2e.test.ts @@ -77,10 +77,10 @@ describe('Place enrichment e2e (real auth guard + real validation pipe)', () => await app.close(); }); - beforeEach(() => { + beforeEach(async () => { db.prepare('DELETE FROM app_settings').run(); db.prepare('DELETE FROM place_details_cache').run(); - app.get(RateLimitService).reset('place_enrichment'); + await app.get(RateLimitService).reset('place_enrichment'); // spyOn hands back the same spy on a second call, so the call counts carry // over between tests unless they are cleared explicitly. vi.spyOn(maps, 'fetchCommonsCandidates').mockClear().mockResolvedValue([]); diff --git a/server/tests/helpers/test-db.ts b/server/tests/helpers/test-db.ts index 2ddcc4221..1ff5b7163 100644 --- a/server/tests/helpers/test-db.ts +++ b/server/tests/helpers/test-db.ts @@ -182,9 +182,9 @@ export function resetTestDb(db: Database.Database): void { * app.get(RateLimitService) is ambiguous and may hand back the wrong instance — we * resolve the auth controller and clear the limiter it actually uses. */ -export function resetRateLimits(app: INestApplication): void { +export function resetRateLimits(app: INestApplication): Promise { const ctrl = app.get(AuthPublicController, { strict: false }) as unknown as { rl: RateLimitService }; - ctrl.rl.reset(); + return ctrl.rl.reset(); } /** Fixed config mock — use with vi.mock('../../src/config', () => TEST_CONFIG) */ diff --git a/server/tests/integration/admin.test.ts b/server/tests/integration/admin.test.ts index 763c9a340..51c72120a 100644 --- a/server/tests/integration/admin.test.ts +++ b/server/tests/integration/admin.test.ts @@ -36,9 +36,9 @@ beforeAll(async () => { app = nestApp.getHttpAdapter().getInstance(); }); -beforeEach(() => { +beforeEach(async () => { resetTestDb(testDb); - resetRateLimits(nestApp); + await resetRateLimits(nestApp); }); afterAll(async () => { diff --git a/server/tests/integration/assignments.test.ts b/server/tests/integration/assignments.test.ts index 4c6d993f7..194e36888 100644 --- a/server/tests/integration/assignments.test.ts +++ b/server/tests/integration/assignments.test.ts @@ -36,9 +36,9 @@ beforeAll(async () => { app = nestApp.getHttpAdapter().getInstance(); }); -beforeEach(() => { +beforeEach(async () => { resetTestDb(testDb); - resetRateLimits(nestApp); + await resetRateLimits(nestApp); }); afterAll(async () => { diff --git a/server/tests/integration/atlas.test.ts b/server/tests/integration/atlas.test.ts index efad02c69..fb586cf09 100644 --- a/server/tests/integration/atlas.test.ts +++ b/server/tests/integration/atlas.test.ts @@ -64,9 +64,9 @@ beforeAll(async () => { await getRegionGeo(['ZZ']); }, 60_000); -beforeEach(() => { +beforeEach(async () => { resetTestDb(testDb); - resetRateLimits(nestApp); + await resetRateLimits(nestApp); }); afterAll(async () => { diff --git a/server/tests/integration/auth.test.ts b/server/tests/integration/auth.test.ts index 9ce40f9dc..c17c80ccc 100644 --- a/server/tests/integration/auth.test.ts +++ b/server/tests/integration/auth.test.ts @@ -42,10 +42,10 @@ beforeAll(async () => { app = nestApp.getHttpAdapter().getInstance(); }); -beforeEach(() => { +beforeEach(async () => { resetTestDb(testDb); // Reset rate limiter state between tests so they don't interfere - resetRateLimits(nestApp); + await resetRateLimits(nestApp); }); afterAll(async () => { diff --git a/server/tests/integration/backup.test.ts b/server/tests/integration/backup.test.ts index 132c1e0a0..5aa88187b 100644 --- a/server/tests/integration/backup.test.ts +++ b/server/tests/integration/backup.test.ts @@ -76,9 +76,9 @@ beforeAll(async () => { app = nestApp.getHttpAdapter().getInstance(); }); -beforeEach(() => { +beforeEach(async () => { resetTestDb(testDb); - resetRateLimits(nestApp); + await resetRateLimits(nestApp); }); afterAll(async () => { diff --git a/server/tests/integration/budget.test.ts b/server/tests/integration/budget.test.ts index ea5676d43..6b56f204c 100644 --- a/server/tests/integration/budget.test.ts +++ b/server/tests/integration/budget.test.ts @@ -36,9 +36,9 @@ beforeAll(async () => { app = nestApp.getHttpAdapter().getInstance(); }); -beforeEach(() => { +beforeEach(async () => { resetTestDb(testDb); - resetRateLimits(nestApp); + await resetRateLimits(nestApp); // GET /budget/settlement reaches ExchangeRatesService.getRates unconditionally // (budget.service.ts settlement()), and that is a real fetch to // api.frankfurter.dev with a 10 s abort. Without this stub the suite talks to @@ -404,7 +404,7 @@ describe('Reorder budget items', () => { // Restrict budget_edit to trip_owner only testDb.prepare("INSERT OR REPLACE INTO app_settings (key, value) VALUES ('perm_budget_edit', 'trip_owner')").run(); const { invalidatePermissionsCache } = await import('../../src/nest/permissions/permissions-cache'); - invalidatePermissionsCache(); + await invalidatePermissionsCache(); const res = await request(app) .put(`/api/trips/${trip.id}/budget/reorder/items`) @@ -414,7 +414,7 @@ describe('Reorder budget items', () => { // Restore default testDb.prepare("DELETE FROM app_settings WHERE key = 'perm_budget_edit'").run(); - invalidatePermissionsCache(); + await invalidatePermissionsCache(); }); it('BUDGET-013 — owner can reorder budget items — returns 200', async () => { @@ -508,7 +508,7 @@ describe('Budget edit permission enforcement', () => { const { invalidatePermissionsCache } = await import('../../src/nest/permissions/permissions-cache'); testDb.prepare("INSERT OR REPLACE INTO app_settings (key, value) VALUES ('perm_budget_edit', 'trip_owner')").run(); - invalidatePermissionsCache(); + await invalidatePermissionsCache(); const res = await request(app) .post(`/api/trips/${trip.id}/budget`) @@ -517,7 +517,7 @@ describe('Budget edit permission enforcement', () => { expect(res.status).toBe(403); testDb.prepare("DELETE FROM app_settings WHERE key = 'perm_budget_edit'").run(); - invalidatePermissionsCache(); + await invalidatePermissionsCache(); }); it('BUDGET-018 — member cannot reorder categories when budget_edit is restricted to trip_owner', async () => { @@ -529,7 +529,7 @@ describe('Budget edit permission enforcement', () => { const { invalidatePermissionsCache } = await import('../../src/nest/permissions/permissions-cache'); testDb.prepare("INSERT OR REPLACE INTO app_settings (key, value) VALUES ('perm_budget_edit', 'trip_owner')").run(); - invalidatePermissionsCache(); + await invalidatePermissionsCache(); const res = await request(app) .put(`/api/trips/${trip.id}/budget/reorder/categories`) @@ -538,6 +538,6 @@ describe('Budget edit permission enforcement', () => { expect(res.status).toBe(403); testDb.prepare("DELETE FROM app_settings WHERE key = 'perm_budget_edit'").run(); - invalidatePermissionsCache(); + await invalidatePermissionsCache(); }); }); diff --git a/server/tests/integration/categories.test.ts b/server/tests/integration/categories.test.ts index 4a928df0d..328ebeeca 100644 --- a/server/tests/integration/categories.test.ts +++ b/server/tests/integration/categories.test.ts @@ -36,9 +36,9 @@ beforeAll(async () => { app = nestApp.getHttpAdapter().getInstance(); }); -beforeEach(() => { +beforeEach(async () => { resetTestDb(testDb); - resetRateLimits(nestApp); + await resetRateLimits(nestApp); }); afterAll(async () => { diff --git a/server/tests/integration/collab.test.ts b/server/tests/integration/collab.test.ts index 45abec366..17dd86230 100644 --- a/server/tests/integration/collab.test.ts +++ b/server/tests/integration/collab.test.ts @@ -55,9 +55,9 @@ beforeAll(async () => { if (!fs.existsSync(uploadsDir)) fs.mkdirSync(uploadsDir, { recursive: true }); }); -beforeEach(() => { +beforeEach(async () => { resetTestDb(testDb); - resetRateLimits(nestApp); + await resetRateLimits(nestApp); }); afterAll(async () => { diff --git a/server/tests/integration/collections.test.ts b/server/tests/integration/collections.test.ts index cba75f71d..d7745e563 100644 --- a/server/tests/integration/collections.test.ts +++ b/server/tests/integration/collections.test.ts @@ -53,9 +53,9 @@ beforeAll(async () => { app = nestApp.getHttpAdapter().getInstance(); }); -beforeEach(() => { +beforeEach(async () => { resetTestDb(testDb); - resetRateLimits(nestApp); + await resetRateLimits(nestApp); // Enable the collections addon (the controller sits behind AddonGuard). testDb.prepare( "INSERT OR REPLACE INTO addons (id, name, description, type, icon, enabled, sort_order) VALUES ('collections', 'Collections', 'Saved places', 'global', 'Bookmark', 1, 40)" diff --git a/server/tests/integration/dayNotes.test.ts b/server/tests/integration/dayNotes.test.ts index 5ba6b9363..d5372183d 100644 --- a/server/tests/integration/dayNotes.test.ts +++ b/server/tests/integration/dayNotes.test.ts @@ -36,9 +36,9 @@ beforeAll(async () => { app = nestApp.getHttpAdapter().getInstance(); }); -beforeEach(() => { +beforeEach(async () => { resetTestDb(testDb); - resetRateLimits(nestApp); + await resetRateLimits(nestApp); }); afterAll(async () => { diff --git a/server/tests/integration/days.test.ts b/server/tests/integration/days.test.ts index bc636fd49..4a47de768 100644 --- a/server/tests/integration/days.test.ts +++ b/server/tests/integration/days.test.ts @@ -37,7 +37,7 @@ beforeAll(async () => { nestApp = await buildApp(); app = nestApp.getHttpAdapter().getInstance(); }); -beforeEach(() => { resetTestDb(testDb); resetRateLimits(nestApp); }); +beforeEach(async () => { resetTestDb(testDb); await resetRateLimits(nestApp); }); afterAll(async () => { await nestApp.close(); testDb.close(); @@ -378,7 +378,7 @@ describe('Delete day', () => { createDay(testDb, trip.id); testDb.prepare("INSERT OR REPLACE INTO app_settings (key, value) VALUES ('perm_day_edit', 'trip_owner')").run(); const { invalidatePermissionsCache } = await import('../../src/nest/permissions/permissions-cache'); - invalidatePermissionsCache(); + await invalidatePermissionsCache(); try { const res = await request(app) .delete(`/api/trips/${trip.id}/days/${day.id}`) @@ -387,7 +387,7 @@ describe('Delete day', () => { expect(testDb.prepare('SELECT id FROM days WHERE id = ?').get(day.id)).toBeDefined(); } finally { testDb.prepare("DELETE FROM app_settings WHERE key = 'perm_day_edit'").run(); - invalidatePermissionsCache(); + await invalidatePermissionsCache(); } }); }); diff --git a/server/tests/integration/files.test.ts b/server/tests/integration/files.test.ts index 135956d01..e001b2e2e 100644 --- a/server/tests/integration/files.test.ts +++ b/server/tests/integration/files.test.ts @@ -51,9 +51,9 @@ beforeAll(async () => { testDb.prepare("INSERT OR REPLACE INTO app_settings (key, value) VALUES ('allowed_file_types', '*')").run(); }); -beforeEach(() => { +beforeEach(async () => { resetTestDb(testDb); - resetRateLimits(nestApp); + await resetRateLimits(nestApp); // Re-seed allowed_file_types after reset testDb.prepare("INSERT OR REPLACE INTO app_settings (key, value) VALUES ('allowed_file_types', '*')").run(); }); diff --git a/server/tests/integration/immich.test.ts b/server/tests/integration/immich.test.ts index df20363dc..ddca46ede 100644 --- a/server/tests/integration/immich.test.ts +++ b/server/tests/integration/immich.test.ts @@ -63,9 +63,9 @@ beforeAll(async () => { app = nestApp.getHttpAdapter().getInstance(); }); -beforeEach(() => { +beforeEach(async () => { resetTestDb(testDb); - resetRateLimits(nestApp); + await resetRateLimits(nestApp); // Providers only count as enabled under an enabled journey addon (migration 84 seeds it off). setAddonEnabled(testDb, 'journey', true); // The migrated snapshot seeds photo_providers.immich.enabled = 0 (an admin must diff --git a/server/tests/integration/journey-place-order.test.ts b/server/tests/integration/journey-place-order.test.ts index 9ddbaeeb4..70bf642a8 100644 --- a/server/tests/integration/journey-place-order.test.ts +++ b/server/tests/integration/journey-place-order.test.ts @@ -54,10 +54,10 @@ beforeAll(async () => { nestApp = await buildApp(); app = nestApp.getHttpAdapter().getInstance(); }); -beforeEach(() => { +beforeEach(async () => { resetTestDb(testDb); - resetRateLimits(nestApp); - invalidatePermissionsCache(); + await resetRateLimits(nestApp); + await invalidatePermissionsCache(); // Enable the journey addon. testDb.prepare( "INSERT OR REPLACE INTO addons (id, name, description, type, icon, enabled, sort_order) VALUES ('journey', 'Journey', 'Travel journal', 'global', 'Compass', 1, 35)" diff --git a/server/tests/integration/journey.test.ts b/server/tests/integration/journey.test.ts index 1d659a459..39762e618 100644 --- a/server/tests/integration/journey.test.ts +++ b/server/tests/integration/journey.test.ts @@ -51,10 +51,10 @@ beforeAll(async () => { nestApp = await buildApp(); app = nestApp.getHttpAdapter().getInstance(); }); -beforeEach(() => { +beforeEach(async () => { resetTestDb(testDb); - resetRateLimits(nestApp); - invalidatePermissionsCache(); + await resetRateLimits(nestApp); + await invalidatePermissionsCache(); // Enable the journey addon testDb.prepare( "INSERT OR REPLACE INTO addons (id, name, description, type, icon, enabled, sort_order) VALUES ('journey', 'Journey', 'Travel journal', 'global', 'Compass', 1, 35)" diff --git a/server/tests/integration/leg-mode-incoming.test.ts b/server/tests/integration/leg-mode-incoming.test.ts index 2971646c0..60a2ceac3 100644 --- a/server/tests/integration/leg-mode-incoming.test.ts +++ b/server/tests/integration/leg-mode-incoming.test.ts @@ -43,9 +43,9 @@ describe('incoming_leg_transport_mode read-path parity', () => { app = nestApp.getHttpAdapter().getInstance(); }); - beforeEach(() => { + beforeEach(async () => { resetTestDb(testDb2); - resetRateLimits(nestApp); + await resetRateLimits(nestApp); }); afterAll(async () => { diff --git a/server/tests/integration/maps.test.ts b/server/tests/integration/maps.test.ts index cce7795dc..9e349d076 100644 --- a/server/tests/integration/maps.test.ts +++ b/server/tests/integration/maps.test.ts @@ -64,9 +64,9 @@ beforeAll(async () => { }; }); -beforeEach(() => { +beforeEach(async () => { resetTestDb(testDb); - resetRateLimits(nestApp); + await resetRateLimits(nestApp); }); afterAll(async () => { diff --git a/server/tests/integration/mcp.test.ts b/server/tests/integration/mcp.test.ts index e68806078..ab7207d07 100644 --- a/server/tests/integration/mcp.test.ts +++ b/server/tests/integration/mcp.test.ts @@ -79,9 +79,9 @@ beforeAll(async () => { oauthSvc = new OauthService(t.repo(OauthClients), t.repo(OauthTokens), t.repo(OauthConsents), await createTestAddonsService(testDb), new AuditService(t.repo(AuditLog), t.repo(Users))); }); -beforeEach(() => { +beforeEach(async () => { resetTestDb(testDb); - resetRateLimits(nestApp); + await resetRateLimits(nestApp); }); afterAll(async () => { diff --git a/server/tests/integration/memories-immich.test.ts b/server/tests/integration/memories-immich.test.ts index d839cc164..ae37b00ba 100644 --- a/server/tests/integration/memories-immich.test.ts +++ b/server/tests/integration/memories-immich.test.ts @@ -214,9 +214,9 @@ beforeAll(async () => { app = nestApp.getHttpAdapter().getInstance(); }); -beforeEach(() => { +beforeEach(async () => { resetTestDb(testDb); - resetRateLimits(nestApp); + await resetRateLimits(nestApp); // Providers only count as enabled under an enabled journey addon (migration 84 seeds it off). setAddonEnabled(testDb, 'journey', true); // The migrated snapshot seeds photo_providers.immich.enabled = 0 (an admin must diff --git a/server/tests/integration/memories-synology.test.ts b/server/tests/integration/memories-synology.test.ts index 10000ea92..92869c18a 100644 --- a/server/tests/integration/memories-synology.test.ts +++ b/server/tests/integration/memories-synology.test.ts @@ -197,9 +197,9 @@ beforeAll(async () => { app = nestApp.getHttpAdapter().getInstance(); }); -beforeEach(() => { +beforeEach(async () => { resetTestDb(testDb); - resetRateLimits(nestApp); + await resetRateLimits(nestApp); }); afterAll(async () => { diff --git a/server/tests/integration/memories-unified.test.ts b/server/tests/integration/memories-unified.test.ts index a82f63a28..210f69263 100644 --- a/server/tests/integration/memories-unified.test.ts +++ b/server/tests/integration/memories-unified.test.ts @@ -52,9 +52,9 @@ beforeAll(async () => { app = nestApp.getHttpAdapter().getInstance(); }); -beforeEach(() => { +beforeEach(async () => { resetTestDb(testDb); - resetRateLimits(nestApp); + await resetRateLimits(nestApp); // Providers only count as enabled under an enabled journey addon (migration 84 seeds it off). setAddonEnabled(testDb, 'journey', true); // The migrated snapshot seeds photo_providers.immich.enabled = 0 (an admin must diff --git a/server/tests/integration/misc.test.ts b/server/tests/integration/misc.test.ts index c2f1fae3a..44ceefe3b 100644 --- a/server/tests/integration/misc.test.ts +++ b/server/tests/integration/misc.test.ts @@ -36,9 +36,9 @@ beforeAll(async () => { app = nestApp.getHttpAdapter().getInstance(); }); -beforeEach(() => { +beforeEach(async () => { resetTestDb(testDb); - resetRateLimits(nestApp); + await resetRateLimits(nestApp); }); afterAll(async () => { diff --git a/server/tests/integration/notifications.test.ts b/server/tests/integration/notifications.test.ts index 07e68f870..44eda1fae 100644 --- a/server/tests/integration/notifications.test.ts +++ b/server/tests/integration/notifications.test.ts @@ -52,9 +52,9 @@ beforeAll(async () => { app = nestApp.getHttpAdapter().getInstance(); }); -beforeEach(() => { +beforeEach(async () => { resetTestDb(testDb); - resetRateLimits(nestApp); + await resetRateLimits(nestApp); }); afterAll(async () => { diff --git a/server/tests/integration/oauth-authorize.test.ts b/server/tests/integration/oauth-authorize.test.ts index 702d3af54..2cd950f83 100644 --- a/server/tests/integration/oauth-authorize.test.ts +++ b/server/tests/integration/oauth-authorize.test.ts @@ -89,9 +89,9 @@ beforeAll(async () => { containerSideOauth = new OauthService(t.repo(OauthClients), t.repo(OauthTokens), t.repo(OauthConsents), await createTestAddonsService(testDb), new AuditService(t.repo(AuditLog), t.repo(Users))); }); -beforeEach(() => { +beforeEach(async () => { resetTestDb(testDb); - resetRateLimits(nestApp); + await resetRateLimits(nestApp); setMcpEnabled(true); }); @@ -273,7 +273,7 @@ describe('GET /oauth/authorize — SDK authorizationHandler over trekOAuthProvid // 2. the consent controller writes the code through the container // singleton; the module-scoped pending-code map is what lets the // SDK-side exchange see it. - const code = containerSideOauth.createAuthCode({ + const code = await containerSideOauth.createAuthCode({ clientId, userId: user.id, redirectUri: 'https://client.example.com/cb', diff --git a/server/tests/integration/oauth.test.ts b/server/tests/integration/oauth.test.ts index 66be9a618..3d9bb2dc4 100644 --- a/server/tests/integration/oauth.test.ts +++ b/server/tests/integration/oauth.test.ts @@ -87,9 +87,9 @@ beforeAll(async () => { containerSideOauth = new OauthService(t.repo(OauthClients), t.repo(OauthTokens), t.repo(OauthConsents), await createTestAddonsService(testDb), new AuditService(t.repo(AuditLog), t.repo(Users))); }); -beforeEach(() => { +beforeEach(async () => { resetTestDb(testDb); - resetRateLimits(nestApp); + await resetRateLimits(nestApp); setMcpEnabled(true); }); @@ -324,7 +324,7 @@ describe('POST /oauth/token — authorization_code grant', () => { const { verifier, challenge } = makePkce(); // Create code for client1 - const code = createAuthCode({ + const code = await createAuthCode({ clientId: r1.client!.client_id as string, userId: user.id, redirectUri: 'https://app1.example.com/cb', @@ -354,7 +354,7 @@ describe('POST /oauth/token — authorization_code grant', () => { const r = await createOAuthClient(user.id, 'App', ['https://app.example.com/cb'], ['trips:read']); const { verifier, challenge } = makePkce(); - const code = createAuthCode({ + const code = await createAuthCode({ clientId: r.client!.client_id as string, userId: user.id, redirectUri: 'https://app.example.com/cb', @@ -383,7 +383,7 @@ describe('POST /oauth/token — authorization_code grant', () => { const r = await createOAuthClient(user.id, 'App', ['https://app.example.com/cb'], ['trips:read']); const { verifier, challenge } = makePkce(); - const code = createAuthCode({ + const code = await createAuthCode({ clientId: r.client!.client_id as string, userId: user.id, redirectUri: 'https://app.example.com/cb', @@ -412,7 +412,7 @@ describe('POST /oauth/token — authorization_code grant', () => { const r = await createOAuthClient(user.id, 'App', ['https://app.example.com/cb'], ['trips:read']); const { challenge } = makePkce(); - const code = createAuthCode({ + const code = await createAuthCode({ clientId: r.client!.client_id as string, userId: user.id, redirectUri: 'https://app.example.com/cb', @@ -441,7 +441,7 @@ describe('POST /oauth/token — authorization_code grant', () => { const r = await createOAuthClient(user.id, 'App', ['https://app.example.com/cb'], ['trips:read']); const { verifier, challenge } = makePkce(); - const code = createAuthCode({ + const code = await createAuthCode({ clientId: r.client!.client_id as string, userId: user.id, redirectUri: 'https://app.example.com/cb', @@ -504,7 +504,7 @@ describe('POST /oauth/token — refresh_token grant', () => { const r = await createOAuthClient(user.id, 'App', ['https://app.example.com/cb'], ['trips:read']); const { verifier, challenge } = makePkce(); - const code = createAuthCode({ + const code = await createAuthCode({ clientId: r.client!.client_id as string, userId: user.id, redirectUri: 'https://app.example.com/cb', @@ -596,7 +596,7 @@ describe('POST /oauth/revoke', () => { const r = await createOAuthClient(user.id, 'App', ['https://app.example.com/cb'], ['trips:read']); const { verifier, challenge } = makePkce(); - const code = createAuthCode({ + const code = await createAuthCode({ clientId: r.client!.client_id as string, userId: user.id, redirectUri: 'https://app.example.com/cb', @@ -1093,7 +1093,7 @@ describe('Sessions — /api/oauth/sessions', () => { const r = await createOAuthClient(user.id, 'App', ['https://app.example.com/cb'], ['trips:read']); const { verifier, challenge } = makePkce(); - const code = createAuthCode({ + const code = await createAuthCode({ clientId: r.client!.client_id as string, userId: user.id, redirectUri: 'https://app.example.com/cb', @@ -1211,7 +1211,7 @@ describe('H1 — PKCE format validation', () => { const r = await createOAuthClient(user.id, 'App', ['https://app.example.com/cb'], ['trips:read']); const { challenge } = makePkce(); - const code = createAuthCode({ + const code = await createAuthCode({ clientId: r.client!.client_id as string, userId: user.id, redirectUri: 'https://app.example.com/cb', @@ -1269,7 +1269,7 @@ describe('H5 — All invalid_grant cases return identical response body', () => const r = await createOAuthClient(user.id, 'App', ['https://app.example.com/cb'], ['trips:read']); const { verifier, challenge } = makePkce(); - const code = createAuthCode({ + const code = await createAuthCode({ clientId: r.client!.client_id as string, userId: user.id, redirectUri: 'https://app.example.com/cb', @@ -1290,7 +1290,7 @@ describe('H5 — All invalid_grant cases return identical response body', () => }); // Redirect URI mismatch (need fresh code since code is single-use) - const code2 = createAuthCode({ + const code2 = await createAuthCode({ clientId: r.client!.client_id as string, userId: user.id, redirectUri: 'https://app.example.com/cb', @@ -1427,7 +1427,7 @@ describe('C3 — Refresh token replay detection', () => { const r = await createOAuthClient(user.id, 'App', ['https://app.example.com/cb'], ['trips:read']); const { verifier, challenge } = makePkce(); - const code = createAuthCode({ + const code = await createAuthCode({ clientId: r.client!.client_id as string, userId: user.id, redirectUri: 'https://app.example.com/cb', @@ -1475,7 +1475,7 @@ describe('C3 — Refresh token replay detection', () => { const r = await createOAuthClient(user.id, 'App', ['https://app.example.com/cb'], ['trips:read']); const { verifier, challenge } = makePkce(); - const code = createAuthCode({ + const code = await createAuthCode({ clientId: r.client!.client_id as string, userId: user.id, redirectUri: 'https://app.example.com/cb', @@ -1515,7 +1515,7 @@ describe('C3 — Refresh token replay detection', () => { const r = await createOAuthClient(user.id, 'App', ['https://app.example.com/cb'], ['trips:read']); const { verifier, challenge } = makePkce(); - const code = createAuthCode({ + const code = await createAuthCode({ clientId: r.client!.client_id as string, userId: user.id, redirectUri: 'https://app.example.com/cb', diff --git a/server/tests/integration/oidc-local-provider.test.ts b/server/tests/integration/oidc-local-provider.test.ts index 6a8836c30..7c920db95 100644 --- a/server/tests/integration/oidc-local-provider.test.ts +++ b/server/tests/integration/oidc-local-provider.test.ts @@ -149,9 +149,9 @@ beforeAll(async () => { app = nestApp.getHttpAdapter().getInstance(); }); -beforeEach(() => { +beforeEach(async () => { resetTestDb(testDb); - resetRateLimits(nestApp); + await resetRateLimits(nestApp); seen.length = 0; answers.clear(); process.env.OIDC_CLIENT_ID = CLIENT_ID; diff --git a/server/tests/integration/oidc.test.ts b/server/tests/integration/oidc.test.ts index b344b23c7..b2d9f1252 100644 --- a/server/tests/integration/oidc.test.ts +++ b/server/tests/integration/oidc.test.ts @@ -80,9 +80,9 @@ beforeAll(async () => { mockVerifyIdToken = vi.spyOn(oidcSvc, 'verifyIdToken').mockImplementation(async () => undefined as never); }); -beforeEach(() => { +beforeEach(async () => { resetTestDb(testDb); - resetRateLimits(nestApp); + await resetRateLimits(nestApp); vi.clearAllMocks(); // Set OIDC environment variables for each test @@ -163,7 +163,7 @@ describe('GET /api/auth/oidc/callback', () => { }); // Create a valid state token - const { state } = oidcSvc.createState('http://localhost:3001/api/auth/oidc/callback'); + const { state } = await oidcSvc.createState('http://localhost:3001/api/auth/oidc/callback'); const res = await request(app).get(`/api/auth/oidc/callback?code=authcode123&state=${state}`).set('Cookie', `trek_oidc_state=${state}`); @@ -177,7 +177,7 @@ describe('GET /api/auth/oidc/callback', () => { mockExchangeCode.mockResolvedValueOnce({ access_token: 'test-access-token', id_token: 'fake.id.token', _ok: true, _status: 200 }); mockVerifyIdToken.mockResolvedValueOnce({ ok: true, claims: { sub: 'sub-audited-1' } }); mockGetUserInfo.mockResolvedValueOnce({ sub: 'sub-audited-1', email: 'audited@example.com', name: 'Audited', email_verified: true }); - const { state } = oidcSvc.createState('http://localhost:3001/api/auth/oidc/callback'); + const { state } = await oidcSvc.createState('http://localhost:3001/api/auth/oidc/callback'); const res = await request(app).get(`/api/auth/oidc/callback?code=authcode123&state=${state}`).set('Cookie', `trek_oidc_state=${state}`); @@ -202,7 +202,7 @@ describe('GET /api/auth/oidc/callback', () => { name: 'New User', }); - const { state } = oidcSvc.createState('http://localhost:3001/api/auth/oidc/callback'); + const { state } = await oidcSvc.createState('http://localhost:3001/api/auth/oidc/callback'); const res = await request(app).get(`/api/auth/oidc/callback?code=code999&state=${state}`).set('Cookie', `trek_oidc_state=${state}`); @@ -245,7 +245,7 @@ describe('GET /api/auth/oidc/callback', () => { mockDiscover.mockResolvedValueOnce(MOCK_DISCOVERY_DOC); mockExchangeCode.mockResolvedValueOnce({ _ok: false, _status: 400 }); - const { state } = oidcSvc.createState('http://localhost:3001/api/auth/oidc/callback'); + const { state } = await oidcSvc.createState('http://localhost:3001/api/auth/oidc/callback'); const res = await request(app).get(`/api/auth/oidc/callback?code=badcode&state=${state}`).set('Cookie', `trek_oidc_state=${state}`); @@ -257,7 +257,7 @@ describe('GET /api/auth/oidc/callback', () => { mockDiscover.mockResolvedValueOnce(MOCK_DISCOVERY_DOC); mockExchangeCode.mockResolvedValueOnce({ access_token: 'tok', _ok: true, _status: 200 }); // no id_token - const { state } = oidcSvc.createState('http://localhost:3001/api/auth/oidc/callback'); + const { state } = await oidcSvc.createState('http://localhost:3001/api/auth/oidc/callback'); const res = await request(app).get(`/api/auth/oidc/callback?code=anycode&state=${state}`).set('Cookie', `trek_oidc_state=${state}`); @@ -270,7 +270,7 @@ describe('GET /api/auth/oidc/callback', () => { mockExchangeCode.mockResolvedValueOnce({ access_token: 'tok', id_token: 'bad.id.token', _ok: true, _status: 200 }); mockVerifyIdToken.mockResolvedValueOnce({ ok: false, error: 'signature_or_claim_mismatch: invalid signature' }); - const { state } = oidcSvc.createState('http://localhost:3001/api/auth/oidc/callback'); + const { state } = await oidcSvc.createState('http://localhost:3001/api/auth/oidc/callback'); const res = await request(app).get(`/api/auth/oidc/callback?code=anycode&state=${state}`).set('Cookie', `trek_oidc_state=${state}`); @@ -288,7 +288,7 @@ describe('GET /api/auth/oidc/callback', () => { name: 'Alice', }); - const { state } = oidcSvc.createState('http://localhost:3001/api/auth/oidc/callback'); + const { state } = await oidcSvc.createState('http://localhost:3001/api/auth/oidc/callback'); const res = await request(app).get(`/api/auth/oidc/callback?code=anycode&state=${state}`).set('Cookie', `trek_oidc_state=${state}`); @@ -311,7 +311,7 @@ describe('GET /api/auth/oidc/callback', () => { name: 'Blocked', }); - const { state } = oidcSvc.createState('http://localhost:3001/api/auth/oidc/callback'); + const { state } = await oidcSvc.createState('http://localhost:3001/api/auth/oidc/callback'); const res = await request(app).get(`/api/auth/oidc/callback?code=anycode&state=${state}`).set('Cookie', `trek_oidc_state=${state}`); @@ -325,7 +325,7 @@ describe('GET /api/auth/oidc/callback', () => { describe('GET /api/auth/oidc/exchange', () => { it('OIDC-011: valid auth code returns JWT and sets cookie', async () => { const fakeToken = 'eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.test.sig'; - const { code, binding } = oidcSvc.createAuthCode(fakeToken); + const { code, binding } = await oidcSvc.createAuthCode(fakeToken); const res = await request(app) .get(`/api/auth/oidc/exchange?code=${code}`) @@ -353,7 +353,7 @@ describe('GET /api/auth/oidc/exchange', () => { }); it('OIDC-013b: a real code without its binding cookie returns 400 and buys nothing', async () => { - const { code } = oidcSvc.createAuthCode('valid.but.unbound'); + const { code } = await oidcSvc.createAuthCode('valid.but.unbound'); const res = await request(app).get(`/api/auth/oidc/exchange?code=${code}`); @@ -365,7 +365,7 @@ describe('GET /api/auth/oidc/exchange', () => { it('OIDC-014: auth code is single-use (second use returns 400)', async () => { const fakeToken = 'test.token.here'; - const { code, binding } = oidcSvc.createAuthCode(fakeToken); + const { code, binding } = await oidcSvc.createAuthCode(fakeToken); const cookie = `trek_oidc_exchange=${binding}`; // First use: success diff --git a/server/tests/integration/packing.test.ts b/server/tests/integration/packing.test.ts index c8a1ff201..46fc37259 100644 --- a/server/tests/integration/packing.test.ts +++ b/server/tests/integration/packing.test.ts @@ -36,9 +36,9 @@ beforeAll(async () => { app = nestApp.getHttpAdapter().getInstance(); }); -beforeEach(() => { +beforeEach(async () => { resetTestDb(testDb); - resetRateLimits(nestApp); + await resetRateLimits(nestApp); }); afterAll(async () => { diff --git a/server/tests/integration/passkey.test.ts b/server/tests/integration/passkey.test.ts index cbd6c2951..42662e466 100644 --- a/server/tests/integration/passkey.test.ts +++ b/server/tests/integration/passkey.test.ts @@ -52,9 +52,9 @@ beforeAll(async () => { app = nestApp.getHttpAdapter().getInstance(); }); -beforeEach(() => { +beforeEach(async () => { resetTestDb(testDb); - resetRateLimits(nestApp); + await resetRateLimits(nestApp); }); afterAll(async () => { diff --git a/server/tests/integration/places.test.ts b/server/tests/integration/places.test.ts index 48226f651..1e9bf5a22 100644 --- a/server/tests/integration/places.test.ts +++ b/server/tests/integration/places.test.ts @@ -56,10 +56,10 @@ beforeAll(async () => { app = nestApp.getHttpAdapter().getInstance(); }); -beforeEach(() => { +beforeEach(async () => { resetTestDb(testDb); - resetRateLimits(nestApp); - invalidatePermissionsCache(); + await resetRateLimits(nestApp); + await invalidatePermissionsCache(); // Re-attached per test: one describe below calls vi.restoreAllMocks() in its // afterEach, which would otherwise strip these for every later test. importGoogleList = vi.spyOn(nestApp.get(PlacesService), 'importGoogleList'); @@ -984,7 +984,7 @@ describe('Delete place — permission edge cases', () => { // Restrict place edits to trip owner only testDb.prepare("INSERT OR REPLACE INTO app_settings (key, value) VALUES ('perm_place_edit', 'trip_owner')").run(); - invalidatePermissionsCache(); + await invalidatePermissionsCache(); const res = await request(app) .delete(`/api/trips/${trip.id}/places/${place.id}`) diff --git a/server/tests/integration/profile.test.ts b/server/tests/integration/profile.test.ts index 45bf7e243..2c03953b4 100644 --- a/server/tests/integration/profile.test.ts +++ b/server/tests/integration/profile.test.ts @@ -39,9 +39,9 @@ beforeAll(async () => { app = nestApp.getHttpAdapter().getInstance(); }); -beforeEach(() => { +beforeEach(async () => { resetTestDb(testDb); - resetRateLimits(nestApp); + await resetRateLimits(nestApp); }); afterAll(async () => { diff --git a/server/tests/integration/reservations.test.ts b/server/tests/integration/reservations.test.ts index 99566f7fa..4fac15285 100644 --- a/server/tests/integration/reservations.test.ts +++ b/server/tests/integration/reservations.test.ts @@ -36,9 +36,9 @@ beforeAll(async () => { app = nestApp.getHttpAdapter().getInstance(); }); -beforeEach(() => { +beforeEach(async () => { resetTestDb(testDb); - resetRateLimits(nestApp); + await resetRateLimits(nestApp); }); afterAll(async () => { diff --git a/server/tests/integration/security.test.ts b/server/tests/integration/security.test.ts index 8ef63af42..89f05115f 100644 --- a/server/tests/integration/security.test.ts +++ b/server/tests/integration/security.test.ts @@ -47,9 +47,9 @@ beforeAll(async () => { testDb.prepare("INSERT OR REPLACE INTO app_settings (key, value) VALUES ('allowed_file_types', '*')").run(); }); -beforeEach(() => { +beforeEach(async () => { resetTestDb(testDb); - resetRateLimits(nestApp); + await resetRateLimits(nestApp); testDb.prepare("INSERT OR REPLACE INTO app_settings (key, value) VALUES ('allowed_file_types', '*')").run(); }); diff --git a/server/tests/integration/settings.test.ts b/server/tests/integration/settings.test.ts index 2fdbf41ea..fc7c62607 100644 --- a/server/tests/integration/settings.test.ts +++ b/server/tests/integration/settings.test.ts @@ -36,9 +36,9 @@ beforeAll(async () => { app = nestApp.getHttpAdapter().getInstance(); }); -beforeEach(() => { +beforeEach(async () => { resetTestDb(testDb); - resetRateLimits(nestApp); + await resetRateLimits(nestApp); }); afterAll(async () => { diff --git a/server/tests/integration/share.test.ts b/server/tests/integration/share.test.ts index 3fb3337ac..e0640eda1 100644 --- a/server/tests/integration/share.test.ts +++ b/server/tests/integration/share.test.ts @@ -69,9 +69,9 @@ beforeAll(async () => { placePhotoCache = new PlacePhotoCacheService(testStorage, t.repo(GooglePlacePhotoMeta), t.repo(Places), t.repo(CollectionPlaces)); }); -beforeEach(() => { +beforeEach(async () => { resetTestDb(testDb); - resetRateLimits(nestApp); + await resetRateLimits(nestApp); }); afterAll(async () => { @@ -309,7 +309,7 @@ describe('Shared trip access', () => { const { invalidatePermissionsCache } = await import('../../src/nest/permissions/permissions-cache'); testDb.prepare("INSERT OR REPLACE INTO app_settings (key, value) VALUES ('perm_share_manage', 'trip_member')").run(); - invalidatePermissionsCache(); + await invalidatePermissionsCache(); try { const res = await request(app).get(`/api/trips/${trip.id}/share-link`).set('Cookie', authCookie(member.id)); expect(res.status).toBe(200); @@ -317,7 +317,7 @@ describe('Shared trip access', () => { } finally { // Module-scoped cache: leaving it set would decide the next file's tests. testDb.prepare("DELETE FROM app_settings WHERE key = 'perm_share_manage'").run(); - invalidatePermissionsCache(); + await invalidatePermissionsCache(); } }); }); diff --git a/server/tests/integration/tags.test.ts b/server/tests/integration/tags.test.ts index 2114000bc..f5c444f41 100644 --- a/server/tests/integration/tags.test.ts +++ b/server/tests/integration/tags.test.ts @@ -36,9 +36,9 @@ beforeAll(async () => { app = nestApp.getHttpAdapter().getInstance(); }); -beforeEach(() => { +beforeEach(async () => { resetTestDb(testDb); - resetRateLimits(nestApp); + await resetRateLimits(nestApp); }); afterAll(async () => { diff --git a/server/tests/integration/todo.test.ts b/server/tests/integration/todo.test.ts index e0ca5d22a..dfeb5147a 100644 --- a/server/tests/integration/todo.test.ts +++ b/server/tests/integration/todo.test.ts @@ -37,10 +37,10 @@ beforeAll(async () => { app = nestApp.getHttpAdapter().getInstance(); }); -beforeEach(() => { +beforeEach(async () => { resetTestDb(testDb); - resetRateLimits(nestApp); - invalidatePermissionsCache(); + await resetRateLimits(nestApp); + await invalidatePermissionsCache(); }); afterAll(async () => { diff --git a/server/tests/integration/trip-access-primitives.test.ts b/server/tests/integration/trip-access-primitives.test.ts index 134d38af1..8639e212e 100644 --- a/server/tests/integration/trip-access-primitives.test.ts +++ b/server/tests/integration/trip-access-primitives.test.ts @@ -55,9 +55,9 @@ beforeAll(async () => { nestApp = await buildApp(); app = nestApp.getHttpAdapter().getInstance(); }); -beforeEach(() => { +beforeEach(async () => { resetTestDb(testDb); - resetRateLimits(nestApp); + await resetRateLimits(nestApp); }); afterAll(async () => { closeMcpSessions(); diff --git a/server/tests/integration/trips.test.ts b/server/tests/integration/trips.test.ts index b10edbcfa..b2ffd8790 100644 --- a/server/tests/integration/trips.test.ts +++ b/server/tests/integration/trips.test.ts @@ -39,10 +39,10 @@ beforeAll(async () => { nestApp = await buildApp(); app = nestApp.getHttpAdapter().getInstance(); }); -beforeEach(() => { +beforeEach(async () => { resetTestDb(testDb); - resetRateLimits(nestApp); - invalidatePermissionsCache(); + await resetRateLimits(nestApp); + await invalidatePermissionsCache(); }); afterAll(async () => { await nestApp.close(); @@ -137,7 +137,7 @@ describe('Create trip', () => { // Restrict trip creation to admins only testDb.prepare("INSERT INTO app_settings (key, value) VALUES ('perm_trip_create', 'admin')").run(); - invalidatePermissionsCache(); + await invalidatePermissionsCache(); const res = await request(app) .post('/api/trips') @@ -152,7 +152,7 @@ describe('Create trip', () => { const { user: admin } = createAdmin(testDb); testDb.prepare("INSERT INTO app_settings (key, value) VALUES ('perm_trip_create', 'admin')").run(); - invalidatePermissionsCache(); + await invalidatePermissionsCache(); const res = await request(app) .post('/api/trips') @@ -375,7 +375,7 @@ describe('Update trip', () => { // Restrict archiving to trip_owner only (this is actually the default, but set explicitly) testDb.prepare("INSERT OR REPLACE INTO app_settings (key, value) VALUES ('perm_trip_archive', 'trip_owner')").run(); - invalidatePermissionsCache(); + await invalidatePermissionsCache(); const res = await request(app) .put(`/api/trips/${trip.id}`) @@ -394,7 +394,7 @@ describe('Update trip', () => { // Default trip_edit is trip_owner — members should be blocked testDb.prepare("INSERT OR REPLACE INTO app_settings (key, value) VALUES ('perm_trip_edit', 'trip_owner')").run(); - invalidatePermissionsCache(); + await invalidatePermissionsCache(); const res = await request(app) .put(`/api/trips/${trip.id}`) @@ -896,7 +896,7 @@ describe('Trip members', () => { // Restrict member management to trip_owner (default) testDb.prepare("INSERT OR REPLACE INTO app_settings (key, value) VALUES ('perm_member_manage', 'trip_owner')").run(); - invalidatePermissionsCache(); + await invalidatePermissionsCache(); const res = await request(app) .post(`/api/trips/${trip.id}/members`) diff --git a/server/tests/integration/vacay.test.ts b/server/tests/integration/vacay.test.ts index 6d118ea45..ee2f1414a 100644 --- a/server/tests/integration/vacay.test.ts +++ b/server/tests/integration/vacay.test.ts @@ -53,9 +53,9 @@ beforeAll(async () => { app = nestApp.getHttpAdapter().getInstance(); }); -beforeEach(() => { +beforeEach(async () => { resetTestDb(testDb); - resetRateLimits(nestApp); + await resetRateLimits(nestApp); }); afterAll(async () => { diff --git a/server/tests/unit/mcp/tools-budget.test.ts b/server/tests/unit/mcp/tools-budget.test.ts index 618f656f4..5c7a6fd5f 100644 --- a/server/tests/unit/mcp/tools-budget.test.ts +++ b/server/tests/unit/mcp/tools-budget.test.ts @@ -1138,13 +1138,13 @@ describe('Tool: freeze_budget_rates', () => { // budget_edit lowered to the owner, the way the admin permission panel does it. testDb.prepare('INSERT OR REPLACE INTO app_settings (key, value) VALUES (?, ?)').run('perm_budget_edit', 'trip_owner'); - invalidatePermissionsCache(); + await invalidatePermissionsCache(); await withHarness(other.id, async (h) => { const result = await h.client.callTool({ name: 'freeze_budget_rates', arguments: { tripId: trip.id } }); expect(result.isError).toBe(true); }); testDb.prepare("DELETE FROM app_settings WHERE key = 'perm_budget_edit'").run(); - invalidatePermissionsCache(); + await invalidatePermissionsCache(); process.env.DEMO_MODE = 'true'; const { user: demo } = createUser(testDb, { email: 'demo@nomad.app' }); @@ -1160,7 +1160,7 @@ describe('Tool: freeze_budget_rates', () => { } finally { spy.mockRestore(); testDb.prepare("DELETE FROM app_settings WHERE key = 'perm_budget_edit'").run(); - invalidatePermissionsCache(); + await invalidatePermissionsCache(); } }); diff --git a/server/tests/unit/mcp/tools-days-accommodations.test.ts b/server/tests/unit/mcp/tools-days-accommodations.test.ts index ca15ca6ad..32ba89463 100644 --- a/server/tests/unit/mcp/tools-days-accommodations.test.ts +++ b/server/tests/unit/mcp/tools-days-accommodations.test.ts @@ -235,7 +235,7 @@ describe('Tool: delete_day', () => { const day = createDay(testDb, trip.id); createDay(testDb, trip.id); testDb.prepare("INSERT OR REPLACE INTO app_settings (key, value) VALUES ('perm_day_edit', 'trip_owner')").run(); - invalidatePermissionsCache(); + await invalidatePermissionsCache(); try { await withHarness(member.id, async (h) => { const result = await h.client.callTool({ name: 'delete_day', arguments: { tripId: trip.id, dayId: day.id } }); @@ -244,7 +244,7 @@ describe('Tool: delete_day', () => { }); } finally { testDb.prepare("DELETE FROM app_settings WHERE key = 'perm_day_edit'").run(); - invalidatePermissionsCache(); + await invalidatePermissionsCache(); } }); diff --git a/server/tests/unit/mcp/tools-feeds-invite.test.ts b/server/tests/unit/mcp/tools-feeds-invite.test.ts index 5302eed4b..9e5f9feac 100644 --- a/server/tests/unit/mcp/tools-feeds-invite.test.ts +++ b/server/tests/unit/mcp/tools-feeds-invite.test.ts @@ -62,7 +62,7 @@ interface FeedResult { feed_url: string | null } interface InviteLink { token: string; expires_at: string | null; created_at: string; url: string } interface InviteResult { invite_link: InviteLink | null } -beforeEach(() => { +beforeEach(async () => { resetTestDb(testDb); // trip_invite_tokens is not in the shared helper's RESET_TABLES, and trip ids // restart after a DELETE-based reset, so a leftover row would attach itself @@ -71,7 +71,7 @@ beforeEach(() => { // The permission levels are cached in module state that resetTestDb knows // nothing about, so the one test that lowers share_manage would otherwise // keep the lowered level alive for every test after it. - invalidatePermissionsCache(); + await invalidatePermissionsCache(); delete process.env.DEMO_MODE; process.env.APP_URL = BASE; }); diff --git a/server/tests/unit/mcp/tools-files.test.ts b/server/tests/unit/mcp/tools-files.test.ts index 498e990ab..ab085a60c 100644 --- a/server/tests/unit/mcp/tools-files.test.ts +++ b/server/tests/unit/mcp/tools-files.test.ts @@ -49,13 +49,13 @@ import { FilesMcp } from '../../../src/nest/files/files.mcp'; import { FILE_CONTENT_MAX } from '../../../src/nest/files/files.service'; import { StorageService } from '../../../src/nest/storage/storage.service'; -beforeEach(() => { +beforeEach(async () => { resetTestDb(testDb); broadcastMock.mockClear(); delete process.env.DEMO_MODE; // resetTestDb truncates app_settings, but the permission cache is module-scoped // and would keep serving whatever the previous case configured. - invalidatePermissionsCache(); + await invalidatePermissionsCache(); }); afterAll(() => { @@ -68,9 +68,9 @@ async function withHarness(userId: number, fn: (h: McpHarness) => Promise) } /** Lower a configurable action the way the admin permission panel does. */ -function setPermission(action: string, level: string) { +async function setPermission(action: string, level: string): Promise { testDb.prepare('INSERT OR REPLACE INTO app_settings (key, value) VALUES (?, ?)').run(`perm_${action}`, level); - invalidatePermissionsCache(); + await invalidatePermissionsCache(); } interface FileRowOverrides { @@ -435,7 +435,7 @@ describe('Tool: update_trip_file', () => { const trip = createTrip(testDb, owner.id); addTripMember(testDb, trip.id, member.id); const file = insertFile(trip.id); - setPermission('file_edit', 'trip_owner'); + await setPermission('file_edit', 'trip_owner'); await withHarness(member.id, async (h) => { const result = await h.client.callTool({ @@ -574,7 +574,7 @@ describe('Tool: upload_trip_file', () => { const { user: member } = createUser(testDb); const trip = createTrip(testDb, owner.id); addTripMember(testDb, trip.id, member.id); - setPermission('file_upload', 'trip_owner'); + await setPermission('file_upload', 'trip_owner'); await withHarness(member.id, async (h) => { const result = await h.client.callTool({ name: 'upload_trip_file', arguments: { tripId: trip.id, filename: 'a.pdf', content: pdf } }); expect(result.isError).toBe(true); @@ -708,7 +708,7 @@ describe('Tool: link_trip_file', () => { addTripMember(testDb, trip.id, member.id); const reservation = createReservation(testDb, trip.id); const file = insertFile(trip.id); - setPermission('file_edit', 'trip_owner'); + await setPermission('file_edit', 'trip_owner'); await withHarness(member.id, async (h) => { const result = await h.client.callTool({ @@ -811,7 +811,7 @@ describe('Tool: unlink_trip_file', () => { const file = insertFile(trip.id); testDb.prepare('INSERT INTO file_links (file_id, reservation_id) VALUES (?, ?)').run(file.id, reservation.id); const linkId = linkRows(file.id)[0].id; - setPermission('file_edit', 'trip_owner'); + await setPermission('file_edit', 'trip_owner'); await withHarness(member.id, async (h) => { const result = await h.client.callTool({ diff --git a/server/tests/unit/mcp/tools-transit.test.ts b/server/tests/unit/mcp/tools-transit.test.ts index 0adee4fad..ded99314f 100644 --- a/server/tests/unit/mcp/tools-transit.test.ts +++ b/server/tests/unit/mcp/tools-transit.test.ts @@ -106,7 +106,7 @@ const itinerary = { ], }; -beforeEach(() => { +beforeEach(async () => { resetTestDb(testDb); geocodeMock.mockReset(); planMock.mockReset(); @@ -114,7 +114,7 @@ beforeEach(() => { // mockReset would fall back to the real notification write — keep it stubbed. notifyBookingChangeMock.mockReset().mockImplementation(async () => {}); delete process.env.DEMO_MODE; - invalidatePermissionsCache(); + await invalidatePermissionsCache(); }); afterAll(() => testDb.close()); diff --git a/server/tests/unit/mcp/tools-trip-members.test.ts b/server/tests/unit/mcp/tools-trip-members.test.ts index 9b5c0b0b9..9cf1d61ab 100644 --- a/server/tests/unit/mcp/tools-trip-members.test.ts +++ b/server/tests/unit/mcp/tools-trip-members.test.ts @@ -26,13 +26,13 @@ import { invalidatePermissionsCache } from '../../../src/nest/permissions/permis import { createUser, createAdmin, createTrip, addTripMember } from '../../helpers/factories'; import { createMcpHarness, parseToolResult, type McpHarness } from '../../helpers/mcp-harness'; -beforeEach(() => { +beforeEach(async () => { resetTestDb(testDb); broadcastMock.mockClear(); delete process.env.DEMO_MODE; // resetTestDb truncates app_settings, but the permission cache is module-scoped // and would keep serving whatever the previous case configured. - invalidatePermissionsCache(); + await invalidatePermissionsCache(); }); afterAll(() => { @@ -45,9 +45,9 @@ async function withHarness(userId: number, fn: (h: McpHarness) => Promise) } /** Lower a configurable action the way the admin permission panel does. */ -function setPermission(action: string, level: string) { +async function setPermission(action: string, level: string): Promise { testDb.prepare('INSERT OR REPLACE INTO app_settings (key, value) VALUES (?, ?)').run(`perm_${action}`, level); - invalidatePermissionsCache(); + await invalidatePermissionsCache(); } // --------------------------------------------------------------------------- @@ -157,7 +157,7 @@ describe('Tool: add_trip_member', () => { const { user: outsider } = createUser(testDb); const trip = createTrip(testDb, owner.id); addTripMember(testDb, trip.id, collaborator.id); - setPermission('member_manage', 'trip_member'); + await setPermission('member_manage', 'trip_member'); await withHarness(collaborator.id, async (h) => { const result = await h.client.callTool({ name: 'add_trip_member', @@ -274,7 +274,7 @@ describe('Tool: remove_trip_member', () => { const trip = createTrip(testDb, owner.id); addTripMember(testDb, trip.id, member.id); addTripMember(testDb, trip.id, other.id); - setPermission('member_manage', 'trip_member'); + await setPermission('member_manage', 'trip_member'); await withHarness(member.id, async (h) => { const result = await h.client.callTool({ name: 'remove_trip_member', @@ -867,7 +867,7 @@ describe('Guest tools stay owner-only', () => { let guestId = 0; await withHarness(owner.id, async (h) => { guestId = await makeGuest(h, trip.id, 'Anna'); }); - setPermission('member_manage', 'trip_member'); + await setPermission('member_manage', 'trip_member'); await withHarness(collaborator.id, async (h) => { expect((await h.client.callTool({ name: 'create_trip_guest', arguments: { tripId: trip.id, name: 'Bea' }, diff --git a/server/tests/unit/nest/accommodations.service.test.ts b/server/tests/unit/nest/accommodations.service.test.ts index ce4056d75..a5e6cf297 100644 --- a/server/tests/unit/nest/accommodations.service.test.ts +++ b/server/tests/unit/nest/accommodations.service.test.ts @@ -606,7 +606,7 @@ describe('trip access and edit permission', () => { // The permission cache is module-scoped and outlives resetTestDb — drop the row and // the cache together, or every later case in this file inherits the raised level. testDb.prepare("DELETE FROM app_settings WHERE key = 'perm_day_edit'").run(); - permissions.invalidatePermissionsCache(); + await permissions.invalidatePermissionsCache(); } }); diff --git a/server/tests/unit/nest/auth-guard.test.ts b/server/tests/unit/nest/auth-guard.test.ts index 6cc6e57a1..0748465b3 100644 --- a/server/tests/unit/nest/auth-guard.test.ts +++ b/server/tests/unit/nest/auth-guard.test.ts @@ -267,11 +267,11 @@ describe('PasskeyController', () => { it('throttles registration and login ceremonies once the bucket is exhausted', async () => { const s = new RateLimitService(); const now = Date.now(); - for (let i = 0; i < 5; i++) s.check('mfa', '9.9.9.9', 5, 15 * 60 * 1000, now); + for (let i = 0; i < 5; i++) await s.check('mfa', '9.9.9.9', 5, 15 * 60 * 1000, now); expect(await thrownAsync(() => pc(s).registerOptions(user, {}, req))).toEqual({ status: 429, body: { error: 'Too many attempts. Please try again later.' } }); const s2 = new RateLimitService(); - for (let i = 0; i < 10; i++) s2.check('login', '9.9.9.9', 10, 15 * 60 * 1000, now); + for (let i = 0; i < 10; i++) await s2.check('login', '9.9.9.9', 10, 15 * 60 * 1000, now); expect(await thrownAsync(() => pc(s2).loginOptions(req))).toEqual({ status: 429, body: { error: 'Too many attempts. Please try again later.' } }); }); diff --git a/server/tests/unit/nest/auth.controller.test.ts b/server/tests/unit/nest/auth.controller.test.ts index 780bd9bbd..af05db057 100644 --- a/server/tests/unit/nest/auth.controller.test.ts +++ b/server/tests/unit/nest/auth.controller.test.ts @@ -60,25 +60,25 @@ beforeEach(() => vi.clearAllMocks()); afterEach(() => { delete process.env.DEMO_MODE; }); describe('RateLimitService', () => { - it('allows up to max then blocks within the window; buckets are isolated', () => { + it('allows up to max then blocks within the window; buckets are isolated', async () => { const s = rl(); - expect(s.check('login', 'ip', 2, 1000, 0)).toBe(true); - expect(s.check('login', 'ip', 2, 1000, 10)).toBe(true); - expect(s.check('login', 'ip', 2, 1000, 20)).toBe(false); // 3rd within window - expect(s.check('mfa', 'ip', 2, 1000, 20)).toBe(true); // different bucket - expect(s.check('login', 'ip', 2, 1000, 2000)).toBe(true); // window elapsed -> reset + expect(await s.check('login', 'ip', 2, 1000, 0)).toBe(true); + expect(await s.check('login', 'ip', 2, 1000, 10)).toBe(true); + expect(await s.check('login', 'ip', 2, 1000, 20)).toBe(false); // 3rd within window + expect(await s.check('mfa', 'ip', 2, 1000, 20)).toBe(true); // different bucket + expect(await s.check('login', 'ip', 2, 1000, 2000)).toBe(true); // window elapsed -> reset }); - it('reset clears a single named bucket, and reset() clears all of them', () => { + it('reset clears a single named bucket, and reset() clears all of them', async () => { const s = rl(); - s.check('login', 'ip', 1, 1000, 0); // login bucket now at its cap - s.check('mfa', 'ip', 1, 1000, 0); // mfa bucket now at its cap - expect(s.check('login', 'ip', 1, 1000, 0)).toBe(false); - s.reset('login'); // only the login bucket - expect(s.check('login', 'ip', 1, 1000, 0)).toBe(true); - expect(s.check('mfa', 'ip', 1, 1000, 0)).toBe(false); // mfa untouched - s.reset(); // everything - expect(s.check('mfa', 'ip', 1, 1000, 0)).toBe(true); + await s.check('login', 'ip', 1, 1000, 0); // login bucket now at its cap + await s.check('mfa', 'ip', 1, 1000, 0); // mfa bucket now at its cap + expect(await s.check('login', 'ip', 1, 1000, 0)).toBe(false); + await s.reset('login'); // only the login bucket + expect(await s.check('login', 'ip', 1, 1000, 0)).toBe(true); + expect(await s.check('mfa', 'ip', 1, 1000, 0)).toBe(false); // mfa untouched + await s.reset(); // everything + expect(await s.check('mfa', 'ip', 1, 1000, 0)).toBe(true); }); }); @@ -102,7 +102,7 @@ describe('AuthPublicController', () => { it('invite 429 when rate-limited', async () => { const s = rl(); - s.check('login', '9.9.9.9', 10, 15 * 60 * 1000, Date.now()); // not exhausted yet + await s.check('login', '9.9.9.9', 10, 15 * 60 * 1000, Date.now()); // not exhausted yet const c = apc(asvc({ validateInviteToken: vi.fn().mockReturnValue({ valid: true, max_uses: 1, used_count: 0, expires_at: null }) } as Partial), s); expect(await c.invite('tok', req)).toEqual({ valid: true, max_uses: 1, used_count: 0, expires_at: null }); }); @@ -179,7 +179,7 @@ describe('AuthPublicController', () => { it('reset-password 429 once the dedicated reset bucket is exhausted', async () => { const s = rl(); const now = Date.now(); - for (let i = 0; i < 5; i++) s.check('reset', '9.9.9.9', 5, 15 * 60 * 1000, now); + for (let i = 0; i < 5; i++) await s.check('reset', '9.9.9.9', 5, 15 * 60 * 1000, now); const c = apc(asvc({ resetPassword: vi.fn() } as Partial), s); expect(await thrownAsync(() => c.resetPassword(anyBody(), req))).toEqual({ status: 429, body: { error: 'Too many attempts. Please try again later.' } }); }); @@ -192,7 +192,7 @@ describe('AuthPublicController', () => { it('demo-login + register + invite throw 429 when the login bucket is exhausted', async () => { const s = rl(); const now = Date.now(); - for (let i = 0; i < 10; i++) s.check('login', '9.9.9.9', 10, 15 * 60 * 1000, now); + for (let i = 0; i < 10; i++) await s.check('login', '9.9.9.9', 10, 15 * 60 * 1000, now); const c = apc(asvc({ registerUser: vi.fn(), validateInviteToken: vi.fn() } as Partial), s); expect(await thrownAsync(() => c.register(anyBody(), req, res))).toEqual({ status: 429, body: { error: 'Too many attempts. Please try again later.' } }); expect(await thrownAsync(() => c.invite('t', req))).toEqual({ status: 429, body: { error: 'Too many attempts. Please try again later.' } }); @@ -248,23 +248,23 @@ describe('AuthController (authenticated)', () => { expect(await tok.createMcpToken(user, { name: 'CLI' }, req)).toEqual({ token: 'mcp_x' }); }); - it('resource-token 503 when unavailable, else returns the token payload', () => { - expect(thrown(() => ac(asvc({}), rl(), { createResourceToken: vi.fn().mockReturnValue(null) }).resourceToken(user, {}))).toEqual({ status: 503, body: { error: 'Service unavailable' } }); - expect(ac(asvc({}), rl(), { createResourceToken: vi.fn().mockReturnValue({ token: 'rt' }) }).resourceToken(user, { purpose: 'download' })).toEqual({ token: 'rt' }); + it('resource-token 503 when unavailable, else returns the token payload', async () => { + expect(await thrownAsync(() => ac(asvc({}), rl(), { createResourceToken: vi.fn().mockReturnValue(null) }).resourceToken(user, {}))).toEqual({ status: 503, body: { error: 'Service unavailable' } }); + expect(await ac(asvc({}), rl(), { createResourceToken: vi.fn().mockReturnValue({ token: 'rt' }) }).resourceToken(user, { purpose: 'download' })).toEqual({ token: 'rt' }); }); it('ws/resource tokens throttle on their own buckets, so a mint loop cannot drain the store or block login', async () => { const s = rl(); const now = Date.now(); - for (let i = 0; i < 120; i++) s.check('ws_token', String(user.id), 120, 15 * 60 * 1000, now); + for (let i = 0; i < 120; i++) await s.check('ws_token', String(user.id), 120, 15 * 60 * 1000, now); expect(await thrownAsync(() => ac(asvc({}), s, { createWsToken: vi.fn().mockReturnValue({ token: 'ws' }) }).wsToken(user))) .toEqual({ status: 429, body: { error: 'Too many attempts. Please try again later.' } }); // The login bucket is untouched: an exhausted socket loop must not lock the account out. - expect(s.check('login', '9.9.9.9', 5, 15 * 60 * 1000, now)).toBe(true); + expect(await s.check('login', '9.9.9.9', 5, 15 * 60 * 1000, now)).toBe(true); const s2 = rl(); - for (let i = 0; i < 120; i++) s2.check('resource_token', String(user.id), 120, 15 * 60 * 1000, now); - expect(thrown(() => ac(asvc({}), s2, { createResourceToken: vi.fn().mockReturnValue({ token: 'rt' }) }).resourceToken(user, {}))) + for (let i = 0; i < 120; i++) await s2.check('resource_token', String(user.id), 120, 15 * 60 * 1000, now); + expect(await thrownAsync(() => ac(asvc({}), s2, { createResourceToken: vi.fn().mockReturnValue({ token: 'rt' }) }).resourceToken(user, {}))) .toEqual({ status: 429, body: { error: 'Too many attempts. Please try again later.' } }); }); @@ -281,14 +281,14 @@ describe('AuthController (authenticated)', () => { .toEqual({ status: 429, body: { error: 'Too many attempts. Please try again later.' } }); expect(await ac(asvc({}), s, { createWsToken: vi.fn().mockReturnValue({ token: 'ws2' }) }).wsToken(other)).toEqual({ token: 'ws2' }); - expect(ac(asvc({}), s, { createResourceToken: vi.fn().mockReturnValue({ token: 'rt2' }) }).resourceToken(other, {})).toEqual({ token: 'rt2' }); + expect(await ac(asvc({}), s, { createResourceToken: vi.fn().mockReturnValue({ token: 'rt2' }) }).resourceToken(other, {})).toEqual({ token: 'rt2' }); }); it('rate-limited account ops throw 429 once the bucket is exhausted', async () => { const s = rl(); const now = Date.now(); // exhaust the shared 'login' bucket for this ip (max 5) - for (let i = 0; i < 5; i++) s.check('login', '9.9.9.9', 5, 15 * 60 * 1000, now); + for (let i = 0; i < 5; i++) await s.check('login', '9.9.9.9', 5, 15 * 60 * 1000, now); const c = ac(asvc({ changePassword: vi.fn() } as Partial), s); expect(await thrownAsync(() => c.changePassword(user, anyBody(), req, res))).toEqual({ status: 429, body: { error: 'Too many attempts. Please try again later.' } }); }); diff --git a/server/tests/unit/nest/collab.service.test.ts b/server/tests/unit/nest/collab.service.test.ts index 96bf75526..eb24870ca 100644 --- a/server/tests/unit/nest/collab.service.test.ts +++ b/server/tests/unit/nest/collab.service.test.ts @@ -740,15 +740,37 @@ describe('linkPreview hardening', () => { }); vi.stubGlobal('fetch', fetchMock); - const service = await freshSvc(); + const limiter = new RateLimitService(); + const check = vi.spyOn(limiter, 'check'); + const service = await buildCollabService(collabFx.storage, limiter); const all = Promise.all(Array.from({ length: 20 }, () => service.linkPreview('https://example.com/same', 9))); release!(); const results = await all; expect(fetchMock).toHaveBeenCalledTimes(1); expect(results.every(r => r.title === 'Geteilt')).toBe(true); - // And the nineteen that joined were not charged for a fetch they did not make. + // And the nineteen that joined were not charged for a fetch they did not + // make, although the budget check awaits its store before the fetch starts. expect(results.some(r => r.rateLimited)).toBe(false); + expect(check).toHaveBeenCalledTimes(1); + }); + + it('COLLAB-SVC-049: an asker that joined someone out of budget asks again on its own budget', async () => { + const fetchMock = stubFetch({ ok: true, text: async () => 'Eigenes Budget' }); + const limiter = new RateLimitService(); + for (let i = 0; i < 60; i++) await limiter.check('collab_link_preview', '42', 60, 60_000, Date.now()); + const service = await buildCollabService(collabFx.storage, limiter); + + // 42 is first in line and out of budget; 43 arrives while 42's check runs. + const [spent, fresh] = await Promise.all([ + service.linkPreview('https://example.com/shared-link', 42), + service.linkPreview('https://example.com/shared-link', 43), + ]); + + expect(spent.rateLimited).toBe(true); + expect(fresh.rateLimited).toBeUndefined(); + expect(fresh.title).toBe('Eigenes Budget'); + expect(fetchMock).toHaveBeenCalledTimes(1); }); it('COLLAB-SVC-047: a body over the cap, and one behind a failed response, are both released', async () => { diff --git a/server/tests/unit/nest/oauth-sdk.provider.test.ts b/server/tests/unit/nest/oauth-sdk.provider.test.ts index 06431e885..6216cfa87 100644 --- a/server/tests/unit/nest/oauth-sdk.provider.test.ts +++ b/server/tests/unit/nest/oauth-sdk.provider.test.ts @@ -25,6 +25,7 @@ import { clientRegistrationHandler } from '@modelcontextprotocol/sdk/server/auth import { InvalidClientMetadataError, ServerError } from '@modelcontextprotocol/sdk/server/auth/errors'; import { TrekClientsStore, TrekOAuthProvider } from '../../../src/nest/oauth/oauth-sdk.provider'; import { OauthModule } from '../../../src/nest/oauth/oauth.module'; +import { processPendingCodes } from '../../../src/nest/oauth/oauth.pending-codes'; import type { AddonsService } from '../../../src/nest/addons/addons.service'; import { ALL_SCOPES, DEFAULT_CLIENT_SCOPES, OPT_IN_ONLY_SCOPES } from '../../../src/mcp/scopes'; import type { OauthService } from '../../../src/nest/oauth/oauth.service'; @@ -362,30 +363,30 @@ describe('TrekOAuthProvider.exchangeAuthorizationCode', () => { const invalid = 'Authorization grant is invalid.'; it('SDKP-040: unknown/expired code throws the uniform grant error', async () => { - const { provider } = makeProvider(makeOauth({ consumeAuthCode: vi.fn(() => null) })); + const { provider } = makeProvider(makeOauth({ consumeAuthCode: vi.fn(async () => null) })); await expect(provider.exchangeAuthorizationCode(clientInfo(), 'code')).rejects.toThrow(invalid); }); it('SDKP-041: a code minted for another client throws the same error', async () => { - const { provider } = makeProvider(makeOauth({ consumeAuthCode: vi.fn(() => pending({ clientId: 'someone-else' })) })); + const { provider } = makeProvider(makeOauth({ consumeAuthCode: vi.fn(async () => pending({ clientId: 'someone-else' })) })); await expect(provider.exchangeAuthorizationCode(clientInfo(), 'code')).rejects.toThrow(invalid); }); it('SDKP-042: redirect_uri mismatch throws the same error', async () => { - const { provider } = makeProvider(makeOauth({ consumeAuthCode: vi.fn(() => pending()) })); + const { provider } = makeProvider(makeOauth({ consumeAuthCode: vi.fn(async () => pending()) })); await expect(provider.exchangeAuthorizationCode(clientInfo(), 'code', undefined, 'https://wrong.example.com/cb')) .rejects.toThrow(invalid); }); it('SDKP-043: resource mismatch throws the same error', async () => { - const { provider } = makeProvider(makeOauth({ consumeAuthCode: vi.fn(() => pending()) })); + const { provider } = makeProvider(makeOauth({ consumeAuthCode: vi.fn(async () => pending()) })); await expect(provider.exchangeAuthorizationCode( clientInfo(), 'code', undefined, 'https://client.example.com/cb', new URL('https://other.example.com/'), )).rejects.toThrow(invalid); }); it('SDKP-044: failed PKCE throws the same error', async () => { - const oauth = makeOauth({ consumeAuthCode: vi.fn(() => pending()), verifyPKCE: vi.fn(() => false) }); + const oauth = makeOauth({ consumeAuthCode: vi.fn(async () => pending()), verifyPKCE: vi.fn(() => false) }); const { provider } = makeProvider(oauth); await expect(provider.exchangeAuthorizationCode(clientInfo(), 'code', 'bad-verifier', 'https://client.example.com/cb')) .rejects.toThrow(invalid); @@ -393,7 +394,7 @@ describe('TrekOAuthProvider.exchangeAuthorizationCode', () => { }); it('SDKP-045: a valid exchange issues tokens and audits the issue', async () => { - const oauth = makeOauth({ consumeAuthCode: vi.fn(() => pending()) }); + const oauth = makeOauth({ consumeAuthCode: vi.fn(async () => pending()) }); const audit = makeAudit(); const { provider } = makeProvider(oauth, audit); const result = await provider.exchangeAuthorizationCode( @@ -410,14 +411,14 @@ describe('TrekOAuthProvider.exchangeAuthorizationCode', () => { }); it('SDKP-046: a pending code without a resource issues a null-audience token', async () => { - const oauth = makeOauth({ consumeAuthCode: vi.fn(() => pending({ resource: null })) }); + const oauth = makeOauth({ consumeAuthCode: vi.fn(async () => pending({ resource: null })) }); const { provider } = makeProvider(oauth); await provider.exchangeAuthorizationCode(clientInfo(), 'code', 'verifier'); expect(oauth.issueTokens).toHaveBeenCalledWith('cid-1', 7, ['trips:read'], null, null); }); it('SDKP-047: a missing codeVerifier is refused, not waved through', async () => { - const oauth = makeOauth({ consumeAuthCode: vi.fn(() => pending()) }); + const oauth = makeOauth({ consumeAuthCode: vi.fn(async () => pending()) }); const { provider } = makeProvider(oauth); await expect(provider.exchangeAuthorizationCode(clientInfo(), 'code')).rejects.toThrow(invalid); expect(oauth.issueTokens).not.toHaveBeenCalled(); @@ -485,7 +486,7 @@ describe('OauthModule.configure', () => { })), }; - new OauthModule(addons, provider, clients).configure(consumer as never); + new OauthModule(addons, provider, clients, processPendingCodes).configure(consumer as never); expect(vi.mocked(authorizationHandler)).toHaveBeenCalledWith({ provider }); expect(vi.mocked(clientRegistrationHandler)).toHaveBeenCalledWith({ clientsStore: clients }); diff --git a/server/tests/unit/nest/oauth.controller.test.ts b/server/tests/unit/nest/oauth.controller.test.ts index 27079891a..aa9e3772a 100644 --- a/server/tests/unit/nest/oauth.controller.test.ts +++ b/server/tests/unit/nest/oauth.controller.test.ts @@ -150,7 +150,7 @@ describe('OauthPublicController /token', () => { it('429 when the token bucket is exhausted (per ip|client)', async () => { const s = rl(); - for (let i = 0; i < 30; i++) s.check('oauth_token', '7.7.7.7|c', 30, 60000, Date.now()); + for (let i = 0; i < 30; i++) await s.check('oauth_token', '7.7.7.7|c', 30, 60000, Date.now()); const res = makeRes(); await opc(osvc(), s).token(reqWith({ client_id: 'c' }), res); expect(res.statusCode).toBe(429); @@ -293,7 +293,7 @@ describe('OauthPublicController /userinfo + /revoke', () => { it('revoke: 429 when the per-ip bucket is exhausted', async () => { const s = rl(); - for (let i = 0; i < 10; i++) s.check('oauth_revoke', '1', 10, 60000, Date.now()); + for (let i = 0; i < 10; i++) await s.check('oauth_revoke', '1', 10, 60000, Date.now()); const res = makeRes(); await opc(osvc(), s).revoke({ ip: '1', body: { token: 't', client_id: 'c' } } as Request, res); expect(res.statusCode).toBe(429); @@ -375,7 +375,7 @@ describe('OauthApiController', () => { it('validate: 429 when the per-ip bucket is exhausted', async () => { const s = rl(); - for (let i = 0; i < 30; i++) s.check('oauth_validate', '1.2.3.4', 30, 60000, Date.now()); + for (let i = 0; i < 30; i++) await s.check('oauth_validate', '1.2.3.4', 30, 60000, Date.now()); const res = makeRes2(); expect(await thrownAsync(() => new OauthApiController(osvc(), s).validate({ ...req } as Request, {}, res))).toEqual({ status: 429, diff --git a/server/tests/unit/nest/oauth.service.test.ts b/server/tests/unit/nest/oauth.service.test.ts index 50ef6d503..826fb2d98 100644 --- a/server/tests/unit/nest/oauth.service.test.ts +++ b/server/tests/unit/nest/oauth.service.test.ts @@ -389,7 +389,7 @@ describe('createAuthCode + consumeAuthCode', () => { const created = await makeClient(user.id); const clientId = created.client!.client_id as string; - const code = createAuthCode({ + const code = await createAuthCode({ clientId, userId: user.id, redirectUri: 'https://example.com/callback', @@ -399,14 +399,14 @@ describe('createAuthCode + consumeAuthCode', () => { codeChallengeMethod: 'S256', }); - const entry = consumeAuthCode(code); + const entry = await consumeAuthCode(code); expect(entry).not.toBeNull(); expect(entry!.userId).toBe(user.id); expect(entry!.clientId).toBe(clientId); }); it('returns null for non-existent code', async () => { - expect(consumeAuthCode('does-not-exist')).toBeNull(); + expect(await consumeAuthCode('does-not-exist')).toBeNull(); }); it('consuming same code twice returns null (one-time use)', async () => { @@ -414,7 +414,7 @@ describe('createAuthCode + consumeAuthCode', () => { const created = await makeClient(user.id); const clientId = created.client!.client_id as string; - const code = createAuthCode({ + const code = await createAuthCode({ clientId, userId: user.id, redirectUri: 'https://example.com/callback', @@ -424,8 +424,8 @@ describe('createAuthCode + consumeAuthCode', () => { codeChallengeMethod: 'S256', }); - consumeAuthCode(code); - expect(consumeAuthCode(code)).toBeNull(); + await consumeAuthCode(code); + expect(await consumeAuthCode(code)).toBeNull(); }); }); @@ -1189,24 +1189,24 @@ describe('pending-code store', () => { }; it('refuses a new code at capacity and the sweep frees it again', async () => { - for (let i = 0; i < MAX_PENDING_CODES; i++) createAuthCode(codeParams); + for (let i = 0; i < MAX_PENDING_CODES; i++) await createAuthCode(codeParams); - expect(createAuthCode(codeParams)).toBeNull(); + expect(await createAuthCode(codeParams)).toBeNull(); // Everything in the store is past its 2-minute TTL by then. - sweepPendingCodes(Date.now() + 3 * 60 * 1000); + await sweepPendingCodes(Date.now() + 3 * 60 * 1000); - const afterSweep = createAuthCode(codeParams); + const afterSweep = await createAuthCode(codeParams); expect(afterSweep).not.toBeNull(); - sweepPendingCodes(Date.now() + 3 * 60 * 1000); + await sweepPendingCodes(Date.now() + 3 * 60 * 1000); }); it('a code past its TTL is consumed as invalid', async () => { - const code = createAuthCode(codeParams)!; + const code = (await createAuthCode(codeParams))!; const realNow = Date.now; Date.now = () => realNow() + 3 * 60 * 1000; try { - expect(consumeAuthCode(code)).toBeNull(); + expect(await consumeAuthCode(code)).toBeNull(); } finally { Date.now = realNow; } @@ -1298,7 +1298,7 @@ describe('module-scoped OAuth state', () => { // the DI singleton, the SDK exchange path reads it back. The map is module- // scoped, so even a second hand-built instance must see it — two maps would // kill the authorization-code flow silently. - const code = createAuthCode({ + const code = (await createAuthCode({ clientId: 'c', userId: 42, redirectUri: 'https://example.com/callback', @@ -1306,10 +1306,10 @@ describe('module-scoped OAuth state', () => { resource: null, codeChallenge: 'x', codeChallengeMethod: 'S256', - })!; + }))!; const secondInstance = new OauthService(clientsRepo, tokensRepo, consentsRepo, addonsStub, new AuditService(auditLogRepo, usersRepo)); - expect(secondInstance.consumeAuthCode(code)?.userId).toBe(42); + expect((await secondInstance.consumeAuthCode(code))?.userId).toBe(42); }); }); diff --git a/server/tests/unit/nest/oidc.controller.test.ts b/server/tests/unit/nest/oidc.controller.test.ts index bc71a1917..d2119c220 100644 --- a/server/tests/unit/nest/oidc.controller.test.ts +++ b/server/tests/unit/nest/oidc.controller.test.ts @@ -18,15 +18,15 @@ function svc(o: Partial = {}): OidcService { getOidcConfig: vi.fn().mockReturnValue({ issuer: 'https://idp', clientId: 'c', clientSecret: 's', discoveryUrl: null }), getAppUrl: vi.fn().mockReturnValue('https://app'), discover: vi.fn().mockResolvedValue({ authorization_endpoint: 'https://idp/auth', userinfo_endpoint: 'https://idp/ui', issuer: 'https://idp' }), - createState: vi.fn().mockReturnValue({ state: 'st', codeChallenge: 'cc' }), - consumeState: vi.fn().mockReturnValue({ redirectUri: 'https://app/api/auth/oidc/callback', codeVerifier: 'cv', inviteToken: undefined }), + createState: vi.fn().mockResolvedValue({ state: 'st', codeChallenge: 'cc' }), + consumeState: vi.fn().mockResolvedValue({ redirectUri: 'https://app/api/auth/oidc/callback', codeVerifier: 'cv', inviteToken: undefined }), exchangeCodeForToken: vi.fn(), verifyIdToken: vi.fn(), getUserInfo: vi.fn(), findOrCreateUser: vi.fn(), touchLastLogin: vi.fn(), generateToken: vi.fn().mockReturnValue('jwt'), - createAuthCode: vi.fn().mockReturnValue({ code: 'ac', binding: 'bnd' }), + createAuthCode: vi.fn().mockResolvedValue({ code: 'ac', binding: 'bnd' }), consumeAuthCode: vi.fn(), frontendUrl: vi.fn((p: string) => 'https://app' + p), setAuthCookie: vi.fn(), @@ -112,7 +112,7 @@ describe('OidcController /login', () => { it('passes the invite token from the query into createState', async () => { const res = makeRes(); - const createState = vi.fn().mockReturnValue({ state: 'st', codeChallenge: 'cc' }); + const createState = vi.fn().mockResolvedValue({ state: 'st', codeChallenge: 'cc' }); const reqInvite = { query: { invite: 'tok123' }, headers: {} } as unknown as Request; await ctl(svc({ createState })).login(reqInvite, res); expect(createState).toHaveBeenCalledWith('https://app/api/auth/oidc/callback', 'tok123', undefined); @@ -124,7 +124,7 @@ describe('OidcController /login', () => { [{ remember: '0' }, false], [{}, undefined], ] as const) { - const createState = vi.fn().mockReturnValue({ state: 'st', codeChallenge: 'cc' }); + const createState = vi.fn().mockResolvedValue({ state: 'st', codeChallenge: 'cc' }); const r = makeRes(); await ctl(svc({ createState })).login({ query, headers: {} } as unknown as Request, r); expect(createState).toHaveBeenCalledWith('https://app/api/auth/oidc/callback', undefined, expected); @@ -132,7 +132,7 @@ describe('OidcController /login', () => { }); it('a malformed remember value never blocks the redirect and falls back to the default duration', async () => { - const createState = vi.fn().mockReturnValue({ state: 'st', codeChallenge: 'cc' }); + const createState = vi.fn().mockResolvedValue({ state: 'st', codeChallenge: 'cc' }); const res = makeRes(); const reqBad = { query: { invite: 'tok123', remember: 'yes' }, headers: {} } as unknown as Request; await ctl(svc({ createState })).login(reqBad, res); @@ -142,8 +142,8 @@ describe('OidcController /login', () => { it('trims a trailing slash off APP_URL when building the redirect uri', async () => { const res = makeRes(); - const createState = vi.fn().mockReturnValue({ state: 'st', codeChallenge: 'cc' }); - await ctl(svc({ getAppUrl: vi.fn().mockReturnValue('https://app///'), createState })).login(req, res); + const createState = vi.fn().mockResolvedValue({ state: 'st', codeChallenge: 'cc' }); + await ctl(svc({ getAppUrl: vi.fn().mockResolvedValue('https://app///'), createState })).login(req, res); expect(createState).toHaveBeenCalledWith('https://app/api/auth/oidc/callback', undefined, undefined); }); @@ -182,7 +182,7 @@ describe('OidcController /callback', () => { await ctl(svc()).callback(undefined, 's', undefined, reqCb('s'), r1); expect(r1.redirectedTo).toBe('https://app/login?oidc_error=missing_params'); const r2 = makeRes(); - await ctl(svc({ consumeState: vi.fn().mockReturnValue(null) })).callback('c', 's', undefined, reqCb('s'), r2); + await ctl(svc({ consumeState: vi.fn().mockResolvedValue(null) })).callback('c', 's', undefined, reqCb('s'), r2); expect(r2.redirectedTo).toBe('https://app/login?oidc_error=invalid_state'); }); @@ -453,10 +453,10 @@ describe('OidcController /callback', () => { // to a browser-session cookie half a lifetime later. for (const remember of [true, false, undefined] as const) { const generateToken = vi.fn().mockReturnValue('jwt'); - const createAuthCode = vi.fn().mockReturnValue({ code: 'ac', binding: 'bnd' }); + const createAuthCode = vi.fn().mockResolvedValue({ code: 'ac', binding: 'bnd' }); const res = makeRes(); await ctl(svc({ - consumeState: vi.fn().mockReturnValue({ redirectUri: 'https://app/api/auth/oidc/callback', codeVerifier: 'cv', inviteToken: undefined, remember }), + consumeState: vi.fn().mockResolvedValue({ redirectUri: 'https://app/api/auth/oidc/callback', codeVerifier: 'cv', inviteToken: undefined, remember }), exchangeCodeForToken: vi.fn().mockResolvedValue({ _ok: true, access_token: 'at', id_token: 'it' }), verifyIdToken: vi.fn().mockResolvedValue({ ok: true, claims: { sub: 'u1' } }), getUserInfo: vi.fn().mockResolvedValue({ email: 'a@b.c', sub: 'u1' }), @@ -471,57 +471,57 @@ describe('OidcController /callback', () => { }); describe('OidcController /exchange', () => { - it('400 without a code, 400 on an invalid code, else sets the cookie + returns the token', () => { + it('400 without a code, 400 on an invalid code, else sets the cookie + returns the token', async () => { const r1 = makeRes(); - ctl(svc()).exchange(undefined, reqEx(), r1); + await ctl(svc()).exchange(undefined, reqEx(), r1); expect(r1.statusCode).toBe(400); expect(r1.body).toEqual({ error: 'Code required' }); const r2 = makeRes(); - ctl(svc({ consumeAuthCode: vi.fn().mockReturnValue({ error: 'invalid_code' }) })).exchange('x', reqEx(), r2); + await ctl(svc({ consumeAuthCode: vi.fn().mockResolvedValue({ error: 'invalid_code' }) })).exchange('x', reqEx(), r2); expect(r2.statusCode).toBe(400); expect(r2.body).toEqual({ error: 'invalid_code' }); const r3 = makeRes(); const setAuthCookie = vi.fn(); const req3 = reqEx(); - ctl(svc({ consumeAuthCode: vi.fn().mockReturnValue({ token: 'jwt' }), setAuthCookie })).exchange('x', req3, r3); + await ctl(svc({ consumeAuthCode: vi.fn().mockResolvedValue({ token: 'jwt' }), setAuthCookie })).exchange('x', req3, r3); expect(setAuthCookie).toHaveBeenCalledWith(r3, 'jwt', req3, undefined); expect(r3.body).toEqual({ token: 'jwt' }); }); - it('forwards the stored remember flag to setAuthCookie', () => { + it('forwards the stored remember flag to setAuthCookie', async () => { for (const remember of [true, false] as const) { const res = makeRes(); const setAuthCookie = vi.fn(); const r = reqEx(); - ctl(svc({ consumeAuthCode: vi.fn().mockReturnValue({ token: 'jwt', remember }), setAuthCookie })).exchange('x', r, res); + await ctl(svc({ consumeAuthCode: vi.fn().mockResolvedValue({ token: 'jwt', remember }), setAuthCookie })).exchange('x', r, res); expect(setAuthCookie).toHaveBeenCalledWith(res, 'jwt', r, remember); expect(res.body).toEqual({ token: 'jwt' }); } }); - it('hands the binding cookie to consumeAuthCode and clears it on the way out', () => { - const consumeAuthCode = vi.fn().mockReturnValue({ token: 'jwt' }); + it('hands the binding cookie to consumeAuthCode and clears it on the way out', async () => { + const consumeAuthCode = vi.fn().mockResolvedValue({ token: 'jwt' }); const res = makeRes(); - ctl(svc({ consumeAuthCode })).exchange('x', reqEx('the-secret'), res); + await ctl(svc({ consumeAuthCode })).exchange('x', reqEx('the-secret'), res); expect(consumeAuthCode).toHaveBeenCalledWith('x', 'the-secret'); expect(res.clearCookie).toHaveBeenCalledWith('trek_oidc_exchange', expect.objectContaining({ httpOnly: true, path: '/' })); }); - it('passes undefined when the browser has no binding cookie, and still clears it', () => { - const consumeAuthCode = vi.fn().mockReturnValue({ error: 'Invalid or expired code' }); + it('passes undefined when the browser has no binding cookie, and still clears it', async () => { + const consumeAuthCode = vi.fn().mockResolvedValue({ error: 'Invalid or expired code' }); const res = makeRes(); // A request from any other context: no cookie jar for this origin at all. - ctl(svc({ consumeAuthCode })).exchange('stolen', { query: {}, headers: {} } as Request, res); + await ctl(svc({ consumeAuthCode })).exchange('stolen', { query: {}, headers: {} } as Request, res); expect(consumeAuthCode).toHaveBeenCalledWith('stolen', undefined); expect(res.statusCode).toBe(400); expect(res.clearCookie).toHaveBeenCalledWith('trek_oidc_exchange', expect.anything()); }); - it('clears the binding cookie even when no code was presented', () => { + it('clears the binding cookie even when no code was presented', async () => { const res = makeRes(); - ctl(svc()).exchange(undefined, reqEx(), res); + await ctl(svc()).exchange(undefined, reqEx(), res); expect(res.clearCookie).toHaveBeenCalledWith('trek_oidc_exchange', expect.anything()); }); }); diff --git a/server/tests/unit/nest/oidc.service.test.ts b/server/tests/unit/nest/oidc.service.test.ts index ab5481d57..3faf79646 100644 --- a/server/tests/unit/nest/oidc.service.test.ts +++ b/server/tests/unit/nest/oidc.service.test.ts @@ -100,6 +100,7 @@ import { UserCleanupService } from '../../../src/nest/auth/user-cleanup.service' import { EphemeralTokenService } from '../../../src/nest/auth/ephemeral-token.service'; import { AllowedFileTypesService } from '../../../src/nest/files/allowed-file-types.service'; import { OidcService } from '../../../src/nest/oidc/oidc.service'; +import { InMemoryOidcFlowStore } from '../../../src/nest/oidc/oidc-flow.store'; import { MailerService } from '../../../src/nest/notifications/mailer/mailer.service'; import { createTestUnitOfWork, @@ -178,51 +179,51 @@ afterAll(() => { // ── createState / consumeState ──────────────────────────────────────────────── describe('createState / consumeState', () => { - it('OIDC-SVC-001: createState returns a hex token + PKCE S256 challenge', () => { - const { state, codeChallenge } = svc.createState('https://example.com/callback'); + it('OIDC-SVC-001: createState returns a hex token + PKCE S256 challenge', async () => { + const { state, codeChallenge } = await svc.createState('https://example.com/callback'); expect(state).toMatch(/^[0-9a-f]{64}$/); expect(codeChallenge).toMatch(/^[A-Za-z0-9_-]{43}$/); // base64url SHA-256, no padding }); - it('OIDC-SVC-002: consumeState returns stored data (incl. verifier) and deletes state', () => { - const { state } = svc.createState('https://example.com/callback', 'invite-abc'); - const data = svc.consumeState(state); + it('OIDC-SVC-002: consumeState returns stored data (incl. verifier) and deletes state', async () => { + const { state } = await svc.createState('https://example.com/callback', 'invite-abc'); + const data = await svc.consumeState(state); expect(data).not.toBeNull(); expect(data!.redirectUri).toBe('https://example.com/callback'); expect(data!.inviteToken).toBe('invite-abc'); expect(typeof data!.codeVerifier).toBe('string'); expect(data!.codeVerifier.length).toBeGreaterThan(20); // State is consumed — second call returns null - expect(svc.consumeState(state)).toBeNull(); + expect(await svc.consumeState(state)).toBeNull(); }); - it('OIDC-SVC-003: consumeState returns null for unknown state', () => { - expect(svc.consumeState('not-a-real-state')).toBeNull(); + it('OIDC-SVC-003: consumeState returns null for unknown state', async () => { + expect(await svc.consumeState('not-a-real-state')).toBeNull(); }); - it('OIDC-SVC-055: createState stores the remember flag and consumeState returns it', () => { - const { state: sTrue } = svc.createState('https://example.com/cb', undefined, true); - const { state: sFalse } = svc.createState('https://example.com/cb', undefined, false); - const { state: sAbsent } = svc.createState('https://example.com/cb'); - expect(svc.consumeState(sTrue)!.remember).toBe(true); - expect(svc.consumeState(sFalse)!.remember).toBe(false); - expect(svc.consumeState(sAbsent)!.remember).toBeUndefined(); + it('OIDC-SVC-055: createState stores the remember flag and consumeState returns it', async () => { + const { state: sTrue } = await svc.createState('https://example.com/cb', undefined, true); + const { state: sFalse } = await svc.createState('https://example.com/cb', undefined, false); + const { state: sAbsent } = await svc.createState('https://example.com/cb'); + expect((await svc.consumeState(sTrue))!.remember).toBe(true); + expect((await svc.consumeState(sFalse))!.remember).toBe(false); + expect((await svc.consumeState(sAbsent))!.remember).toBeUndefined(); }); - it('OIDC-SVC-004: two different states do not conflict', () => { - const { state: s1 } = svc.createState('http://a.example.com'); - const { state: s2 } = svc.createState('http://b.example.com'); + it('OIDC-SVC-004: two different states do not conflict', async () => { + const { state: s1 } = await svc.createState('http://a.example.com'); + const { state: s2 } = await svc.createState('http://b.example.com'); expect(s1).not.toBe(s2); - expect(svc.consumeState(s1)!.redirectUri).toBe('http://a.example.com'); - expect(svc.consumeState(s2)!.redirectUri).toBe('http://b.example.com'); + expect((await svc.consumeState(s1))!.redirectUri).toBe('http://a.example.com'); + expect((await svc.consumeState(s2))!.redirectUri).toBe('http://b.example.com'); }); }); // ── createAuthCode / consumeAuthCode ───────────────────────────────────────── describe('createAuthCode / consumeAuthCode', () => { - it('OIDC-SVC-005: createAuthCode returns a code and the secret that redeems it', () => { - const { code, binding } = svc.createAuthCode('my.jwt.token'); + it('OIDC-SVC-005: createAuthCode returns a code and the secret that redeems it', async () => { + const { code, binding } = await svc.createAuthCode('my.jwt.token'); expect(typeof code).toBe('string'); expect(code.length).toBeGreaterThan(0); // 32 random bytes as base64url; the code alone must never be enough. @@ -230,62 +231,62 @@ describe('createAuthCode / consumeAuthCode', () => { expect(binding).not.toBe(code); }); - it('OIDC-SVC-005b: every code gets its own binding secret', () => { - const a = svc.createAuthCode('t'); - const b = svc.createAuthCode('t'); + it('OIDC-SVC-005b: every code gets its own binding secret', async () => { + const a = await svc.createAuthCode('t'); + const b = await svc.createAuthCode('t'); expect(a.binding).not.toBe(b.binding); expect(a.code).not.toBe(b.code); }); - it('OIDC-SVC-006: consumeAuthCode returns the stored token', () => { - const { code, binding } = svc.createAuthCode('real.jwt.here'); - const result = svc.consumeAuthCode(code, binding); + it('OIDC-SVC-006: consumeAuthCode returns the stored token', async () => { + const { code, binding } = await svc.createAuthCode('real.jwt.here'); + const result = await svc.consumeAuthCode(code, binding); expect('token' in result).toBe(true); expect((result as { token: string }).token).toBe('real.jwt.here'); }); - it('OIDC-SVC-007: auth code is single-use (second consume returns error)', () => { - const { code, binding } = svc.createAuthCode('single.use.token'); - svc.consumeAuthCode(code, binding); // first use - const second = svc.consumeAuthCode(code, binding); + it('OIDC-SVC-007: auth code is single-use (second consume returns error)', async () => { + const { code, binding } = await svc.createAuthCode('single.use.token'); + await svc.consumeAuthCode(code, binding); // first use + const second = await svc.consumeAuthCode(code, binding); expect('error' in second).toBe(true); }); - it('OIDC-SVC-008: consumeAuthCode returns error for unknown code', () => { - const result = svc.consumeAuthCode('not-a-real-code', 'whatever'); + it('OIDC-SVC-008: consumeAuthCode returns error for unknown code', async () => { + const result = await svc.consumeAuthCode('not-a-real-code', 'whatever'); expect('error' in result).toBe(true); }); - it('OIDC-SVC-008b: a valid code without its binding is refused, and indistinguishable from an unknown one', () => { - const { code } = svc.createAuthCode('bound.token'); - expect(svc.consumeAuthCode(code, undefined)).toEqual({ error: 'Invalid or expired code' }); - expect(svc.consumeAuthCode('not-a-real-code', undefined)).toEqual({ error: 'Invalid or expired code' }); + it('OIDC-SVC-008b: a valid code without its binding is refused, and indistinguishable from an unknown one', async () => { + const { code } = await svc.createAuthCode('bound.token'); + expect(await svc.consumeAuthCode(code, undefined)).toEqual({ error: 'Invalid or expired code' }); + expect(await svc.consumeAuthCode('not-a-real-code', undefined)).toEqual({ error: 'Invalid or expired code' }); }); - it('OIDC-SVC-008c: another browser binding is refused', () => { - const mine = svc.createAuthCode('mine'); - const theirs = svc.createAuthCode('theirs'); - expect('error' in svc.consumeAuthCode(mine.code, theirs.binding)).toBe(true); + it('OIDC-SVC-008c: another browser binding is refused', async () => { + const mine = await svc.createAuthCode('mine'); + const theirs = await svc.createAuthCode('theirs'); + expect('error' in (await svc.consumeAuthCode(mine.code, theirs.binding))).toBe(true); }); - it('OIDC-SVC-008d: a wrong binding burns the code, so it cannot be retried', () => { - const { code, binding } = svc.createAuthCode('burn.me'); - svc.consumeAuthCode(code, 'not-the-binding'); - expect('error' in svc.consumeAuthCode(code, binding)).toBe(true); + it('OIDC-SVC-008d: a wrong binding burns the code, so it cannot be retried', async () => { + const { code, binding } = await svc.createAuthCode('burn.me'); + await svc.consumeAuthCode(code, 'not-the-binding'); + expect('error' in (await svc.consumeAuthCode(code, binding))).toBe(true); }); - it('OIDC-SVC-008e: a binding that is not even base64 is refused instead of throwing', () => { - const { code } = svc.createAuthCode('t'); - expect('error' in svc.consumeAuthCode(code, '!!!!')).toBe(true); + it('OIDC-SVC-008e: a binding that is not even base64 is refused instead of throwing', async () => { + const { code } = await svc.createAuthCode('t'); + expect('error' in (await svc.consumeAuthCode(code, '!!!!'))).toBe(true); }); - it('OIDC-SVC-056: auth code round-trips the remember flag', () => { - const cTrue = svc.createAuthCode('t1', true); - const cFalse = svc.createAuthCode('t2', false); - const cAbsent = svc.createAuthCode('t3'); - expect((svc.consumeAuthCode(cTrue.code, cTrue.binding) as { remember?: boolean }).remember).toBe(true); - expect((svc.consumeAuthCode(cFalse.code, cFalse.binding) as { remember?: boolean }).remember).toBe(false); - expect((svc.consumeAuthCode(cAbsent.code, cAbsent.binding) as { remember?: boolean }).remember).toBeUndefined(); + it('OIDC-SVC-056: auth code round-trips the remember flag', async () => { + const cTrue = await svc.createAuthCode('t1', true); + const cFalse = await svc.createAuthCode('t2', false); + const cAbsent = await svc.createAuthCode('t3'); + expect(((await svc.consumeAuthCode(cTrue.code, cTrue.binding)) as { remember?: boolean }).remember).toBe(true); + expect(((await svc.consumeAuthCode(cFalse.code, cFalse.binding)) as { remember?: boolean }).remember).toBe(false); + expect(((await svc.consumeAuthCode(cAbsent.code, cAbsent.binding)) as { remember?: boolean }).remember).toBeUndefined(); }); }); @@ -1401,3 +1402,24 @@ describe('OIDC settings — the lockout guard', () => { toggles.mockRestore(); }); }); + +describe('the flow-store sweeps', () => { + it('OIDC-SVC-093: the interval sweeps go through the injected store, and a failed sweep is logged, not thrown', async () => { + vi.useFakeTimers({ toFake: ['setInterval', 'clearInterval'] }); + const error = vi.spyOn(console, 'error').mockImplementation(() => {}); + const flows = new InMemoryOidcFlowStore(); + const sweepStates = vi.spyOn(flows, 'sweepStates').mockRejectedValue(new Error('store offline')); + const sweepCodes = vi.spyOn(flows, 'sweepCodes'); + const fresh = new OidcService(auth, membership, await createTestUnitOfWork(testDb), await createTestUsersRepo(testDb), await createTestInviteTokensRepo(testDb), await createTestAppSettingsRepo(testDb), flows); + try { + vi.advanceTimersByTime(60_000); + expect(sweepCodes).toHaveBeenCalled(); + expect(sweepStates).toHaveBeenCalledTimes(1); + await vi.waitFor(() => expect(error).toHaveBeenCalledWith(expect.stringContaining('OIDC flow sweep failed: store offline'))); + } finally { + fresh.onModuleDestroy(); + error.mockRestore(); + vi.useRealTimers(); + } + }); +}); diff --git a/server/tests/unit/nest/permissions.service.test.ts b/server/tests/unit/nest/permissions.service.test.ts index 19e4073ad..ab41d7dd0 100644 --- a/server/tests/unit/nest/permissions.service.test.ts +++ b/server/tests/unit/nest/permissions.service.test.ts @@ -65,11 +65,11 @@ beforeAll(async () => { svc = new PermissionsService(appSettings, await createTestUnitOfWork(testDb)); }); -beforeEach(() => { +beforeEach(async () => { vi.clearAllMocks(); testDb.prepare("DELETE FROM app_settings WHERE key LIKE 'perm_%'").run(); t.clear(); - svc.invalidatePermissionsCache(); + await svc.invalidatePermissionsCache(); }); afterAll(async () => { @@ -172,7 +172,7 @@ describe('corrupt stored levels', () => { it('PERM-SVC-013: an unrecognized stored level is ignored — every reader falls back to the default', async () => { testDb.prepare('INSERT OR REPLACE INTO app_settings (key, value) VALUES (?, ?)').run('perm_trip_edit', 'unknown_level'); testDb.prepare('INSERT OR REPLACE INTO app_settings (key, value) VALUES (?, ?)').run('perm_trip_delete', ''); - svc.invalidatePermissionsCache(); + await svc.invalidatePermissionsCache(); // Since the quirk fix the corrupt rows never enter the cache, so // getPermissionLevel, getAllPermissions and checkPermission agree on the // default instead of the old display-default/deny-in-check split. @@ -186,7 +186,7 @@ describe('corrupt stored levels', () => { it('PERM-SVC-021: a stored level outside the action\'s allowedLevels is ignored too', async () => { // trip_edit only allows trip_owner/trip_member — a raw 'everybody' row must not widen it. testDb.prepare('INSERT OR REPLACE INTO app_settings (key, value) VALUES (?, ?)').run('perm_trip_edit', 'everybody'); - svc.invalidatePermissionsCache(); + await svc.invalidatePermissionsCache(); expect(await svc.getPermissionLevel('trip_edit')).toBe('trip_owner'); expect(await svc.checkPermission('trip_edit', 'user', 10, 30, false)).toBe(false); }); @@ -206,32 +206,32 @@ describe('load failures', () => { const spy = vi .spyOn(appSettings, 'findByKeyPrefix') .mockRejectedValueOnce(new Error('no such table: app_settings')); - svc.invalidatePermissionsCache(); + await svc.invalidatePermissionsCache(); expect(await svc.getPermissionLevel('trip_edit')).toBe('trip_owner'); expect(logError).toHaveBeenCalledWith('Permissions load failed: no such table: app_settings'); spy.mockRestore(); - svc.invalidatePermissionsCache(); // don't leak the failed-read (uncached) state + await svc.invalidatePermissionsCache(); // don't leak the failed-read (uncached) state }); it('PERM-SVC-024: a failed read serves defaults without installing them, and a later read populates the cache', async () => { const spy = vi .spyOn(appSettings, 'findByKeyPrefix') .mockRejectedValueOnce(new Error('database connection is closed')); - svc.invalidatePermissionsCache(); + await svc.invalidatePermissionsCache(); expect(await svc.getPermissionLevel('trip_edit')).toBe('trip_owner'); expect(logError).toHaveBeenCalledWith('Permissions load failed: database connection is closed'); // Nothing installed: an admin's stricter stored level would otherwise stay // invisible until somebody invalidated by hand. - expect(getPermissionsCache()).toBe(null); + expect(await getPermissionsCache()).toBe(null); // The mock is exhausted after the one queued rejection — the next call // falls through to the real (now-working) repository read. testDb.prepare('INSERT OR REPLACE INTO app_settings (key, value) VALUES (?, ?)').run('perm_trip_edit', 'trip_member'); expect(await svc.getPermissionLevel('trip_edit')).toBe('trip_member'); - expect(getPermissionsCache()).not.toBe(null); + expect(await getPermissionsCache()).not.toBe(null); spy.mockRestore(); - svc.invalidatePermissionsCache(); // don't leak this test's cache to later tests + await svc.invalidatePermissionsCache(); // don't leak this test's cache to later tests }); it('PERM-SVC-025: a MikroORM ValidationError (context misuse) is a programming error and propagates, uncached — it does not get the "log and serve defaults" treatment', async () => { @@ -244,14 +244,14 @@ describe('load failures', () => { const spy = vi .spyOn(appSettings, 'findByKeyPrefix') .mockRejectedValueOnce(ValidationError.cannotUseGlobalContext()); - svc.invalidatePermissionsCache(); + await svc.invalidatePermissionsCache(); await expect(svc.getPermissionLevel('trip_edit')).rejects.toThrow(ValidationError); expect(logError).not.toHaveBeenCalled(); - expect(getPermissionsCache()).toBe(null); + expect(await getPermissionsCache()).toBe(null); spy.mockRestore(); - svc.invalidatePermissionsCache(); + await svc.invalidatePermissionsCache(); }); it('PERM-SVC-023: an all-skipped save writes nothing and leaves the cache untouched', async () => { @@ -262,7 +262,7 @@ describe('load failures', () => { // No valid entries → no transaction and no cache flush: the raw row above // stays invisible until an explicit invalidation. expect(await svc.getPermissionLevel('trip_edit')).toBe('trip_owner'); - svc.invalidatePermissionsCache(); + await svc.invalidatePermissionsCache(); expect(await svc.getPermissionLevel('trip_edit')).toBe('trip_member'); }); }); @@ -272,7 +272,7 @@ describe('load failures', () => { describe('stored overrides + cache', () => { it('PERM-SVC-014: stored perm_ row overrides the default after invalidation', async () => { testDb.prepare('INSERT OR REPLACE INTO app_settings (key, value) VALUES (?, ?)').run('perm_trip_edit', 'trip_member'); - svc.invalidatePermissionsCache(); + await svc.invalidatePermissionsCache(); expect(await svc.getPermissionLevel('trip_edit')).toBe('trip_member'); // A plain member now passes what defaults to a trip_owner-only action. expect(await svc.checkPermission('trip_edit', 'user', 10, 20, true)).toBe(true); @@ -294,7 +294,7 @@ describe('stored overrides + cache', () => { // Raw SQL write bypasses savePermissions' self-invalidation → stale value served. testDb.prepare('INSERT OR REPLACE INTO app_settings (key, value) VALUES (?, ?)').run('perm_trip_edit', 'trip_member'); expect(await svc.getPermissionLevel('trip_edit')).toBe('trip_owner'); - svc.invalidatePermissionsCache(); + await svc.invalidatePermissionsCache(); expect(await svc.getPermissionLevel('trip_edit')).toBe('trip_member'); }); }); @@ -327,7 +327,7 @@ describe('module-scoped permissions cache', () => { // must serve the fresh value afterwards. testDb.prepare('INSERT OR REPLACE INTO app_settings (key, value) VALUES (?, ?)').run('perm_trip_edit', 'trip_owner'); expect(await svc.getPermissionLevel('trip_edit')).toBe('trip_member'); // still cached - invalidateSharedCache(); + await invalidateSharedCache(); expect(await svc.getPermissionLevel('trip_edit')).toBe('trip_owner'); expect(await secondInstance.checkPermission('trip_edit', 'user', 10, 20, true)).toBe(false); }); diff --git a/server/tests/unit/nest/place-enrichment.controller.test.ts b/server/tests/unit/nest/place-enrichment.controller.test.ts index b7a0bbc46..d53d82dd6 100644 --- a/server/tests/unit/nest/place-enrichment.controller.test.ts +++ b/server/tests/unit/nest/place-enrichment.controller.test.ts @@ -40,7 +40,7 @@ describe('PlaceEnrichmentController', () => { it('ENRICH-042: carries the 429 status and message', async () => { const rl = new RateLimitService(); const { controller } = make({}, rl); - vi.spyOn(rl, 'check').mockReturnValue(false); + vi.spyOn(rl, 'check').mockResolvedValue(false); const err = await controller.enrich(USER, REQ, BODY).catch((e: HttpException) => e); diff --git a/server/tests/unit/nest/process-state-ports.test.ts b/server/tests/unit/nest/process-state-ports.test.ts index 9bfbfcd66..9536cdcc3 100644 --- a/server/tests/unit/nest/process-state-ports.test.ts +++ b/server/tests/unit/nest/process-state-ports.test.ts @@ -4,32 +4,53 @@ * cache and the WebSocket rooms. Each has an in-memory implementation that * behaves exactly as the state did before, is provided by its module, and is * what a hand-built consumer falls back to; a shared store can replace it in - * the module. PORTS-001 through PORTS-014. + * the module. The ports a shared store could back are async. The process-wide + * ones are also read outside the container (the broadcasts, the backup + * restore's cache flush, the pending-code sweep), so their module installs the + * store the container resolved in a slot those readers use: swapping the + * provider changes every reader. PORTS-001 through PORTS-020. */ import { describe, it, expect, vi } from 'vitest'; import { Test } from '@nestjs/testing'; import { RateLimitModule } from '../../../src/nest/common/rate-limit.module'; import { RateLimitService } from '../../../src/nest/common/rate-limit.service'; import { InMemoryRateLimitStore, RateLimitStore } from '../../../src/nest/common/rate-limit.store'; +import { ProcessStoreSlot } from '../../../src/nest/common/process-store-slot'; import { InMemoryPendingCodeStore, MAX_PENDING_CODES, PendingCodeStore, + pendingCodesSlot, processPendingCodes, + runPendingCodeSweep, + sweepPendingCodes, type PendingCode, } from '../../../src/nest/oauth/oauth.pending-codes'; +import { OauthModule } from '../../../src/nest/oauth/oauth.module'; import { OauthService } from '../../../src/nest/oauth/oauth.service'; import { InMemoryOidcFlowStore, OidcFlowStore } from '../../../src/nest/oidc/oidc-flow.store'; import { OidcModule } from '../../../src/nest/oidc/oidc.module'; import { InMemoryPermissionsCacheStore, PermissionsCacheStore, + getPermissionsCache, invalidatePermissionsCache, + permissionsCacheSlot, processPermissionsCache, } from '../../../src/nest/permissions/permissions-cache'; import { PermissionsModule } from '../../../src/nest/permissions/permissions.module'; import { PermissionsService } from '../../../src/nest/permissions/permissions.service'; -import { InMemoryRoomRegistry, RoomRegistry, processRooms, type TrekWebSocket } from '../../../src/nest/realtime/ws-state'; +import { + InMemoryRoomRegistry, + RoomRegistry, + bookPeers, + broadcast, + broadcastToBook, + joinRoom, + processRooms, + roomsSlot, + type TrekWebSocket, +} from '../../../src/nest/realtime/ws-state'; import { RealtimeGatewayModule } from '../../../src/nest/realtime/realtime-gateway.module'; import { expectRegisteredProvider } from '../../helpers/module-providers'; @@ -50,12 +71,12 @@ function code(over: Partial = {}): PendingCode { } describe('rate limits', () => { - it('PORTS-001: RateLimitService counts through the store it is given', () => { + it('PORTS-001: RateLimitService counts through the store it is given', async () => { const store = { hit: vi.fn(() => false), reset: vi.fn() } as unknown as RateLimitStore; const service = new RateLimitService(store); - expect(service.check('login', '1.2.3.4', 5, WINDOW, 1_000)).toBe(false); + expect(await service.check('login', '1.2.3.4', 5, WINDOW, 1_000)).toBe(false); expect(store.hit).toHaveBeenCalledWith('login', '1.2.3.4', 5, WINDOW, 1_000); - service.reset('login'); + await service.reset('login'); expect(store.reset).toHaveBeenCalledWith('login'); }); @@ -64,44 +85,44 @@ describe('rate limits', () => { try { const service = moduleRef.get(RateLimitService); expect((service as unknown as { store: unknown }).store).toBeInstanceOf(InMemoryRateLimitStore); - for (let i = 0; i < 3; i++) expect(service.check('mfa', 'k', 3, WINDOW, 1_000)).toBe(true); - expect(service.check('mfa', 'k', 3, WINDOW, 1_000)).toBe(false); + for (let i = 0; i < 3; i++) expect(await service.check('mfa', 'k', 3, WINDOW, 1_000)).toBe(true); + expect(await service.check('mfa', 'k', 3, WINDOW, 1_000)).toBe(false); } finally { await moduleRef.close(); } }); - it('PORTS-003: two hand-built services keep their own counters, as before', () => { + it('PORTS-003: two hand-built services keep their own counters, as before', async () => { const a = new RateLimitService(); const b = new RateLimitService(); - a.check('login', 'k', 1, WINDOW, 1_000); - expect(a.check('login', 'k', 1, WINDOW, 1_000)).toBe(false); - expect(b.check('login', 'k', 1, WINDOW, 1_000)).toBe(true); + await a.check('login', 'k', 1, WINDOW, 1_000); + expect(await a.check('login', 'k', 1, WINDOW, 1_000)).toBe(false); + expect(await b.check('login', 'k', 1, WINDOW, 1_000)).toBe(true); }); }); describe('OAuth pending codes', () => { - it('PORTS-004: a code is single use, and an expired one is burnt and refused', () => { + it('PORTS-004: a code is single use, and an expired one is burnt and refused', async () => { const store = new InMemoryPendingCodeStore(); - expect(store.put('a', code())).toBe(true); - expect(store.take('a')?.clientId).toBe('c'); - expect(store.take('a')).toBeNull(); - store.put('old', code({ expiresAt: Date.now() - 1 })); - expect(store.take('old')).toBeNull(); + expect(await store.put('a', code())).toBe(true); + expect((await store.take('a'))?.clientId).toBe('c'); + expect(await store.take('a')).toBeNull(); + await store.put('old', code({ expiresAt: Date.now() - 1 })); + expect(await store.take('old')).toBeNull(); }); - it('PORTS-005: the store refuses past its capacity and the sweep frees expired entries', () => { + it('PORTS-005: the store refuses past its capacity and the sweep frees expired entries', async () => { const store = new InMemoryPendingCodeStore(); - for (let i = 0; i < MAX_PENDING_CODES; i++) store.put(`c${i}`, code({ expiresAt: 10 })); - expect(store.put('one-more', code())).toBe(false); - store.sweep(11); - expect(store.put('one-more', code())).toBe(true); + for (let i = 0; i < MAX_PENDING_CODES; i++) await store.put(`c${i}`, code({ expiresAt: 10 })); + expect(await store.put('one-more', code())).toBe(false); + await store.sweep(11); + expect(await store.put('one-more', code())).toBe(true); }); - it('PORTS-006: OauthService issues and redeems codes through the injected store', () => { + it('PORTS-006: OauthService issues and redeems codes through the injected store', async () => { const store = new InMemoryPendingCodeStore(); const service = new OauthService({} as never, {} as never, {} as never, {} as never, {} as never, store); - const issued = service.createAuthCode({ + const issued = await service.createAuthCode({ clientId: 'c', userId: 1, redirectUri: 'https://app.example/cb', @@ -111,41 +132,70 @@ describe('OAuth pending codes', () => { codeChallengeMethod: 'S256', }); expect(issued).toMatch(/^[0-9a-f]{64}$/); - expect(processPendingCodes.take(issued!)).toBeNull(); - expect(service.consumeAuthCode(issued!)?.clientId).toBe('c'); + expect(await processPendingCodes.take(issued!)).toBeNull(); + expect((await service.consumeAuthCode(issued!))?.clientId).toBe('c'); }); - it('PORTS-007: OauthModule provides the process-wide store, the one a hand-built service defaults to', async () => { - const { OauthModule } = await import('../../../src/nest/oauth/oauth.module'); + it('PORTS-007: OauthModule provides the process-wide store, the one a hand-built service defaults to', () => { expectRegisteredProvider(OauthModule, { provide: PendingCodeStore, useValue: processPendingCodes }); const service = new OauthService({} as never, {} as never, {} as never, {} as never, {} as never); expect((service as unknown as { pendingCodes: unknown }).pendingCodes).toBe(processPendingCodes); }); + + it('PORTS-018: a swapped store reaches the sweep and a hand-built service until the module goes away', async () => { + const fake = new InMemoryPendingCodeStore(); + const sweep = vi.spyOn(fake, 'sweep'); + const oauthModule = new OauthModule({} as never, {} as never, {} as never, fake); + try { + expect(pendingCodesSlot.get()).toBe(fake); + await sweepPendingCodes(5); + expect(sweep).toHaveBeenCalledWith(5); + const service = new OauthService({} as never, {} as never, {} as never, {} as never, {} as never); + expect((service as unknown as { pendingCodes: unknown }).pendingCodes).toBe(fake); + } finally { + oauthModule.onModuleDestroy(); + } + expect(pendingCodesSlot.get()).toBe(processPendingCodes); + }); + + it('PORTS-020: the interval body logs a failed sweep instead of throwing it into the timer', async () => { + const fake = new InMemoryPendingCodeStore(); + vi.spyOn(fake, 'sweep').mockRejectedValue(new Error('store offline')); + const error = vi.spyOn(console, 'error').mockImplementation(() => {}); + pendingCodesSlot.install(fake); + try { + await expect(runPendingCodeSweep(5)).resolves.toBeUndefined(); + expect(error).toHaveBeenCalledWith(expect.stringContaining('OAuth pending-code sweep failed: store offline')); + } finally { + pendingCodesSlot.release(fake); + error.mockRestore(); + } + }); }); describe('OIDC login states and codes', () => { - it('PORTS-008: states and codes are single use', () => { + it('PORTS-008: states and codes are single use', async () => { const store = new InMemoryOidcFlowStore(); - store.putState('s', { createdAt: 1, redirectUri: '/', codeVerifier: 'v' }); - expect(store.takeState('s')?.codeVerifier).toBe('v'); - expect(store.takeState('s')).toBeNull(); - store.putCode('c', { token: 't', created: 1, bindingHash: 'h' }); - expect(store.takeCode('c')?.token).toBe('t'); - expect(store.takeCode('c')).toBeNull(); + await store.putState('s', { createdAt: 1, redirectUri: '/', codeVerifier: 'v' }); + expect((await store.takeState('s'))?.codeVerifier).toBe('v'); + expect(await store.takeState('s')).toBeNull(); + await store.putCode('c', { token: 't', created: 1, bindingHash: 'h' }); + expect((await store.takeCode('c'))?.token).toBe('t'); + expect(await store.takeCode('c')).toBeNull(); }); - it('PORTS-009: the sweeps drop only what is past its TTL', () => { + it('PORTS-009: the sweeps drop only what is past its TTL', async () => { const store = new InMemoryOidcFlowStore(); - store.putState('old', { createdAt: 0, redirectUri: '/', codeVerifier: 'v' }); - store.putState('new', { createdAt: 900, redirectUri: '/', codeVerifier: 'v' }); - store.putCode('old', { token: 't', created: 0, bindingHash: 'h' }); - store.putCode('new', { token: 't', created: 900, bindingHash: 'h' }); - store.sweepStates(1_000, 500); - store.sweepCodes(1_000, 500); - expect(store.takeState('old')).toBeNull(); - expect(store.takeState('new')).not.toBeNull(); - expect(store.takeCode('old')).toBeNull(); - expect(store.takeCode('new')).not.toBeNull(); + await store.putState('old', { createdAt: 0, redirectUri: '/', codeVerifier: 'v' }); + await store.putState('new', { createdAt: 900, redirectUri: '/', codeVerifier: 'v' }); + await store.putCode('old', { token: 't', created: 0, bindingHash: 'h' }); + await store.putCode('new', { token: 't', created: 900, bindingHash: 'h' }); + await store.sweepStates(1_000, 500); + await store.sweepCodes(1_000, 500); + expect(await store.takeState('old')).toBeNull(); + expect(await store.takeState('new')).not.toBeNull(); + expect(await store.takeCode('old')).toBeNull(); + expect(await store.takeCode('new')).not.toBeNull(); }); it('PORTS-010: OidcModule provides the in-memory store', () => { @@ -159,20 +209,43 @@ describe('the permissions cache', () => { const appSettings = { findByKeyPrefix: vi.fn(async () => [{ key: 'perm_trip_create', value: 'admin' }]) }; const service = new PermissionsService(appSettings as never, {} as never, store); expect(await service.getPermissionLevel('trip_create')).toBe('admin'); - expect(store.get()?.get('trip_create')).toBe('admin'); + expect((await store.get())?.get('trip_create')).toBe('admin'); await service.getPermissionLevel('trip_create'); expect(appSettings.findByKeyPrefix).toHaveBeenCalledTimes(1); - service.invalidatePermissionsCache(); - expect(store.get()).toBeNull(); + await service.invalidatePermissionsCache(); + expect(await store.get()).toBeNull(); }); - it('PORTS-012: the module, a hand-built service and the restore path share the process-wide store', () => { + it('PORTS-012: the module, a hand-built service and the restore path share the process-wide store', async () => { expectRegisteredProvider(PermissionsModule, { provide: PermissionsCacheStore, useValue: processPermissionsCache }); const service = new PermissionsService({} as never, {} as never); expect((service as unknown as { cacheStore: unknown }).cacheStore).toBe(processPermissionsCache); - processPermissionsCache.set(new Map([['trip_create', 'admin']])); - invalidatePermissionsCache(); - expect(processPermissionsCache.get()).toBeNull(); + await processPermissionsCache.set(new Map([['trip_create', 'admin']])); + await invalidatePermissionsCache(); + expect(await processPermissionsCache.get()).toBeNull(); + }); + + it('PORTS-016: a swapped store is the one the backup restore flushes, until the module goes away', async () => { + const fake = new InMemoryPermissionsCacheStore(); + await fake.set(new Map([['trip_create', 'admin']])); + const invalidate = vi.spyOn(fake, 'invalidate'); + await processPermissionsCache.set(new Map([['trip_edit', 'trip_member']])); + const permissionsModule = new PermissionsModule(fake); + try { + expect(permissionsCacheSlot.get()).toBe(fake); + // backup.impl.ts flushes through this free function after a restore. + await invalidatePermissionsCache(); + expect(invalidate).toHaveBeenCalledTimes(1); + expect(await getPermissionsCache()).toBeNull(); + // The default store was not the one flushed. + expect(await processPermissionsCache.get()).not.toBeNull(); + const service = new PermissionsService({} as never, {} as never); + expect((service as unknown as { cacheStore: unknown }).cacheStore).toBe(fake); + } finally { + permissionsModule.onModuleDestroy(); + await processPermissionsCache.invalidate(); + } + expect(permissionsCacheSlot.get()).toBe(processPermissionsCache); }); }); @@ -200,5 +273,64 @@ describe('the WebSocket rooms', () => { it('PORTS-014: RealtimeGatewayModule provides the process-wide registry the broadcasts read', () => { expectRegisteredProvider(RealtimeGatewayModule, { provide: RoomRegistry, useValue: processRooms }); + expect(roomsSlot.get()).toBe(processRooms); + }); + + it('PORTS-017: a swapped registry is the one the broadcasts join and deliver through, until the module goes away', () => { + const fake = new InMemoryRoomRegistry(); + const members = vi.spyOn(fake, 'members'); + const bookMembers = vi.spyOn(fake, 'bookMembers'); + const send = vi.fn(); + const live = { readyState: 1, send } as unknown as TrekWebSocket; + const gatewayModule = new RealtimeGatewayModule(fake); + try { + expect(roomsSlot.get()).toBe(fake); + fake.register(live); + joinRoom(live, 41); + fake.joinBook(live, 9); + + broadcast(41, 'place:created', { place: { id: 1 } }); + expect(members).toHaveBeenCalledWith(41); + expect(send).toHaveBeenCalledWith(JSON.stringify({ type: 'place:created', tripId: 41, place: { id: 1 } })); + + broadcastToBook(9, { type: 'book:pointer' }); + expect(bookMembers).toHaveBeenCalledWith(9); + expect(send).toHaveBeenCalledWith(JSON.stringify({ journeyId: 9, type: 'book:pointer' })); + // The socket was never registered with a user, so it is not listed as a peer. + expect(bookPeers(9)).toEqual([]); + + // Nothing reached the default registry. + expect(processRooms.members(41)).toBeUndefined(); + } finally { + gatewayModule.onModuleDestroy(); + } + expect(roomsSlot.get()).toBe(processRooms); + }); +}); + +describe('ProcessStoreSlot', () => { + it('PORTS-015: install swaps the store, and release only restores the default for the store that is installed', () => { + const fallback = { name: 'fallback' }; + const first = { name: 'first' }; + const second = { name: 'second' }; + const slot = new ProcessStoreSlot(fallback); + expect(slot.get()).toBe(fallback); + slot.install(first); + slot.install(second); + // A module torn down after a later one installed its own leaves that one in place. + slot.release(first); + expect(slot.get()).toBe(second); + slot.release(second); + expect(slot.get()).toBe(fallback); + }); +}); + +describe('the rate-limit store', () => { + it('PORTS-019: the in-memory store answers through promises with the old counting', async () => { + const store = new InMemoryRateLimitStore(); + await expect(store.hit('login', 'k', 1, WINDOW, 1_000)).resolves.toBe(true); + await expect(store.hit('login', 'k', 1, WINDOW, 1_000)).resolves.toBe(false); + await store.reset('login'); + await expect(store.hit('login', 'k', 1, WINDOW, 1_000)).resolves.toBe(true); }); }); diff --git a/server/tests/unit/nest/public-api-scopes.test.ts b/server/tests/unit/nest/public-api-scopes.test.ts index 3d5a90d23..b95e752b0 100644 --- a/server/tests/unit/nest/public-api-scopes.test.ts +++ b/server/tests/unit/nest/public-api-scopes.test.ts @@ -134,7 +134,7 @@ async function thrownAsync(fn: () => Promise): Promise<{ status: number } function apiController(svc: Partial) { - const rl = { check: vi.fn().mockReturnValue(true) } as unknown as RateLimitService; + const rl = { check: vi.fn().mockResolvedValue(true) } as unknown as RateLimitService; return new PublicApiController(svc as PublicApiService, rl); } diff --git a/server/tests/unit/nest/public-api.controller.test.ts b/server/tests/unit/nest/public-api.controller.test.ts index 67311f56c..1776a34c7 100644 --- a/server/tests/unit/nest/public-api.controller.test.ts +++ b/server/tests/unit/nest/public-api.controller.test.ts @@ -36,7 +36,7 @@ const ITEM = { /** The limiter always allows unless a test says otherwise. */ function makeController(svc: Partial, allow = true) { - const rl = { check: vi.fn().mockReturnValue(allow) } as unknown as RateLimitService; + const rl = { check: vi.fn().mockResolvedValue(allow) } as unknown as RateLimitService; return new PublicApiController(svc as PublicApiService, rl); } diff --git a/server/tests/unit/nest/rate-limit.service.test.ts b/server/tests/unit/nest/rate-limit.service.test.ts index 8c75d9415..4fdbac048 100644 --- a/server/tests/unit/nest/rate-limit.service.test.ts +++ b/server/tests/unit/nest/rate-limit.service.test.ts @@ -17,61 +17,61 @@ function bucketSize(service: RateLimitService, bucket: string): number { } describe('RateLimitService', () => { - it('allows up to max attempts in a window, then refuses', () => { + it('allows up to max attempts in a window, then refuses', async () => { const s = new RateLimitService(); const now = Date.now(); - for (let i = 0; i < 5; i++) expect(s.check('login', '1.2.3.4', 5, WINDOW, now)).toBe(true); - expect(s.check('login', '1.2.3.4', 5, WINDOW, now)).toBe(false); + for (let i = 0; i < 5; i++) expect(await s.check('login', '1.2.3.4', 5, WINDOW, now)).toBe(true); + expect(await s.check('login', '1.2.3.4', 5, WINDOW, now)).toBe(false); }); - it('starts a fresh window once the old one elapsed', () => { + it('starts a fresh window once the old one elapsed', async () => { const s = new RateLimitService(); const now = Date.now(); - for (let i = 0; i < 5; i++) s.check('login', '1.2.3.4', 5, WINDOW, now); - expect(s.check('login', '1.2.3.4', 5, WINDOW, now + WINDOW)).toBe(true); + for (let i = 0; i < 5; i++) await s.check('login', '1.2.3.4', 5, WINDOW, now); + expect(await s.check('login', '1.2.3.4', 5, WINDOW, now + WINDOW)).toBe(true); }); - it('drops expired records instead of keeping one per address that ever knocked', () => { + it('drops expired records instead of keeping one per address that ever knocked', async () => { const s = new RateLimitService(); const now = Date.now(); - for (let i = 0; i < 100; i++) s.check('login', `10.0.0.${i}`, 5, WINDOW, now); + for (let i = 0; i < 100; i++) await s.check('login', `10.0.0.${i}`, 5, WINDOW, now); expect(bucketSize(s, 'login')).toBe(100); // One request a window later sweeps the hundred stale keys and leaves its own. - s.check('login', '10.0.0.0', 5, WINDOW, now + WINDOW); + await s.check('login', '10.0.0.0', 5, WINDOW, now + WINDOW); expect(bucketSize(s, 'login')).toBe(1); }); - it('sweeps each bucket on its own clock, so a busy bucket cannot starve a quiet one', () => { + it('sweeps each bucket on its own clock, so a busy bucket cannot starve a quiet one', async () => { const s = new RateLimitService(); const now = Date.now(); - s.check('login', '1.2.3.4', 5, WINDOW, now); - for (let i = 0; i < 3; i++) s.check('mfa', `10.0.0.${i}`, 5, WINDOW, now + WINDOW * (i + 1)); + await s.check('login', '1.2.3.4', 5, WINDOW, now); + for (let i = 0; i < 3; i++) await s.check('mfa', `10.0.0.${i}`, 5, WINDOW, now + WINDOW * (i + 1)); expect(bucketSize(s, 'login')).toBe(1); - s.check('login', '5.6.7.8', 5, WINDOW, now + WINDOW); + await s.check('login', '5.6.7.8', 5, WINDOW, now + WINDOW); expect(bucketSize(s, 'login')).toBe(1); }); - it('keeps a record that is still inside its window when the sweep runs', () => { + it('keeps a record that is still inside its window when the sweep runs', async () => { const s = new RateLimitService(); const now = Date.now(); - s.check('login', '1.2.3.4', 5, WINDOW, now); - s.check('login', '5.6.7.8', 5, WINDOW, now + WINDOW); + await s.check('login', '1.2.3.4', 5, WINDOW, now); + await s.check('login', '5.6.7.8', 5, WINDOW, now + WINDOW); // The second key was written after the sweep, the first one was expired. expect(bucketSize(s, 'login')).toBe(1); - expect(s.check('login', '1.2.3.4', 5, WINDOW, now + WINDOW)).toBe(true); + expect(await s.check('login', '1.2.3.4', 5, WINDOW, now + WINDOW)).toBe(true); }); - it('reset clears a single bucket, or all of them', () => { + it('reset clears a single bucket, or all of them', async () => { const s = new RateLimitService(); const now = Date.now(); - s.check('login', '1.2.3.4', 5, WINDOW, now); - s.check('mfa', '1.2.3.4', 5, WINDOW, now); - s.reset('login'); + await s.check('login', '1.2.3.4', 5, WINDOW, now); + await s.check('mfa', '1.2.3.4', 5, WINDOW, now); + await s.reset('login'); expect(bucketSize(s, 'login')).toBe(0); expect(bucketSize(s, 'mfa')).toBe(1); - s.reset(); + await s.reset(); expect(bucketSize(s, 'mfa')).toBe(0); }); }); diff --git a/server/tests/unit/plugins/plugin-guards.test.ts b/server/tests/unit/plugins/plugin-guards.test.ts index ec214f3d5..94963d736 100644 --- a/server/tests/unit/plugins/plugin-guards.test.ts +++ b/server/tests/unit/plugins/plugin-guards.test.ts @@ -206,7 +206,7 @@ describe('PluginGuards — canEditAs/canCreateAs against a real UsersRepository // The permissions cache is module-scoped (permissions-cache.ts), not // per-instance — invalidate before every case so a stored override from // another describe block in this same worker can't leak in. - permissions.invalidatePermissionsCache(); + await permissions.invalidatePermissionsCache(); guards = new PluginGuards(db, permissions, {} as AddonsService, orm.repo(Users)); createUser(testDb, { role: 'user' }); // id 1, the trip owner — unused by these cases directly memberId = createUser(testDb, { role: 'user' }).user.id; // id 2, a non-owner trip member diff --git a/server/tests/unit/plugins/request-context.test.ts b/server/tests/unit/plugins/request-context.test.ts index c342f4ff6..f0a00b5cc 100644 --- a/server/tests/unit/plugins/request-context.test.ts +++ b/server/tests/unit/plugins/request-context.test.ts @@ -120,7 +120,7 @@ function makeSupervisor(resolveOrm?: () => { em: EntityManager } | undefined) { describe('PluginSupervisor request context (D6, C3)', () => { it('CTX-PLUGIN-001: without a resolveOrm thunk, the dispatch THROWS (host-side visibility) but the child still gets answered first (task-6-rereview.md §5 RULING: an unanswered \'req\' hangs the plugin forever — plugin-host-entry.ts\'s pending map has no timeout)', async () => { - permissions.invalidatePermissionsCache(); + await permissions.invalidatePermissionsCache(); const { supervisor, sup, sent } = makeSupervisor(); // no resolveOrm — the bug's exact reproduction const canCreateAsSpy = vi.spyOn(guards, 'canCreateAs'); try { @@ -147,7 +147,7 @@ describe('PluginSupervisor request context (D6, C3)', () => { }); it('CTX-PLUGIN-002: wrapped in withRequestContext, the same dispatch enforces the stored override — a "user" role is refused', async () => { - permissions.invalidatePermissionsCache(); + await permissions.invalidatePermissionsCache(); const { supervisor, sup, sent } = makeSupervisor(() => t.orm); await supervisor.onMessage(sup, { k: 'req', id: 'r2', method: 'trips.create', params: { _inv: 'inv-1' } }); diff --git a/server/tests/websocket/connection.test.ts b/server/tests/websocket/connection.test.ts index ede3f801f..d9322dcdb 100644 --- a/server/tests/websocket/connection.test.ts +++ b/server/tests/websocket/connection.test.ts @@ -75,9 +75,9 @@ afterAll(async () => { testDb.close(); }); -beforeEach(() => { +beforeEach(async () => { resetTestDb(testDb); - resetRateLimits(nestApp); + await resetRateLimits(nestApp); }); /** Buffered WebSocket wrapper that never drops messages. */