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(); - }); -});