From 03493a2eddfadde9d966c6fb5a10ad2a3732939e Mon Sep 17 00:00:00 2001 From: Maurice <61554723+mauriceboe@users.noreply.github.com> Date: Sat, 8 Aug 2026 11:14:24 +0200 Subject: [PATCH] refactor(server): fold webauthn, account erasure, oauth and the wiki out of services/ (#1840) * refactor(server): move the webauthn config and the account erasure into nest/auth webauthnConfig resolved the RP ID through its own getSetting() on the db singleton; it is a WebauthnConfigService over DatabaseService now, injected into AuthService (passkey_configured) and PasskeyService (four ceremonies). readEnv() is still read per call, so an operator flipping the RP ID at runtime takes effect on the next ceremony instead of after a restart. userCleanupService follows as UserCleanupService. It keeps taking removeUserFromBudgetItems from budget.bridge rather than injecting BudgetService: BudgetModule imports AuthModule for the demo guard in BudgetMcp, so AuthModule cannot import BudgetModule back, and breaking that edge the other way drags BudgetController, BudgetMcp and its trips.bridge chain into every graph that imports AuthModule. The legacy module had no tests of its own - the erasure only ever ran through AdminService.deleteUser and TripsService.deleteGuest - so USER-CLEANUP-001..009 pin both halves, including the orphan data dir that no permissions row can describe and the all-or-nothing deletion transaction. * refactor(server): fold the oauth 2.1 server out of services/ 694 lines move into OauthService over DatabaseService, AddonsService and AuditService. The thin wrapper that only forwarded is gone, and so is the second path to the MCP addon flag - validateAuthorizeRequest read it through addons.bridge while the wrapper read it through the injected service, which would have let the two drift. The pending authorization codes stay a module singleton in oauth.pending-codes.ts instead of becoming instance state. The SDK still serves /oauth/authorize, /oauth/register and /oauth/consent from its own router, mounted on Express before app.init(), so it reaches the domain through the new oauth.bridge.ts - two maps would break the authorization-code flow silently, since the SDK only ever answers "Authorization grant is invalid.". The import-time sweep keeps its .unref() and is a named function now so a test can drive it. scopes and sessionManager are imported deep, never through the src/mcp barrel: the barrel re-exports revokeUserSessionsForClient and pulls the domain bridges back in, which is the nest -> mcp -> nest cycle. Same comment as admin.service and auth.service carry. The 70 cases of the legacy suite moved over unchanged, bound to a hand-built service. Fifteen more cover what free functions never let anyone reach: the code store at capacity, an expired code, an expired access token, refresh against a public and an unknown client, the RFC 8707 resource checks, and that a code written through the container is read back through the bridge. * refactor(server): move the wiki reader into the help module wikiService had exactly one consumer, HelpController, and touches no database - it stays a plain module rather than becoming a provider, the way auth/ephemeral-tokens.ts does, so the text and asset caches keep being one per process without any ceremony around it. The WIKI_DIR anchor needed one more level: src/services sits three below the repo root, src/nest/help sits four. The existing path-resolution case caught it, which is what it is there for - a wrong anchor makes help degrade to the GitHub fallback without anybody noticing. Also brings the nest README back in line: services/ is down to five top-level files, and the passkey and budget entries no longer point at paths that were deleted since they were written. --- server/src/mcp/index.ts | 2 +- server/src/mcp/oauthProvider.ts | 2 +- server/src/nest/README.md | 19 +- server/src/nest/admin/admin.bridge.ts | 9 +- server/src/nest/admin/admin.service.ts | 5 +- server/src/nest/auth/auth.bridge.ts | 16 +- server/src/nest/auth/auth.module.ts | 16 +- server/src/nest/auth/auth.service.ts | 10 +- server/src/nest/auth/passkey.service.ts | 11 +- server/src/nest/auth/user-cleanup.service.ts | 88 ++ .../src/nest/auth/webauthn-config.service.ts | 95 ++ server/src/nest/budget/budget.bridge.ts | 12 +- server/src/nest/help/help.controller.ts | 4 +- server/src/nest/help/help.module.ts | 2 +- .../wikiService.ts => nest/help/wiki.ts} | 9 +- server/src/nest/oauth/oauth-api.controller.ts | 2 +- server/src/nest/oauth/oauth.bridge.ts | 59 + server/src/nest/oauth/oauth.helpers.ts | 70 + server/src/nest/oauth/oauth.module.ts | 10 +- server/src/nest/oauth/oauth.pending-codes.ts | 55 + server/src/nest/oauth/oauth.service.ts | 652 ++++++++- server/src/nest/trips/trips.bridge.ts | 2 + server/src/nest/trips/trips.service.ts | 5 +- server/src/services/oauthService.ts | 694 --------- server/src/services/userCleanupService.ts | 65 - server/src/services/webauthnConfig.ts | 86 -- server/tests/e2e/oauth.e2e.test.ts | 7 +- server/tests/integration/oauth.test.ts | 2 +- server/tests/unit/nest/admin.service.test.ts | 10 +- .../nest/admin.version-notification.test.ts | 10 +- server/tests/unit/nest/auth.service.test.ts | 4 + .../tests/unit/nest/budget.service.db.test.ts | 15 +- server/tests/unit/nest/oauth.service.test.ts | 1250 +++++++++++++++-- .../tests/unit/nest/passkey.service.test.ts | 18 +- server/tests/unit/nest/trips.service.test.ts | 5 + .../unit/nest/user-cleanup.service.test.ts | 223 +++ .../webauthn-config.service.test.ts} | 63 +- .../wikiService.test.ts => nest/wiki.test.ts} | 12 +- .../tests/unit/services/oauthService.test.ts | 958 ------------- 39 files changed, 2528 insertions(+), 2049 deletions(-) create mode 100644 server/src/nest/auth/user-cleanup.service.ts create mode 100644 server/src/nest/auth/webauthn-config.service.ts rename server/src/{services/wikiService.ts => nest/help/wiki.ts} (97%) create mode 100644 server/src/nest/oauth/oauth.bridge.ts create mode 100644 server/src/nest/oauth/oauth.helpers.ts create mode 100644 server/src/nest/oauth/oauth.pending-codes.ts delete mode 100644 server/src/services/oauthService.ts delete mode 100644 server/src/services/userCleanupService.ts delete mode 100644 server/src/services/webauthnConfig.ts create mode 100644 server/tests/unit/nest/user-cleanup.service.test.ts rename server/tests/unit/{services/webauthnConfig.test.ts => nest/webauthn-config.service.test.ts} (66%) rename server/tests/unit/{services/wikiService.test.ts => nest/wiki.test.ts} (95%) delete mode 100644 server/tests/unit/services/oauthService.test.ts diff --git a/server/src/mcp/index.ts b/server/src/mcp/index.ts index 3e2203e6e..7d2b60bdf 100644 --- a/server/src/mcp/index.ts +++ b/server/src/mcp/index.ts @@ -4,7 +4,7 @@ import { McpServer } from '@modelcontextprotocol/sdk/server/mcp'; import { StreamableHTTPServerTransport } from '@modelcontextprotocol/sdk/server/streamableHttp'; import { User } from '../types'; import { verifyMcpToken, verifyJwtToken } from '../nest/auth/auth.bridge'; -import { getUserByAccessToken } from '../services/oauthService'; +import { getUserByAccessToken } from '../nest/oauth/oauth.bridge'; import { isAddonEnabled } from '../nest/addons/addons.bridge'; import { ADDON_IDS } from '../addons'; import { registerResources } from './resources'; diff --git a/server/src/mcp/oauthProvider.ts b/server/src/mcp/oauthProvider.ts index e49edc0d0..bd6843a30 100644 --- a/server/src/mcp/oauthProvider.ts +++ b/server/src/mcp/oauthProvider.ts @@ -14,7 +14,7 @@ import { revokeToken as serviceRevokeToken, verifyPKCE, getUserByAccessToken, -} from '../services/oauthService'; +} from '../nest/oauth/oauth.bridge'; import { ALL_SCOPES } from './scopes'; import { getMcpSafeUrl } from '../app-config'; import { writeAudit } from '../nest/audit/audit.bridge'; diff --git a/server/src/nest/README.md b/server/src/nest/README.md index 305f20711..44943376f 100644 --- a/server/src/nest/README.md +++ b/server/src/nest/README.md @@ -29,7 +29,8 @@ remains as the platform underneath `@nestjs/platform-express`. todo, packing, day-notes, trip-invite, assignments, share, settings, files, collab, vacay, reservations, day, permissions, audit, budget, trip, maps, transit, place, transit-itinerary, collections, atlas, auth, oidc, passkey, - notifications, admin — see the migration recipe below. + notifications, admin, webauthn-config, user-cleanup, oauth, wiki — see the + migration recipe below. - **Foundation (BE-Phase 1, complete):** the eight stateless helpers moved to `common/`; every trip-access check routes through `DatabaseService` (`services/tripAccess.ts` and the dead `middleware/tripAccess.ts` are gone); @@ -40,7 +41,9 @@ remains as the platform underneath `@nestjs/platform-express`. addon guards are one `AddonGuard` + `@RequireAddon`; the demo-mode block is one condition. Still open: `TripAccessGuard`, default-deny, the MFA policy. -`src/services/` is down to 19 top-level files. Note that the trip-access and +`src/services/` is down to five top-level files (journey, journeyShare, +notifications, notificationPreferences, backup) plus the `airtrail/`, +`memories/` and `notifications/` directories. Note that the trip-access and `canEdit` methods on the domain services are **not** dead weight waiting for a guard: their callers are overwhelmingly the `*.mcp.ts` tools, which never pass through an HTTP guard. In the five domains piloted for `TripAccessGuard`, 40 of @@ -266,8 +269,11 @@ composites kept their wrapper names while the raw settlement writes became freeze; the 11-tool `mcp/tools/budget.ts` registrar + 3 `resources.ts` budget resources moved to `budget.mcp.ts`; TripsService, ReservationsService (+ its MCP class) and BookingImportService inject `BudgetService`; a 4-export -`budget.bridge.ts` serves the legacy tripService/userCleanupService and the -trips/transports registrars; `exchange-rates.bridge.ts` was deleted with its +`budget.bridge.ts` served the legacy tripService/userCleanupService and the +trips/transports registrars (down to two exports since; `UserCleanupService` +still takes `removeUserFromBudgetItems` from it, because BudgetModule imports +AuthModule for BudgetMcp's demo guard and AuthModule therefore cannot import +BudgetModule back); `exchange-rates.bridge.ts` was deleted with its last consumers, and the controller adopted `budget.dto.ts` — all nine allow-list entries removed). tripService followed (the 1121-line Wave-4 hub — the biggest fold — moved into @@ -463,8 +469,9 @@ passkeyService followed (the last frontier member of the auth stack: the `PasskeyService` over DatabaseService + injected AuthService — its three `auth.bridge` imports resolved to `this.auth.generateToken` plus plain `stripUserForClient`/`avatarUrl` helper imports, and `resolveWebauthnConfig` -stays a plain `services/webauthnConfig` import (the helper also feeds -auth.service's `isPasskeyConfigured`). No MCP registrar, no plugin-host +stayed a plain `services/webauthnConfig` import at the time (it has since +become the injected `WebauthnConfigService`, which also feeds auth.service's +`passkey_configured`). No MCP registrar, no plugin-host import, and **no bridge at all** — both consumers were already in-container: `PasskeyController` swapped its `import *` shim for the injection, and `AdminService` swapped its `adminResetPasskeys` function import for the diff --git a/server/src/nest/admin/admin.bridge.ts b/server/src/nest/admin/admin.bridge.ts index d6c66965c..9ca06c5ba 100644 --- a/server/src/nest/admin/admin.bridge.ts +++ b/server/src/nest/admin/admin.bridge.ts @@ -6,6 +6,8 @@ import { SettingsService } from '../settings/settings.service'; import { AuthService } from '../auth/auth.service'; import { PasskeyService } from '../auth/passkey.service'; import { AtlasService } from '../atlas/atlas.service'; +import { UserCleanupService } from '../auth/user-cleanup.service'; +import { WebauthnConfigService } from '../auth/webauthn-config.service'; import { TripMembershipService } from '../trip-membership/trip-membership.service'; import { PackingService } from '../packing/packing.service'; import { PermissionsService } from '../permissions/permissions.service'; @@ -31,16 +33,19 @@ import { AdminService } from './admin.service'; const dbs = new DatabaseService(db); const realtime = new RealtimeService(); const permissions = new PermissionsService(dbs); -const auth = new AuthService(dbs, permissions, new AtlasService(dbs), new TripMembershipService(dbs)); +const webauthn = new WebauthnConfigService(dbs); +const userCleanup = new UserCleanupService(dbs); +const auth = new AuthService(dbs, permissions, new AtlasService(dbs), new TripMembershipService(dbs), webauthn, userCleanup); const admin = new AdminService( dbs, new SettingsService(dbs), new AddonsService(dbs), - new PasskeyService(dbs, auth), + new PasskeyService(dbs, auth, webauthn), new PackingService(dbs, permissions, realtime), auth, permissions, new NotificationsService(dbs, realtime), + userCleanup, ); export function checkAndNotifyVersion(): Promise { diff --git a/server/src/nest/admin/admin.service.ts b/server/src/nest/admin/admin.service.ts index ce7e3e5ae..69b5fff92 100644 --- a/server/src/nest/admin/admin.service.ts +++ b/server/src/nest/admin/admin.service.ts @@ -20,7 +20,7 @@ import { avatarUrl } from '../common/avatarUrl'; import { prepareLlmAddonConfigForWrite, maskLlmAddonConfig } from '../llm-parse/llm-config'; import { getPhotoProviderConfig } from '../../services/memories/helpersService'; import { validatePassword } from '../common/passwordPolicy'; -import { deleteUserCompletely } from '../../services/userCleanupService'; +import { UserCleanupService } from '../auth/user-cleanup.service'; import { getPreferencesMatrix, setAdminPreferences } from '../../services/notificationPreferencesService'; import { DatabaseService } from '../database/database.service'; import { AddonsService } from '../addons/addons.service'; @@ -77,6 +77,7 @@ export class AdminService { private readonly auth: AuthService, private readonly permissions: PermissionsService, private readonly notifications: NotificationsService, + private readonly userCleanup: UserCleanupService, ) {} // ── User CRUD ────────────────────────────────────────────────────────────── @@ -231,7 +232,7 @@ export class AdminService { const userToDel = this.db.get<{ id: number; email: string }>('SELECT id, email FROM users WHERE id = ?', id); if (!userToDel) return { error: 'User not found', status: 404 }; - deleteUserCompletely(userToDel.id); + this.userCleanup.deleteUserCompletely(userToDel.id); emitUserDeleted(userToDel.id); // let plugins erase their own per-user data return { email: userToDel.email }; } diff --git a/server/src/nest/auth/auth.bridge.ts b/server/src/nest/auth/auth.bridge.ts index ecf6f7d31..574b4d436 100644 --- a/server/src/nest/auth/auth.bridge.ts +++ b/server/src/nest/auth/auth.bridge.ts @@ -4,6 +4,8 @@ import { PermissionsService } from '../permissions/permissions.service'; import { AtlasService } from '../atlas/atlas.service'; import { TripMembershipService } from '../trip-membership/trip-membership.service'; import { AuthService } from './auth.service'; +import { UserCleanupService } from './user-cleanup.service'; +import { WebauthnConfigService } from './webauthn-config.service'; import { User } from '../../types'; /** @@ -28,12 +30,18 @@ import { User } from '../../types'; * * Module-level construction is safe: `db` is the reinitialize-proof Proxy onto * the shared better-sqlite3 singleton (same pattern as permissions.bridge.ts). + * WebauthnConfigService and UserCleanupService are constructed only because + * AuthService takes them — nothing this file exports reaches either one. */ +const dbs = new DatabaseService(db); +const permissions = new PermissionsService(dbs); const auth = new AuthService( - new DatabaseService(db), - new PermissionsService(new DatabaseService(db)), - new AtlasService(new DatabaseService(db)), - new TripMembershipService(new DatabaseService(db)), + dbs, + permissions, + new AtlasService(dbs), + new TripMembershipService(dbs), + new WebauthnConfigService(dbs), + new UserCleanupService(dbs), ); export { stripUserForClient } from './auth.helpers'; diff --git a/server/src/nest/auth/auth.module.ts b/server/src/nest/auth/auth.module.ts index 5f756b25b..10d472c5d 100644 --- a/server/src/nest/auth/auth.module.ts +++ b/server/src/nest/auth/auth.module.ts @@ -5,6 +5,8 @@ import { AuthController } from './auth.controller'; import { PasskeyController } from './passkey.controller'; import { AuthService } from './auth.service'; import { PasskeyService } from './passkey.service'; +import { UserCleanupService } from './user-cleanup.service'; +import { WebauthnConfigService } from './webauthn-config.service'; import { AppConfigModule } from '../app-config/app-config.module'; import { AuditModule } from '../audit/audit.module'; import { PermissionsModule } from '../permissions/permissions.module'; @@ -18,15 +20,17 @@ import { TripMembershipModule } from '../trip-membership/trip-membership.module' * sub-paths explicitly rather than claiming all of /api/auth. * * PermissionsModule feeds getAppConfig's permissions block; AtlasModule feeds - * getTravelStats' hidden-countries subtraction. AuthService is exported for - * the in-container consumers (the domain *.mcp.ts demo guards, OidcService, - * PasskeyEnabledGuard); PasskeyService for AdminService's passkey reset; - * everything outside the container goes through auth.bridge.ts. + * getTravelStats' hidden-countries subtraction. AuthService is + * exported for the in-container consumers (the domain *.mcp.ts demo guards, + * OidcService, PasskeyEnabledGuard); PasskeyService for AdminService's passkey + * reset; UserCleanupService for the two account-deletion paths (AdminService) + * and the guest deletion in TripsService; everything outside the container goes + * through auth.bridge.ts. */ @Module({ imports: [RateLimitModule, AuditModule, PermissionsModule, AtlasModule, TripMembershipModule, AppConfigModule], controllers: [AuthPublicController, AuthController, PasskeyController], - providers: [AuthService, PasskeyService], - exports: [AuthService, PasskeyService], + providers: [AuthService, PasskeyService, UserCleanupService, WebauthnConfigService], + exports: [AuthService, PasskeyService, UserCleanupService], }) export class AuthModule {} diff --git a/server/src/nest/auth/auth.service.ts b/server/src/nest/auth/auth.service.ts index c19657119..aa5f56e91 100644 --- a/server/src/nest/auth/auth.service.ts +++ b/server/src/nest/auth/auth.service.ts @@ -22,7 +22,7 @@ import { createEphemeralToken, startTokenCleanup, stopTokenCleanup } from './eph // every consumer of this module — a nest→mcp→nest module cycle. import { revokeUserSessions } from '../../mcp/sessionManager'; import { startTripReminders } from '../../scheduler'; -import { deleteUserCompletely } from '../../services/userCleanupService'; +import { UserCleanupService } from './user-cleanup.service'; import { emitUserDeleted } from '../../plugin-user-lifecycle'; import { haversineKm } from '../common/geo'; import { verifyJwtAndLoadUser } from './jwt-verify'; @@ -30,7 +30,7 @@ import { User } from '../../types'; import { DEMO_EMAIL_PRIMARY, isDemoEmail } from '../common/demo'; import { avatarUrl } from '../common/avatarUrl'; import { TripMembershipService } from '../trip-membership/trip-membership.service'; -import { isPasskeyConfigured } from '../../services/webauthnConfig'; +import { WebauthnConfigService } from './webauthn-config.service'; import { setAuthCookie, clearAuthCookie } from '../common/cookie'; import { sendPasswordResetEmail } from '../../services/notifications'; import { getAppUrl } from '../../app-config'; @@ -138,6 +138,8 @@ export class AuthService implements OnModuleInit, OnModuleDestroy { private readonly permissions: PermissionsService, private readonly atlas: AtlasService, private readonly membership: TripMembershipService, + private readonly webauthn: WebauthnConfigService, + private readonly userCleanup: UserCleanupService, ) {} // Cookie @@ -279,7 +281,7 @@ export class AuthService implements OnModuleInit, OnModuleDestroy { // are true. `passkey_configured` stays a pure boolean — it never leaks the // resolved RP ID / origin / APP_URL on this unauthenticated endpoint. passkey_login: toggles.passkey_login, - passkey_configured: isPasskeyConfigured(), + passkey_configured: this.webauthn.isConfigured(), env_override_oidc_only: readEnv().oidc.only, has_users: userCount > 0, setup_complete: setupComplete, @@ -566,7 +568,7 @@ export class AuthService implements OnModuleInit, OnModuleDestroy { return { error: 'Cannot delete the last admin account', status: 400 }; } } - deleteUserCompletely(userId); + this.userCleanup.deleteUserCompletely(userId); emitUserDeleted(userId); // let plugins erase their own per-user data return { success: true }; } diff --git a/server/src/nest/auth/passkey.service.ts b/server/src/nest/auth/passkey.service.ts index ea944599a..880d7f89f 100644 --- a/server/src/nest/auth/passkey.service.ts +++ b/server/src/nest/auth/passkey.service.ts @@ -7,7 +7,7 @@ import { verifyAuthenticationResponse, type AuthenticatorTransportFuture, } from '@simplewebauthn/server'; -import { resolveWebauthnConfig } from '../../services/webauthnConfig'; +import { WebauthnConfigService } from './webauthn-config.service'; import { avatarUrl } from '../common/avatarUrl'; import { stripUserForClient } from './auth.helpers'; import { AuthService } from './auth.service'; @@ -94,6 +94,7 @@ export class PasskeyService { constructor( private readonly db: DatabaseService, private readonly auth: AuthService, + private readonly webauthn: WebauthnConfigService, ) {} // ------------------------------------------------------------------------- @@ -133,7 +134,7 @@ export class PasskeyService { userId: number, password: string | undefined, ): Promise<{ error?: string; status?: number; options?: Awaited> }> { - const cfg = resolveWebauthnConfig(); + const cfg = this.webauthn.resolve(); if (!cfg) return { ...NOT_CONFIGURED }; const user = this.db.get('SELECT * FROM users WHERE id = ?', userId); @@ -174,7 +175,7 @@ export class PasskeyService { userId: number, body: { attestationResponse?: unknown; name?: unknown }, ): Promise<{ error?: string; status?: number; success?: boolean; credential?: unknown }> { - const cfg = resolveWebauthnConfig(); + const cfg = this.webauthn.resolve(); if (!cfg) return { ...NOT_CONFIGURED }; const resp = body?.attestationResponse; @@ -257,7 +258,7 @@ export class PasskeyService { status?: number; options?: Awaited>; }> { - const cfg = resolveWebauthnConfig(); + const cfg = this.webauthn.resolve(); if (!cfg) return { ...NOT_CONFIGURED }; const now = Date.now(); @@ -282,7 +283,7 @@ export class PasskeyService { auditUserId?: number | null; auditAction?: string; }> { - const cfg = resolveWebauthnConfig(); + const cfg = this.webauthn.resolve(); if (!cfg) return { ...NOT_CONFIGURED }; const resp = body?.assertionResponse; diff --git a/server/src/nest/auth/user-cleanup.service.ts b/server/src/nest/auth/user-cleanup.service.ts new file mode 100644 index 000000000..0313070c6 --- /dev/null +++ b/server/src/nest/auth/user-cleanup.service.ts @@ -0,0 +1,88 @@ +import fs from 'node:fs'; +import { Injectable } from '@nestjs/common'; +import { DatabaseService } from '../database/database.service'; +// budget.bridge, not an injected BudgetService: BudgetModule imports AuthModule +// (BudgetMcp's demo guard), so AuthModule importing BudgetModule back would +// close a module cycle — and breaking it the other way would drag +// BudgetController, BudgetMcp and its trips.bridge/MCP `_shared` chain into +// every graph that imports AuthModule, including the single-domain e2e +// TestingModules. Same documented in-container exception as atlas.mcp.ts. +// The bridge instance runs its SQL over the shared db Proxy, and the re-split +// keeps no state, so the second BudgetService is behaviourally identical. +import { removeUserFromBudgetItems } from '../budget/budget.bridge'; +import { pluginsDataRoot } from '../plugins/paths'; + +/** + * Account erasure — everything that has to happen around `DELETE FROM users` + * and is not covered by a foreign key. + * + * Lives in the auth domain because it owns the `users` table; both deletion + * paths (a user deleting their own account, an admin deleting someone else's) + * go through here, and `TripsService.deleteGuest` reuses the plugin half for a + * guest id. The plugin erasure runs in the CORE deletion path, not via the + * plugin runtime, so it works even when TREK_PLUGINS_ENABLED=false or before + * the runtime has booted — otherwise a deletion in those windows would leave + * the user's plugin data behind forever. + */ +@Injectable() +export class UserCleanupService { + constructor(private readonly db: DatabaseService) {} + + /** + * Erase a user's PLUGIN-held data on account deletion. Two parts: + * 1. Host-side per-user plugin tables (encrypted config values, OAuth access/refresh + * tokens, in-flight OAuth state) — deleted directly; these live in trek.db, not in + * a plugin's own db, so nothing else ever removes them. + * 2. A durable erasure row per plugin that holds `hook:user-data`, so its OWN db is + * purged of the user (drained to the plugin when it is next active). + * + * Best-effort per table so a slimmed-down schema (some tests) can't fail the user + * deletion itself. + */ + erasePluginUserData(userId: number): void { + for (const table of ['plugin_user_config', 'plugin_oauth_tokens', 'plugin_oauth_state']) { + try { this.db.run(`DELETE FROM ${table} WHERE user_id = ?`, userId); } catch { /* table absent (slim schema) */ } + } + try { + const rows = this.db.all<{ id: string; permissions: string | null }>('SELECT id, permissions FROM plugins'); + const installed = new Set(rows.map((r) => r.id)); + const insert = this.db.prepare('INSERT OR IGNORE INTO plugin_user_erasure_queue (plugin_id, user_id) VALUES (?, ?)'); + for (const r of rows) { + let perms: unknown; + try { perms = JSON.parse(r.permissions ?? '[]'); } catch { perms = []; } + if (Array.isArray(perms) && perms.includes('hook:user-data')) insert.run(r.id, userId); + } + // Also enqueue for plugins UNINSTALLED with their data retained (deleteData=false): + // their data dir still holds the user's rows and a same-id reinstall would re-adopt + // them. No permissions record survives uninstall, so we can't check hook:user-data — + // enqueue for every orphan data dir; the row sits inert (no FK) and drains only if + // that id is reinstalled + active (erasure delivery is a duty, not grant-gated). + try { + for (const entry of fs.readdirSync(pluginsDataRoot(), { withFileTypes: true })) { + if (entry.isDirectory() && !installed.has(entry.name)) insert.run(entry.name, userId); + } + } catch { /* no plugin data root yet */ } + } catch { /* plugins / queue table absent (slim schema) */ } + } + + private cleanupUserReferences(userId: number): void { + this.db.run('UPDATE trip_members SET invited_by = NULL WHERE invited_by = ?', userId); + this.db.run('UPDATE budget_items SET paid_by_user_id = NULL WHERE paid_by_user_id = ?', userId); + removeUserFromBudgetItems(userId); + this.db.run('DELETE FROM share_tokens WHERE created_by = ?', userId); + this.db.run('DELETE FROM journey_share_tokens WHERE created_by = ?', userId); + // Owned journeys cascade-delete their entries/contributors/share_tokens/photos via journey_id FKs + this.db.run('DELETE FROM journeys WHERE user_id = ?', userId); + // Entries authored on other users' journeys (not covered by the cascade above) + this.db.run('DELETE FROM journey_entries WHERE author_id = ?', userId); + this.db.run('DELETE FROM journey_contributors WHERE user_id = ?', userId); + } + + deleteUserCompletely(userId: number): void { + this.db.transaction(() => { + this.cleanupUserReferences(userId); + this.erasePluginUserData(userId); + this.db.run('DELETE FROM users WHERE id = ?', userId); + }); + } +} diff --git a/server/src/nest/auth/webauthn-config.service.ts b/server/src/nest/auth/webauthn-config.service.ts new file mode 100644 index 000000000..831911110 --- /dev/null +++ b/server/src/nest/auth/webauthn-config.service.ts @@ -0,0 +1,95 @@ +import { Injectable } from '@nestjs/common'; +import { getAppUrl, readEnv } from '../../app-config'; +import { DatabaseService } from '../database/database.service'; + +function hostOf(url: string): string | null { + try { + return new URL(url).hostname || null; + } catch { + return null; + } +} + +/** WebAuthn RP IDs must be registrable domains — never bare IP literals. */ +function isIpHost(host: string): boolean { + if (/^\d{1,3}(\.\d{1,3}){3}$/.test(host)) return true; // IPv4 + if (host.includes(':')) return true; // IPv6 (hostname keeps the colons) + return false; +} + +export interface WebauthnConfig { + rpID: string; + rpName: string; + /** Exact allowed origins (scheme + host + port). One in prod; localhost dev adds the Vite/API ports. */ + origins: string[]; +} + +/** + * Resolves the WebAuthn Relying Party ID + allowed origins for this deployment. + * + * SECURITY: the RP ID and the allowed origins are derived ONLY from server-side + * configuration — the `webauthn_rp_id` / `webauthn_origins` admin settings (or + * the matching env vars), falling back to APP_URL. They are NEVER taken from the + * request `Host` / `X-Forwarded-Host` header: a forged forwarded host would + * otherwise let an attacker bind credentials to a domain they control, or brick + * every enrolled user. This mirrors how OIDC derives its redirect URI from + * APP_URL (oidc.controller.ts) rather than from request input. + * + * Nothing is cached: `readEnv()` and the app-settings row are read on every + * resolve so an operator flipping the RP ID at runtime takes effect on the next + * ceremony instead of after a restart. + */ +@Injectable() +export class WebauthnConfigService { + constructor(private readonly db: DatabaseService) {} + + private setting(key: string): string | null { + const raw = this.db.get<{ value: string }>('SELECT value FROM app_settings WHERE key = ?', key)?.value; + const trimmed = raw?.trim(); + return trimmed ? trimmed : null; + } + + /** + * Returns null when no usable RP ID can be resolved (bare IP host, or nothing + * configured) — the feature then reports itself as "not configured" and stays + * disabled so nobody can enrol a credential bound to the wrong origin. + */ + resolve(): WebauthnConfig | null { + // 1. Explicit operator config always wins. + const explicitRpId = (readEnv().webauthn.rpId || this.setting('webauthn_rp_id'))?.trim() || null; + const explicitOrigins = (readEnv().webauthn.origins || this.setting('webauthn_origins') || '') + .split(',') + .map((o) => o.trim().replace(/\/+$/, '')) + .filter(Boolean); + + const appUrl = getAppUrl(); + const appHost = hostOf(appUrl); + + // 2. Derive the RP ID from APP_URL when not explicitly set. + let rpID = explicitRpId; + if (!rpID && appHost && !isIpHost(appHost)) { + rpID = appHost; // a real domain, or "localhost" + } + if (!rpID) return null; // bare IP / unresolved → WebAuthn cannot be used here + + // 3. Resolve the allowed origins. Explicit list wins verbatim (operator's + // responsibility). Otherwise derive a SINGLE origin from APP_URL — we never + // silently union dev localhost origins into a production allow-list. + let origins = explicitOrigins; + if (origins.length === 0) { + if (appHost) origins = [appUrl.replace(/\/+$/, '')]; + if (rpID === 'localhost') { + // Dev: the browser origin is the Vite dev server (:5173), not the API port. + origins = Array.from(new Set([...origins, 'http://localhost:5173', 'http://localhost:3001'])); + } + } + if (origins.length === 0) return null; + + return { rpID, rpName: 'TREK', origins }; + } + + /** True when a usable RP ID resolves for this deployment (exposed as a pure boolean on app-config). */ + isConfigured(): boolean { + return this.resolve() !== null; + } +} diff --git a/server/src/nest/budget/budget.bridge.ts b/server/src/nest/budget/budget.bridge.ts index 9404de576..67368a609 100644 --- a/server/src/nest/budget/budget.bridge.ts +++ b/server/src/nest/budget/budget.bridge.ts @@ -6,17 +6,19 @@ import { PermissionsService } from '../permissions/permissions.service'; import { ExchangeRatesService } from './exchange-rates.service'; /** - * Non-Nest entry point for the budget domain — for code running OUTSIDE the - * Nest container (services/userCleanupService.ts and the legacy - * create_transport registrar in src/mcp/tools/transports.ts; the budget MCP + * Non-Nest entry point for the budget domain — for the legacy create_transport + * registrar in src/mcp/tools/transports.ts, plus one in-container exception: + * UserCleanupService (nest/auth) takes removeUserFromBudgetItems from here + * because BudgetModule imports AuthModule for BudgetMcp's demo guard, so + * AuthModule cannot import BudgetModule back — see the note there. (The budget MCP * tools and resources moved to the DI-discovered budget.mcp.ts, the plugin * RPC host injects BudgetService via PluginHostDepsFactory, and the trip * domain folded into the DI-native TripsService, which injects BudgetService — * its bridge exports were pruned with it). Exports only the legacy * services/budgetService names still consumed outside the container, 1:1, so * repointing a consumer is an import-path-only diff. Inside the container, - * inject BudgetService instead. Delete this file when - * userCleanupService and the transports registrar migrate. + * inject BudgetService instead. Delete this file when the transports registrar + * migrates and BudgetMcp stops needing AuthService. * * Module-level construction is safe: `db` is the reinitialize-proof Proxy onto * the shared better-sqlite3 singleton, and ExchangeRatesService keeps its FX diff --git a/server/src/nest/help/help.controller.ts b/server/src/nest/help/help.controller.ts index ace688be3..9058431ee 100644 --- a/server/src/nest/help/help.controller.ts +++ b/server/src/nest/help/help.controller.ts @@ -8,11 +8,11 @@ import { WikiNotFound, type WikiPage, type WikiNavSection, -} from '../../services/wikiService'; +} from './wiki'; /** * /api/help — embedded TREK wiki, served from the `wiki/` directory that ships - * with the app (see wikiService for the GitHub fallback). Content is public docs, + * with the app (see wiki.ts for the GitHub fallback). Content is public docs, * so these endpoints are unauthenticated; that also lets tags load the * proxied assets without sending credentials. */ diff --git a/server/src/nest/help/help.module.ts b/server/src/nest/help/help.module.ts index 7a9ce9271..0637cf68d 100644 --- a/server/src/nest/help/help.module.ts +++ b/server/src/nest/help/help.module.ts @@ -1,7 +1,7 @@ import { Module } from '@nestjs/common'; import { HelpController } from './help.controller'; -/** /api/help — the bundled `wiki/` directory, read via wikiService. */ +/** /api/help — the bundled `wiki/` directory, read via ./wiki. */ @Module({ controllers: [HelpController], }) diff --git a/server/src/services/wikiService.ts b/server/src/nest/help/wiki.ts similarity index 97% rename from server/src/services/wikiService.ts rename to server/src/nest/help/wiki.ts index b35c344e4..757f25905 100644 --- a/server/src/services/wikiService.ts +++ b/server/src/nest/help/wiki.ts @@ -1,6 +1,6 @@ import { existsSync, promises as fs } from 'fs'; import path from 'path'; -import { readEnv } from '../app-config'; +import { readEnv } from '../../app-config'; /** * In-app Help/Wiki content, sourced from the `wiki/**` directory that ships with @@ -20,12 +20,13 @@ const TTL_MS = 60 * 60 * 1000; // remote fallback only: refresh from GitHub at m const SLUG_RE = /^[A-Za-z0-9][A-Za-z0-9._-]*$/; /** - * `server/{src,dist}/services` both sit three levels under the repo root, so this + * `server/{src,dist}/nest/help` both sit four levels under the repo root, so this * one anchor resolves in dev, a built source install, vitest, and Docker (where * the Dockerfile copies `wiki/` to /app/wiki). `process.cwd()` would not — Docker - * runs the server from /app/server. + * runs the server from /app/server. The depth is counted from THIS file: it was + * three while the module lived in `src/services`. */ -const WIKI_DIR = readEnv().paths.wikiDir ?? path.join(__dirname, '..', '..', '..', 'wiki'); +const WIKI_DIR = readEnv().paths.wikiDir ?? path.join(__dirname, '..', '..', '..', '..', 'wiki'); /** * Probe for the sidebar rather than the bare directory: an empty or half-copied diff --git a/server/src/nest/oauth/oauth-api.controller.ts b/server/src/nest/oauth/oauth-api.controller.ts index d4bbe8547..96727dde7 100644 --- a/server/src/nest/oauth/oauth-api.controller.ts +++ b/server/src/nest/oauth/oauth-api.controller.ts @@ -8,7 +8,7 @@ import { OptionalJwtGuard } from '../auth/optional-jwt.guard'; import { CurrentUser } from '../auth/current-user.decorator'; import { getClientIp } from '../audit/client-ip'; import type { User } from '../../types'; -import type { AuthorizeParams } from '../../services/oauthService'; +import type { AuthorizeParams } from './oauth.service'; const MIN = 60_000; diff --git a/server/src/nest/oauth/oauth.bridge.ts b/server/src/nest/oauth/oauth.bridge.ts new file mode 100644 index 000000000..6b3ceb6bd --- /dev/null +++ b/server/src/nest/oauth/oauth.bridge.ts @@ -0,0 +1,59 @@ +import { db } from '../../db/database'; +import { AddonsService } from '../addons/addons.service'; +import { AuditService } from '../audit/audit.service'; +import { DatabaseService } from '../database/database.service'; +import { OauthService } from './oauth.service'; + +/** + * Non-Nest entry point for the OAuth domain — for code running OUTSIDE the + * Nest container: the MCP transport (src/mcp/index.ts token verification) and + * the SDK provider adapter (src/mcp/oauthProvider.ts), which + * platform.routes.ts mounts on the Express app BEFORE app.init(), so + * `app.get(OauthService)` is not available to it. Exports the legacy + * services/oauthService names 1:1 so repointing a consumer is an + * import-path-only diff. Inside the container, inject OauthService instead. + * Delete this file when the SDK mount moves behind the container. + * + * Module-level construction is safe: `db` is the reinitialize-proof Proxy onto + * the shared better-sqlite3 singleton. The one piece of shared state — the + * pending authorization codes — is module-scoped in oauth.pending-codes.ts, so + * this instance and the container singleton see the same map. That is load- + * bearing: the consent controller writes the code through the container and + * the SDK reads it back through here. + */ +const dbs = new DatabaseService(db); +const oauth = new OauthService(dbs, new AddonsService(dbs), new AuditService(dbs)); + +export type { OAuthTokenInfo, PendingCode } from './oauth.service'; + +export function getUserByAccessToken(rawToken: string) { + return oauth.getUserByAccessToken(rawToken); +} + +export function createOAuthClient(...args: Parameters) { + return oauth.createOAuthClient(...args); +} + +export function createAuthCode(params: Parameters[0]) { + return oauth.createAuthCode(params); +} + +export function consumeAuthCode(code: string) { + return oauth.consumeAuthCode(code); +} + +export function issueTokens(...args: Parameters) { + return oauth.issueTokens(...args); +} + +export function refreshTokens(...args: Parameters) { + return oauth.refreshTokens(...args); +} + +export function revokeToken(...args: Parameters) { + return oauth.revokeToken(...args); +} + +export function verifyPKCE(codeVerifier: string, codeChallenge: string): boolean { + return oauth.verifyPKCE(codeVerifier, codeChallenge); +} diff --git a/server/src/nest/oauth/oauth.helpers.ts b/server/src/nest/oauth/oauth.helpers.ts new file mode 100644 index 000000000..6846e886a --- /dev/null +++ b/server/src/nest/oauth/oauth.helpers.ts @@ -0,0 +1,70 @@ +import crypto, { createHash, randomBytes } from 'crypto'; + +/** Pure helpers and row shapes for the OAuth 2.1 domain — no DB, no DI. */ + +// --------------------------------------------------------------------------- +// Constants +// --------------------------------------------------------------------------- + +export const ACCESS_TOKEN_TTL_S = 60 * 60; // 1 hour +export const REFRESH_TOKEN_TTL_MS = 30 * 24 * 60 * 60 * 1000; // 30 days rolling + +// PKCE format (RFC 7636) +export const CODE_CHALLENGE_RE = /^[A-Za-z0-9_-]{43}$/; +export const CODE_VERIFIER_RE = /^[A-Za-z0-9\-._~]{43,128}$/; + +// --------------------------------------------------------------------------- +// DB row types +// --------------------------------------------------------------------------- + +export interface OAuthClientRow { + id: string; + user_id: number; + name: string; + client_id: string; + client_secret_hash: string; + redirect_uris: string; // JSON array + allowed_scopes: string; // JSON array + created_at: string; + is_public: number; // 0 | 1 (SQLite boolean) + created_via: string; // 'settings_ui' | 'browser-registration' + allows_client_credentials: number; // 0 | 1 +} + +export interface OAuthTokenRow { + id: number; + client_id: string; + user_id: number; + access_token_hash: string; + refresh_token_hash: string; + scopes: string; // JSON array + audience: string | null; + access_token_expires_at: string; + refresh_token_expires_at: string; + revoked_at: string | null; + parent_token_id: number | null; +} + +// --------------------------------------------------------------------------- +// Token helpers +// --------------------------------------------------------------------------- + +export function hashToken(raw: string): string { + return createHash('sha256').update(raw).digest('hex'); +} + +/** Constant-time comparison of two hex-encoded SHA-256 hashes. */ +export function timingSafeEqualHex(a: string, b: string): boolean { + if (a.length !== b.length) return false; + try { + return crypto.timingSafeEqual(Buffer.from(a, 'hex'), Buffer.from(b, 'hex')); + } catch { return false; } +} + +export function generateAccessToken(): string { + return 'trekoa_' + randomBytes(32).toString('hex'); +} + +export function generateRefreshToken(): string { + return 'trekrf_' + randomBytes(32).toString('hex'); +} diff --git a/server/src/nest/oauth/oauth.module.ts b/server/src/nest/oauth/oauth.module.ts index fb3dfe722..60e404913 100644 --- a/server/src/nest/oauth/oauth.module.ts +++ b/server/src/nest/oauth/oauth.module.ts @@ -8,9 +8,13 @@ import { AddonsModule } from '../addons/addons.module'; /** * OAuth 2.1 server (MCP). Public token/userinfo/revoke endpoints + the SPA's - * authenticated consent/client/session management. The SDK-mounted - * /oauth/authorize, /oauth/register and /oauth/consent stay on Express, so the - * strangler lists /oauth/token, /oauth/userinfo, /oauth/revoke explicitly. + * authenticated consent/client/session management. /oauth/authorize, + * /oauth/register and /oauth/consent are still served by the MCP SDK's own + * router, mounted on the Express platform before app.init() — that path + * reaches the domain through oauth.bridge.ts, which is why the pending + * authorization codes live in a module, not on the provider. + * + * No `exports`: nothing inside the container injects this domain. */ @Module({ imports: [RateLimitModule, AuditModule, AddonsModule], diff --git a/server/src/nest/oauth/oauth.pending-codes.ts b/server/src/nest/oauth/oauth.pending-codes.ts new file mode 100644 index 000000000..5f74374aa --- /dev/null +++ b/server/src/nest/oauth/oauth.pending-codes.ts @@ -0,0 +1,55 @@ +/** + * The short-lived authorization-code store. + * + * Module-scoped + import-time interval on purpose (legacy parity, shared + * between bridge and container instances — atlas-geo interval precedent). + * This is not a stylistic choice: the SDK-mounted /oauth/authorize path reads + * a code through oauth.bridge (mcp/oauthProvider.ts) that the Nest consent + * controller wrote through the container singleton. Two instances would mean + * two maps, and the authorization-code flow would fail *silently* — the SDK + * only ever answers "Authorization grant is invalid." + * + * No DB persistence: codes live two minutes and a restart invalidating them + * is the correct behaviour. + */ + +export interface PendingCode { + clientId: string; + userId: number; + redirectUri: string; + scopes: string[]; + resource: string | null; + codeChallenge: string; + codeChallengeMethod: 'S256'; + expiresAt: number; +} + +export const MAX_PENDING_CODES = 500; +export const AUTH_CODE_TTL_MS = 2 * 60 * 1000; // 2 minutes + +const pendingCodes = new Map(); + +/** Drop everything past its TTL. Named so the interval body is reachable from a test. */ +export function sweepPendingCodes(now = Date.now()): void { + for (const [key, entry] of pendingCodes) { + if (now > entry.expiresAt) pendingCodes.delete(key); + } +} + +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 { + if (pendingCodes.size >= MAX_PENDING_CODES) return false; + pendingCodes.set(code, entry); + return true; +} + +/** Single-use: the entry is removed even when it turns out to be expired. */ +export function takePendingCode(code: string): PendingCode | null { + const entry = pendingCodes.get(code); + if (!entry) return null; + pendingCodes.delete(code); + if (Date.now() > entry.expiresAt) return null; + return entry; +} diff --git a/server/src/nest/oauth/oauth.service.ts b/server/src/nest/oauth/oauth.service.ts index 4f4b5bee6..b28577d1b 100644 --- a/server/src/nest/oauth/oauth.service.ts +++ b/server/src/nest/oauth/oauth.service.ts @@ -1,38 +1,644 @@ +import crypto, { randomBytes, randomUUID } from 'crypto'; import { Injectable } from '@nestjs/common'; -import * as oauth from '../../services/oauthService'; -import { AddonsService } from '../addons/addons.service'; import { ADDON_IDS } from '../../addons'; import { getMcpSafeUrl } from '../../app-config'; +// Import from scopes/sessionManager directly, NOT the ../../mcp barrel: the +// barrel pulls the whole tools fan-out (and via the domain bridges, the Nest +// services) into every consumer of this module — a nest→mcp→nest module cycle. +// Both files themselves are back-edge free (scopes has no imports at all, +// sessionManager only the MCP SDK). +import { validateScopes } from '../../mcp/scopes'; +import { revokeUserSessionsForClient } from '../../mcp/sessionManager'; +import { User } from '../../types'; +import { AddonsService } from '../addons/addons.service'; +import { AuditService } from '../audit/audit.service'; +import { logWarn } from '../audit/audit-log.logger'; +import { DatabaseService } from '../database/database.service'; +import { + ACCESS_TOKEN_TTL_S, + CODE_CHALLENGE_RE, + CODE_VERIFIER_RE, + REFRESH_TOKEN_TTL_MS, + generateAccessToken, + generateRefreshToken, + hashToken, + timingSafeEqualHex, + type OAuthClientRow, + type OAuthTokenRow, +} from './oauth.helpers'; +import { AUTH_CODE_TTL_MS, putPendingCode, takePendingCode, type PendingCode } from './oauth.pending-codes'; + +export type { OAuthClientRow, OAuthTokenRow } from './oauth.helpers'; +export type { PendingCode } from './oauth.pending-codes'; + +export interface OAuthTokenInfo { + user: User; + scopes: string[]; + clientId: string; + audience: string | null; +} + +export interface AuthorizeParams { + response_type: string; + client_id: string; + redirect_uri: string; + scope: string; + state?: string; + code_challenge: string; + code_challenge_method: string; + resource?: string; +} + +export interface ValidateAuthorizeResult { + valid: boolean; + error?: string; + error_description?: string; + client?: { name: string; allowed_scopes: string[] }; + scopes?: string[]; + resource?: string | null; + /** true when user is logged in but consent UI must be shown */ + consentRequired?: boolean; + /** true when the request is valid but user is not authenticated */ + loginRequired?: boolean; + /** true when the client was registered via machine DCR — user may adjust scopes on the consent screen */ + scopeSelectable?: boolean; +} /** - * Thin Nest wrapper around the existing OAuth 2.1 service. The grant handling, - * PKCE, client auth, consent storage, token issue/refresh/revoke and the - * client/session CRUD all reuse the legacy code unchanged. + * OAuth 2.1 server for MCP: client management, the authorization-code grant + * with PKCE, consent storage, token issue/refresh/revoke with rotation and + * replay detection, and the client_credentials grant. + * + * The authorization-code store lives in oauth.pending-codes.ts rather than on + * this instance — the SDK-mounted /oauth/authorize path reaches it through + * oauth.bridge.ts, outside the container, and must see the same map. */ @Injectable() export class OauthService { - constructor(private readonly addons: AddonsService) {} + constructor( + private readonly db: DatabaseService, + private readonly addons: AddonsService, + private readonly audit: AuditService, + ) {} mcpEnabled(): boolean { return this.addons.isAddonEnabled(ADDON_IDS.MCP); } mcpSafeUrl(): string { return getMcpSafeUrl(); } - consumeAuthCode(code: string) { return oauth.consumeAuthCode(code); } - authenticateClient(clientId: string, clientSecret?: string) { return oauth.authenticateClient(clientId, clientSecret); } - verifyPKCE(verifier: string, challenge: string) { return oauth.verifyPKCE(verifier, challenge); } - issueTokens(...args: Parameters) { return oauth.issueTokens(...args); } - issueClientCredentialsToken(...args: Parameters) { return oauth.issueClientCredentialsToken(...args); } - refreshTokens(...args: Parameters) { return oauth.refreshTokens(...args); } - revokeToken(...args: Parameters) { return oauth.revokeToken(...args); } - getUserByAccessToken(token: string) { return oauth.getUserByAccessToken(token); } + // ------------------------------------------------------------------------- + // Client management (self-service, gated by MCP addon) + // ------------------------------------------------------------------------- - validateAuthorizeRequest(params: oauth.AuthorizeParams, userId: number | null) { return oauth.validateAuthorizeRequest(params, userId); } - saveConsent(...args: Parameters) { return oauth.saveConsent(...args); } - createAuthCode(...args: Parameters) { return oauth.createAuthCode(...args); } + listOAuthClients(userId: number): Record[] { + const rows = this.db.all( + 'SELECT id, user_id, name, client_id, redirect_uris, allowed_scopes, created_at, is_public, created_via, allows_client_credentials FROM oauth_clients WHERE user_id = ? ORDER BY created_at DESC', + userId, + ); + return rows.map(r => ({ + ...r, + is_public: Boolean(r.is_public), + allows_client_credentials: Boolean(r.allows_client_credentials), + redirect_uris: JSON.parse(r.redirect_uris), + allowed_scopes: JSON.parse(r.allowed_scopes), + })); + } - listOAuthClients(userId: number) { return oauth.listOAuthClients(userId); } - createOAuthClient(...args: Parameters) { return oauth.createOAuthClient(...args); } - rotateOAuthClientSecret(userId: number, id: string, ip: string | undefined) { return oauth.rotateOAuthClientSecret(userId, id, ip); } - deleteOAuthClient(userId: number, id: string, ip: string | undefined) { return oauth.deleteOAuthClient(userId, id, ip); } - listOAuthSessions(userId: number) { return oauth.listOAuthSessions(userId); } - revokeSession(userId: number, id: number, ip: string | undefined) { return oauth.revokeSession(userId, id, ip); } + createOAuthClient( + userId: number | null, + name: string, + redirectUris: string[], + allowedScopes: string[], + ip?: string | null, + options?: { isPublic?: boolean; createdVia?: string; allowsClientCredentials?: boolean }, + ): { error?: string; status?: number; client?: Record } { + if (!name?.trim()) return { error: 'Name is required', status: 400 }; + if (name.trim().length > 100) return { error: 'Name must be 100 characters or less', status: 400 }; + const isMachineClient = Boolean(options?.allowsClientCredentials); + if (!isMachineClient && (!redirectUris || redirectUris.length === 0)) return { error: 'At least one redirect URI is required', status: 400 }; + if (redirectUris.length > 10) return { error: 'Maximum 10 redirect URIs per client', status: 400 }; + + for (const uri of redirectUris) { + let parsed: URL; + try { + parsed = new URL(uri); + } catch { + return { error: `Invalid redirect URI: ${uri}`, status: 400 }; + } + if (parsed.protocol !== 'https:' && parsed.hostname !== 'localhost' && parsed.hostname !== '127.0.0.1') { + return { error: `Redirect URI must use HTTPS (localhost exempt): ${uri}`, status: 400 }; + } + } + + if (!allowedScopes || allowedScopes.length === 0) return { error: 'At least one scope is required', status: 400 }; + const { valid, invalid } = validateScopes(allowedScopes); + if (!valid) return { error: `Invalid scopes: ${invalid.join(', ')}`, status: 400 }; + + if (userId !== null) { + const count = this.db.get<{ count: number }>('SELECT COUNT(*) as count FROM oauth_clients WHERE user_id = ?', userId)!.count; + if (count >= 10) return { error: 'Maximum of 10 OAuth clients per user', status: 400 }; + } else { + // Anonymous DCR clients: enforce a global cap to prevent unbounded registration abuse + const count = this.db.get<{ count: number }>('SELECT COUNT(*) as count FROM oauth_clients WHERE user_id IS NULL')!.count; + if (count >= 500) return { error: 'server_error', status: 503 }; + } + + // Machine clients (client_credentials) must always be confidential — ignore isPublic for them. + const isPublic = isMachineClient ? false : (options?.isPublic ?? false); + const createdVia = options?.createdVia ?? 'settings_ui'; + const id = randomUUID(); + const clientId = randomUUID(); + // Public clients have no usable secret; store an opaque random value to satisfy NOT NULL. + const rawSecret = isPublic ? null : 'trekcs_' + randomBytes(24).toString('hex'); + const secretHash = rawSecret ? hashToken(rawSecret) : randomBytes(32).toString('hex'); + + this.db.run( + 'INSERT INTO oauth_clients (id, user_id, name, client_id, client_secret_hash, redirect_uris, allowed_scopes, is_public, created_via, allows_client_credentials) VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?)', + id, userId, name.trim(), clientId, secretHash, JSON.stringify(redirectUris), JSON.stringify(allowedScopes), isPublic ? 1 : 0, createdVia, isMachineClient ? 1 : 0, + ); + + const row = this.db.get( + 'SELECT id, user_id, name, client_id, redirect_uris, allowed_scopes, created_at, is_public, created_via, allows_client_credentials FROM oauth_clients WHERE id = ?', + id, + )!; + + this.audit.writeAudit({ userId, action: 'oauth.client.create', details: { client_id: clientId, name: name.trim(), is_public: isPublic, allows_client_credentials: isMachineClient }, ip }); + + return { + client: { + id: row.id, + user_id: row.user_id, + name: row.name, + client_id: row.client_id, + redirect_uris: JSON.parse(row.redirect_uris), + allowed_scopes: JSON.parse(row.allowed_scopes), + created_at: row.created_at, + is_public: Boolean(row.is_public), + allows_client_credentials: Boolean(row.allows_client_credentials), + created_via: row.created_via, + // client_secret only present for confidential clients — shown once, not stored in plain text + ...(rawSecret ? { client_secret: rawSecret } : {}), + }, + }; + } + + rotateOAuthClientSecret( + userId: number, + clientRowId: string, + ip?: string | null, + ): { error?: string; status?: number; client_secret?: string } { + const row = this.db.get('SELECT id, client_id, is_public FROM oauth_clients WHERE id = ? AND user_id = ?', clientRowId, userId); + if (!row) return { error: 'Client not found', status: 404 }; + if (row.is_public) return { error: 'Public clients do not use a client secret', status: 400 }; + + const rawSecret = 'trekcs_' + randomBytes(24).toString('hex'); + const secretHash = hashToken(rawSecret); + + this.db.run('UPDATE oauth_clients SET client_secret_hash = ? WHERE id = ?', secretHash, clientRowId); + + // Revoke all existing tokens for this client so old sessions are invalidated + this.db.run("UPDATE oauth_tokens SET revoked_at = datetime('now') WHERE client_id = ? AND revoked_at IS NULL", row.client_id); + + // Terminate active MCP sessions for this (user, client) pair + revokeUserSessionsForClient(userId, row.client_id); + + this.audit.writeAudit({ userId, action: 'oauth.client.rotate_secret', details: { client_id: row.client_id }, ip }); + + return { client_secret: rawSecret }; + } + + deleteOAuthClient( + userId: number, + clientRowId: string, + ip?: string | null, + ): { error?: string; status?: number; success?: boolean } { + const row = this.db.get('SELECT id, client_id FROM oauth_clients WHERE id = ? AND user_id = ?', clientRowId, userId); + if (!row) return { error: 'Client not found', status: 404 }; + this.db.run('DELETE FROM oauth_clients WHERE id = ?', clientRowId); + this.audit.writeAudit({ userId, action: 'oauth.client.delete', details: { client_id: row.client_id }, ip }); + return { success: true }; + } + + // ------------------------------------------------------------------------- + // Auth code (in-memory, 2-minute TTL) + // ------------------------------------------------------------------------- + + createAuthCode(params: { + clientId: string; + userId: number; + redirectUri: string; + scopes: string[]; + resource: string | null; + codeChallenge: string; + codeChallengeMethod: 'S256'; + }): string | null { + const rawCode = randomBytes(32).toString('hex'); + const stored = putPendingCode(rawCode, { ...params, expiresAt: Date.now() + AUTH_CODE_TTL_MS }); + return stored ? rawCode : null; + } + + consumeAuthCode(code: string): PendingCode | null { + return takePendingCode(code); + } + + // ------------------------------------------------------------------------- + // Consent management + // ------------------------------------------------------------------------- + + getConsent(clientId: string, userId: number): string[] | null { + const row = this.db.get<{ scopes: string }>( + 'SELECT scopes FROM oauth_consents WHERE client_id = ? AND user_id = ?', clientId, userId, + ); + return row ? JSON.parse(row.scopes) : null; + } + + saveConsent(clientId: string, userId: number, scopes: string[], ip?: string | null): void { + // Union existing consent with newly approved scopes (M5: never narrow stored consent) + const existing = this.getConsent(clientId, userId) ?? []; + const merged = Array.from(new Set([...existing, ...scopes])); + this.db.run( + 'INSERT OR REPLACE INTO oauth_consents (client_id, user_id, scopes, updated_at) VALUES (?, ?, ?, CURRENT_TIMESTAMP)', + clientId, userId, JSON.stringify(merged), + ); + this.audit.writeAudit({ userId, action: 'oauth.consent.grant', details: { client_id: clientId, scopes: merged }, ip }); + } + + isConsentSufficient(existingScopes: string[], requestedScopes: string[]): boolean { + return requestedScopes.every(s => existingScopes.includes(s)); + } + + // ------------------------------------------------------------------------- + // Token issuance + // ------------------------------------------------------------------------- + + issueTokens( + clientId: string, + userId: number, + scopes: string[], + parentTokenId: number | null = null, + audience: string | null = null, + ): { + access_token: string; + refresh_token: string; + token_type: 'Bearer'; + expires_in: number; + scope: string; + } { + const rawAccess = generateAccessToken(); + const rawRefresh = generateRefreshToken(); + const accessHash = hashToken(rawAccess); + const refreshHash = hashToken(rawRefresh); + + const now = new Date(); + const accessExpiry = new Date(now.getTime() + ACCESS_TOKEN_TTL_S * 1000); + const refreshExpiry = new Date(now.getTime() + REFRESH_TOKEN_TTL_MS); + + this.db.run(` + INSERT INTO oauth_tokens + (client_id, user_id, access_token_hash, refresh_token_hash, scopes, audience, access_token_expires_at, refresh_token_expires_at, parent_token_id) + VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?) + `, clientId, userId, accessHash, refreshHash, JSON.stringify(scopes), audience, accessExpiry.toISOString(), refreshExpiry.toISOString(), parentTokenId); + + return { + access_token: rawAccess, + refresh_token: rawRefresh, + token_type: 'Bearer', + expires_in: ACCESS_TOKEN_TTL_S, + scope: scopes.join(' '), + }; + } + + /** + * Issues an access token only — no refresh token (RFC 6749 §4.4.3). + * Used exclusively for the client_credentials grant. A random opaque hash is + * stored in refresh_token_hash to satisfy the NOT NULL/UNIQUE constraint; it + * can never be presented as a valid refresh token (same precedent as public + * client secret hashes stored in client_secret_hash). + */ + issueClientCredentialsToken( + clientId: string, + userId: number, + scopes: string[], + audience: string, + ): { + access_token: string; + token_type: 'Bearer'; + expires_in: number; + scope: string; + } { + const rawAccess = generateAccessToken(); + const accessHash = hashToken(rawAccess); + const placeholderHash = randomBytes(32).toString('hex'); + + const now = new Date(); + const accessExpiry = new Date(now.getTime() + ACCESS_TOKEN_TTL_S * 1000); + + this.db.run(` + INSERT INTO oauth_tokens + (client_id, user_id, access_token_hash, refresh_token_hash, scopes, audience, access_token_expires_at, refresh_token_expires_at, parent_token_id) + VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?) + `, clientId, userId, accessHash, placeholderHash, JSON.stringify(scopes), audience, accessExpiry.toISOString(), now.toISOString(), null); + + return { + access_token: rawAccess, + token_type: 'Bearer', + expires_in: ACCESS_TOKEN_TTL_S, + scope: scopes.join(' '), + }; + } + + // ------------------------------------------------------------------------- + // Token verification (used by MCP handler on every request) + // ------------------------------------------------------------------------- + + getUserByAccessToken(rawToken: string): OAuthTokenInfo | null { + const hash = hashToken(rawToken); + const row = this.db.get(` + SELECT ot.scopes, ot.audience, ot.revoked_at, ot.access_token_expires_at, + ot.user_id, ot.client_id, u.username, u.email, u.role + FROM oauth_tokens ot + JOIN users u ON ot.user_id = u.id + WHERE ot.access_token_hash = ? + `, hash); + + if (!row) return null; + if (row.revoked_at) return null; + if (new Date(row.access_token_expires_at) < new Date()) return null; + + return { + user: { id: row.user_id, username: row.username, email: row.email, role: row.role as 'admin' | 'user' }, + scopes: JSON.parse(row.scopes), + clientId: row.client_id, + audience: row.audience ?? null, + }; + } + + // ------------------------------------------------------------------------- + // Token refresh (rotation + replay detection) + // ------------------------------------------------------------------------- + + /** Walk parent_token_id upward to find the root token id of this rotation chain. */ + private findChainRoot(tokenId: number): number { + let current = tokenId; + for (let i = 0; i < 100; i++) { + const row = this.db.get<{ id: number; parent_token_id: number | null }>('SELECT id, parent_token_id FROM oauth_tokens WHERE id = ?', current); + if (!row || row.parent_token_id === null) return current; + current = row.parent_token_id; + } + return current; + } + + /** Revoke all tokens in the rotation chain rooted at rootId. Returns affected ids. */ + private revokeChain(rootId: number): number[] { + const rows = this.db.all<{ id: number }>(` + WITH RECURSIVE chain(id) AS ( + SELECT id FROM oauth_tokens WHERE id = ? + UNION ALL + SELECT t.id FROM oauth_tokens t JOIN chain c ON t.parent_token_id = c.id + ) + SELECT id FROM chain + `, rootId); + const ids = rows.map(r => r.id); + if (ids.length > 0) { + this.db.run( + `UPDATE oauth_tokens SET revoked_at = CURRENT_TIMESTAMP WHERE id IN (${ids.map(() => '?').join(',')}) AND revoked_at IS NULL`, + ...ids, + ); + } + return ids; + } + + refreshTokens( + rawRefreshToken: string, + clientId: string, + clientSecret: string | undefined, + ip?: string | null, + ): { error?: string; status?: number; tokens?: ReturnType } { + const client = this.db.get('SELECT client_id, client_secret_hash, is_public FROM oauth_clients WHERE client_id = ?', clientId); + if (!client) return { error: 'invalid_client', status: 401 }; + if (!client.is_public) { + if (!clientSecret || !timingSafeEqualHex(hashToken(clientSecret), client.client_secret_hash)) { + return { error: 'invalid_client', status: 401 }; + } + } + + const hash = hashToken(rawRefreshToken); + const row = this.db.get(` + SELECT id, client_id, user_id, scopes, audience, refresh_token_expires_at, revoked_at, parent_token_id + FROM oauth_tokens WHERE refresh_token_hash = ? + `, hash); + + if (!row) return { error: 'invalid_grant', status: 400 }; + if (row.client_id !== clientId) return { error: 'invalid_grant', status: 400 }; + + // ---- Replay detection (C3) ---- + if (row.revoked_at) { + // A revoked refresh token was replayed — assume token theft. Cascade-revoke the chain. + const rootId = this.findChainRoot(row.id); + this.revokeChain(rootId); + + revokeUserSessionsForClient(row.user_id, clientId); + + this.audit.writeAudit({ + userId: row.user_id, + action: 'oauth.token.replay_detected', + details: { client_id: clientId }, + ip, + }); + logWarn(`[OAuth] Refresh token replay detected for user=${row.user_id} client=${clientId} ip=${ip ?? '-'}`); + + return { error: 'invalid_grant', status: 400 }; + } + + if (new Date(row.refresh_token_expires_at) < new Date()) return { error: 'invalid_grant', status: 400 }; + + // Revoke old pair immediately (rotation) and issue new pair linked to old row. + // Do NOT revoke active MCP sessions here: a legitimate refresh isn't a security + // event (that's handled above, in the replay-detection branch), and mcpHandler + // already re-validates session.userId/clientId against the new token on every + // request. Killing the session on every routine hourly refresh broke long-lived + // MCP connections (#1475). + this.db.run('UPDATE oauth_tokens SET revoked_at = CURRENT_TIMESTAMP WHERE id = ?', row.id); + + const tokens = this.issueTokens(clientId, row.user_id, JSON.parse(row.scopes), row.id, row.audience ?? null); + this.audit.writeAudit({ userId: row.user_id, action: 'oauth.token.refresh', details: { client_id: clientId }, ip }); + + return { tokens }; + } + + // ------------------------------------------------------------------------- + // Token revocation + // ------------------------------------------------------------------------- + + revokeToken(rawToken: string, clientId: string, userId?: number, ip?: string | null): void { + const hash = hashToken(rawToken); + + // Get the user_id for the token so we can revoke its MCP sessions + const row = this.db.get<{ user_id: number }>( + 'SELECT user_id FROM oauth_tokens WHERE (access_token_hash = ? OR refresh_token_hash = ?) AND client_id = ?', + hash, hash, clientId, + ); + + this.db.run(` + UPDATE oauth_tokens + SET revoked_at = CURRENT_TIMESTAMP + WHERE (access_token_hash = ? OR refresh_token_hash = ?) AND client_id = ? + `, hash, hash, clientId); + + const affectedUserId = row?.user_id ?? userId; + if (affectedUserId) { + revokeUserSessionsForClient(affectedUserId, clientId); + this.audit.writeAudit({ userId: affectedUserId, action: 'oauth.token.revoke', details: { client_id: clientId, method: 'token' }, ip }); + } + } + + // ------------------------------------------------------------------------- + // Active session listing (for user settings page) + // ------------------------------------------------------------------------- + + listOAuthSessions(userId: number): Record[] { + const rows = this.db.all>(` + SELECT ot.id, ot.client_id, oc.name AS client_name, ot.scopes, + ot.access_token_expires_at, ot.refresh_token_expires_at, ot.created_at + FROM oauth_tokens ot + JOIN oauth_clients oc ON ot.client_id = oc.client_id + WHERE ot.user_id = ? + AND ot.revoked_at IS NULL + AND ot.refresh_token_expires_at > CURRENT_TIMESTAMP + ORDER BY ot.created_at DESC + `, userId); + return rows.map(r => ({ ...r, scopes: JSON.parse(r.scopes as string) })); + } + + revokeSession( + userId: number, + sessionId: number, + ip?: string | null, + ): { error?: string; status?: number; success?: boolean } { + const row = this.db.get<{ id: number; client_id: string }>('SELECT id, client_id FROM oauth_tokens WHERE id = ? AND user_id = ?', sessionId, userId); + if (!row) return { error: 'Session not found', status: 404 }; + + this.db.run('UPDATE oauth_tokens SET revoked_at = CURRENT_TIMESTAMP WHERE id = ?', sessionId); + + revokeUserSessionsForClient(userId, row.client_id); + + this.audit.writeAudit({ userId, action: 'oauth.token.revoke', details: { client_id: row.client_id, method: 'session' }, ip }); + + return { success: true }; + } + + // ------------------------------------------------------------------------- + // Authorize request validation (option A: called by SPA via GET /api/oauth/authorize/validate) + // ------------------------------------------------------------------------- + + validateAuthorizeRequest( + params: AuthorizeParams, + userId: number | null, + ): ValidateAuthorizeResult { + if (!this.addons.isAddonEnabled(ADDON_IDS.MCP)) { + return { valid: false, error: 'mcp_disabled', error_description: 'MCP is not enabled on this server' }; + } + + if (params.response_type !== 'code') { + return { valid: false, error: 'unsupported_response_type', error_description: 'Only response_type=code is supported' }; + } + + if (!params.code_challenge || params.code_challenge_method !== 'S256') { + return { valid: false, error: 'invalid_request', error_description: 'PKCE with code_challenge_method=S256 is required (OAuth 2.1)' }; + } + + // H1: Enforce code_challenge format (RFC 7636 §4.2) + if (!CODE_CHALLENGE_RE.test(params.code_challenge)) { + return { valid: false, error: 'invalid_request', error_description: 'code_challenge must be 43 base64url characters (S256)' }; + } + + if (!params.client_id) { + return { valid: false, error: 'invalid_request', error_description: 'client_id is required' }; + } + + const client = this.db.get('SELECT * FROM oauth_clients WHERE client_id = ?', params.client_id); + if (!client) { + return { valid: false, error: 'invalid_client', error_description: 'Unknown client_id' }; + } + + const allowedUris: string[] = JSON.parse(client.redirect_uris); + if (!params.redirect_uri || !allowedUris.includes(params.redirect_uri)) { + return { valid: false, error: 'invalid_redirect_uri', error_description: 'redirect_uri does not match any registered URI' }; + } + + // RFC 8707 resource indicator: if provided, must identify the TREK + // MCP endpoint exactly. If the client didn't supply `resource`, we + // bind the token to the MCP endpoint by default — previously this + // left `audience = null`, and the audience-bind check on MCP requests + // then treated a null audience as "valid for any resource". + const mcpResource = `${getMcpSafeUrl().replace(/\/+$/, '')}/mcp`; + const resource = params.resource + ? params.resource.replace(/\/+$/, '') + : mcpResource; + if (resource !== mcpResource) { + return { valid: false, error: 'invalid_target', error_description: 'Requested resource must be the TREK MCP endpoint' }; + } + + const requestedScopes = (params.scope || '').split(' ').filter(Boolean); + if (requestedScopes.length === 0) { + return { valid: false, error: 'invalid_scope', error_description: 'At least one scope is required' }; + } + + const allowedScopes: string[] = JSON.parse(client.allowed_scopes); + // Narrow to the intersection: drop scopes the client isn't permitted for rather + // than rejecting the whole request (per OAuth 2.0 §3.3 scope narrowing). + const grantedScopes = requestedScopes.filter(s => allowedScopes.includes(s)); + if (grantedScopes.length === 0) { + return { valid: false, error: 'invalid_scope', error_description: 'None of the requested scopes are permitted for this client' }; + } + + if (userId === null) { + // H3: return only the minimum required fields — do NOT expose scopes, client.name, or + // allowed_scopes to unauthenticated callers to prevent client enumeration. + return { valid: true, loginRequired: true }; + } + + const existingConsent = this.getConsent(params.client_id, userId); + const consentRequired = !existingConsent || !this.isConsentSufficient(existingConsent, grantedScopes); + + return { + valid: true, + client: { name: client.name, allowed_scopes: allowedScopes }, + scopes: grantedScopes, + resource: resource ?? mcpResource, + consentRequired, + scopeSelectable: client.created_via === 'dcr', + }; + } + + // ------------------------------------------------------------------------- + // PKCE verification + // ------------------------------------------------------------------------- + + verifyPKCE(codeVerifier: string, codeChallenge: string): boolean { + // H1: validate code_verifier format before hashing + if (!CODE_VERIFIER_RE.test(codeVerifier)) return false; + + const expected = crypto.createHash('sha256').update(codeVerifier).digest('base64url'); + // Constant-time compare (both are base64url strings of equal length for S256) + if (expected.length !== codeChallenge.length) return false; + try { + return crypto.timingSafeEqual(Buffer.from(expected), Buffer.from(codeChallenge)); + } catch { return false; } + } + + // ------------------------------------------------------------------------- + // Client authentication (for token endpoint) + // ------------------------------------------------------------------------- + + authenticateClient(clientId: string, clientSecret: string | undefined): OAuthClientRow | null { + const client = this.db.get('SELECT * FROM oauth_clients WHERE client_id = ?', clientId); + if (!client) return null; + if (client.is_public) { + // Public clients are identified by client_id alone — PKCE provides the security guarantee. + return client; + } + // H4: constant-time comparison to prevent timing side-channel + if (!clientSecret) return null; + if (!timingSafeEqualHex(hashToken(clientSecret), client.client_secret_hash)) return null; + return client; + } } diff --git a/server/src/nest/trips/trips.bridge.ts b/server/src/nest/trips/trips.bridge.ts index 254cc1730..9dad573d6 100644 --- a/server/src/nest/trips/trips.bridge.ts +++ b/server/src/nest/trips/trips.bridge.ts @@ -18,6 +18,7 @@ import { CollabService } from '../collab/collab.service'; import { VacayService } from '../vacay/vacay.service'; import { PlacesService } from '../places/places.service'; import { MapsService } from '../maps/maps.service'; +import { UserCleanupService } from '../auth/user-cleanup.service'; /** * Non-Nest entry point for the trip domain — for the two consumers that cannot @@ -53,6 +54,7 @@ const trips = new TripsService( new RealtimeService(), new PlacesService(dbs(), new PermissionsService(dbs()), new RealtimeService(), new MapsService(dbs(), photoCache), new QueryHelpersService(dbs()), new UnsplashService(dbs(), new RuntimeEnvService()), photoCache), new UnsplashService(dbs(), new RuntimeEnvService()), + new UserCleanupService(dbs()), ); export function getTripOwner(tripId: string | number) { diff --git a/server/src/nest/trips/trips.service.ts b/server/src/nest/trips/trips.service.ts index f24d3c0cc..7cb89218c 100644 --- a/server/src/nest/trips/trips.service.ts +++ b/server/src/nest/trips/trips.service.ts @@ -8,7 +8,7 @@ import { RealtimeService } from '../realtime/realtime.service'; import { PermissionsService } from '../permissions/permissions.service'; import type { Trip, User } from '../../types'; import { avatarUrl } from '../common/avatarUrl'; -import { erasePluginUserData } from '../../services/userCleanupService'; +import { UserCleanupService } from '../auth/user-cleanup.service'; import { emitUserDeleted } from '../../plugin-user-lifecycle'; import { resolveTimeZone } from '../common/timezoneService'; import { PlacesService } from '../places/places.service'; @@ -228,6 +228,7 @@ export class TripsService { private readonly realtime: RealtimeService, private readonly places: PlacesService, private readonly unsplash: UnsplashService, + private readonly userCleanup: UserCleanupService, ) {} private get db() { @@ -897,7 +898,7 @@ export class TripsService { // A guest is still a user id a plugin may hold data for, so erase that too — the // host-side per-user tables + a durable own-db erasure per granted plugin — exactly // like a full account deletion (otherwise a deleted guest's plugin data lingers). - erasePluginUserData(guestUserId); + this.userCleanup.erasePluginUserData(guestUserId); // Quirk fix on top of the 1:1 move: the budget re-split and the user delete // run in one transaction, so a failure mid-flow can't leave the expense // divisors re-derived for a guest that still exists (or vice versa). The diff --git a/server/src/services/oauthService.ts b/server/src/services/oauthService.ts deleted file mode 100644 index d313e5710..000000000 --- a/server/src/services/oauthService.ts +++ /dev/null @@ -1,694 +0,0 @@ -import crypto, { randomBytes, createHash, randomUUID } from 'crypto'; -import { db } from '../db/database'; -import { isAddonEnabled } from '../nest/addons/addons.bridge'; -import { validateScopes } from '../mcp/scopes'; -import { ADDON_IDS } from '../addons'; -import { User } from '../types'; -import { writeAudit, logWarn } from '../nest/audit/audit.bridge'; -import { revokeUserSessionsForClient } from '../mcp/sessionManager'; -import { getMcpSafeUrl } from '../app-config'; - -// --------------------------------------------------------------------------- -// Constants -// --------------------------------------------------------------------------- - -const ACCESS_TOKEN_TTL_S = 60 * 60; // 1 hour -const REFRESH_TOKEN_TTL_MS = 30 * 24 * 60 * 60 * 1000; // 30 days rolling -const AUTH_CODE_TTL_MS = 2 * 60 * 1000; // 2 minutes - -// PKCE format (RFC 7636) -const CODE_CHALLENGE_RE = /^[A-Za-z0-9_-]{43}$/; -const CODE_VERIFIER_RE = /^[A-Za-z0-9\-._~]{43,128}$/; - -// --------------------------------------------------------------------------- -// In-memory auth code store (short-lived, no need for DB persistence) -// --------------------------------------------------------------------------- - -interface PendingCode { - clientId: string; - userId: number; - redirectUri: string; - scopes: string[]; - resource: string | null; - codeChallenge: string; - codeChallengeMethod: 'S256'; - expiresAt: number; -} - -const MAX_PENDING_CODES = 500; -const pendingCodes = new Map(); - -setInterval(() => { - const now = Date.now(); - for (const [key, entry] of pendingCodes) { - if (now > entry.expiresAt) pendingCodes.delete(key); - } -}, 60_000).unref(); - -// --------------------------------------------------------------------------- -// DB row types -// --------------------------------------------------------------------------- - -interface OAuthClientRow { - id: string; - user_id: number; - name: string; - client_id: string; - client_secret_hash: string; - redirect_uris: string; // JSON array - allowed_scopes: string; // JSON array - created_at: string; - is_public: number; // 0 | 1 (SQLite boolean) - created_via: string; // 'settings_ui' | 'browser-registration' - allows_client_credentials: number; // 0 | 1 -} - -interface OAuthTokenRow { - id: number; - client_id: string; - user_id: number; - access_token_hash: string; - refresh_token_hash: string; - scopes: string; // JSON array - audience: string | null; - access_token_expires_at: string; - refresh_token_expires_at: string; - revoked_at: string | null; - parent_token_id: number | null; -} - -// --------------------------------------------------------------------------- -// Token helpers -// --------------------------------------------------------------------------- - -function hashToken(raw: string): string { - return createHash('sha256').update(raw).digest('hex'); -} - -/** Constant-time comparison of two hex-encoded SHA-256 hashes. */ -function timingSafeEqualHex(a: string, b: string): boolean { - if (a.length !== b.length) return false; - try { - return crypto.timingSafeEqual(Buffer.from(a, 'hex'), Buffer.from(b, 'hex')); - } catch { return false; } -} - -function generateAccessToken(): string { - return 'trekoa_' + randomBytes(32).toString('hex'); -} - -function generateRefreshToken(): string { - return 'trekrf_' + randomBytes(32).toString('hex'); -} - -// --------------------------------------------------------------------------- -// Client management (self-service, gated by MCP addon) -// --------------------------------------------------------------------------- - -export function listOAuthClients(userId: number): Record[] { - const rows = db.prepare( - 'SELECT id, user_id, name, client_id, redirect_uris, allowed_scopes, created_at, is_public, created_via, allows_client_credentials FROM oauth_clients WHERE user_id = ? ORDER BY created_at DESC' - ).all(userId) as OAuthClientRow[]; - return rows.map(r => ({ - ...r, - is_public: Boolean(r.is_public), - allows_client_credentials: Boolean(r.allows_client_credentials), - redirect_uris: JSON.parse(r.redirect_uris), - allowed_scopes: JSON.parse(r.allowed_scopes), - })); -} - -export function createOAuthClient( - userId: number | null, - name: string, - redirectUris: string[], - allowedScopes: string[], - ip?: string | null, - options?: { isPublic?: boolean; createdVia?: string; allowsClientCredentials?: boolean }, -): { error?: string; status?: number; client?: Record } { - if (!name?.trim()) return { error: 'Name is required', status: 400 }; - if (name.trim().length > 100) return { error: 'Name must be 100 characters or less', status: 400 }; - const isMachineClient = Boolean(options?.allowsClientCredentials); - if (!isMachineClient && (!redirectUris || redirectUris.length === 0)) return { error: 'At least one redirect URI is required', status: 400 }; - if (redirectUris.length > 10) return { error: 'Maximum 10 redirect URIs per client', status: 400 }; - - for (const uri of redirectUris) { - let parsed: URL; - try { - parsed = new URL(uri); - } catch { - return { error: `Invalid redirect URI: ${uri}`, status: 400 }; - } - if (parsed.protocol !== 'https:' && parsed.hostname !== 'localhost' && parsed.hostname !== '127.0.0.1') { - return { error: `Redirect URI must use HTTPS (localhost exempt): ${uri}`, status: 400 }; - } - } - - if (!allowedScopes || allowedScopes.length === 0) return { error: 'At least one scope is required', status: 400 }; - const { valid, invalid } = validateScopes(allowedScopes); - if (!valid) return { error: `Invalid scopes: ${invalid.join(', ')}`, status: 400 }; - - if (userId !== null) { - const count = (db.prepare('SELECT COUNT(*) as count FROM oauth_clients WHERE user_id = ?').get(userId) as { count: number }).count; - if (count >= 10) return { error: 'Maximum of 10 OAuth clients per user', status: 400 }; - } else { - // Anonymous DCR clients: enforce a global cap to prevent unbounded registration abuse - const count = (db.prepare('SELECT COUNT(*) as count FROM oauth_clients WHERE user_id IS NULL').get() as { count: number }).count; - if (count >= 500) return { error: 'server_error', status: 503 }; - } - - // Machine clients (client_credentials) must always be confidential — ignore isPublic for them. - const isPublic = isMachineClient ? false : (options?.isPublic ?? false); - const createdVia = options?.createdVia ?? 'settings_ui'; - const id = randomUUID(); - const clientId = randomUUID(); - // Public clients have no usable secret; store an opaque random value to satisfy NOT NULL. - const rawSecret = isPublic ? null : 'trekcs_' + randomBytes(24).toString('hex'); - const secretHash = rawSecret ? hashToken(rawSecret) : randomBytes(32).toString('hex'); - - db.prepare( - 'INSERT INTO oauth_clients (id, user_id, name, client_id, client_secret_hash, redirect_uris, allowed_scopes, is_public, created_via, allows_client_credentials) VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?)' - ).run(id, userId, name.trim(), clientId, secretHash, JSON.stringify(redirectUris), JSON.stringify(allowedScopes), isPublic ? 1 : 0, createdVia, isMachineClient ? 1 : 0); - - const row = db.prepare( - 'SELECT id, user_id, name, client_id, redirect_uris, allowed_scopes, created_at, is_public, created_via, allows_client_credentials FROM oauth_clients WHERE id = ?' - ).get(id) as OAuthClientRow; - - writeAudit({ userId, action: 'oauth.client.create', details: { client_id: clientId, name: name.trim(), is_public: isPublic, allows_client_credentials: isMachineClient }, ip }); - - return { - client: { - id: row.id, - user_id: row.user_id, - name: row.name, - client_id: row.client_id, - redirect_uris: JSON.parse(row.redirect_uris), - allowed_scopes: JSON.parse(row.allowed_scopes), - created_at: row.created_at, - is_public: Boolean(row.is_public), - allows_client_credentials: Boolean(row.allows_client_credentials), - created_via: row.created_via, - // client_secret only present for confidential clients — shown once, not stored in plain text - ...(rawSecret ? { client_secret: rawSecret } : {}), - }, - }; -} - -export function rotateOAuthClientSecret( - userId: number, - clientRowId: string, - ip?: string | null, -): { error?: string; status?: number; client_secret?: string } { - const row = db.prepare('SELECT id, client_id, is_public FROM oauth_clients WHERE id = ? AND user_id = ?').get(clientRowId, userId) as OAuthClientRow | undefined; - if (!row) return { error: 'Client not found', status: 404 }; - if (row.is_public) return { error: 'Public clients do not use a client secret', status: 400 }; - - const rawSecret = 'trekcs_' + randomBytes(24).toString('hex'); - const secretHash = hashToken(rawSecret); - - db.prepare('UPDATE oauth_clients SET client_secret_hash = ? WHERE id = ?').run(secretHash, clientRowId); - - // Revoke all existing tokens for this client so old sessions are invalidated - db.prepare("UPDATE oauth_tokens SET revoked_at = datetime('now') WHERE client_id = ? AND revoked_at IS NULL").run(row.client_id); - - // Terminate active MCP sessions for this (user, client) pair - - revokeUserSessionsForClient(userId, row.client_id); - - writeAudit({ userId, action: 'oauth.client.rotate_secret', details: { client_id: row.client_id }, ip }); - - return { client_secret: rawSecret }; -} - -export function deleteOAuthClient( - userId: number, - clientRowId: string, - ip?: string | null, -): { error?: string; status?: number; success?: boolean } { - const row = db.prepare('SELECT id, client_id FROM oauth_clients WHERE id = ? AND user_id = ?').get(clientRowId, userId) as OAuthClientRow | undefined; - if (!row) return { error: 'Client not found', status: 404 }; - db.prepare('DELETE FROM oauth_clients WHERE id = ?').run(clientRowId); - writeAudit({ userId, action: 'oauth.client.delete', details: { client_id: row.client_id }, ip }); - return { success: true }; -} - -// --------------------------------------------------------------------------- -// Auth code (in-memory, 2-minute TTL) -// --------------------------------------------------------------------------- - -export function createAuthCode(params: { - clientId: string; - userId: number; - redirectUri: string; - scopes: string[]; - resource: string | null; - codeChallenge: string; - codeChallengeMethod: 'S256'; -}): string | null { - if (pendingCodes.size >= MAX_PENDING_CODES) return null; - const rawCode = randomBytes(32).toString('hex'); - pendingCodes.set(rawCode, { ...params, expiresAt: Date.now() + AUTH_CODE_TTL_MS }); - return rawCode; -} - -export function consumeAuthCode(code: string): PendingCode | null { - const entry = pendingCodes.get(code); - if (!entry) return null; - pendingCodes.delete(code); - if (Date.now() > entry.expiresAt) return null; - return entry; -} - -// --------------------------------------------------------------------------- -// Consent management -// --------------------------------------------------------------------------- - -export function getConsent(clientId: string, userId: number): string[] | null { - const row = db.prepare( - 'SELECT scopes FROM oauth_consents WHERE client_id = ? AND user_id = ?' - ).get(clientId, userId) as { scopes: string } | undefined; - return row ? JSON.parse(row.scopes) : null; -} - -export function saveConsent(clientId: string, userId: number, scopes: string[], ip?: string | null): void { - // Union existing consent with newly approved scopes (M5: never narrow stored consent) - const existing = getConsent(clientId, userId) ?? []; - const merged = Array.from(new Set([...existing, ...scopes])); - db.prepare( - 'INSERT OR REPLACE INTO oauth_consents (client_id, user_id, scopes, updated_at) VALUES (?, ?, ?, CURRENT_TIMESTAMP)' - ).run(clientId, userId, JSON.stringify(merged)); - writeAudit({ userId, action: 'oauth.consent.grant', details: { client_id: clientId, scopes: merged }, ip }); -} - -export function isConsentSufficient(existingScopes: string[], requestedScopes: string[]): boolean { - return requestedScopes.every(s => existingScopes.includes(s)); -} - -// --------------------------------------------------------------------------- -// Token issuance -// --------------------------------------------------------------------------- - -export function issueTokens( - clientId: string, - userId: number, - scopes: string[], - parentTokenId: number | null = null, - audience: string | null = null, -): { - access_token: string; - refresh_token: string; - token_type: 'Bearer'; - expires_in: number; - scope: string; -} { - const rawAccess = generateAccessToken(); - const rawRefresh = generateRefreshToken(); - const accessHash = hashToken(rawAccess); - const refreshHash = hashToken(rawRefresh); - - const now = new Date(); - const accessExpiry = new Date(now.getTime() + ACCESS_TOKEN_TTL_S * 1000); - const refreshExpiry = new Date(now.getTime() + REFRESH_TOKEN_TTL_MS); - - db.prepare(` - INSERT INTO oauth_tokens - (client_id, user_id, access_token_hash, refresh_token_hash, scopes, audience, access_token_expires_at, refresh_token_expires_at, parent_token_id) - VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?) - `).run(clientId, userId, accessHash, refreshHash, JSON.stringify(scopes), audience, accessExpiry.toISOString(), refreshExpiry.toISOString(), parentTokenId); - - return { - access_token: rawAccess, - refresh_token: rawRefresh, - token_type: 'Bearer', - expires_in: ACCESS_TOKEN_TTL_S, - scope: scopes.join(' '), - }; -} - -// Issues an access token only — no refresh token (RFC 6749 §4.4.3). -// Used exclusively for the client_credentials grant. A random opaque hash is -// stored in refresh_token_hash to satisfy the NOT NULL/UNIQUE constraint; it -// can never be presented as a valid refresh token (same precedent as public -// client secret hashes stored in client_secret_hash). -export function issueClientCredentialsToken( - clientId: string, - userId: number, - scopes: string[], - audience: string, -): { - access_token: string; - token_type: 'Bearer'; - expires_in: number; - scope: string; -} { - const rawAccess = generateAccessToken(); - const accessHash = hashToken(rawAccess); - const placeholderHash = randomBytes(32).toString('hex'); - - const now = new Date(); - const accessExpiry = new Date(now.getTime() + ACCESS_TOKEN_TTL_S * 1000); - - db.prepare(` - INSERT INTO oauth_tokens - (client_id, user_id, access_token_hash, refresh_token_hash, scopes, audience, access_token_expires_at, refresh_token_expires_at, parent_token_id) - VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?) - `).run(clientId, userId, accessHash, placeholderHash, JSON.stringify(scopes), audience, accessExpiry.toISOString(), now.toISOString(), null); - - return { - access_token: rawAccess, - token_type: 'Bearer', - expires_in: ACCESS_TOKEN_TTL_S, - scope: scopes.join(' '), - }; -} - -// --------------------------------------------------------------------------- -// Token verification (used by MCP handler on every request) -// --------------------------------------------------------------------------- - -export interface OAuthTokenInfo { - user: User; - scopes: string[]; - clientId: string; - audience: string | null; -} - -export function getUserByAccessToken(rawToken: string): OAuthTokenInfo | null { - const hash = hashToken(rawToken); - const row = db.prepare(` - SELECT ot.scopes, ot.audience, ot.revoked_at, ot.access_token_expires_at, - ot.user_id, ot.client_id, u.username, u.email, u.role - FROM oauth_tokens ot - JOIN users u ON ot.user_id = u.id - WHERE ot.access_token_hash = ? - `).get(hash) as (OAuthTokenRow & { username: string; email: string; role: string }) | undefined; - - if (!row) return null; - if (row.revoked_at) return null; - if (new Date(row.access_token_expires_at) < new Date()) return null; - - return { - user: { id: row.user_id, username: row.username, email: row.email, role: row.role as 'admin' | 'user' }, - scopes: JSON.parse(row.scopes), - clientId: row.client_id, - audience: row.audience ?? null, - }; -} - -// --------------------------------------------------------------------------- -// Token refresh (rotation + replay detection) -// --------------------------------------------------------------------------- - -/** Walk parent_token_id upward to find the root token id of this rotation chain. */ -function findChainRoot(tokenId: number): number { - let current = tokenId; - for (let i = 0; i < 100; i++) { - const row = db.prepare('SELECT id, parent_token_id FROM oauth_tokens WHERE id = ?').get(current) as { id: number; parent_token_id: number | null } | undefined; - if (!row || row.parent_token_id === null) return current; - current = row.parent_token_id; - } - return current; -} - -/** Revoke all tokens in the rotation chain rooted at rootId. Returns affected ids. */ -function revokeChain(rootId: number): number[] { - const rows = db.prepare(` - WITH RECURSIVE chain(id) AS ( - SELECT id FROM oauth_tokens WHERE id = ? - UNION ALL - SELECT t.id FROM oauth_tokens t JOIN chain c ON t.parent_token_id = c.id - ) - SELECT id FROM chain - `).all(rootId) as { id: number }[]; - const ids = rows.map(r => r.id); - if (ids.length > 0) { - db.prepare( - `UPDATE oauth_tokens SET revoked_at = CURRENT_TIMESTAMP WHERE id IN (${ids.map(() => '?').join(',')}) AND revoked_at IS NULL` - ).run(...ids); - } - return ids; -} - -export function refreshTokens( - rawRefreshToken: string, - clientId: string, - clientSecret: string | undefined, - ip?: string | null, -): { error?: string; status?: number; tokens?: ReturnType } { - const client = db.prepare('SELECT client_id, client_secret_hash, is_public FROM oauth_clients WHERE client_id = ?').get(clientId) as OAuthClientRow | undefined; - if (!client) return { error: 'invalid_client', status: 401 }; - if (!client.is_public) { - if (!clientSecret || !timingSafeEqualHex(hashToken(clientSecret), client.client_secret_hash)) { - return { error: 'invalid_client', status: 401 }; - } - } - - const hash = hashToken(rawRefreshToken); - const row = db.prepare(` - SELECT id, client_id, user_id, scopes, audience, refresh_token_expires_at, revoked_at, parent_token_id - FROM oauth_tokens WHERE refresh_token_hash = ? - `).get(hash) as OAuthTokenRow | undefined; - - if (!row) return { error: 'invalid_grant', status: 400 }; - if (row.client_id !== clientId) return { error: 'invalid_grant', status: 400 }; - - // ---- Replay detection (C3) ---- - if (row.revoked_at) { - // A revoked refresh token was replayed — assume token theft. Cascade-revoke the chain. - const rootId = findChainRoot(row.id); - revokeChain(rootId); - - - revokeUserSessionsForClient(row.user_id, clientId); - - writeAudit({ - userId: row.user_id, - action: 'oauth.token.replay_detected', - details: { client_id: clientId }, - ip, - }); - logWarn(`[OAuth] Refresh token replay detected for user=${row.user_id} client=${clientId} ip=${ip ?? '-'}`); - - return { error: 'invalid_grant', status: 400 }; - } - - if (new Date(row.refresh_token_expires_at) < new Date()) return { error: 'invalid_grant', status: 400 }; - - // Revoke old pair immediately (rotation) and issue new pair linked to old row. - // Do NOT revoke active MCP sessions here: a legitimate refresh isn't a security - // event (that's handled above, in the replay-detection branch), and mcpHandler - // already re-validates session.userId/clientId against the new token on every - // request. Killing the session on every routine hourly refresh broke long-lived - // MCP connections (#1475). - db.prepare('UPDATE oauth_tokens SET revoked_at = CURRENT_TIMESTAMP WHERE id = ?').run(row.id); - - const tokens = issueTokens(clientId, row.user_id, JSON.parse(row.scopes), row.id, row.audience ?? null); - writeAudit({ userId: row.user_id, action: 'oauth.token.refresh', details: { client_id: clientId }, ip }); - - return { tokens }; -} - -// --------------------------------------------------------------------------- -// Token revocation -// --------------------------------------------------------------------------- - -export function revokeToken(rawToken: string, clientId: string, userId?: number, ip?: string | null): void { - const hash = hashToken(rawToken); - - // Get the user_id for the token so we can revoke its MCP sessions - const row = db.prepare( - 'SELECT user_id FROM oauth_tokens WHERE (access_token_hash = ? OR refresh_token_hash = ?) AND client_id = ?' - ).get(hash, hash, clientId) as { user_id: number } | undefined; - - db.prepare(` - UPDATE oauth_tokens - SET revoked_at = CURRENT_TIMESTAMP - WHERE (access_token_hash = ? OR refresh_token_hash = ?) AND client_id = ? - `).run(hash, hash, clientId); - - const affectedUserId = row?.user_id ?? userId; - if (affectedUserId) { - - revokeUserSessionsForClient(affectedUserId, clientId); - writeAudit({ userId: affectedUserId, action: 'oauth.token.revoke', details: { client_id: clientId, method: 'token' }, ip }); - } -} - -// --------------------------------------------------------------------------- -// Active session listing (for user settings page) -// --------------------------------------------------------------------------- - -export function listOAuthSessions(userId: number): Record[] { - const rows = db.prepare(` - SELECT ot.id, ot.client_id, oc.name AS client_name, ot.scopes, - ot.access_token_expires_at, ot.refresh_token_expires_at, ot.created_at - FROM oauth_tokens ot - JOIN oauth_clients oc ON ot.client_id = oc.client_id - WHERE ot.user_id = ? - AND ot.revoked_at IS NULL - AND ot.refresh_token_expires_at > CURRENT_TIMESTAMP - ORDER BY ot.created_at DESC - `).all(userId) as Record[]; - return rows.map(r => ({ ...r, scopes: JSON.parse(r.scopes as string) })); -} - -export function revokeSession( - userId: number, - sessionId: number, - ip?: string | null, -): { error?: string; status?: number; success?: boolean } { - const row = db.prepare('SELECT id, client_id FROM oauth_tokens WHERE id = ? AND user_id = ?').get(sessionId, userId) as { id: number; client_id: string } | undefined; - if (!row) return { error: 'Session not found', status: 404 }; - - db.prepare('UPDATE oauth_tokens SET revoked_at = CURRENT_TIMESTAMP WHERE id = ?').run(sessionId); - - - revokeUserSessionsForClient(userId, row.client_id); - - writeAudit({ userId, action: 'oauth.token.revoke', details: { client_id: row.client_id, method: 'session' }, ip }); - - return { success: true }; -} - -// --------------------------------------------------------------------------- -// Authorize request validation (option A: called by SPA via GET /api/oauth/authorize/validate) -// --------------------------------------------------------------------------- - -export interface AuthorizeParams { - response_type: string; - client_id: string; - redirect_uri: string; - scope: string; - state?: string; - code_challenge: string; - code_challenge_method: string; - resource?: string; -} - -export interface ValidateAuthorizeResult { - valid: boolean; - error?: string; - error_description?: string; - client?: { name: string; allowed_scopes: string[] }; - scopes?: string[]; - resource?: string | null; - /** true when user is logged in but consent UI must be shown */ - consentRequired?: boolean; - /** true when the request is valid but user is not authenticated */ - loginRequired?: boolean; - /** true when the client was registered via machine DCR — user may adjust scopes on the consent screen */ - scopeSelectable?: boolean; -} - -export function validateAuthorizeRequest( - params: AuthorizeParams, - userId: number | null, -): ValidateAuthorizeResult { - if (!isAddonEnabled(ADDON_IDS.MCP)) { - return { valid: false, error: 'mcp_disabled', error_description: 'MCP is not enabled on this server' }; - } - - if (params.response_type !== 'code') { - return { valid: false, error: 'unsupported_response_type', error_description: 'Only response_type=code is supported' }; - } - - if (!params.code_challenge || params.code_challenge_method !== 'S256') { - return { valid: false, error: 'invalid_request', error_description: 'PKCE with code_challenge_method=S256 is required (OAuth 2.1)' }; - } - - // H1: Enforce code_challenge format (RFC 7636 §4.2) - if (!CODE_CHALLENGE_RE.test(params.code_challenge)) { - return { valid: false, error: 'invalid_request', error_description: 'code_challenge must be 43 base64url characters (S256)' }; - } - - if (!params.client_id) { - return { valid: false, error: 'invalid_request', error_description: 'client_id is required' }; - } - - const client = db.prepare('SELECT * FROM oauth_clients WHERE client_id = ?').get(params.client_id) as OAuthClientRow | undefined; - if (!client) { - return { valid: false, error: 'invalid_client', error_description: 'Unknown client_id' }; - } - - const allowedUris: string[] = JSON.parse(client.redirect_uris); - if (!params.redirect_uri || !allowedUris.includes(params.redirect_uri)) { - return { valid: false, error: 'invalid_redirect_uri', error_description: 'redirect_uri does not match any registered URI' }; - } - - // RFC 8707 resource indicator: if provided, must identify the TREK - // MCP endpoint exactly. If the client didn't supply `resource`, we - // bind the token to the MCP endpoint by default — previously this - // left `audience = null`, and the audience-bind check on MCP requests - // then treated a null audience as "valid for any resource". - const mcpResource = `${getMcpSafeUrl().replace(/\/+$/, '')}/mcp`; - const resource = params.resource - ? params.resource.replace(/\/+$/, '') - : mcpResource; - if (resource !== mcpResource) { - return { valid: false, error: 'invalid_target', error_description: 'Requested resource must be the TREK MCP endpoint' }; - } - - const requestedScopes = (params.scope || '').split(' ').filter(Boolean); - if (requestedScopes.length === 0) { - return { valid: false, error: 'invalid_scope', error_description: 'At least one scope is required' }; - } - - const allowedScopes: string[] = JSON.parse(client.allowed_scopes); - // Narrow to the intersection: drop scopes the client isn't permitted for rather - // than rejecting the whole request (per OAuth 2.0 §3.3 scope narrowing). - const grantedScopes = requestedScopes.filter(s => allowedScopes.includes(s)); - if (grantedScopes.length === 0) { - return { valid: false, error: 'invalid_scope', error_description: 'None of the requested scopes are permitted for this client' }; - } - - if (userId === null) { - // H3: return only the minimum required fields — do NOT expose scopes, client.name, or - // allowed_scopes to unauthenticated callers to prevent client enumeration. - return { valid: true, loginRequired: true }; - } - - const existingConsent = getConsent(params.client_id, userId); - const consentRequired = !existingConsent || !isConsentSufficient(existingConsent, grantedScopes); - - return { - valid: true, - client: { name: client.name, allowed_scopes: allowedScopes }, - scopes: grantedScopes, - resource: resource ?? mcpResource, - consentRequired, - scopeSelectable: client.created_via === 'dcr', - }; -} - -// --------------------------------------------------------------------------- -// PKCE verification -// --------------------------------------------------------------------------- - -export function verifyPKCE(codeVerifier: string, codeChallenge: string): boolean { - // H1: validate code_verifier format before hashing - if (!CODE_VERIFIER_RE.test(codeVerifier)) return false; - - const expected = crypto.createHash('sha256').update(codeVerifier).digest('base64url'); - // Constant-time compare (both are base64url strings of equal length for S256) - if (expected.length !== codeChallenge.length) return false; - try { - return crypto.timingSafeEqual(Buffer.from(expected), Buffer.from(codeChallenge)); - } catch { return false; } -} - -// --------------------------------------------------------------------------- -// Client authentication (for token endpoint) -// --------------------------------------------------------------------------- - -export function authenticateClient(clientId: string, clientSecret: string | undefined): OAuthClientRow | null { - const client = db.prepare('SELECT * FROM oauth_clients WHERE client_id = ?').get(clientId) as OAuthClientRow | undefined; - if (!client) return null; - if (client.is_public) { - // Public clients are identified by client_id alone — PKCE provides the security guarantee. - return client; - } - // H4: constant-time comparison to prevent timing side-channel - if (!clientSecret) return null; - if (!timingSafeEqualHex(hashToken(clientSecret), client.client_secret_hash)) return null; - return client; -} diff --git a/server/src/services/userCleanupService.ts b/server/src/services/userCleanupService.ts deleted file mode 100644 index 10d50016f..000000000 --- a/server/src/services/userCleanupService.ts +++ /dev/null @@ -1,65 +0,0 @@ -import fs from 'node:fs'; -import { db } from '../db/database'; -import { pluginsDataRoot } from '../nest/plugins/paths'; -import { removeUserFromBudgetItems } from '../nest/budget/budget.bridge'; - -/** - * Erase a user's PLUGIN-held data on account deletion. Two parts: - * 1. Host-side per-user plugin tables (encrypted config values, OAuth access/refresh - * tokens, in-flight OAuth state) — deleted directly; these live in trek.db, not in - * a plugin's own db, so nothing else ever removes them. - * 2. A durable erasure row per plugin that holds `hook:user-data`, so its OWN db is - * purged of the user (drained to the plugin when it is next active). - * - * Runs in the core deletion path (NOT via the plugin runtime), so it works even when - * TREK_PLUGINS_ENABLED=false or before the runtime has booted — otherwise a deletion in - * those windows would leave the user's plugin data behind forever. Best-effort per table - * so a slimmed-down schema (some tests) can't fail the user deletion itself. - */ -export function erasePluginUserData(userId: number): void { - for (const table of ['plugin_user_config', 'plugin_oauth_tokens', 'plugin_oauth_state']) { - try { db.prepare(`DELETE FROM ${table} WHERE user_id = ?`).run(userId); } catch { /* table absent (slim schema) */ } - } - try { - const rows = db.prepare('SELECT id, permissions FROM plugins').all() as Array<{ id: string; permissions: string | null }>; - const installed = new Set(rows.map((r) => r.id)); - const insert = db.prepare('INSERT OR IGNORE INTO plugin_user_erasure_queue (plugin_id, user_id) VALUES (?, ?)'); - for (const r of rows) { - let perms: unknown; - try { perms = JSON.parse(r.permissions ?? '[]'); } catch { perms = []; } - if (Array.isArray(perms) && perms.includes('hook:user-data')) insert.run(r.id, userId); - } - // Also enqueue for plugins UNINSTALLED with their data retained (deleteData=false): - // their data dir still holds the user's rows and a same-id reinstall would re-adopt - // them. No permissions record survives uninstall, so we can't check hook:user-data — - // enqueue for every orphan data dir; the row sits inert (no FK) and drains only if - // that id is reinstalled + active (erasure delivery is a duty, not grant-gated). - try { - for (const entry of fs.readdirSync(pluginsDataRoot(), { withFileTypes: true })) { - if (entry.isDirectory() && !installed.has(entry.name)) insert.run(entry.name, userId); - } - } catch { /* no plugin data root yet */ } - } catch { /* plugins / queue table absent (slim schema) */ } -} - -function cleanupUserReferences(userId: number): void { - db.prepare('UPDATE trip_members SET invited_by = NULL WHERE invited_by = ?').run(userId); - db.prepare('UPDATE budget_items SET paid_by_user_id = NULL WHERE paid_by_user_id = ?').run(userId); - removeUserFromBudgetItems(userId); - db.prepare('DELETE FROM share_tokens WHERE created_by = ?').run(userId); - db.prepare('DELETE FROM journey_share_tokens WHERE created_by = ?').run(userId); - // Owned journeys cascade-delete their entries/contributors/share_tokens/photos via journey_id FKs - db.prepare('DELETE FROM journeys WHERE user_id = ?').run(userId); - // Entries authored on other users' journeys (not covered by the cascade above) - db.prepare('DELETE FROM journey_entries WHERE author_id = ?').run(userId); - db.prepare('DELETE FROM journey_contributors WHERE user_id = ?').run(userId); -} - -export function deleteUserCompletely(userId: number): void { - const tx = db.transaction((id: number) => { - cleanupUserReferences(id); - erasePluginUserData(id); - db.prepare('DELETE FROM users WHERE id = ?').run(id); - }); - tx(userId); -} diff --git a/server/src/services/webauthnConfig.ts b/server/src/services/webauthnConfig.ts deleted file mode 100644 index a7f68c969..000000000 --- a/server/src/services/webauthnConfig.ts +++ /dev/null @@ -1,86 +0,0 @@ -import { readEnv } from '../app-config'; -import { db } from '../db/database'; -import { getAppUrl } from '../app-config'; - -/** - * Resolves the WebAuthn Relying Party ID + allowed origins for this deployment. - * - * SECURITY: the RP ID and the allowed origins are derived ONLY from server-side - * configuration — the `webauthn_rp_id` / `webauthn_origins` admin settings (or - * the matching env vars), falling back to APP_URL. They are NEVER taken from the - * request `Host` / `X-Forwarded-Host` header: a forged forwarded host would - * otherwise let an attacker bind credentials to a domain they control, or brick - * every enrolled user. This mirrors how OIDC derives its redirect URI from - * APP_URL (oidc.controller.ts) rather than from request input. - * - * Returns null when no usable RP ID can be resolved (bare IP host, or nothing - * configured) — the feature then reports itself as "not configured" and stays - * disabled so nobody can enrol a credential bound to the wrong origin. - */ - -function getSetting(key: string): string | null { - const raw = (db.prepare('SELECT value FROM app_settings WHERE key = ?').get(key) as { value: string } | undefined)?.value; - const trimmed = raw?.trim(); - return trimmed ? trimmed : null; -} - -function hostOf(url: string): string | null { - try { - return new URL(url).hostname || null; - } catch { - return null; - } -} - -/** WebAuthn RP IDs must be registrable domains — never bare IP literals. */ -function isIpHost(host: string): boolean { - if (/^\d{1,3}(\.\d{1,3}){3}$/.test(host)) return true; // IPv4 - if (host.includes(':')) return true; // IPv6 (hostname keeps the colons) - return false; -} - -export interface WebauthnConfig { - rpID: string; - rpName: string; - /** Exact allowed origins (scheme + host + port). One in prod; localhost dev adds the Vite/API ports. */ - origins: string[]; -} - -export function resolveWebauthnConfig(): WebauthnConfig | null { - // 1. Explicit operator config always wins. - const explicitRpId = (readEnv().webauthn.rpId || getSetting('webauthn_rp_id'))?.trim() || null; - const explicitOrigins = (readEnv().webauthn.origins || getSetting('webauthn_origins') || '') - .split(',') - .map((o) => o.trim().replace(/\/+$/, '')) - .filter(Boolean); - - const appUrl = getAppUrl(); - const appHost = hostOf(appUrl); - - // 2. Derive the RP ID from APP_URL when not explicitly set. - let rpID = explicitRpId; - if (!rpID && appHost && !isIpHost(appHost)) { - rpID = appHost; // a real domain, or "localhost" - } - if (!rpID) return null; // bare IP / unresolved → WebAuthn cannot be used here - - // 3. Resolve the allowed origins. Explicit list wins verbatim (operator's - // responsibility). Otherwise derive a SINGLE origin from APP_URL — we never - // silently union dev localhost origins into a production allow-list. - let origins = explicitOrigins; - if (origins.length === 0) { - if (appHost) origins = [appUrl.replace(/\/+$/, '')]; - if (rpID === 'localhost') { - // Dev: the browser origin is the Vite dev server (:5173), not the API port. - origins = Array.from(new Set([...origins, 'http://localhost:5173', 'http://localhost:3001'])); - } - } - if (origins.length === 0) return null; - - return { rpID, rpName: 'TREK', origins }; -} - -/** True when a usable RP ID resolves for this deployment (exposed as a pure boolean on app-config). */ -export function isPasskeyConfigured(): boolean { - return resolveWebauthnConfig() !== null; -} diff --git a/server/tests/e2e/oauth.e2e.test.ts b/server/tests/e2e/oauth.e2e.test.ts index de686c1a3..3635b0bde 100644 --- a/server/tests/e2e/oauth.e2e.test.ts +++ b/server/tests/e2e/oauth.e2e.test.ts @@ -38,6 +38,9 @@ vi.mock('../../src/app-config', async (importOriginal) => { const { isAddonEnabled } = vi.hoisted(() => ({ isAddonEnabled: vi.fn(() => true) })); +// Stubbed at the provider, not the module path: the domain folded into +// OauthService, so there is no legacy module left to vi.mock. Same six cases, +// same assertions — these pin the guard/gate layer, not the OAuth logic. const { oauthSvc } = vi.hoisted(() => ({ oauthSvc: { validateAuthorizeRequest: vi.fn(), createAuthCode: vi.fn(), consumeAuthCode: vi.fn(), saveConsent: vi.fn(), @@ -47,9 +50,9 @@ const { oauthSvc } = vi.hoisted(() => ({ getUserByAccessToken: vi.fn(), }, })); -vi.mock('../../src/services/oauthService', () => oauthSvc); import { OauthModule } from '../../src/nest/oauth/oauth.module'; +import { OauthService } from '../../src/nest/oauth/oauth.service'; import { AddonsService } from '../../src/nest/addons/addons.service'; import { DatabaseModule } from '../../src/nest/database/database.module'; import { TrekExceptionFilter } from '../../src/nest/common/trek-exception.filter'; @@ -62,6 +65,8 @@ describe('OAuth e2e (real guards + temp SQLite)', () => { const moduleRef = await Test.createTestingModule({ imports: [DatabaseModule, OauthModule] }) .overrideProvider(AddonsService) .useValue({ isAddonEnabled }) + .overrideProvider(OauthService) + .useValue({ ...oauthSvc, mcpEnabled: () => isAddonEnabled(), mcpSafeUrl: () => 'https://mcp.test' }) .compile(); const nest = moduleRef.createNestApplication(); nest.use(cookieParser()); diff --git a/server/tests/integration/oauth.test.ts b/server/tests/integration/oauth.test.ts index db5f773f2..ef0f053e3 100644 --- a/server/tests/integration/oauth.test.ts +++ b/server/tests/integration/oauth.test.ts @@ -58,7 +58,7 @@ import { runMigrations } from '../../src/db/migrations'; import { resetTestDb, resetRateLimits } from '../helpers/test-db'; import { createUser } from '../helpers/factories'; import { authCookie } from '../helpers/auth'; -import { createOAuthClient, createAuthCode, getUserByAccessToken } from '../../src/services/oauthService'; +import { createOAuthClient, createAuthCode, getUserByAccessToken } from '../../src/nest/oauth/oauth.bridge'; let nestApp: INestApplication; let app: Application; diff --git a/server/tests/unit/nest/admin.service.test.ts b/server/tests/unit/nest/admin.service.test.ts index 03731278f..e6fc77b41 100644 --- a/server/tests/unit/nest/admin.service.test.ts +++ b/server/tests/unit/nest/admin.service.test.ts @@ -61,6 +61,9 @@ import { RealtimeService } from '../../../src/nest/realtime/realtime.service'; import { AddonsService } from '../../../src/nest/addons/addons.service'; import { SettingsService } from '../../../src/nest/settings/settings.service'; import { AtlasService } from '../../../src/nest/atlas/atlas.service'; +import { TripMembershipService } from '../../../src/nest/trip-membership/trip-membership.service'; +import { UserCleanupService } from '../../../src/nest/auth/user-cleanup.service'; +import { WebauthnConfigService } from '../../../src/nest/auth/webauthn-config.service'; import { AuthService } from '../../../src/nest/auth/auth.service'; import { PasskeyService } from '../../../src/nest/auth/passkey.service'; import { PackingService } from '../../../src/nest/packing/packing.service'; @@ -73,16 +76,19 @@ import { __clearVersionCacheForTests } from '../../../src/nest/admin/admin.helpe const dbs = new DatabaseService(testDb); const realtime = new RealtimeService(); const permissions = new PermissionsService(dbs); -const auth = new AuthService(dbs, permissions, new AtlasService(dbs)); +const webauthn = new WebauthnConfigService(dbs); +const userCleanup = new UserCleanupService(dbs); +const auth = new AuthService(dbs, permissions, new AtlasService(dbs), new TripMembershipService(dbs), webauthn, userCleanup); const svc = new AdminService( dbs, new SettingsService(dbs), new AddonsService(dbs), - new PasskeyService(dbs, auth), + new PasskeyService(dbs, auth, webauthn), new PackingService(dbs, permissions, realtime), auth, permissions, new NotificationsService(dbs, realtime), + userCleanup, ); // Legacy free-function names bound to the service, so the moved cases below read diff --git a/server/tests/unit/nest/admin.version-notification.test.ts b/server/tests/unit/nest/admin.version-notification.test.ts index b264f44ac..d24aea36c 100644 --- a/server/tests/unit/nest/admin.version-notification.test.ts +++ b/server/tests/unit/nest/admin.version-notification.test.ts @@ -46,6 +46,9 @@ import { RealtimeService } from '../../../src/nest/realtime/realtime.service'; import { AddonsService } from '../../../src/nest/addons/addons.service'; import { SettingsService } from '../../../src/nest/settings/settings.service'; import { AtlasService } from '../../../src/nest/atlas/atlas.service'; +import { TripMembershipService } from '../../../src/nest/trip-membership/trip-membership.service'; +import { UserCleanupService } from '../../../src/nest/auth/user-cleanup.service'; +import { WebauthnConfigService } from '../../../src/nest/auth/webauthn-config.service'; import { AuthService } from '../../../src/nest/auth/auth.service'; import { PasskeyService } from '../../../src/nest/auth/passkey.service'; import { PackingService } from '../../../src/nest/packing/packing.service'; @@ -56,16 +59,19 @@ import { AdminService } from '../../../src/nest/admin/admin.service'; const dbs = new DatabaseService(testDb); const realtime = new RealtimeService(); const permissions = new PermissionsService(dbs); -const auth = new AuthService(dbs, permissions, new AtlasService(dbs)); +const webauthn = new WebauthnConfigService(dbs); +const userCleanup = new UserCleanupService(dbs); +const auth = new AuthService(dbs, permissions, new AtlasService(dbs), new TripMembershipService(dbs), webauthn, userCleanup); const svc = new AdminService( dbs, new SettingsService(dbs), new AddonsService(dbs), - new PasskeyService(dbs, auth), + new PasskeyService(dbs, auth, webauthn), new PackingService(dbs, permissions, realtime), auth, permissions, new NotificationsService(dbs, realtime), + userCleanup, ); const checkAndNotifyVersion = () => svc.checkAndNotifyVersion(); diff --git a/server/tests/unit/nest/auth.service.test.ts b/server/tests/unit/nest/auth.service.test.ts index 5f0706410..0eca17696 100644 --- a/server/tests/unit/nest/auth.service.test.ts +++ b/server/tests/unit/nest/auth.service.test.ts @@ -82,6 +82,8 @@ import { authenticator } from 'otplib'; import { hashBackupCode } from '../../../src/nest/auth/auth.helpers'; import { createEphemeralToken } from '../../../src/nest/auth/ephemeral-tokens'; import { TripMembershipService } from '../../../src/nest/trip-membership/trip-membership.service'; +import { UserCleanupService } from '../../../src/nest/auth/user-cleanup.service'; +import { WebauthnConfigService } from '../../../src/nest/auth/webauthn-config.service'; import { revokeUserSessions } from '../../../src/mcp/sessionManager'; // Stubbed rather than real: these tests assert that the invite path CALLS the @@ -93,6 +95,8 @@ const svc = new AuthService( new PermissionsService(new DatabaseService(testDb)), new AtlasService(new DatabaseService(testDb)), membershipStub, + new WebauthnConfigService(new DatabaseService(testDb)), + new UserCleanupService(new DatabaseService(testDb)), ); // --------------------------------------------------------------------------- diff --git a/server/tests/unit/nest/budget.service.db.test.ts b/server/tests/unit/nest/budget.service.db.test.ts index d5f88a97e..4f569d433 100644 --- a/server/tests/unit/nest/budget.service.db.test.ts +++ b/server/tests/unit/nest/budget.service.db.test.ts @@ -61,6 +61,7 @@ import { DatabaseService } from '../../../src/nest/database/database.service'; import { PermissionsService } from '../../../src/nest/permissions/permissions.service'; import { ExchangeRatesService } from '../../../src/nest/budget/exchange-rates.service'; import * as bridge from '../../../src/nest/budget/budget.bridge'; +import { UserCleanupService } from '../../../src/nest/auth/user-cleanup.service'; import { RealtimeService } from '../../../src/nest/realtime/realtime.service'; import { TripsService } from '../../../src/nest/trips/trips.service'; import { TodoService } from '../../../src/nest/todo/todo.service'; @@ -95,6 +96,9 @@ const tripsSvc = new TripsService( new CollabService(dbs(), new PermissionsService(dbs()), new RealtimeService()), new VacayService(dbs(), new RealtimeService()), new RealtimeService(), + undefined as never, // places — not exercised here + undefined as never, // unsplash — not exercised here + new UserCleanupService(dbs()), ); const createGuest = tripsSvc.createGuest.bind(tripsSvc); const deleteGuest = tripsSvc.deleteGuest.bind(tripsSvc); @@ -404,9 +408,10 @@ describe('rebaseTripCurrency', () => { }); describe('budget.bridge delegation', () => { - // The listBudgetItems / rebaseTripCurrency bridge exports were pruned when - // their last outside-container consumers (legacy tripService + trips MCP - // registrar) migrated — 015/017 pin the same behavior on the service. + // The listBudgetItems / rebaseTripCurrency / removeUserFromBudgetItems bridge + // exports were pruned when their last outside-container consumers (legacy + // tripService, the trips MCP registrar, services/userCleanupService) migrated + // — 015/016/017 pin the same behavior on the service. it('BUDGET-SVC-DB-015: listBudgetItems returns the hydrated list', () => { const { user } = createUser(testDb); const trip = createTrip(testDb, user.id); @@ -418,13 +423,13 @@ describe('budget.bridge delegation', () => { expect(items[0].members.map(m => m.user_id)).toEqual([user.id]); }); - it('BUDGET-SVC-DB-016: removeUserFromBudgetItems re-derives persons through the bridge', () => { + it('BUDGET-SVC-DB-016: removeUserFromBudgetItems re-derives persons', () => { const { user: owner } = createUser(testDb); const { user: other } = createUser(testDb, { username: 'other' }); const trip = createTrip(testDb, owner.id); const item = budget.createBudgetItem(trip.id, { name: 'Dinner', total_price: 80, member_ids: [owner.id, other.id] }); - bridge.removeUserFromBudgetItems(other.id); + budget.removeUserFromBudgetItems(other.id); const row = testDb.prepare('SELECT persons FROM budget_items WHERE id = ?').get(item.id) as { persons: number | null }; expect(row.persons).toBe(1); diff --git a/server/tests/unit/nest/oauth.service.test.ts b/server/tests/unit/nest/oauth.service.test.ts index b2c89b638..5b101caf8 100644 --- a/server/tests/unit/nest/oauth.service.test.ts +++ b/server/tests/unit/nest/oauth.service.test.ts @@ -1,162 +1,1168 @@ -import { describe, it, expect, vi, beforeEach } from 'vitest'; +/** + * Unit tests for the OAuth 2.1 domain service. + * + * Moved 1:1 from tests/unit/services/oauthService.test.ts with the fold — the + * cases and their names are unchanged; the free-function imports became + * methods bound to a hand-constructed OauthService (the repo's DB-backed + * service-test shape). The delegation cases of the old thin wrapper died with + * it; mcpEnabled/mcpSafeUrl, the module metadata and the bridge seam are + * pinned at the bottom. + */ +import { describe, it, expect, vi, beforeAll, beforeEach, afterAll } from 'vitest'; +import crypto from 'crypto'; -// The Nest service is a thin wrapper that forwards to the legacy oauthService -// plus the addon/notification helpers. Mock those and assert the delegation. -const { oauth } = vi.hoisted(() => ({ - oauth: { - consumeAuthCode: vi.fn(), - authenticateClient: vi.fn(), - verifyPKCE: vi.fn(), - issueTokens: vi.fn(), - issueClientCredentialsToken: vi.fn(), - refreshTokens: vi.fn(), - revokeToken: vi.fn(), - getUserByAccessToken: vi.fn(), - validateAuthorizeRequest: vi.fn(), - saveConsent: vi.fn(), - createAuthCode: vi.fn(), - listOAuthClients: vi.fn(), - createOAuthClient: vi.fn(), - rotateOAuthClientSecret: vi.fn(), - deleteOAuthClient: vi.fn(), - listOAuthSessions: vi.fn(), - revokeSession: vi.fn(), - }, -})); -vi.mock('../../../src/services/oauthService', () => oauth); - -const { isAddonEnabled } = vi.hoisted(() => ({ isAddonEnabled: vi.fn() })); - -const { getMcpSafeUrl } = vi.hoisted(() => ({ getMcpSafeUrl: vi.fn() })); -vi.mock('../../../src/app-config', async (importOriginal) => { - const actual = await importOriginal(); - return { ...actual, getMcpSafeUrl }; +const { testDb, dbMock } = vi.hoisted(() => { + const Database = require('better-sqlite3'); + const db = new Database(':memory:'); + db.exec('PRAGMA journal_mode = WAL'); + db.exec('PRAGMA foreign_keys = ON'); + db.exec('PRAGMA busy_timeout = 5000'); + const mock = { + db, + closeDb: () => {}, + reinitialize: () => {}, + getPlaceWithTags: () => null, + canAccessTrip: (tripId: any, userId: number) => + db.prepare(`SELECT t.id, t.user_id FROM trips t LEFT JOIN trip_members m ON m.trip_id = t.id AND m.user_id = ? WHERE t.id = ? AND (t.user_id = ? OR m.user_id IS NOT NULL)`).get(userId, tripId, userId), + isOwner: (tripId: any, userId: number) => + !!db.prepare('SELECT id FROM trips WHERE id = ? AND user_id = ?').get(tripId, userId), + }; + return { testDb: db, dbMock: mock }; }); +vi.mock('../../../src/db/database', () => dbMock); +vi.mock('../../../src/config', () => ({ + JWT_SECRET: 'test-jwt-secret-for-trek-testing-only', + ENCRYPTION_KEY: 'a1b2c3d4e5f6a7b8c9d0e1f2a3b4c5d6a7b8c9d0e1f2a3b4c5d6a7b8c9d0e1f2', + updateJwtSecret: () => {}, +})); +vi.mock('../../../src/nest/common/crypto/apiKeyCrypto', () => ({ + encrypt_api_key: (v: string) => v, + decrypt_api_key: (v: string) => v, + maybe_encrypt_api_key: (v: string) => v, +})); +vi.mock('../../../src/mcp/sessionManager', () => ({ revokeUserSessions: vi.fn(), revokeUserSessionsForClient: vi.fn(), sessions: new Map() })); +import { revokeUserSessionsForClient } from '../../../src/mcp/sessionManager'; +vi.mock('../../../src/demo/demo-reset', () => ({ saveBaseline: vi.fn() })); + +const { isAddonEnabled } = vi.hoisted(() => ({ isAddonEnabled: vi.fn().mockReturnValue(true) })); + +import { createTables } from '../../../src/db/schema'; +import { runMigrations } from '../../../src/db/migrations'; +import { resetTestDb } from '../../helpers/test-db'; +import { createUser } from '../../helpers/factories'; +// PKCE helper — generates a valid code_verifier + code_challenge pair (RFC 7636) +function makePkce() { + const verifier = crypto.randomBytes(32).toString('base64url'); // 43 chars + const challenge = crypto.createHash('sha256').update(verifier).digest('base64url'); // 43 chars + return { verifier, challenge }; +} + import { OauthService } from '../../../src/nest/oauth/oauth.service'; +import { DatabaseService } from '../../../src/nest/database/database.service'; +import { AuditService } from '../../../src/nest/audit/audit.service'; import type { AddonsService } from '../../../src/nest/addons/addons.service'; +import { getMcpSafeUrl } from '../../../src/app-config'; import { ADDON_IDS } from '../../../src/addons'; +import * as bridge from '../../../src/nest/oauth/oauth.bridge'; +import { MAX_PENDING_CODES, sweepPendingCodes } from '../../../src/nest/oauth/oauth.pending-codes'; +const dbs = new DatabaseService(testDb); +// Stubbed rather than real: every case drives the MCP gate through this one +// flag, exactly as the addons.bridge mock did before the fold. const addonsStub = { isAddonEnabled } as unknown as AddonsService; -function svc() { return new OauthService(addonsStub); } +const svc = new OauthService(dbs, addonsStub, new AuditService(dbs)); -beforeEach(() => vi.clearAllMocks()); +// Legacy free-function names bound to the service, so the moved cases below read +// exactly as they did before the fold. +const createOAuthClient = svc.createOAuthClient.bind(svc); +const listOAuthClients = svc.listOAuthClients.bind(svc); +const deleteOAuthClient = svc.deleteOAuthClient.bind(svc); +const rotateOAuthClientSecret = svc.rotateOAuthClientSecret.bind(svc); +const createAuthCode = svc.createAuthCode.bind(svc); +const consumeAuthCode = svc.consumeAuthCode.bind(svc); +const issueTokens = svc.issueTokens.bind(svc); +const getUserByAccessToken = svc.getUserByAccessToken.bind(svc); +const refreshTokens = svc.refreshTokens.bind(svc); +const revokeToken = svc.revokeToken.bind(svc); +const listOAuthSessions = svc.listOAuthSessions.bind(svc); +const revokeSession = svc.revokeSession.bind(svc); +const validateAuthorizeRequest = svc.validateAuthorizeRequest.bind(svc); +const verifyPKCE = svc.verifyPKCE.bind(svc); +const authenticateClient = svc.authenticateClient.bind(svc); +const saveConsent = svc.saveConsent.bind(svc); +const getConsent = svc.getConsent.bind(svc); +const isConsentSufficient = svc.isConsentSufficient.bind(svc); -describe('OauthService', () => { +beforeAll(() => { + createTables(testDb); + runMigrations(testDb); +}); + +beforeEach(() => { + resetTestDb(testDb); + // Clear oauth tables manually since they're not in the standard reset list + testDb.exec('DELETE FROM oauth_tokens'); + testDb.exec('DELETE FROM oauth_consents'); + testDb.exec('DELETE FROM oauth_clients'); + isAddonEnabled.mockReturnValue(true); +}); + +afterAll(() => { + testDb.close(); +}); + +// --------------------------------------------------------------------------- +// Helper +// --------------------------------------------------------------------------- + +function makeClient( + userId: number, + overrides: Partial<{ name: string; redirectUris: string[]; scopes: string[] }> = {} +) { + return createOAuthClient( + userId, + overrides.name ?? 'Test Client', + overrides.redirectUris ?? ['https://example.com/callback'], + overrides.scopes ?? ['trips:read'], + ); +} + +// --------------------------------------------------------------------------- +// createOAuthClient +// --------------------------------------------------------------------------- + +describe('createOAuthClient', () => { + it('creates a client successfully and returns client_secret only on creation', () => { + const { user } = createUser(testDb); + const result = makeClient(user.id); + expect(result.error).toBeUndefined(); + expect(result.client).toBeDefined(); + expect(typeof result.client!.client_secret).toBe('string'); + expect((result.client!.client_secret as string).startsWith('trekcs_')).toBe(true); + }); + + it('client_id is a UUID', () => { + const { user } = createUser(testDb); + const result = makeClient(user.id); + expect(result.client!.client_id).toMatch( + /^[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}$/ + ); + }); + + it('returns 400 error if name is empty', () => { + const { user } = createUser(testDb); + const result = createOAuthClient(user.id, '', ['https://example.com/cb'], ['trips:read']); + expect(result.status).toBe(400); + expect(result.error).toContain('Name'); + }); + + it('returns 400 error if name exceeds 100 characters', () => { + const { user } = createUser(testDb); + const longName = 'A'.repeat(101); + const result = createOAuthClient(user.id, longName, ['https://example.com/cb'], ['trips:read']); + expect(result.status).toBe(400); + expect(result.error).toContain('100'); + }); + + it('returns 400 error if no redirect URIs provided', () => { + const { user } = createUser(testDb); + const result = createOAuthClient(user.id, 'Test', [], ['trips:read']); + expect(result.status).toBe(400); + expect(result.error).toContain('redirect URI'); + }); + + it('returns 400 error if more than 10 redirect URIs provided', () => { + const { user } = createUser(testDb); + const uris = Array.from({ length: 11 }, (_, i) => `https://example${i}.com/cb`); + const result = createOAuthClient(user.id, 'Test', uris, ['trips:read']); + expect(result.status).toBe(400); + expect(result.error).toContain('10'); + }); + + it('returns 400 error for invalid URI format', () => { + const { user } = createUser(testDb); + const result = createOAuthClient(user.id, 'Test', ['not-a-url'], ['trips:read']); + expect(result.status).toBe(400); + expect(result.error).toContain('Invalid redirect URI'); + }); + + it('returns 400 error for non-https URI (not localhost)', () => { + const { user } = createUser(testDb); + const result = createOAuthClient(user.id, 'Test', ['http://example.com/cb'], ['trips:read']); + expect(result.status).toBe(400); + expect(result.error).toContain('HTTPS'); + }); + + it('allows http://localhost redirect URI', () => { + const { user } = createUser(testDb); + const result = createOAuthClient(user.id, 'Test', ['http://localhost:3000/callback'], ['trips:read']); + expect(result.error).toBeUndefined(); + expect(result.client).toBeDefined(); + }); + + it('allows http://127.0.0.1 redirect URI', () => { + const { user } = createUser(testDb); + const result = createOAuthClient(user.id, 'Test', ['http://127.0.0.1:5000/callback'], ['trips:read']); + expect(result.error).toBeUndefined(); + expect(result.client).toBeDefined(); + }); + + it('returns 400 error if no scopes provided', () => { + const { user } = createUser(testDb); + const result = createOAuthClient(user.id, 'Test', ['https://example.com/cb'], []); + expect(result.status).toBe(400); + expect(result.error).toContain('scope'); + }); + + it('returns 400 error for invalid scopes', () => { + const { user } = createUser(testDb); + const result = createOAuthClient(user.id, 'Test', ['https://example.com/cb'], ['invalid:scope']); + expect(result.status).toBe(400); + expect(result.error).toContain('Invalid scopes'); + }); + + it('enforces max 10 clients per user', () => { + const { user } = createUser(testDb); + for (let i = 0; i < 10; i++) { + const r = makeClient(user.id, { name: `Client ${i}` }); + expect(r.error).toBeUndefined(); + } + const eleventh = makeClient(user.id, { name: 'Eleventh' }); + expect(eleventh.status).toBe(400); + expect(eleventh.error).toContain('10'); + }); +}); + +// --------------------------------------------------------------------------- +// listOAuthClients +// --------------------------------------------------------------------------- + +describe('listOAuthClients', () => { + it('returns empty array for user with no clients', () => { + const { user } = createUser(testDb); + expect(listOAuthClients(user.id)).toEqual([]); + }); + + it('returns created clients with redirect_uris and allowed_scopes as arrays', () => { + const { user } = createUser(testDb); + makeClient(user.id, { name: 'Client A', redirectUris: ['https://a.com/cb'], scopes: ['trips:read', 'budget:read'] }); + const clients = listOAuthClients(user.id); + expect(clients).toHaveLength(1); + expect(clients[0].name).toBe('Client A'); + expect(Array.isArray(clients[0].redirect_uris)).toBe(true); + expect(Array.isArray(clients[0].allowed_scopes)).toBe(true); + expect(clients[0].allowed_scopes).toContain('trips:read'); + }); +}); + +// --------------------------------------------------------------------------- +// deleteOAuthClient +// --------------------------------------------------------------------------- + +describe('deleteOAuthClient', () => { + it('deletes own client successfully', () => { + const { user } = createUser(testDb); + const created = makeClient(user.id); + const clientRowId = created.client!.id as string; + const result = deleteOAuthClient(user.id, clientRowId); + expect(result.success).toBe(true); + expect(listOAuthClients(user.id)).toHaveLength(0); + }); + + it('returns 404 for non-existent client', () => { + const { user } = createUser(testDb); + const result = deleteOAuthClient(user.id, 'non-existent-id'); + expect(result.status).toBe(404); + }); + + it("returns 404 for another user's client", () => { + const { user: owner } = createUser(testDb); + const { user: other } = createUser(testDb); + const created = makeClient(owner.id); + const result = deleteOAuthClient(other.id, created.client!.id as string); + expect(result.status).toBe(404); + }); +}); + +// --------------------------------------------------------------------------- +// rotateOAuthClientSecret +// --------------------------------------------------------------------------- + +describe('rotateOAuthClientSecret', () => { + it('rotates secret and returns new client_secret starting with trekcs_', () => { + const { user } = createUser(testDb); + const created = makeClient(user.id); + const oldSecret = created.client!.client_secret as string; + const result = rotateOAuthClientSecret(user.id, created.client!.id as string); + expect(result.error).toBeUndefined(); + expect(result.client_secret).toBeDefined(); + expect((result.client_secret as string).startsWith('trekcs_')).toBe(true); + expect(result.client_secret).not.toBe(oldSecret); + }); + + it('returns 404 for non-existent client', () => { + const { user } = createUser(testDb); + const result = rotateOAuthClientSecret(user.id, 'non-existent-id'); + expect(result.status).toBe(404); + }); + + it('revokes old tokens after rotation', () => { + const { user } = createUser(testDb); + const created = makeClient(user.id); + const clientId = created.client!.client_id as string; + const { access_token } = issueTokens(clientId, user.id, ['trips:read']); + expect(getUserByAccessToken(access_token)).not.toBeNull(); + + rotateOAuthClientSecret(user.id, created.client!.id as string); + + expect(getUserByAccessToken(access_token)).toBeNull(); + }); +}); + +// --------------------------------------------------------------------------- +// createAuthCode + consumeAuthCode +// --------------------------------------------------------------------------- + +describe('createAuthCode + consumeAuthCode', () => { + it('create code and consume it once returns the pending entry', () => { + const { user } = createUser(testDb); + const created = makeClient(user.id); + const clientId = created.client!.client_id as string; + + const code = createAuthCode({ + clientId, + userId: user.id, + redirectUri: 'https://example.com/callback', + scopes: ['trips:read'], + codeChallenge: 'abc123', + codeChallengeMethod: 'S256', + }); + + const entry = consumeAuthCode(code); + expect(entry).not.toBeNull(); + expect(entry!.userId).toBe(user.id); + expect(entry!.clientId).toBe(clientId); + }); + + it('returns null for non-existent code', () => { + expect(consumeAuthCode('does-not-exist')).toBeNull(); + }); + + it('consuming same code twice returns null (one-time use)', () => { + const { user } = createUser(testDb); + const created = makeClient(user.id); + const clientId = created.client!.client_id as string; + + const code = createAuthCode({ + clientId, + userId: user.id, + redirectUri: 'https://example.com/callback', + scopes: ['trips:read'], + codeChallenge: 'abc123', + codeChallengeMethod: 'S256', + }); + + consumeAuthCode(code); + expect(consumeAuthCode(code)).toBeNull(); + }); +}); + +// --------------------------------------------------------------------------- +// issueTokens + getUserByAccessToken +// --------------------------------------------------------------------------- + +describe('issueTokens + getUserByAccessToken', () => { + it('issues tokens with correct prefixes', () => { + const { user } = createUser(testDb); + const created = makeClient(user.id); + const clientId = created.client!.client_id as string; + + const tokens = issueTokens(clientId, user.id, ['trips:read']); + expect(tokens.access_token.startsWith('trekoa_')).toBe(true); + expect(tokens.refresh_token.startsWith('trekrf_')).toBe(true); + expect(tokens.token_type).toBe('Bearer'); + expect(typeof tokens.expires_in).toBe('number'); + }); + + it('getUserByAccessToken returns user and scopes for a valid token', () => { + const { user } = createUser(testDb); + const created = makeClient(user.id); + const clientId = created.client!.client_id as string; + + const { access_token } = issueTokens(clientId, user.id, ['trips:read', 'budget:write']); + const info = getUserByAccessToken(access_token); + expect(info).not.toBeNull(); + expect(info!.user.email).toBe(user.email); + expect(info!.scopes).toContain('trips:read'); + expect(info!.scopes).toContain('budget:write'); + }); + + it('getUserByAccessToken returns null for unknown token', () => { + expect(getUserByAccessToken('trekoa_unknown')).toBeNull(); + }); + + it('getUserByAccessToken returns null for revoked token', () => { + const { user } = createUser(testDb); + const created = makeClient(user.id); + const clientId = created.client!.client_id as string; + + const { access_token } = issueTokens(clientId, user.id, ['trips:read']); + revokeToken(access_token, clientId); + expect(getUserByAccessToken(access_token)).toBeNull(); + }); +}); + +// --------------------------------------------------------------------------- +// refreshTokens +// --------------------------------------------------------------------------- + +describe('refreshTokens', () => { + it('exchanges a refresh token for a new token pair', () => { + const { user } = createUser(testDb); + const created = makeClient(user.id); + const clientId = created.client!.client_id as string; + const rawSecret = created.client!.client_secret as string; + + const { refresh_token } = issueTokens(clientId, user.id, ['trips:read']); + const result = refreshTokens(refresh_token, clientId, rawSecret); + expect(result.error).toBeUndefined(); + expect(result.tokens).toBeDefined(); + expect(result.tokens!.access_token.startsWith('trekoa_')).toBe(true); + }); + + it('old tokens are revoked after refresh (rotation)', () => { + const { user } = createUser(testDb); + const created = makeClient(user.id); + const clientId = created.client!.client_id as string; + const rawSecret = created.client!.client_secret as string; + + const { access_token, refresh_token } = issueTokens(clientId, user.id, ['trips:read']); + refreshTokens(refresh_token, clientId, rawSecret); + expect(getUserByAccessToken(access_token)).toBeNull(); + }); + + it('does not revoke the active MCP session on a normal (non-replayed) refresh (#1475)', () => { + const { user } = createUser(testDb); + const created = makeClient(user.id); + const clientId = created.client!.client_id as string; + const rawSecret = created.client!.client_secret as string; + + const { refresh_token } = issueTokens(clientId, user.id, ['trips:read']); + const callsBefore = vi.mocked(revokeUserSessionsForClient).mock.calls.length; + const result = refreshTokens(refresh_token, clientId, rawSecret); + expect(result.error).toBeUndefined(); + expect(vi.mocked(revokeUserSessionsForClient).mock.calls.length).toBe(callsBefore); + }); + + it('returns invalid_grant for unknown refresh token', () => { + const { user } = createUser(testDb); + const created = makeClient(user.id); + const clientId = created.client!.client_id as string; + const rawSecret = created.client!.client_secret as string; + + const result = refreshTokens('trekrf_unknown', clientId, rawSecret); + expect(result.error).toBe('invalid_grant'); + expect(result.status).toBe(400); + }); + + it('returns invalid_grant for revoked token', () => { + const { user } = createUser(testDb); + const created = makeClient(user.id); + const clientId = created.client!.client_id as string; + const rawSecret = created.client!.client_secret as string; + + const { access_token, refresh_token } = issueTokens(clientId, user.id, ['trips:read']); + revokeToken(access_token, clientId); + const result = refreshTokens(refresh_token, clientId, rawSecret); + expect(result.error).toBe('invalid_grant'); + }); + + it('returns invalid_client for wrong client_secret', () => { + const { user } = createUser(testDb); + const created = makeClient(user.id); + const clientId = created.client!.client_id as string; + + const { refresh_token } = issueTokens(clientId, user.id, ['trips:read']); + const result = refreshTokens(refresh_token, clientId, 'wrong-secret'); + expect(result.error).toBe('invalid_client'); + expect(result.status).toBe(401); + }); +}); + +// --------------------------------------------------------------------------- +// revokeToken +// --------------------------------------------------------------------------- + +describe('revokeToken', () => { + it('after revoking access token, getUserByAccessToken returns null', () => { + const { user } = createUser(testDb); + const created = makeClient(user.id); + const clientId = created.client!.client_id as string; + + const { access_token } = issueTokens(clientId, user.id, ['trips:read']); + expect(getUserByAccessToken(access_token)).not.toBeNull(); + + revokeToken(access_token, clientId); + expect(getUserByAccessToken(access_token)).toBeNull(); + }); +}); + +// --------------------------------------------------------------------------- +// listOAuthSessions + revokeSession +// --------------------------------------------------------------------------- + +describe('listOAuthSessions + revokeSession', () => { + it('lists active sessions', () => { + const { user } = createUser(testDb); + const created = makeClient(user.id); + const clientId = created.client!.client_id as string; + + issueTokens(clientId, user.id, ['trips:read']); + const sessions = listOAuthSessions(user.id); + expect(sessions).toHaveLength(1); + expect(sessions[0].client_id).toBe(clientId); + }); + + it('revoked session is not listed', () => { + const { user } = createUser(testDb); + const created = makeClient(user.id); + const clientId = created.client!.client_id as string; + + const { access_token } = issueTokens(clientId, user.id, ['trips:read']); + revokeToken(access_token, clientId); + const sessions = listOAuthSessions(user.id); + expect(sessions).toHaveLength(0); + }); + + it('revokeSession returns 404 for unknown session', () => { + const { user } = createUser(testDb); + const result = revokeSession(user.id, 99999); + expect(result.status).toBe(404); + }); + + it('revokeSession by session id removes session from list', () => { + const { user } = createUser(testDb); + const created = makeClient(user.id); + const clientId = created.client!.client_id as string; + + issueTokens(clientId, user.id, ['trips:read']); + const sessions = listOAuthSessions(user.id); + const sessionId = sessions[0].id as number; + + const result = revokeSession(user.id, sessionId); + expect(result.success).toBe(true); + expect(listOAuthSessions(user.id)).toHaveLength(0); + }); +}); + +// --------------------------------------------------------------------------- +// validateAuthorizeRequest +// --------------------------------------------------------------------------- + +describe('validateAuthorizeRequest', () => { + // Use a proper 43-char S256 code_challenge to pass H1 format validation + const { challenge: VALID_CHALLENGE } = makePkce(); + + function makeParams(overrides: Partial<{ + response_type: string; + client_id: string; + redirect_uri: string; + scope: string; + code_challenge: string; + code_challenge_method: string; + }> = {}) { + return { + response_type: 'code', + client_id: '', + redirect_uri: 'https://example.com/callback', + scope: 'trips:read', + code_challenge: VALID_CHALLENGE, + code_challenge_method: 'S256', + ...overrides, + }; + } + + it('returns mcp_disabled when isAddonEnabled returns false', () => { + vi.mocked(isAddonEnabled).mockReturnValue(false); + const result = validateAuthorizeRequest(makeParams({ client_id: 'x' }), null); + expect(result.valid).toBe(false); + expect(result.error).toBe('mcp_disabled'); + }); + + it('requires response_type=code', () => { + const { user } = createUser(testDb); + const result = validateAuthorizeRequest(makeParams({ response_type: 'token', client_id: 'x' }), user.id); + expect(result.valid).toBe(false); + expect(result.error).toBe('unsupported_response_type'); + }); + + it('requires PKCE with S256', () => { + const { user } = createUser(testDb); + const result = validateAuthorizeRequest(makeParams({ client_id: 'x', code_challenge_method: 'plain' }), user.id); + expect(result.valid).toBe(false); + expect(result.error).toBe('invalid_request'); + }); + + it('requires valid client_id', () => { + const { user } = createUser(testDb); + const result = validateAuthorizeRequest(makeParams({ client_id: 'nonexistent' }), user.id); + expect(result.valid).toBe(false); + expect(result.error).toBe('invalid_client'); + }); + + it('validates redirect_uri against registered URIs', () => { + const { user } = createUser(testDb); + const created = makeClient(user.id, { redirectUris: ['https://example.com/callback'] }); + const clientId = created.client!.client_id as string; + + const result = validateAuthorizeRequest( + makeParams({ client_id: clientId, redirect_uri: 'https://evil.com/callback' }), + user.id + ); + expect(result.valid).toBe(false); + expect(result.error).toBe('invalid_redirect_uri'); + }); + + it('validates scope against client allowed_scopes', () => { + const { user } = createUser(testDb); + const created = makeClient(user.id, { scopes: ['trips:read'] }); + const clientId = created.client!.client_id as string; + + const result = validateAuthorizeRequest( + makeParams({ client_id: clientId, scope: 'budget:write' }), + user.id + ); + expect(result.valid).toBe(false); + expect(result.error).toBe('invalid_scope'); + }); + + it('returns loginRequired when userId is null', () => { + const { user } = createUser(testDb); + const created = makeClient(user.id); + const clientId = created.client!.client_id as string; + + const result = validateAuthorizeRequest(makeParams({ client_id: clientId }), null); + expect(result.valid).toBe(true); + expect(result.loginRequired).toBe(true); + }); + + it('returns consentRequired=true when consent not yet saved', () => { + const { user } = createUser(testDb); + const created = makeClient(user.id); + const clientId = created.client!.client_id as string; + + const result = validateAuthorizeRequest(makeParams({ client_id: clientId }), user.id); + expect(result.valid).toBe(true); + expect(result.consentRequired).toBe(true); + }); + + it('returns consentRequired=false when consent already saved and sufficient', () => { + const { user } = createUser(testDb); + const created = makeClient(user.id); + const clientId = created.client!.client_id as string; + + saveConsent(clientId, user.id, ['trips:read']); + const result = validateAuthorizeRequest(makeParams({ client_id: clientId }), user.id); + expect(result.valid).toBe(true); + expect(result.consentRequired).toBe(false); + }); +}); + +// --------------------------------------------------------------------------- +// verifyPKCE +// --------------------------------------------------------------------------- + +describe('verifyPKCE', () => { + it('returns true for valid code_verifier / code_challenge pair (SHA256 base64url)', () => { + const verifier = 'dBjftJeZ4CVP-mB92K27uhbUJU1p1r_wW1gFWFOEjXk'; + const challenge = crypto.createHash('sha256').update(verifier).digest('base64url'); + expect(verifyPKCE(verifier, challenge)).toBe(true); + }); + + it('returns false for wrong verifier', () => { + const verifier = 'correct-verifier'; + const challenge = crypto.createHash('sha256').update(verifier).digest('base64url'); + expect(verifyPKCE('wrong-verifier', challenge)).toBe(false); + }); +}); + +// --------------------------------------------------------------------------- +// authenticateClient +// --------------------------------------------------------------------------- + +describe('authenticateClient', () => { + it('returns client row for correct credentials', () => { + const { user } = createUser(testDb); + const created = makeClient(user.id); + const clientId = created.client!.client_id as string; + const rawSecret = created.client!.client_secret as string; + + const client = authenticateClient(clientId, rawSecret); + expect(client).not.toBeNull(); + expect(client!.client_id).toBe(clientId); + }); + + it('returns null for wrong secret', () => { + const { user } = createUser(testDb); + const created = makeClient(user.id); + const clientId = created.client!.client_id as string; + + expect(authenticateClient(clientId, 'wrong-secret')).toBeNull(); + }); + + it('returns null for unknown client_id', () => { + expect(authenticateClient('unknown-client-id', 'any-secret')).toBeNull(); + }); +}); + +// --------------------------------------------------------------------------- +// saveConsent + getConsent + isConsentSufficient +// --------------------------------------------------------------------------- + +describe('saveConsent + getConsent + isConsentSufficient', () => { + it('saves and retrieves consent', () => { + const { user } = createUser(testDb); + const created = makeClient(user.id); + const clientId = created.client!.client_id as string; + + saveConsent(clientId, user.id, ['trips:read', 'budget:write']); + const consent = getConsent(clientId, user.id); + expect(consent).not.toBeNull(); + expect(consent).toContain('trips:read'); + expect(consent).toContain('budget:write'); + }); + + it('isConsentSufficient returns true when all requested scopes are in existing', () => { + expect(isConsentSufficient(['trips:read', 'budget:write'], ['trips:read'])).toBe(true); + expect(isConsentSufficient(['trips:read', 'budget:write'], ['trips:read', 'budget:write'])).toBe(true); + }); + + it('isConsentSufficient returns false when some scopes are missing', () => { + expect(isConsentSufficient(['trips:read'], ['trips:read', 'budget:write'])).toBe(false); + expect(isConsentSufficient([], ['trips:read'])).toBe(false); + }); +}); + +// --------------------------------------------------------------------------- +// M5 — saveConsent unions instead of replacing +// --------------------------------------------------------------------------- + +describe('saveConsent — scope union (M5)', () => { + it('unioning scopes: approving B after A leaves both in consent', () => { + const { user } = createUser(testDb); + const created = makeClient(user.id, { scopes: ['trips:read', 'budget:write'] }); + const clientId = created.client!.client_id as string; + + saveConsent(clientId, user.id, ['trips:read']); + saveConsent(clientId, user.id, ['budget:write']); + + const consent = getConsent(clientId, user.id); + expect(consent).toContain('trips:read'); + expect(consent).toContain('budget:write'); + }); + + it('re-approving a superset scope still preserves previously-consented scopes', () => { + const { user } = createUser(testDb); + const created = makeClient(user.id, { scopes: ['trips:read', 'trips:write'] }); + const clientId = created.client!.client_id as string; + + saveConsent(clientId, user.id, ['trips:read', 'trips:write']); + // approve only trips:read on a later request + saveConsent(clientId, user.id, ['trips:read']); + + const consent = getConsent(clientId, user.id); + // trips:write should NOT be removed (union semantics) + expect(consent).toContain('trips:read'); + expect(consent).toContain('trips:write'); + }); + + it('consent is sufficient after sequential approvals — no re-prompt needed', () => { + const { user } = createUser(testDb); + const created = makeClient(user.id, { scopes: ['trips:read', 'budget:write'] }); + const clientId = created.client!.client_id as string; + + saveConsent(clientId, user.id, ['trips:read']); + saveConsent(clientId, user.id, ['budget:write']); + + // Should not require consent again for either scope + expect(isConsentSufficient(getConsent(clientId, user.id)!, ['trips:read'])).toBe(true); + expect(isConsentSufficient(getConsent(clientId, user.id)!, ['budget:write'])).toBe(true); + expect(isConsentSufficient(getConsent(clientId, user.id)!, ['trips:read', 'budget:write'])).toBe(true); + }); +}); + +// --------------------------------------------------------------------------- +// C2 — getUserByAccessToken returns clientId +// --------------------------------------------------------------------------- + +describe('getUserByAccessToken — includes clientId (C2)', () => { + it('returns clientId matching the issuing OAuth client', () => { + const { user } = createUser(testDb); + const created = makeClient(user.id); + const clientId = created.client!.client_id as string; + + const { access_token } = issueTokens(clientId, user.id, ['trips:read']); + const info = getUserByAccessToken(access_token); + expect(info).not.toBeNull(); + expect(info!.clientId).toBe(clientId); + }); +}); + +// --------------------------------------------------------------------------- +// C3 — Refresh token replay detection and chain revocation +// --------------------------------------------------------------------------- + +describe('refreshTokens — replay detection (C3)', () => { + it('replaying a revoked refresh token returns invalid_grant', () => { + const { user } = createUser(testDb); + const created = makeClient(user.id); + const clientId = created.client!.client_id as string; + const rawSecret = created.client!.client_secret as string; + + // Issue tokens, then rotate once (old token becomes revoked) + const { refresh_token: firstRefresh } = issueTokens(clientId, user.id, ['trips:read']); + const rotateResult = refreshTokens(firstRefresh, clientId, rawSecret); + expect(rotateResult.error).toBeUndefined(); + const { refresh_token: secondRefresh } = rotateResult.tokens!; + + // Replay the FIRST (now revoked) refresh token + const callsBefore = vi.mocked(revokeUserSessionsForClient).mock.calls.length; + const replayResult = refreshTokens(firstRefresh, clientId, rawSecret); + expect(replayResult.error).toBe('invalid_grant'); + expect(replayResult.status).toBe(400); + // Replay IS a security event — sessions must still be torn down here. + expect(vi.mocked(revokeUserSessionsForClient).mock.calls.length).toBe(callsBefore + 1); + expect(vi.mocked(revokeUserSessionsForClient)).toHaveBeenLastCalledWith(user.id, clientId); + }); + + it('replaying a revoked token also revokes the entire rotation chain', () => { + const { user } = createUser(testDb); + const created = makeClient(user.id); + const clientId = created.client!.client_id as string; + const rawSecret = created.client!.client_secret as string; + + // Issue → rotate once + const { refresh_token: first } = issueTokens(clientId, user.id, ['trips:read']); + const r1 = refreshTokens(first, clientId, rawSecret); + const { access_token: access2, refresh_token: second } = r1.tokens!; + + // Replay first (revoked) refresh token → chain revoke + refreshTokens(first, clientId, rawSecret); + + // The rotated access token should also be dead now + expect(getUserByAccessToken(access2)).toBeNull(); + + // The second refresh token should also be revoked + const r2 = refreshTokens(second, clientId, rawSecret); + expect(r2.error).toBe('invalid_grant'); + }); + + it('new rotation chain after replay is independent', () => { + const { user } = createUser(testDb); + const created = makeClient(user.id); + const clientId = created.client!.client_id as string; + const rawSecret = created.client!.client_secret as string; + + const { refresh_token: first } = issueTokens(clientId, user.id, ['trips:read']); + // Rotate once + const r1 = refreshTokens(first, clientId, rawSecret); + const { refresh_token: second } = r1.tokens!; + // Rotate again on the second token + const r2 = refreshTokens(second, clientId, rawSecret); + expect(r2.error).toBeUndefined(); + const { refresh_token: third } = r2.tokens!; + + // Replay the first revoked token → revokes chain containing first+second+third + refreshTokens(first, clientId, rawSecret); + + // third should now be revoked too (it's in the same chain) + const r3 = refreshTokens(third, clientId, rawSecret); + expect(r3.error).toBe('invalid_grant'); + }); +}); + +// --------------------------------------------------------------------------- +// H1 — PKCE code_challenge / code_verifier format validation +// --------------------------------------------------------------------------- + +describe('verifyPKCE — format validation (H1)', () => { + it('returns false for a code_verifier that is too short (< 43 chars)', () => { + const { challenge } = makePkce(); + expect(verifyPKCE('short', challenge)).toBe(false); + }); + + it('returns false for a code_verifier that is too long (> 128 chars)', () => { + const { challenge } = makePkce(); + const longVerifier = 'a'.repeat(129); + expect(verifyPKCE(longVerifier, challenge)).toBe(false); + }); + + it('returns false for a code_verifier with invalid characters', () => { + const { challenge } = makePkce(); + const badVerifier = 'A'.repeat(42) + ' '; // space is not allowed + expect(verifyPKCE(badVerifier, challenge)).toBe(false); + }); + + it('returns true for a valid 43-char verifier matching its challenge', () => { + const { verifier, challenge } = makePkce(); + expect(verifyPKCE(verifier, challenge)).toBe(true); + }); +}); + +describe('validateAuthorizeRequest — PKCE format (H1)', () => { + it('returns invalid_request when code_challenge is shorter than 43 chars', () => { + const { user } = createUser(testDb); + const created = makeClient(user.id); + const clientId = created.client!.client_id as string; + + const result = validateAuthorizeRequest({ + response_type: 'code', + client_id: clientId, + redirect_uri: 'https://example.com/callback', + scope: 'trips:read', + code_challenge: 'tooshort', + code_challenge_method: 'S256', + }, user.id); + expect(result.valid).toBe(false); + expect(result.error).toBe('invalid_request'); + }); + + it('returns invalid_request when code_challenge contains invalid characters', () => { + const { user } = createUser(testDb); + const created = makeClient(user.id); + const clientId = created.client!.client_id as string; + + // 43 chars but includes '=' which is not base64url + const badChallenge = '='.repeat(43); + const result = validateAuthorizeRequest({ + response_type: 'code', + client_id: clientId, + redirect_uri: 'https://example.com/callback', + scope: 'trips:read', + code_challenge: badChallenge, + code_challenge_method: 'S256', + }, user.id); + expect(result.valid).toBe(false); + expect(result.error).toBe('invalid_request'); + }); +}); + +// --------------------------------------------------------------------------- +// H3 — validateAuthorizeRequest: loginRequired response strips client info +// --------------------------------------------------------------------------- + +describe('validateAuthorizeRequest — unauthenticated strips client info (H3)', () => { + it('loginRequired response does not include client.name or allowed_scopes', () => { + const { user } = createUser(testDb); + const created = makeClient(user.id); + const clientId = created.client!.client_id as string; + const { challenge } = makePkce(); + + const result = validateAuthorizeRequest({ + response_type: 'code', + client_id: clientId, + redirect_uri: 'https://example.com/callback', + scope: 'trips:read', + code_challenge: challenge, + code_challenge_method: 'S256', + }, null /* unauthenticated */); + + expect(result.valid).toBe(true); + expect(result.loginRequired).toBe(true); + // Must NOT expose client metadata to unauthenticated callers + expect(result.client).toBeUndefined(); + expect(result.scopes).toBeUndefined(); + }); +}); + +// --------------------------------------------------------------------------- +// Wrapper surface that survived the fold, plus the branches the legacy suite +// never reached (they were free functions nobody could drive in isolation). +// --------------------------------------------------------------------------- + +describe('addon gate and MCP endpoint', () => { it('mcpEnabled checks the MCP addon flag', () => { isAddonEnabled.mockReturnValue(true); - expect(svc().mcpEnabled()).toBe(true); + expect(svc.mcpEnabled()).toBe(true); expect(isAddonEnabled).toHaveBeenCalledWith(ADDON_IDS.MCP); isAddonEnabled.mockReturnValue(false); - expect(svc().mcpEnabled()).toBe(false); + expect(svc.mcpEnabled()).toBe(false); }); it('mcpSafeUrl forwards to the app-config helper', () => { - getMcpSafeUrl.mockReturnValue('https://safe'); - expect(svc().mcpSafeUrl()).toBe('https://safe'); - expect(getMcpSafeUrl).toHaveBeenCalled(); + expect(svc.mcpSafeUrl()).toBe(getMcpSafeUrl()); + }); +}); + +describe('pending-code store', () => { + const codeParams = { + clientId: 'c', + userId: 42, + redirectUri: 'https://example.com/callback', + scopes: ['trips:read'], + resource: null, + codeChallenge: 'x', + codeChallengeMethod: 'S256' as const, + }; + + it('refuses a new code at capacity and the sweep frees it again', () => { + for (let i = 0; i < MAX_PENDING_CODES; i++) createAuthCode(codeParams); + + expect(createAuthCode(codeParams)).toBeNull(); + + // Everything in the store is past its 2-minute TTL by then. + sweepPendingCodes(Date.now() + 3 * 60 * 1000); + + const afterSweep = createAuthCode(codeParams); + expect(afterSweep).not.toBeNull(); + sweepPendingCodes(Date.now() + 3 * 60 * 1000); }); - it('consumeAuthCode delegates', () => { - oauth.consumeAuthCode.mockReturnValue({ clientId: 'c' }); - expect(svc().consumeAuthCode('code')).toEqual({ clientId: 'c' }); - expect(oauth.consumeAuthCode).toHaveBeenCalledWith('code'); + it('a code past its TTL is consumed as invalid', () => { + const code = createAuthCode(codeParams)!; + const realNow = Date.now; + Date.now = () => realNow() + 3 * 60 * 1000; + try { + expect(consumeAuthCode(code)).toBeNull(); + } finally { + Date.now = realNow; + } + }); +}); + +describe('branches the legacy suite could not reach', () => { + it('rejects an expired access token', () => { + const { user } = createUser(testDb); + const created = makeClient(user.id); + const clientId = (created.client as { client_id: string }).client_id; + const tokens = issueTokens(clientId, user.id, ['trips:read']); + testDb.prepare("UPDATE oauth_tokens SET access_token_expires_at = '2000-01-01T00:00:00.000Z'").run(); + + expect(getUserByAccessToken(tokens.access_token)).toBeNull(); }); - it('authenticateClient delegates with both args', () => { - oauth.authenticateClient.mockReturnValue({ id: 'c' }); - expect(svc().authenticateClient('c', 'secret')).toEqual({ id: 'c' }); - expect(oauth.authenticateClient).toHaveBeenCalledWith('c', 'secret'); + it('refreshTokens rejects an unknown client before touching the token', () => { + expect(refreshTokens('trekrf_whatever', 'no-such-client', 'secret')).toEqual({ error: 'invalid_client', status: 401 }); }); - it('verifyPKCE delegates', () => { - oauth.verifyPKCE.mockReturnValue(true); - expect(svc().verifyPKCE('v', 'ch')).toBe(true); - expect(oauth.verifyPKCE).toHaveBeenCalledWith('v', 'ch'); + it('refreshTokens skips the secret check for a public client', () => { + const { user } = createUser(testDb); + const created = createOAuthClient(user.id, 'Public', ['https://example.com/callback'], ['trips:read'], null, { isPublic: true }); + const clientId = (created.client as { client_id: string }).client_id; + const tokens = issueTokens(clientId, user.id, ['trips:read']); + + const result = refreshTokens(tokens.refresh_token, clientId, undefined); + + expect(result.error).toBeUndefined(); + expect(result.tokens?.access_token).toMatch(/^trekoa_/); }); - it('issueTokens forwards the full argument list', () => { - oauth.issueTokens.mockReturnValue({ access_token: 'at' }); - expect(svc().issueTokens('c', 1, ['s'], null, 'aud')).toEqual({ access_token: 'at' }); - expect(oauth.issueTokens).toHaveBeenCalledWith('c', 1, ['s'], null, 'aud'); + it('authenticateClient identifies a public client by id alone and rejects a missing secret otherwise', () => { + const { user } = createUser(testDb); + const pub = createOAuthClient(user.id, 'Public', ['https://example.com/callback'], ['trips:read'], null, { isPublic: true }); + const pubId = (pub.client as { client_id: string }).client_id; + const conf = makeClient(user.id, { name: 'Confidential' }); + const confId = (conf.client as { client_id: string }).client_id; + + expect(authenticateClient(pubId, undefined)?.client_id).toBe(pubId); + expect(authenticateClient(confId, undefined)).toBeNull(); }); - it('issueClientCredentialsToken forwards the full argument list', () => { - oauth.issueClientCredentialsToken.mockReturnValue({ access_token: 'cc' }); - expect(svc().issueClientCredentialsToken('c', 1, ['s'], 'aud')).toEqual({ access_token: 'cc' }); - expect(oauth.issueClientCredentialsToken).toHaveBeenCalledWith('c', 1, ['s'], 'aud'); + it('validateAuthorizeRequest rejects a resource that is not the MCP endpoint', () => { + const { user } = createUser(testDb); + const created = makeClient(user.id); + const clientId = (created.client as { client_id: string }).client_id; + const { challenge } = makePkce(); + + const result = validateAuthorizeRequest({ + response_type: 'code', + client_id: clientId, + redirect_uri: 'https://example.com/callback', + scope: 'trips:read', + code_challenge: challenge, + code_challenge_method: 'S256', + resource: 'https://evil.example.org/mcp', + }, user.id); + + expect(result).toEqual({ valid: false, error: 'invalid_target', error_description: 'Requested resource must be the TREK MCP endpoint' }); }); - it('refreshTokens forwards the full argument list', () => { - oauth.refreshTokens.mockReturnValue({ tokens: { access_token: 'new' } }); - expect(svc().refreshTokens('rt', 'c', 's', '1.2.3.4')).toEqual({ tokens: { access_token: 'new' } }); - expect(oauth.refreshTokens).toHaveBeenCalledWith('rt', 'c', 's', '1.2.3.4'); + it('validateAuthorizeRequest accepts the MCP endpoint passed explicitly, trailing slash and all', () => { + const { user } = createUser(testDb); + const created = makeClient(user.id); + const clientId = (created.client as { client_id: string }).client_id; + const { challenge } = makePkce(); + const mcpResource = getMcpSafeUrl().replace(/\/+$/, '') + '/mcp'; + + const result = validateAuthorizeRequest({ + response_type: 'code', + client_id: clientId, + redirect_uri: 'https://example.com/callback', + scope: 'trips:read', + code_challenge: challenge, + code_challenge_method: 'S256', + resource: mcpResource + '/', + }, user.id); + + expect(result.valid).toBe(true); + expect(result.resource).toBe(mcpResource); + }); +}); + +describe('oauth.bridge delegation', () => { + // The bridge is what the SDK-mounted /oauth/authorize path and the MCP + // transport reach, both outside the container. + it('issues and verifies a token through the bridge instance', () => { + const { user } = createUser(testDb); + const created = makeClient(user.id); + const clientId = (created.client as { client_id: string }).client_id; + + const tokens = bridge.issueTokens(clientId, user.id, ['trips:read']); + + expect(bridge.getUserByAccessToken(tokens.access_token)?.user.id).toBe(user.id); }); - it('revokeToken forwards the full argument list', () => { - svc().revokeToken('t', 'c', undefined, '1.2.3.4'); - expect(oauth.revokeToken).toHaveBeenCalledWith('t', 'c', undefined, '1.2.3.4'); + it('shares the pending-code map with the container instance', () => { + // The load-bearing invariant: the consent controller writes the code through + // the DI singleton, the SDK provider reads it back through the bridge. Two + // maps here would kill the authorization-code flow silently. + const code = createAuthCode({ + clientId: 'c', + userId: 42, + redirectUri: 'https://example.com/callback', + scopes: ['trips:read'], + resource: null, + codeChallenge: 'x', + codeChallengeMethod: 'S256', + })!; + + expect(bridge.consumeAuthCode(code)?.userId).toBe(42); }); - it('getUserByAccessToken delegates', () => { - oauth.getUserByAccessToken.mockReturnValue({ user: { id: 1 } }); - expect(svc().getUserByAccessToken('tok')).toEqual({ user: { id: 1 } }); - expect(oauth.getUserByAccessToken).toHaveBeenCalledWith('tok'); + it('verifyPKCE and revokeToken reach the same rows', () => { + const { verifier, challenge } = makePkce(); + expect(bridge.verifyPKCE(verifier, challenge)).toBe(true); + + const { user } = createUser(testDb); + const created = makeClient(user.id); + const clientId = (created.client as { client_id: string }).client_id; + const tokens = issueTokens(clientId, user.id, ['trips:read']); + + bridge.revokeToken(tokens.access_token, clientId, user.id); + + expect(getUserByAccessToken(tokens.access_token)).toBeNull(); }); - it('validateAuthorizeRequest delegates with the user id', () => { - oauth.validateAuthorizeRequest.mockReturnValue({ valid: true }); - const params = { response_type: 'code' } as never; - expect(svc().validateAuthorizeRequest(params, 5)).toEqual({ valid: true }); - expect(oauth.validateAuthorizeRequest).toHaveBeenCalledWith(params, 5); - }); + it('registers a client and refreshes a token pair through the bridge', () => { + const { user } = createUser(testDb); + const created = bridge.createOAuthClient(user.id, 'Via bridge', ['https://example.com/callback'], ['trips:read']); + const client = created.client as { client_id: string; client_secret: string }; + const tokens = issueTokens(client.client_id, user.id, ['trips:read']); - it('saveConsent forwards the full argument list', () => { - svc().saveConsent('c', 1, ['s'], '1.2.3.4'); - expect(oauth.saveConsent).toHaveBeenCalledWith('c', 1, ['s'], '1.2.3.4'); - }); + const refreshed = bridge.refreshTokens(tokens.refresh_token, client.client_id, client.client_secret); - it('createAuthCode forwards the params object', () => { - oauth.createAuthCode.mockReturnValue('the_code'); - const p = { clientId: 'c', userId: 1, redirectUri: 'u', scopes: ['s'], resource: null, codeChallenge: 'cc', codeChallengeMethod: 'S256' } as const; - expect(svc().createAuthCode(p)).toBe('the_code'); - expect(oauth.createAuthCode).toHaveBeenCalledWith(p); - }); - - it('listOAuthClients delegates', () => { - oauth.listOAuthClients.mockReturnValue([{ id: 'c1' }]); - expect(svc().listOAuthClients(1)).toEqual([{ id: 'c1' }]); - expect(oauth.listOAuthClients).toHaveBeenCalledWith(1); - }); - - it('createOAuthClient forwards the full argument list', () => { - oauth.createOAuthClient.mockReturnValue({ client_id: 'c1' }); - expect(svc().createOAuthClient(1, 'CLI', ['https://cb'], ['a'], '1.2.3.4', { allowsClientCredentials: true })).toEqual({ client_id: 'c1' }); - expect(oauth.createOAuthClient).toHaveBeenCalledWith(1, 'CLI', ['https://cb'], ['a'], '1.2.3.4', { allowsClientCredentials: true }); - }); - - it('rotateOAuthClientSecret delegates', () => { - oauth.rotateOAuthClientSecret.mockReturnValue({ client_secret: 'new' }); - expect(svc().rotateOAuthClientSecret(1, 'c1', '1.2.3.4')).toEqual({ client_secret: 'new' }); - expect(oauth.rotateOAuthClientSecret).toHaveBeenCalledWith(1, 'c1', '1.2.3.4'); - }); - - it('deleteOAuthClient delegates', () => { - oauth.deleteOAuthClient.mockReturnValue({}); - expect(svc().deleteOAuthClient(1, 'c1', '1.2.3.4')).toEqual({}); - expect(oauth.deleteOAuthClient).toHaveBeenCalledWith(1, 'c1', '1.2.3.4'); - }); - - it('listOAuthSessions delegates', () => { - oauth.listOAuthSessions.mockReturnValue([{ id: 1 }]); - expect(svc().listOAuthSessions(1)).toEqual([{ id: 1 }]); - expect(oauth.listOAuthSessions).toHaveBeenCalledWith(1); - }); - - it('revokeSession delegates', () => { - oauth.revokeSession.mockReturnValue({}); - expect(svc().revokeSession(1, 7, '1.2.3.4')).toEqual({}); - expect(oauth.revokeSession).toHaveBeenCalledWith(1, 7, '1.2.3.4'); + expect(refreshed.tokens?.refresh_token).toMatch(/^trekrf_/); }); }); diff --git a/server/tests/unit/nest/passkey.service.test.ts b/server/tests/unit/nest/passkey.service.test.ts index 0c9df4916..8a6344f4d 100644 --- a/server/tests/unit/nest/passkey.service.test.ts +++ b/server/tests/unit/nest/passkey.service.test.ts @@ -63,15 +63,6 @@ vi.mock('../../../src/scheduler', () => ({ VALID_INTERVALS: ['daily', 'weekly', 'monthly'], })); -// The RP config resolver is a plain helper with its own suite -// (webauthnConfig.test.ts); here it is a switch for the configured/ -// not-configured branches. isPasskeyConfigured stays real for auth.service. -const { resolveWebauthnConfigMock } = vi.hoisted(() => ({ resolveWebauthnConfigMock: vi.fn() })); -vi.mock('../../../src/services/webauthnConfig', async (importOriginal) => { - const actual = await importOriginal(); - return { ...actual, resolveWebauthnConfig: resolveWebauthnConfigMock }; -}); - // The WebAuthn ceremony verdicts are the mock boundary — the service must // persist ONLY what the verifier vouches for, so the tests hand it verdicts. const { swMock } = vi.hoisted(() => ({ @@ -94,13 +85,20 @@ import { PermissionsService } from '../../../src/nest/permissions/permissions.se import { AtlasService } from '../../../src/nest/atlas/atlas.service'; import { AuthService } from '../../../src/nest/auth/auth.service'; import { PasskeyService } from '../../../src/nest/auth/passkey.service'; +import type { WebauthnConfigService } from '../../../src/nest/auth/webauthn-config.service'; + +// The RP config resolver has its own suite (webauthn-config.service.test.ts); +// here it is a switch for the configured / not-configured branches, handed in +// as a stub now that PasskeyService injects it. +const resolveWebauthnConfigMock = vi.fn(); +const webauthn = { resolve: resolveWebauthnConfigMock } as unknown as WebauthnConfigService; const auth = new AuthService( new DatabaseService(testDb), new PermissionsService(new DatabaseService(testDb)), new AtlasService(new DatabaseService(testDb)), ); -const svc = new PasskeyService(new DatabaseService(testDb), auth); +const svc = new PasskeyService(new DatabaseService(testDb), auth, webauthn); const CFG = { rpID: 'trek.example.com', rpName: 'TREK', origins: ['https://trek.example.com'] }; const NOT_CONFIGURED_ERROR = 'Passkey login is not configured for this server.'; diff --git a/server/tests/unit/nest/trips.service.test.ts b/server/tests/unit/nest/trips.service.test.ts index 888a2e80a..68c1c4229 100644 --- a/server/tests/unit/nest/trips.service.test.ts +++ b/server/tests/unit/nest/trips.service.test.ts @@ -64,6 +64,7 @@ import { CollabService } from '../../../src/nest/collab/collab.service'; import { VacayService } from '../../../src/nest/vacay/vacay.service'; import { TripsService } from '../../../src/nest/trips/trips.service'; import { PlacesService } from '../../../src/nest/places/places.service'; +import { UserCleanupService } from '../../../src/nest/auth/user-cleanup.service'; import { MapsService } from '../../../src/nest/maps/maps.service'; import { getTripOwner, listMembers as bridgeListMembers } from '../../../src/nest/trips/trips.bridge'; import { QueryHelpersService } from '../../../src/nest/query-helpers/query-helpers.service'; @@ -90,6 +91,8 @@ const svc = new TripsService( new VacayService(dbs(), new RealtimeService()), new RealtimeService(), placesSvc, + undefined as never, // unsplash — not exercised here + new UserCleanupService(dbs()), ); beforeAll(() => { @@ -1206,6 +1209,8 @@ describe('quirk fixes', () => { new VacayService(dbs(), new RealtimeService()), new RealtimeService(), placesSvc, + undefined as never, // unsplash — not exercised here + new UserCleanupService(dbs()), ); } diff --git a/server/tests/unit/nest/user-cleanup.service.test.ts b/server/tests/unit/nest/user-cleanup.service.test.ts new file mode 100644 index 000000000..ec19e4888 --- /dev/null +++ b/server/tests/unit/nest/user-cleanup.service.test.ts @@ -0,0 +1,223 @@ +/** + * DB-backed unit tests for UserCleanupService (USER-CLEANUP-001+). + * + * The legacy services/userCleanupService had no tests of its own; the erasure + * path only ever ran incidentally through AdminService.deleteUser and + * TripsService.deleteGuest. Both halves are pinned here directly: the plugin + * erasure (host-side tables + the durable per-plugin queue, including the + * orphan-data-dir case that no permissions row can describe) and the + * account-deletion transaction (reference cleanup + budget re-split + the + * users row, all or nothing). + */ +import { describe, it, expect, vi, beforeAll, beforeEach, afterAll } from 'vitest'; + +const { testDb, dbMock, dataRootRef } = vi.hoisted(() => { + const Database = require('better-sqlite3'); + const db = new Database(':memory:'); + db.exec('PRAGMA journal_mode = WAL'); + db.exec('PRAGMA foreign_keys = ON'); + return { + testDb: db, + dbMock: { + db, + closeDb: () => {}, + reinitialize: () => {}, + getPlaceWithTags: () => null, + canAccessTrip: () => null, + isOwner: () => false, + }, + // Points at a directory that does not exist by default, so the orphan scan + // takes its "no plugin data root yet" branch unless a test says otherwise. + dataRootRef: { value: '' }, + }; +}); + +vi.mock('../../../src/db/database', () => dbMock); +vi.mock('../../../src/websocket', () => ({ broadcast: vi.fn() })); +vi.mock('../../../src/nest/plugins/paths', () => ({ pluginsDataRoot: () => dataRootRef.value })); + +import fs from 'node:fs'; +import os from 'node:os'; +import path from 'node:path'; +import { createTables } from '../../../src/db/schema'; +import { runMigrations } from '../../../src/db/migrations'; +import { resetTestDb } from '../../helpers/test-db'; +import { createUser, createTrip } from '../../helpers/factories'; +import { DatabaseService } from '../../../src/nest/database/database.service'; +import { PermissionsService } from '../../../src/nest/permissions/permissions.service'; +import { ExchangeRatesService } from '../../../src/nest/budget/exchange-rates.service'; +import { RealtimeService } from '../../../src/nest/realtime/realtime.service'; +import { BudgetService } from '../../../src/nest/budget/budget.service'; +import { UserCleanupService } from '../../../src/nest/auth/user-cleanup.service'; + +const dbs = new DatabaseService(testDb); +const budget = new BudgetService(dbs, new PermissionsService(dbs), new ExchangeRatesService(), new RealtimeService()); +const svc = new UserCleanupService(dbs); + +const installPlugin = (id: string, permissions: string[] | null) => { + testDb.prepare('INSERT INTO plugins (id, name, version, permissions) VALUES (?, ?, ?, ?)') + .run(id, id, '1.0.0', permissions === null ? null : JSON.stringify(permissions)); +}; + +const createJourney = (userId: number, title: string): number => + Number(testDb.prepare("INSERT INTO journeys (user_id, title, status, created_at, updated_at) VALUES (?, ?, 'draft', 0, 0)") + .run(userId, title).lastInsertRowid); + +const queuedFor = (userId: number): string[] => + (testDb.prepare('SELECT plugin_id FROM plugin_user_erasure_queue WHERE user_id = ? ORDER BY plugin_id') + .all(userId) as Array<{ plugin_id: string }>).map(r => r.plugin_id); + +beforeAll(() => { + createTables(testDb); + runMigrations(testDb); +}); + +beforeEach(() => { + resetTestDb(testDb); + // The plugin tables are not user data, so resetTestDb leaves them alone — + // these tests own them and must not leak rows into each other. + for (const t of ['plugin_user_erasure_queue', 'plugin_user_config', 'plugins']) { + testDb.prepare(`DELETE FROM ${t}`).run(); + } + dataRootRef.value = path.join(os.tmpdir(), 'trek-user-cleanup-absent'); +}); + +afterAll(() => { + testDb.close(); +}); + +describe('erasePluginUserData', () => { + it('USER-CLEANUP-001: deletes the host-side per-user plugin rows', () => { + const { user } = createUser(testDb); + const { user: other } = createUser(testDb, { username: 'other' }); + installPlugin('demo', []); + testDb.prepare('INSERT INTO plugin_user_config (plugin_id, user_id, config) VALUES (?, ?, ?)') + .run('demo', user.id, '{"token":"secret"}'); + testDb.prepare('INSERT INTO plugin_user_config (plugin_id, user_id, config) VALUES (?, ?, ?)') + .run('demo', other.id, '{"token":"keep-me"}'); + + svc.erasePluginUserData(user.id); + + const rows = testDb.prepare('SELECT user_id FROM plugin_user_config').all() as Array<{ user_id: number }>; + expect(rows.map(r => r.user_id)).toEqual([other.id]); + }); + + it('USER-CLEANUP-002: enqueues an erasure only for plugins holding hook:user-data', () => { + const { user } = createUser(testDb); + installPlugin('with-hook', ['hook:user-data', 'trips:read']); + installPlugin('without-hook', ['trips:read']); + installPlugin('no-permissions', null); + + svc.erasePluginUserData(user.id); + + expect(queuedFor(user.id)).toEqual(['with-hook']); + }); + + it('USER-CLEANUP-003: treats an unparseable permissions column as no permissions', () => { + const { user } = createUser(testDb); + testDb.prepare('INSERT INTO plugins (id, name, version, permissions) VALUES (?, ?, ?, ?)') + .run('broken', 'broken', '1.0.0', '{not json'); + + svc.erasePluginUserData(user.id); + + expect(queuedFor(user.id)).toEqual([]); + }); + + it('USER-CLEANUP-004: enqueues every orphan data dir — an uninstall keeps no permissions row', () => { + const { user } = createUser(testDb); + const root = fs.mkdtempSync(path.join(os.tmpdir(), 'trek-plugins-data-')); + dataRootRef.value = root; + fs.mkdirSync(path.join(root, 'uninstalled-but-retained')); + fs.writeFileSync(path.join(root, 'stray-file'), ''); // not a directory → ignored + installPlugin('installed', ['hook:user-data']); + fs.mkdirSync(path.join(root, 'installed')); + + try { + svc.erasePluginUserData(user.id); + // 'installed' comes from the permissions scan, not the orphan scan, and the + // INSERT OR IGNORE keeps it single. + expect(queuedFor(user.id)).toEqual(['installed', 'uninstalled-but-retained']); + } finally { + fs.rmSync(root, { recursive: true, force: true }); + } + }); + + it('USER-CLEANUP-005: survives a slim schema without the plugin tables', () => { + const slim = new (require('better-sqlite3'))(':memory:'); + slim.exec('CREATE TABLE users (id INTEGER PRIMARY KEY)'); + slim.prepare('INSERT INTO users (id) VALUES (1)').run(); + const slimSvc = new UserCleanupService(new DatabaseService(slim)); + + expect(() => slimSvc.erasePluginUserData(1)).not.toThrow(); + + slim.close(); + }); +}); + +describe('deleteUserCompletely', () => { + it('USER-CLEANUP-006: removes the user and nulls the references that have no cascade', () => { + const { user: owner } = createUser(testDb); + const { user: victim } = createUser(testDb, { username: 'victim' }); + const trip = createTrip(testDb, owner.id); + testDb.prepare('INSERT INTO trip_members (trip_id, user_id, invited_by) VALUES (?, ?, ?)') + .run(trip.id, owner.id, victim.id); + testDb.prepare("INSERT INTO share_tokens (trip_id, token, created_by) VALUES (?, 'tok', ?)") + .run(trip.id, victim.id); + + svc.deleteUserCompletely(victim.id); + + expect(testDb.prepare('SELECT id FROM users WHERE id = ?').get(victim.id)).toBeUndefined(); + expect((testDb.prepare('SELECT invited_by FROM trip_members WHERE user_id = ?').get(owner.id) as { invited_by: number | null }).invited_by).toBeNull(); + expect(testDb.prepare('SELECT COUNT(*) AS c FROM share_tokens').get()).toEqual({ c: 0 }); + }); + + it('USER-CLEANUP-007: deletes their journeys and the entries they authored elsewhere', () => { + const { user: owner } = createUser(testDb); + const { user: victim } = createUser(testDb, { username: 'victim' }); + const ownJourney = createJourney(victim.id, 'Mine'); + const foreignJourney = createJourney(owner.id, 'Theirs'); + testDb.prepare("INSERT INTO journey_entries (journey_id, author_id, type, title, entry_date, created_at, updated_at) VALUES (?, ?, 'note', 'Guest post', '2026-08-08', 0, 0)") + .run(foreignJourney, victim.id); + + svc.deleteUserCompletely(victim.id); + + expect(testDb.prepare('SELECT id FROM journeys WHERE id = ?').get(ownJourney)).toBeUndefined(); + expect(testDb.prepare('SELECT id FROM journeys WHERE id = ?').get(foreignJourney)).toBeDefined(); + expect(testDb.prepare('SELECT COUNT(*) AS c FROM journey_entries').get()).toEqual({ c: 0 }); + }); + + it('USER-CLEANUP-008: re-derives the expense divisor before the member rows cascade away', () => { + const { user: owner } = createUser(testDb); + const { user: victim } = createUser(testDb, { username: 'victim' }); + const trip = createTrip(testDb, owner.id); + testDb.prepare('INSERT INTO trip_members (trip_id, user_id) VALUES (?, ?)').run(trip.id, victim.id); + const item = budget.createBudgetItem(trip.id, { name: 'Dinner', total_price: 80, member_ids: [owner.id, victim.id] }); + testDb.prepare('UPDATE budget_items SET paid_by_user_id = ? WHERE id = ?').run(victim.id, item.id); + + svc.deleteUserCompletely(victim.id); + + const row = testDb.prepare('SELECT persons, paid_by_user_id FROM budget_items WHERE id = ?').get(item.id) as { persons: number | null; paid_by_user_id: number | null }; + expect(row).toEqual({ persons: 1, paid_by_user_id: null }); + }); + + it('USER-CLEANUP-009: is atomic — a failing users DELETE rolls the reference cleanup back', () => { + const { user: owner } = createUser(testDb); + const { user: victim } = createUser(testDb, { username: 'victim' }); + const trip = createTrip(testDb, owner.id); + testDb.prepare('INSERT INTO trip_members (trip_id, user_id, invited_by) VALUES (?, ?, ?)') + .run(trip.id, owner.id, victim.id); + + // Fail on the final statement only, after the reference cleanup has written. + const failing = new DatabaseService(testDb); + const realRun = failing.run.bind(failing); + vi.spyOn(failing, 'run').mockImplementation((sql: string, ...params: unknown[]) => { + if (sql.startsWith('DELETE FROM users')) throw new Error('boom'); + return realRun(sql, ...params); + }); + + expect(() => new UserCleanupService(failing).deleteUserCompletely(victim.id)).toThrow('boom'); + + expect(testDb.prepare('SELECT id FROM users WHERE id = ?').get(victim.id)).toBeDefined(); + expect((testDb.prepare('SELECT invited_by FROM trip_members WHERE user_id = ?').get(owner.id) as { invited_by: number | null }).invited_by).toBe(victim.id); + }); +}); diff --git a/server/tests/unit/services/webauthnConfig.test.ts b/server/tests/unit/nest/webauthn-config.service.test.ts similarity index 66% rename from server/tests/unit/services/webauthnConfig.test.ts rename to server/tests/unit/nest/webauthn-config.service.test.ts index 6e2f0f904..090391b37 100644 --- a/server/tests/unit/services/webauthnConfig.test.ts +++ b/server/tests/unit/nest/webauthn-config.service.test.ts @@ -1,5 +1,5 @@ /** - * webauthnConfig.test.ts + * webauthn-config.service.test.ts * * The RP-ID / allowed-origin resolver is the single highest-risk piece of the * passkey feature: a wrong RP ID permanently bricks every enrolled credential. @@ -13,24 +13,28 @@ const { settingsStore, appUrlRef } = vi.hoisted(() => ({ appUrlRef: { value: '' }, })); -vi.mock('../../../src/db/database', () => ({ - db: { - prepare: (_sql: string) => ({ - get: (key: string) => { - const v = settingsStore.get(key); - return v === undefined ? undefined : { value: v }; - }, - }), - }, -})); - vi.mock('../../../src/app-config', async (importOriginal) => { const actual = await importOriginal(); return { ...actual, getAppUrl: () => appUrlRef.value }; }); import { describe, it, expect, beforeEach, afterEach, vi } from 'vitest'; -import { resolveWebauthnConfig, isPasskeyConfigured } from '../../../src/services/webauthnConfig'; +import { DatabaseService } from '../../../src/nest/database/database.service'; +import { WebauthnConfigService } from '../../../src/nest/auth/webauthn-config.service'; + +// Injected instead of vi.mocking src/db/database: the service reads exactly one +// row shape (app_settings.value), so a stub connection keeps the suite free of +// a schema and states the only DB contact the resolver has. +const conn = { + prepare: (_sql: string) => ({ + get: (key: string) => { + const v = settingsStore.get(key); + return v === undefined ? undefined : { value: v }; + }, + }), +} as unknown as ConstructorParameters[0]; + +const svc = new WebauthnConfigService(new DatabaseService(conn)); beforeEach(() => { settingsStore.clear(); @@ -41,30 +45,30 @@ afterEach(() => { vi.unstubAllEnvs(); }); -describe('resolveWebauthnConfig', () => { +describe('WebauthnConfigService.resolve', () => { it('WAC-001: derives the RP ID and single origin from a real APP_URL domain', () => { appUrlRef.value = 'https://trek.example.org'; - const cfg = resolveWebauthnConfig(); + const cfg = svc.resolve(); expect(cfg).not.toBeNull(); expect(cfg!.rpID).toBe('trek.example.org'); expect(cfg!.origins).toEqual(['https://trek.example.org']); - expect(isPasskeyConfigured()).toBe(true); + expect(svc.isConfigured()).toBe(true); }); it('WAC-002: returns null for a bare-IP host (IPs are not valid RP IDs)', () => { appUrlRef.value = 'http://192.168.1.50:3001'; - expect(resolveWebauthnConfig()).toBeNull(); - expect(isPasskeyConfigured()).toBe(false); + expect(svc.resolve()).toBeNull(); + expect(svc.isConfigured()).toBe(false); }); it('WAC-003: returns null when nothing is configured', () => { - expect(resolveWebauthnConfig()).toBeNull(); - expect(isPasskeyConfigured()).toBe(false); + expect(svc.resolve()).toBeNull(); + expect(svc.isConfigured()).toBe(false); }); it('WAC-004: localhost dev uses the browser (Vite :5173) origin, not just the API port', () => { appUrlRef.value = 'http://localhost:3001'; - const cfg = resolveWebauthnConfig(); + const cfg = svc.resolve(); expect(cfg!.rpID).toBe('localhost'); expect(cfg!.origins).toContain('http://localhost:5173'); expect(cfg!.origins).toContain('http://localhost:3001'); @@ -74,7 +78,7 @@ describe('resolveWebauthnConfig', () => { appUrlRef.value = 'https://internal.example.org'; settingsStore.set('webauthn_rp_id', 'public.example.org'); settingsStore.set('webauthn_origins', 'https://public.example.org'); - const cfg = resolveWebauthnConfig(); + const cfg = svc.resolve(); expect(cfg!.rpID).toBe('public.example.org'); expect(cfg!.origins).toEqual(['https://public.example.org']); }); @@ -82,7 +86,7 @@ describe('resolveWebauthnConfig', () => { it('WAC-006: webauthn_origins is parsed as a comma-separated, trimmed list', () => { settingsStore.set('webauthn_rp_id', 'example.org'); settingsStore.set('webauthn_origins', 'https://a.example.org , https://b.example.org/'); - const cfg = resolveWebauthnConfig(); + const cfg = svc.resolve(); expect(cfg!.origins).toEqual(['https://a.example.org', 'https://b.example.org']); }); @@ -90,7 +94,7 @@ describe('resolveWebauthnConfig', () => { vi.stubEnv('WEBAUTHN_RP_ID', 'env.example.org'); vi.stubEnv('WEBAUTHN_ORIGINS', 'https://env.example.org'); appUrlRef.value = 'https://ignored.example.org'; - const cfg = resolveWebauthnConfig(); + const cfg = svc.resolve(); expect(cfg!.rpID).toBe('env.example.org'); expect(cfg!.origins).toEqual(['https://env.example.org']); }); @@ -98,7 +102,16 @@ describe('resolveWebauthnConfig', () => { it('WAC-008: a configured RP ID with no origins falls back to the APP_URL origin', () => { appUrlRef.value = 'https://trek.example.org'; settingsStore.set('webauthn_rp_id', 'trek.example.org'); - const cfg = resolveWebauthnConfig(); + const cfg = svc.resolve(); expect(cfg!.origins).toEqual(['https://trek.example.org']); }); + + it('WAC-009: an operator flipping the RP ID at runtime takes effect on the next resolve', () => { + appUrlRef.value = 'https://trek.example.org'; + expect(svc.resolve()!.rpID).toBe('trek.example.org'); + + settingsStore.set('webauthn_rp_id', 'moved.example.org'); + + expect(svc.resolve()!.rpID).toBe('moved.example.org'); + }); }); diff --git a/server/tests/unit/services/wikiService.test.ts b/server/tests/unit/nest/wiki.test.ts similarity index 95% rename from server/tests/unit/services/wikiService.test.ts rename to server/tests/unit/nest/wiki.test.ts index bad4fefdf..99b212130 100644 --- a/server/tests/unit/services/wikiService.test.ts +++ b/server/tests/unit/nest/wiki.test.ts @@ -1,18 +1,18 @@ import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest'; import path from 'path'; -// wikiService picks its source (local disk vs GitHub) once at module load, so each +// The wiki module picks its source (local disk vs GitHub) once at module load, so each // mode needs a fresh import with TREK_WIKI_DIR already set. const FIXTURE_WIKI = path.join(__dirname, '..', '..', 'fixtures', 'wiki'); const MISSING_WIKI = path.join(__dirname, '..', '..', 'fixtures', 'no-such-wiki'); -type WikiModule = typeof import('../../../src/services/wikiService'); +type WikiModule = typeof import('../../../src/nest/help/wiki'); async function loadWiki(dir: string | undefined): Promise { vi.resetModules(); if (dir === undefined) delete process.env.TREK_WIKI_DIR; else process.env.TREK_WIKI_DIR = dir; - return import('../../../src/services/wikiService'); + return import('../../../src/nest/help/wiki'); } const fetchSpy = vi.fn(); @@ -27,7 +27,7 @@ afterEach(() => { delete process.env.TREK_WIKI_DIR; }); -describe('wikiService — local wiki on disk', () => { +describe('wiki — local wiki on disk', () => { it('serves the sidebar from disk and never touches the network', async () => { const wiki = await loadWiki(FIXTURE_WIKI); expect(wiki.isLocalWiki()).toBe(true); @@ -144,7 +144,7 @@ describe('wikiService — local wiki on disk', () => { }); }); -describe('wikiService — GitHub fallback when no wiki is on disk', () => { +describe('wiki — GitHub fallback when no wiki is on disk', () => { const ok = (body: string): Response => ({ ok: true, status: 200, text: async () => body, arrayBuffer: async () => new ArrayBuffer(4) }) as unknown as Response; @@ -183,7 +183,7 @@ describe('wikiService — GitHub fallback when no wiki is on disk', () => { }); }); -describe('wikiService — default path resolution', () => { +describe('wiki — default path resolution', () => { it("resolves the repo's wiki/ with no override, so a release never ships without docs", async () => { // Guards the __dirname anchor and the Dockerfile COPY: if either breaks, help // silently degrades to the GitHub fallback and nobody notices. diff --git a/server/tests/unit/services/oauthService.test.ts b/server/tests/unit/services/oauthService.test.ts deleted file mode 100644 index 0b97c9759..000000000 --- a/server/tests/unit/services/oauthService.test.ts +++ /dev/null @@ -1,958 +0,0 @@ -/** - * Unit tests for server/src/services/oauthService.ts. - */ -import { describe, it, expect, vi, beforeAll, beforeEach, afterAll } from 'vitest'; -import crypto from 'crypto'; - -const { testDb, dbMock } = vi.hoisted(() => { - const Database = require('better-sqlite3'); - const db = new Database(':memory:'); - db.exec('PRAGMA journal_mode = WAL'); - db.exec('PRAGMA foreign_keys = ON'); - db.exec('PRAGMA busy_timeout = 5000'); - const mock = { - db, - closeDb: () => {}, - reinitialize: () => {}, - getPlaceWithTags: () => null, - canAccessTrip: (tripId: any, userId: number) => - db.prepare(`SELECT t.id, t.user_id FROM trips t LEFT JOIN trip_members m ON m.trip_id = t.id AND m.user_id = ? WHERE t.id = ? AND (t.user_id = ? OR m.user_id IS NOT NULL)`).get(userId, tripId, userId), - isOwner: (tripId: any, userId: number) => - !!db.prepare('SELECT id FROM trips WHERE id = ? AND user_id = ?').get(tripId, userId), - }; - return { testDb: db, dbMock: mock }; -}); - -vi.mock('../../../src/db/database', () => dbMock); -vi.mock('../../../src/config', () => ({ - JWT_SECRET: 'test-jwt-secret-for-trek-testing-only', - ENCRYPTION_KEY: 'a1b2c3d4e5f6a7b8c9d0e1f2a3b4c5d6a7b8c9d0e1f2a3b4c5d6a7b8c9d0e1f2', - updateJwtSecret: () => {}, -})); -vi.mock('../../../src/nest/common/crypto/apiKeyCrypto', () => ({ - encrypt_api_key: (v: string) => v, - decrypt_api_key: (v: string) => v, - maybe_encrypt_api_key: (v: string) => v, -})); -vi.mock('../../../src/mcp/sessionManager', () => ({ revokeUserSessions: vi.fn(), revokeUserSessionsForClient: vi.fn(), sessions: new Map() })); -import { revokeUserSessionsForClient } from '../../../src/mcp/sessionManager'; -vi.mock('../../../src/demo/demo-reset', () => ({ saveBaseline: vi.fn() })); -vi.mock('../../../src/nest/addons/addons.bridge', () => ({ - isAddonEnabled: vi.fn().mockReturnValue(true), - getCollabFeatures: vi.fn().mockReturnValue({ chat: true, notes: true, polls: true, whatsnext: true }), -})); - -import { createTables } from '../../../src/db/schema'; -import { runMigrations } from '../../../src/db/migrations'; -import { resetTestDb } from '../../helpers/test-db'; -import { createUser } from '../../helpers/factories'; -// PKCE helper — generates a valid code_verifier + code_challenge pair (RFC 7636) -function makePkce() { - const verifier = crypto.randomBytes(32).toString('base64url'); // 43 chars - const challenge = crypto.createHash('sha256').update(verifier).digest('base64url'); // 43 chars - return { verifier, challenge }; -} - -import { - createOAuthClient, - listOAuthClients, - deleteOAuthClient, - rotateOAuthClientSecret, - createAuthCode, - consumeAuthCode, - issueTokens, - getUserByAccessToken, - refreshTokens, - revokeToken, - listOAuthSessions, - revokeSession, - validateAuthorizeRequest, - verifyPKCE, - authenticateClient, - saveConsent, - getConsent, - isConsentSufficient, -} from '../../../src/services/oauthService'; -import { isAddonEnabled } from '../../../src/nest/addons/addons.bridge'; - -beforeAll(() => { - createTables(testDb); - runMigrations(testDb); -}); - -beforeEach(() => { - resetTestDb(testDb); - // Clear oauth tables manually since they're not in the standard reset list - testDb.exec('DELETE FROM oauth_tokens'); - testDb.exec('DELETE FROM oauth_consents'); - testDb.exec('DELETE FROM oauth_clients'); - vi.mocked(isAddonEnabled).mockReturnValue(true); -}); - -afterAll(() => { - testDb.close(); -}); - -// --------------------------------------------------------------------------- -// Helper -// --------------------------------------------------------------------------- - -function makeClient( - userId: number, - overrides: Partial<{ name: string; redirectUris: string[]; scopes: string[] }> = {} -) { - return createOAuthClient( - userId, - overrides.name ?? 'Test Client', - overrides.redirectUris ?? ['https://example.com/callback'], - overrides.scopes ?? ['trips:read'], - ); -} - -// --------------------------------------------------------------------------- -// createOAuthClient -// --------------------------------------------------------------------------- - -describe('createOAuthClient', () => { - it('creates a client successfully and returns client_secret only on creation', () => { - const { user } = createUser(testDb); - const result = makeClient(user.id); - expect(result.error).toBeUndefined(); - expect(result.client).toBeDefined(); - expect(typeof result.client!.client_secret).toBe('string'); - expect((result.client!.client_secret as string).startsWith('trekcs_')).toBe(true); - }); - - it('client_id is a UUID', () => { - const { user } = createUser(testDb); - const result = makeClient(user.id); - expect(result.client!.client_id).toMatch( - /^[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}$/ - ); - }); - - it('returns 400 error if name is empty', () => { - const { user } = createUser(testDb); - const result = createOAuthClient(user.id, '', ['https://example.com/cb'], ['trips:read']); - expect(result.status).toBe(400); - expect(result.error).toContain('Name'); - }); - - it('returns 400 error if name exceeds 100 characters', () => { - const { user } = createUser(testDb); - const longName = 'A'.repeat(101); - const result = createOAuthClient(user.id, longName, ['https://example.com/cb'], ['trips:read']); - expect(result.status).toBe(400); - expect(result.error).toContain('100'); - }); - - it('returns 400 error if no redirect URIs provided', () => { - const { user } = createUser(testDb); - const result = createOAuthClient(user.id, 'Test', [], ['trips:read']); - expect(result.status).toBe(400); - expect(result.error).toContain('redirect URI'); - }); - - it('returns 400 error if more than 10 redirect URIs provided', () => { - const { user } = createUser(testDb); - const uris = Array.from({ length: 11 }, (_, i) => `https://example${i}.com/cb`); - const result = createOAuthClient(user.id, 'Test', uris, ['trips:read']); - expect(result.status).toBe(400); - expect(result.error).toContain('10'); - }); - - it('returns 400 error for invalid URI format', () => { - const { user } = createUser(testDb); - const result = createOAuthClient(user.id, 'Test', ['not-a-url'], ['trips:read']); - expect(result.status).toBe(400); - expect(result.error).toContain('Invalid redirect URI'); - }); - - it('returns 400 error for non-https URI (not localhost)', () => { - const { user } = createUser(testDb); - const result = createOAuthClient(user.id, 'Test', ['http://example.com/cb'], ['trips:read']); - expect(result.status).toBe(400); - expect(result.error).toContain('HTTPS'); - }); - - it('allows http://localhost redirect URI', () => { - const { user } = createUser(testDb); - const result = createOAuthClient(user.id, 'Test', ['http://localhost:3000/callback'], ['trips:read']); - expect(result.error).toBeUndefined(); - expect(result.client).toBeDefined(); - }); - - it('allows http://127.0.0.1 redirect URI', () => { - const { user } = createUser(testDb); - const result = createOAuthClient(user.id, 'Test', ['http://127.0.0.1:5000/callback'], ['trips:read']); - expect(result.error).toBeUndefined(); - expect(result.client).toBeDefined(); - }); - - it('returns 400 error if no scopes provided', () => { - const { user } = createUser(testDb); - const result = createOAuthClient(user.id, 'Test', ['https://example.com/cb'], []); - expect(result.status).toBe(400); - expect(result.error).toContain('scope'); - }); - - it('returns 400 error for invalid scopes', () => { - const { user } = createUser(testDb); - const result = createOAuthClient(user.id, 'Test', ['https://example.com/cb'], ['invalid:scope']); - expect(result.status).toBe(400); - expect(result.error).toContain('Invalid scopes'); - }); - - it('enforces max 10 clients per user', () => { - const { user } = createUser(testDb); - for (let i = 0; i < 10; i++) { - const r = makeClient(user.id, { name: `Client ${i}` }); - expect(r.error).toBeUndefined(); - } - const eleventh = makeClient(user.id, { name: 'Eleventh' }); - expect(eleventh.status).toBe(400); - expect(eleventh.error).toContain('10'); - }); -}); - -// --------------------------------------------------------------------------- -// listOAuthClients -// --------------------------------------------------------------------------- - -describe('listOAuthClients', () => { - it('returns empty array for user with no clients', () => { - const { user } = createUser(testDb); - expect(listOAuthClients(user.id)).toEqual([]); - }); - - it('returns created clients with redirect_uris and allowed_scopes as arrays', () => { - const { user } = createUser(testDb); - makeClient(user.id, { name: 'Client A', redirectUris: ['https://a.com/cb'], scopes: ['trips:read', 'budget:read'] }); - const clients = listOAuthClients(user.id); - expect(clients).toHaveLength(1); - expect(clients[0].name).toBe('Client A'); - expect(Array.isArray(clients[0].redirect_uris)).toBe(true); - expect(Array.isArray(clients[0].allowed_scopes)).toBe(true); - expect(clients[0].allowed_scopes).toContain('trips:read'); - }); -}); - -// --------------------------------------------------------------------------- -// deleteOAuthClient -// --------------------------------------------------------------------------- - -describe('deleteOAuthClient', () => { - it('deletes own client successfully', () => { - const { user } = createUser(testDb); - const created = makeClient(user.id); - const clientRowId = created.client!.id as string; - const result = deleteOAuthClient(user.id, clientRowId); - expect(result.success).toBe(true); - expect(listOAuthClients(user.id)).toHaveLength(0); - }); - - it('returns 404 for non-existent client', () => { - const { user } = createUser(testDb); - const result = deleteOAuthClient(user.id, 'non-existent-id'); - expect(result.status).toBe(404); - }); - - it("returns 404 for another user's client", () => { - const { user: owner } = createUser(testDb); - const { user: other } = createUser(testDb); - const created = makeClient(owner.id); - const result = deleteOAuthClient(other.id, created.client!.id as string); - expect(result.status).toBe(404); - }); -}); - -// --------------------------------------------------------------------------- -// rotateOAuthClientSecret -// --------------------------------------------------------------------------- - -describe('rotateOAuthClientSecret', () => { - it('rotates secret and returns new client_secret starting with trekcs_', () => { - const { user } = createUser(testDb); - const created = makeClient(user.id); - const oldSecret = created.client!.client_secret as string; - const result = rotateOAuthClientSecret(user.id, created.client!.id as string); - expect(result.error).toBeUndefined(); - expect(result.client_secret).toBeDefined(); - expect((result.client_secret as string).startsWith('trekcs_')).toBe(true); - expect(result.client_secret).not.toBe(oldSecret); - }); - - it('returns 404 for non-existent client', () => { - const { user } = createUser(testDb); - const result = rotateOAuthClientSecret(user.id, 'non-existent-id'); - expect(result.status).toBe(404); - }); - - it('revokes old tokens after rotation', () => { - const { user } = createUser(testDb); - const created = makeClient(user.id); - const clientId = created.client!.client_id as string; - const { access_token } = issueTokens(clientId, user.id, ['trips:read']); - expect(getUserByAccessToken(access_token)).not.toBeNull(); - - rotateOAuthClientSecret(user.id, created.client!.id as string); - - expect(getUserByAccessToken(access_token)).toBeNull(); - }); -}); - -// --------------------------------------------------------------------------- -// createAuthCode + consumeAuthCode -// --------------------------------------------------------------------------- - -describe('createAuthCode + consumeAuthCode', () => { - it('create code and consume it once returns the pending entry', () => { - const { user } = createUser(testDb); - const created = makeClient(user.id); - const clientId = created.client!.client_id as string; - - const code = createAuthCode({ - clientId, - userId: user.id, - redirectUri: 'https://example.com/callback', - scopes: ['trips:read'], - codeChallenge: 'abc123', - codeChallengeMethod: 'S256', - }); - - const entry = consumeAuthCode(code); - expect(entry).not.toBeNull(); - expect(entry!.userId).toBe(user.id); - expect(entry!.clientId).toBe(clientId); - }); - - it('returns null for non-existent code', () => { - expect(consumeAuthCode('does-not-exist')).toBeNull(); - }); - - it('consuming same code twice returns null (one-time use)', () => { - const { user } = createUser(testDb); - const created = makeClient(user.id); - const clientId = created.client!.client_id as string; - - const code = createAuthCode({ - clientId, - userId: user.id, - redirectUri: 'https://example.com/callback', - scopes: ['trips:read'], - codeChallenge: 'abc123', - codeChallengeMethod: 'S256', - }); - - consumeAuthCode(code); - expect(consumeAuthCode(code)).toBeNull(); - }); -}); - -// --------------------------------------------------------------------------- -// issueTokens + getUserByAccessToken -// --------------------------------------------------------------------------- - -describe('issueTokens + getUserByAccessToken', () => { - it('issues tokens with correct prefixes', () => { - const { user } = createUser(testDb); - const created = makeClient(user.id); - const clientId = created.client!.client_id as string; - - const tokens = issueTokens(clientId, user.id, ['trips:read']); - expect(tokens.access_token.startsWith('trekoa_')).toBe(true); - expect(tokens.refresh_token.startsWith('trekrf_')).toBe(true); - expect(tokens.token_type).toBe('Bearer'); - expect(typeof tokens.expires_in).toBe('number'); - }); - - it('getUserByAccessToken returns user and scopes for a valid token', () => { - const { user } = createUser(testDb); - const created = makeClient(user.id); - const clientId = created.client!.client_id as string; - - const { access_token } = issueTokens(clientId, user.id, ['trips:read', 'budget:write']); - const info = getUserByAccessToken(access_token); - expect(info).not.toBeNull(); - expect(info!.user.email).toBe(user.email); - expect(info!.scopes).toContain('trips:read'); - expect(info!.scopes).toContain('budget:write'); - }); - - it('getUserByAccessToken returns null for unknown token', () => { - expect(getUserByAccessToken('trekoa_unknown')).toBeNull(); - }); - - it('getUserByAccessToken returns null for revoked token', () => { - const { user } = createUser(testDb); - const created = makeClient(user.id); - const clientId = created.client!.client_id as string; - - const { access_token } = issueTokens(clientId, user.id, ['trips:read']); - revokeToken(access_token, clientId); - expect(getUserByAccessToken(access_token)).toBeNull(); - }); -}); - -// --------------------------------------------------------------------------- -// refreshTokens -// --------------------------------------------------------------------------- - -describe('refreshTokens', () => { - it('exchanges a refresh token for a new token pair', () => { - const { user } = createUser(testDb); - const created = makeClient(user.id); - const clientId = created.client!.client_id as string; - const rawSecret = created.client!.client_secret as string; - - const { refresh_token } = issueTokens(clientId, user.id, ['trips:read']); - const result = refreshTokens(refresh_token, clientId, rawSecret); - expect(result.error).toBeUndefined(); - expect(result.tokens).toBeDefined(); - expect(result.tokens!.access_token.startsWith('trekoa_')).toBe(true); - }); - - it('old tokens are revoked after refresh (rotation)', () => { - const { user } = createUser(testDb); - const created = makeClient(user.id); - const clientId = created.client!.client_id as string; - const rawSecret = created.client!.client_secret as string; - - const { access_token, refresh_token } = issueTokens(clientId, user.id, ['trips:read']); - refreshTokens(refresh_token, clientId, rawSecret); - expect(getUserByAccessToken(access_token)).toBeNull(); - }); - - it('does not revoke the active MCP session on a normal (non-replayed) refresh (#1475)', () => { - const { user } = createUser(testDb); - const created = makeClient(user.id); - const clientId = created.client!.client_id as string; - const rawSecret = created.client!.client_secret as string; - - const { refresh_token } = issueTokens(clientId, user.id, ['trips:read']); - const callsBefore = vi.mocked(revokeUserSessionsForClient).mock.calls.length; - const result = refreshTokens(refresh_token, clientId, rawSecret); - expect(result.error).toBeUndefined(); - expect(vi.mocked(revokeUserSessionsForClient).mock.calls.length).toBe(callsBefore); - }); - - it('returns invalid_grant for unknown refresh token', () => { - const { user } = createUser(testDb); - const created = makeClient(user.id); - const clientId = created.client!.client_id as string; - const rawSecret = created.client!.client_secret as string; - - const result = refreshTokens('trekrf_unknown', clientId, rawSecret); - expect(result.error).toBe('invalid_grant'); - expect(result.status).toBe(400); - }); - - it('returns invalid_grant for revoked token', () => { - const { user } = createUser(testDb); - const created = makeClient(user.id); - const clientId = created.client!.client_id as string; - const rawSecret = created.client!.client_secret as string; - - const { access_token, refresh_token } = issueTokens(clientId, user.id, ['trips:read']); - revokeToken(access_token, clientId); - const result = refreshTokens(refresh_token, clientId, rawSecret); - expect(result.error).toBe('invalid_grant'); - }); - - it('returns invalid_client for wrong client_secret', () => { - const { user } = createUser(testDb); - const created = makeClient(user.id); - const clientId = created.client!.client_id as string; - - const { refresh_token } = issueTokens(clientId, user.id, ['trips:read']); - const result = refreshTokens(refresh_token, clientId, 'wrong-secret'); - expect(result.error).toBe('invalid_client'); - expect(result.status).toBe(401); - }); -}); - -// --------------------------------------------------------------------------- -// revokeToken -// --------------------------------------------------------------------------- - -describe('revokeToken', () => { - it('after revoking access token, getUserByAccessToken returns null', () => { - const { user } = createUser(testDb); - const created = makeClient(user.id); - const clientId = created.client!.client_id as string; - - const { access_token } = issueTokens(clientId, user.id, ['trips:read']); - expect(getUserByAccessToken(access_token)).not.toBeNull(); - - revokeToken(access_token, clientId); - expect(getUserByAccessToken(access_token)).toBeNull(); - }); -}); - -// --------------------------------------------------------------------------- -// listOAuthSessions + revokeSession -// --------------------------------------------------------------------------- - -describe('listOAuthSessions + revokeSession', () => { - it('lists active sessions', () => { - const { user } = createUser(testDb); - const created = makeClient(user.id); - const clientId = created.client!.client_id as string; - - issueTokens(clientId, user.id, ['trips:read']); - const sessions = listOAuthSessions(user.id); - expect(sessions).toHaveLength(1); - expect(sessions[0].client_id).toBe(clientId); - }); - - it('revoked session is not listed', () => { - const { user } = createUser(testDb); - const created = makeClient(user.id); - const clientId = created.client!.client_id as string; - - const { access_token } = issueTokens(clientId, user.id, ['trips:read']); - revokeToken(access_token, clientId); - const sessions = listOAuthSessions(user.id); - expect(sessions).toHaveLength(0); - }); - - it('revokeSession returns 404 for unknown session', () => { - const { user } = createUser(testDb); - const result = revokeSession(user.id, 99999); - expect(result.status).toBe(404); - }); - - it('revokeSession by session id removes session from list', () => { - const { user } = createUser(testDb); - const created = makeClient(user.id); - const clientId = created.client!.client_id as string; - - issueTokens(clientId, user.id, ['trips:read']); - const sessions = listOAuthSessions(user.id); - const sessionId = sessions[0].id as number; - - const result = revokeSession(user.id, sessionId); - expect(result.success).toBe(true); - expect(listOAuthSessions(user.id)).toHaveLength(0); - }); -}); - -// --------------------------------------------------------------------------- -// validateAuthorizeRequest -// --------------------------------------------------------------------------- - -describe('validateAuthorizeRequest', () => { - // Use a proper 43-char S256 code_challenge to pass H1 format validation - const { challenge: VALID_CHALLENGE } = makePkce(); - - function makeParams(overrides: Partial<{ - response_type: string; - client_id: string; - redirect_uri: string; - scope: string; - code_challenge: string; - code_challenge_method: string; - }> = {}) { - return { - response_type: 'code', - client_id: '', - redirect_uri: 'https://example.com/callback', - scope: 'trips:read', - code_challenge: VALID_CHALLENGE, - code_challenge_method: 'S256', - ...overrides, - }; - } - - it('returns mcp_disabled when isAddonEnabled returns false', () => { - vi.mocked(isAddonEnabled).mockReturnValue(false); - const result = validateAuthorizeRequest(makeParams({ client_id: 'x' }), null); - expect(result.valid).toBe(false); - expect(result.error).toBe('mcp_disabled'); - }); - - it('requires response_type=code', () => { - const { user } = createUser(testDb); - const result = validateAuthorizeRequest(makeParams({ response_type: 'token', client_id: 'x' }), user.id); - expect(result.valid).toBe(false); - expect(result.error).toBe('unsupported_response_type'); - }); - - it('requires PKCE with S256', () => { - const { user } = createUser(testDb); - const result = validateAuthorizeRequest(makeParams({ client_id: 'x', code_challenge_method: 'plain' }), user.id); - expect(result.valid).toBe(false); - expect(result.error).toBe('invalid_request'); - }); - - it('requires valid client_id', () => { - const { user } = createUser(testDb); - const result = validateAuthorizeRequest(makeParams({ client_id: 'nonexistent' }), user.id); - expect(result.valid).toBe(false); - expect(result.error).toBe('invalid_client'); - }); - - it('validates redirect_uri against registered URIs', () => { - const { user } = createUser(testDb); - const created = makeClient(user.id, { redirectUris: ['https://example.com/callback'] }); - const clientId = created.client!.client_id as string; - - const result = validateAuthorizeRequest( - makeParams({ client_id: clientId, redirect_uri: 'https://evil.com/callback' }), - user.id - ); - expect(result.valid).toBe(false); - expect(result.error).toBe('invalid_redirect_uri'); - }); - - it('validates scope against client allowed_scopes', () => { - const { user } = createUser(testDb); - const created = makeClient(user.id, { scopes: ['trips:read'] }); - const clientId = created.client!.client_id as string; - - const result = validateAuthorizeRequest( - makeParams({ client_id: clientId, scope: 'budget:write' }), - user.id - ); - expect(result.valid).toBe(false); - expect(result.error).toBe('invalid_scope'); - }); - - it('returns loginRequired when userId is null', () => { - const { user } = createUser(testDb); - const created = makeClient(user.id); - const clientId = created.client!.client_id as string; - - const result = validateAuthorizeRequest(makeParams({ client_id: clientId }), null); - expect(result.valid).toBe(true); - expect(result.loginRequired).toBe(true); - }); - - it('returns consentRequired=true when consent not yet saved', () => { - const { user } = createUser(testDb); - const created = makeClient(user.id); - const clientId = created.client!.client_id as string; - - const result = validateAuthorizeRequest(makeParams({ client_id: clientId }), user.id); - expect(result.valid).toBe(true); - expect(result.consentRequired).toBe(true); - }); - - it('returns consentRequired=false when consent already saved and sufficient', () => { - const { user } = createUser(testDb); - const created = makeClient(user.id); - const clientId = created.client!.client_id as string; - - saveConsent(clientId, user.id, ['trips:read']); - const result = validateAuthorizeRequest(makeParams({ client_id: clientId }), user.id); - expect(result.valid).toBe(true); - expect(result.consentRequired).toBe(false); - }); -}); - -// --------------------------------------------------------------------------- -// verifyPKCE -// --------------------------------------------------------------------------- - -describe('verifyPKCE', () => { - it('returns true for valid code_verifier / code_challenge pair (SHA256 base64url)', () => { - const verifier = 'dBjftJeZ4CVP-mB92K27uhbUJU1p1r_wW1gFWFOEjXk'; - const challenge = crypto.createHash('sha256').update(verifier).digest('base64url'); - expect(verifyPKCE(verifier, challenge)).toBe(true); - }); - - it('returns false for wrong verifier', () => { - const verifier = 'correct-verifier'; - const challenge = crypto.createHash('sha256').update(verifier).digest('base64url'); - expect(verifyPKCE('wrong-verifier', challenge)).toBe(false); - }); -}); - -// --------------------------------------------------------------------------- -// authenticateClient -// --------------------------------------------------------------------------- - -describe('authenticateClient', () => { - it('returns client row for correct credentials', () => { - const { user } = createUser(testDb); - const created = makeClient(user.id); - const clientId = created.client!.client_id as string; - const rawSecret = created.client!.client_secret as string; - - const client = authenticateClient(clientId, rawSecret); - expect(client).not.toBeNull(); - expect(client!.client_id).toBe(clientId); - }); - - it('returns null for wrong secret', () => { - const { user } = createUser(testDb); - const created = makeClient(user.id); - const clientId = created.client!.client_id as string; - - expect(authenticateClient(clientId, 'wrong-secret')).toBeNull(); - }); - - it('returns null for unknown client_id', () => { - expect(authenticateClient('unknown-client-id', 'any-secret')).toBeNull(); - }); -}); - -// --------------------------------------------------------------------------- -// saveConsent + getConsent + isConsentSufficient -// --------------------------------------------------------------------------- - -describe('saveConsent + getConsent + isConsentSufficient', () => { - it('saves and retrieves consent', () => { - const { user } = createUser(testDb); - const created = makeClient(user.id); - const clientId = created.client!.client_id as string; - - saveConsent(clientId, user.id, ['trips:read', 'budget:write']); - const consent = getConsent(clientId, user.id); - expect(consent).not.toBeNull(); - expect(consent).toContain('trips:read'); - expect(consent).toContain('budget:write'); - }); - - it('isConsentSufficient returns true when all requested scopes are in existing', () => { - expect(isConsentSufficient(['trips:read', 'budget:write'], ['trips:read'])).toBe(true); - expect(isConsentSufficient(['trips:read', 'budget:write'], ['trips:read', 'budget:write'])).toBe(true); - }); - - it('isConsentSufficient returns false when some scopes are missing', () => { - expect(isConsentSufficient(['trips:read'], ['trips:read', 'budget:write'])).toBe(false); - expect(isConsentSufficient([], ['trips:read'])).toBe(false); - }); -}); - -// --------------------------------------------------------------------------- -// M5 — saveConsent unions instead of replacing -// --------------------------------------------------------------------------- - -describe('saveConsent — scope union (M5)', () => { - it('unioning scopes: approving B after A leaves both in consent', () => { - const { user } = createUser(testDb); - const created = makeClient(user.id, { scopes: ['trips:read', 'budget:write'] }); - const clientId = created.client!.client_id as string; - - saveConsent(clientId, user.id, ['trips:read']); - saveConsent(clientId, user.id, ['budget:write']); - - const consent = getConsent(clientId, user.id); - expect(consent).toContain('trips:read'); - expect(consent).toContain('budget:write'); - }); - - it('re-approving a superset scope still preserves previously-consented scopes', () => { - const { user } = createUser(testDb); - const created = makeClient(user.id, { scopes: ['trips:read', 'trips:write'] }); - const clientId = created.client!.client_id as string; - - saveConsent(clientId, user.id, ['trips:read', 'trips:write']); - // approve only trips:read on a later request - saveConsent(clientId, user.id, ['trips:read']); - - const consent = getConsent(clientId, user.id); - // trips:write should NOT be removed (union semantics) - expect(consent).toContain('trips:read'); - expect(consent).toContain('trips:write'); - }); - - it('consent is sufficient after sequential approvals — no re-prompt needed', () => { - const { user } = createUser(testDb); - const created = makeClient(user.id, { scopes: ['trips:read', 'budget:write'] }); - const clientId = created.client!.client_id as string; - - saveConsent(clientId, user.id, ['trips:read']); - saveConsent(clientId, user.id, ['budget:write']); - - // Should not require consent again for either scope - expect(isConsentSufficient(getConsent(clientId, user.id)!, ['trips:read'])).toBe(true); - expect(isConsentSufficient(getConsent(clientId, user.id)!, ['budget:write'])).toBe(true); - expect(isConsentSufficient(getConsent(clientId, user.id)!, ['trips:read', 'budget:write'])).toBe(true); - }); -}); - -// --------------------------------------------------------------------------- -// C2 — getUserByAccessToken returns clientId -// --------------------------------------------------------------------------- - -describe('getUserByAccessToken — includes clientId (C2)', () => { - it('returns clientId matching the issuing OAuth client', () => { - const { user } = createUser(testDb); - const created = makeClient(user.id); - const clientId = created.client!.client_id as string; - - const { access_token } = issueTokens(clientId, user.id, ['trips:read']); - const info = getUserByAccessToken(access_token); - expect(info).not.toBeNull(); - expect(info!.clientId).toBe(clientId); - }); -}); - -// --------------------------------------------------------------------------- -// C3 — Refresh token replay detection and chain revocation -// --------------------------------------------------------------------------- - -describe('refreshTokens — replay detection (C3)', () => { - it('replaying a revoked refresh token returns invalid_grant', () => { - const { user } = createUser(testDb); - const created = makeClient(user.id); - const clientId = created.client!.client_id as string; - const rawSecret = created.client!.client_secret as string; - - // Issue tokens, then rotate once (old token becomes revoked) - const { refresh_token: firstRefresh } = issueTokens(clientId, user.id, ['trips:read']); - const rotateResult = refreshTokens(firstRefresh, clientId, rawSecret); - expect(rotateResult.error).toBeUndefined(); - const { refresh_token: secondRefresh } = rotateResult.tokens!; - - // Replay the FIRST (now revoked) refresh token - const callsBefore = vi.mocked(revokeUserSessionsForClient).mock.calls.length; - const replayResult = refreshTokens(firstRefresh, clientId, rawSecret); - expect(replayResult.error).toBe('invalid_grant'); - expect(replayResult.status).toBe(400); - // Replay IS a security event — sessions must still be torn down here. - expect(vi.mocked(revokeUserSessionsForClient).mock.calls.length).toBe(callsBefore + 1); - expect(vi.mocked(revokeUserSessionsForClient)).toHaveBeenLastCalledWith(user.id, clientId); - }); - - it('replaying a revoked token also revokes the entire rotation chain', () => { - const { user } = createUser(testDb); - const created = makeClient(user.id); - const clientId = created.client!.client_id as string; - const rawSecret = created.client!.client_secret as string; - - // Issue → rotate once - const { refresh_token: first } = issueTokens(clientId, user.id, ['trips:read']); - const r1 = refreshTokens(first, clientId, rawSecret); - const { access_token: access2, refresh_token: second } = r1.tokens!; - - // Replay first (revoked) refresh token → chain revoke - refreshTokens(first, clientId, rawSecret); - - // The rotated access token should also be dead now - expect(getUserByAccessToken(access2)).toBeNull(); - - // The second refresh token should also be revoked - const r2 = refreshTokens(second, clientId, rawSecret); - expect(r2.error).toBe('invalid_grant'); - }); - - it('new rotation chain after replay is independent', () => { - const { user } = createUser(testDb); - const created = makeClient(user.id); - const clientId = created.client!.client_id as string; - const rawSecret = created.client!.client_secret as string; - - const { refresh_token: first } = issueTokens(clientId, user.id, ['trips:read']); - // Rotate once - const r1 = refreshTokens(first, clientId, rawSecret); - const { refresh_token: second } = r1.tokens!; - // Rotate again on the second token - const r2 = refreshTokens(second, clientId, rawSecret); - expect(r2.error).toBeUndefined(); - const { refresh_token: third } = r2.tokens!; - - // Replay the first revoked token → revokes chain containing first+second+third - refreshTokens(first, clientId, rawSecret); - - // third should now be revoked too (it's in the same chain) - const r3 = refreshTokens(third, clientId, rawSecret); - expect(r3.error).toBe('invalid_grant'); - }); -}); - -// --------------------------------------------------------------------------- -// H1 — PKCE code_challenge / code_verifier format validation -// --------------------------------------------------------------------------- - -describe('verifyPKCE — format validation (H1)', () => { - it('returns false for a code_verifier that is too short (< 43 chars)', () => { - const { challenge } = makePkce(); - expect(verifyPKCE('short', challenge)).toBe(false); - }); - - it('returns false for a code_verifier that is too long (> 128 chars)', () => { - const { challenge } = makePkce(); - const longVerifier = 'a'.repeat(129); - expect(verifyPKCE(longVerifier, challenge)).toBe(false); - }); - - it('returns false for a code_verifier with invalid characters', () => { - const { challenge } = makePkce(); - const badVerifier = 'A'.repeat(42) + ' '; // space is not allowed - expect(verifyPKCE(badVerifier, challenge)).toBe(false); - }); - - it('returns true for a valid 43-char verifier matching its challenge', () => { - const { verifier, challenge } = makePkce(); - expect(verifyPKCE(verifier, challenge)).toBe(true); - }); -}); - -describe('validateAuthorizeRequest — PKCE format (H1)', () => { - it('returns invalid_request when code_challenge is shorter than 43 chars', () => { - const { user } = createUser(testDb); - const created = makeClient(user.id); - const clientId = created.client!.client_id as string; - - const result = validateAuthorizeRequest({ - response_type: 'code', - client_id: clientId, - redirect_uri: 'https://example.com/callback', - scope: 'trips:read', - code_challenge: 'tooshort', - code_challenge_method: 'S256', - }, user.id); - expect(result.valid).toBe(false); - expect(result.error).toBe('invalid_request'); - }); - - it('returns invalid_request when code_challenge contains invalid characters', () => { - const { user } = createUser(testDb); - const created = makeClient(user.id); - const clientId = created.client!.client_id as string; - - // 43 chars but includes '=' which is not base64url - const badChallenge = '='.repeat(43); - const result = validateAuthorizeRequest({ - response_type: 'code', - client_id: clientId, - redirect_uri: 'https://example.com/callback', - scope: 'trips:read', - code_challenge: badChallenge, - code_challenge_method: 'S256', - }, user.id); - expect(result.valid).toBe(false); - expect(result.error).toBe('invalid_request'); - }); -}); - -// --------------------------------------------------------------------------- -// H3 — validateAuthorizeRequest: loginRequired response strips client info -// --------------------------------------------------------------------------- - -describe('validateAuthorizeRequest — unauthenticated strips client info (H3)', () => { - it('loginRequired response does not include client.name or allowed_scopes', () => { - const { user } = createUser(testDb); - const created = makeClient(user.id); - const clientId = created.client!.client_id as string; - const { challenge } = makePkce(); - - const result = validateAuthorizeRequest({ - response_type: 'code', - client_id: clientId, - redirect_uri: 'https://example.com/callback', - scope: 'trips:read', - code_challenge: challenge, - code_challenge_method: 'S256', - }, null /* unauthenticated */); - - expect(result.valid).toBe(true); - expect(result.loginRequired).toBe(true); - // Must NOT expose client metadata to unauthenticated callers - expect(result.client).toBeUndefined(); - expect(result.scopes).toBeUndefined(); - }); -});