From e81efcbc57e82eef94dfceca8af179389313204b Mon Sep 17 00:00:00 2001 From: Maurice Date: Sat, 10 Oct 2026 15:12:44 +0200 Subject: [PATCH] refactor(auth): auth, passkey, profile, token and admin services refuse with DomainError instead of returning error envelopes --- server/src/nest/admin/admin.controller.ts | 35 ++-- server/src/nest/admin/admin.service.ts | 60 +++---- .../src/nest/auth/auth-public.controller.ts | 21 +-- server/src/nest/auth/auth.controller.ts | 51 +----- server/src/nest/auth/auth.service.ts | 137 ++++++++-------- server/src/nest/auth/passkey.controller.ts | 9 +- server/src/nest/auth/passkey.service.ts | 60 +++---- .../nest/auth/registration-invites.service.ts | 3 +- server/src/nest/auth/user-profile.service.ts | 23 +-- server/src/nest/common/domain-error.ts | 9 +- server/src/nest/oauth/oauth.service.ts | 5 +- server/src/nest/tokens/token.service.ts | 23 +-- server/tests/helpers/domain-error.ts | 20 +++ .../tests/unit/nest/admin.controller.test.ts | 25 ++- server/tests/unit/nest/admin.service.test.ts | 19 +-- .../unit/nest/auth-core/auth-guard.test.ts | 11 +- .../tests/unit/nest/auth.controller.test.ts | 41 ++--- server/tests/unit/nest/auth.service.test.ts | 153 +++++++++--------- server/tests/unit/nest/domain-error.test.ts | 4 +- server/tests/unit/nest/oidc.service.test.ts | 3 +- .../tests/unit/nest/passkey.service.test.ts | 117 +++++++------- .../nest/registration-invites.service.test.ts | 7 +- server/tests/unit/nest/token.service.test.ts | 87 +++++----- .../unit/nest/user-profile.service.test.ts | 53 +++--- 24 files changed, 460 insertions(+), 516 deletions(-) create mode 100644 server/tests/helpers/domain-error.ts diff --git a/server/src/nest/admin/admin.controller.ts b/server/src/nest/admin/admin.controller.ts index c74376b1d..bc5d6245e 100644 --- a/server/src/nest/admin/admin.controller.ts +++ b/server/src/nest/admin/admin.controller.ts @@ -29,15 +29,6 @@ import { NotificationsService } from '../notifications/notifications.service'; import type { User } from '../../types'; import { ManagedForbidden } from '../common/managed'; -/** Throw the legacy {error,status} envelope when a service call reports failure. */ -function ok(result: T): Exclude { - if (result && typeof result === 'object' && 'error' in (result as Record)) { - const r = result as unknown as { error: string; status?: number }; - throw new HttpException({ error: r.error }, r.status ?? 400); - } - return result as Exclude; -} - /** * /api/admin — admin-only control surface (users, stats, permissions, audit log, * OIDC settings, invites, feature toggles, packing templates, addons, MCP/OAuth @@ -76,14 +67,14 @@ export class AdminController { @Post('users') @HttpCode(201) async createUser(@CurrentUser() user: User, @Body() body: AdminUserCreateDto, @Req() req: Request) { - const result = ok(await this.admin.createUser(body as Parameters[0])); + const result = await this.admin.createUser(body as Parameters[0]); await this.audit.writeAudit({ userId: user.id, action: 'admin.user_create', resource: String(result.insertedId), ip: getClientIp(req), details: result.auditDetails }); return { user: result.user }; } @Put('users/:id') async updateUser(@CurrentUser() user: User, @Param('id') id: string, @Body() body: AdminUserUpdateDto, @Req() req: Request) { - const result = ok(await this.admin.updateUser(id, body)); + const result = await this.admin.updateUser(id, body); await this.audit.writeAudit({ userId: user.id, action: 'admin.user_update', resource: String(id), ip: getClientIp(req), details: { targetUser: result.previousEmail, fields: result.changed } }); logInfo(`Admin ${user.email} edited user ${result.previousEmail} (fields: ${result.changed.join(', ')})`); return { user: result.user }; @@ -91,7 +82,7 @@ export class AdminController { @Delete('users/:id') async deleteUser(@CurrentUser() user: User, @Param('id') id: string, @Req() req: Request) { - const result = ok(await this.admin.deleteUser(id, user.id)); + const result = await this.admin.deleteUser(id, user.id); await this.audit.writeAudit({ userId: user.id, action: 'admin.user_delete', resource: String(id), ip: getClientIp(req), details: { targetUser: result.email } }); logInfo(`Admin ${user.email} deleted user ${result.email}`); return { success: true }; @@ -99,14 +90,14 @@ export class AdminController { @Delete('users/:id/passkeys') async resetUserPasskeys(@CurrentUser() user: User, @Param('id') id: string, @Req() req: Request) { - const result = ok(await this.admin.resetUserPasskeys(id)); + const result = await this.admin.resetUserPasskeys(id); await this.audit.writeAudit({ userId: user.id, action: 'admin.user_passkeys_reset', resource: String(id), ip: getClientIp(req), details: { targetUser: result.email, deleted: result.deleted } }); return { success: true, deleted: result.deleted }; } @Delete('users/:id/mfa') async resetUserMfa(@CurrentUser() user: User, @Param('id') id: string, @Req() req: Request) { - const result = ok(await this.admin.resetUserMfa(id, user.id)); + const result = await this.admin.resetUserMfa(id, user.id); await this.audit.writeAudit({ userId: user.id, action: 'admin.user_mfa_reset', resource: String(id), ip: getClientIp(req), details: { targetUser: result.email } }); return { success: true }; } @@ -132,9 +123,6 @@ export class AdminController { @HttpCode(200) async saveDemoBaseline(@CurrentUser() user: User, @Req() req: Request) { const result = await this.admin.saveDemoBaseline(); - if (result.error) { - throw new HttpException({ error: result.error }, result.status!); - } await this.audit.writeAudit({ userId: user.id, action: 'admin.demo_baseline_save', ip: getClientIp(req) }); return { success: true, message: result.message }; } @@ -175,7 +163,7 @@ export class AdminController { @Delete('invites/:id') async deleteInvite(@CurrentUser() user: User, @Param('id') id: string, @Req() req: Request) { - ok(await this.invites.deleteInvite(id)); + await this.invites.deleteInvite(id); await this.audit.writeAudit({ userId: user.id, action: 'admin.invite_delete', resource: String(id), ip: getClientIp(req) }); return { success: true }; } @@ -290,7 +278,7 @@ export class AdminController { @Put('addons/:id') async updateAddon(@CurrentUser() user: User, @Param('id') id: string, @Body() body: AdminAddonUpdateDto, @Req() req: Request) { - const result = await ok(await this.admin.updateAddon(id, body)); + const result = await this.admin.updateAddon(id, body); await this.audit.writeAudit({ userId: user.id, action: 'admin.addon_update', resource: String(id), ip: getClientIp(req), details: result.auditDetails }); // Sessions only need re-creating when the registered MCP surface can // actually change — an enabled-flip of an MCP-relevant addon. Config-only @@ -316,7 +304,7 @@ export class AdminController { @Delete('mcp-tokens/:id') async deleteMcpToken(@CurrentUser() user: User, @Param('id') id: string, @Req() req: Request) { - ok(await this.tokens.adminDeleteMcpToken(id)); + await this.tokens.adminDeleteMcpToken(id); await this.audit.writeAudit({ userId: user.id, action: 'admin.mcp_token_delete', resource: String(id), ip: getClientIp(req) }); return { success: true }; } @@ -326,7 +314,7 @@ export class AdminController { @Delete('oauth-sessions/:id') async revokeOAuthSession(@CurrentUser() user: User, @Param('id') id: string, @Req() req: Request) { - ok(await this.oauth.adminRevokeOAuthSession(id)); + await this.oauth.adminRevokeOAuthSession(id); await this.audit.writeAudit({ userId: user.id, action: 'admin.oauth_session_revoke', resource: String(id), ip: getClientIp(req) }); return { success: true }; } @@ -336,10 +324,7 @@ export class AdminController { @Post('rotate-jwt-secret') @HttpCode(200) async rotateJwtSecret(@CurrentUser() user: User, @Req() req: Request) { - const result = this.admin.rotateJwtSecret(); - if (result.error) { - throw new HttpException({ error: result.error }, result.status!); - } + this.admin.rotateJwtSecret(); await this.audit.writeAudit({ userId: user.id, action: 'admin.rotate_jwt_secret', ip: getClientIp(req) }); return { success: true }; } diff --git a/server/src/nest/admin/admin.service.ts b/server/src/nest/admin/admin.service.ts index 47400793d..a01cd8404 100644 --- a/server/src/nest/admin/admin.service.ts +++ b/server/src/nest/admin/admin.service.ts @@ -1,3 +1,4 @@ +import { DomainError } from '../common/domain-error'; import { Inject, Injectable } from '@nestjs/common'; import { InjectRepository } from '@mikro-orm/nestjs'; import bcrypt from 'bcryptjs'; @@ -167,22 +168,22 @@ export class AdminService { const password = data.password?.trim(); if (!username || !email || !password) { - return { error: 'Username, email and password are required', status: 400 }; + throw new DomainError(400, 'Username, email and password are required'); } const pwCheck = validatePassword(password); - if (!pwCheck.ok) return { error: pwCheck.reason, status: 400 }; + if (!pwCheck.ok) throw new DomainError(400, pwCheck.reason); if (data.role && !['user', 'admin'].includes(data.role)) { - return { error: 'Invalid role', status: 400 }; + throw new DomainError(400, 'Invalid role'); } // Guests (#1362) live in a reserved synthetic namespace; never let one block a real account. const existingUsername = await this.users.findIdByUsernameExact(username); - if (existingUsername) return { error: 'Username already taken', status: 409 }; + if (existingUsername) throw new DomainError(409, 'Username already taken'); const existingEmail = await this.users.findIdByEmailCI(email); - if (existingEmail) return { error: 'Email already taken', status: 409 }; + if (existingEmail) throw new DomainError(409, 'Email already taken'); const passwordHash = bcrypt.hashSync(password, BCRYPT_COST); @@ -193,7 +194,7 @@ export class AdminService { }); } catch (err) { // Another write took the name or address between the check and the insert. - if (err instanceof UserIdentityTakenError) return { error: identityTakenMessage(err), status: 409 }; + if (err instanceof UserIdentityTakenError) throw new DomainError(409, identityTakenMessage(err)); throw err; } @@ -213,29 +214,29 @@ export class AdminService { const { role, password } = data; const user = await this.users.findById(userId); - if (!user) return { error: 'User not found', status: 404 }; + if (!user) throw new DomainError(404, 'User not found'); if (role && !['user', 'admin'].includes(role)) { - return { error: 'Invalid role', status: 400 }; + throw new DomainError(400, 'Invalid role'); } // An empty string used to fall through `username || null` into COALESCE and // silently mean "leave unchanged". Say so instead of pretending it worked. - if (username === '') return { error: 'Username cannot be empty', status: 400 }; - if (email === '') return { error: 'Email cannot be empty', status: 400 }; + if (username === '') throw new DomainError(400, 'Username cannot be empty'); + if (email === '') throw new DomainError(400, 'Email cannot be empty'); if (username && username !== user.username) { const conflict = await this.users.findIdByUsernameExactExcluding(username, userId); - if (conflict) return { error: 'Username already taken', status: 409 }; + if (conflict) throw new DomainError(409, 'Username already taken'); } if (email && email !== user.email) { const conflict = await this.users.findIdByEmailCI(email, userId); - if (conflict) return { error: 'Email already taken', status: 409 }; + if (conflict) throw new DomainError(409, 'Email already taken'); } if (password) { const pwCheck = validatePassword(password); - if (!pwCheck.ok) return { error: pwCheck.reason, status: 400 }; + if (!pwCheck.ok) throw new DomainError(400, pwCheck.reason); } const passwordHash = password ? bcrypt.hashSync(password, BCRYPT_COST) : null; @@ -248,7 +249,7 @@ export class AdminService { const currentRole = await this.users.getRole(userId); if (currentRole === 'admin') { const adminCount = await this.users.countAdmins(); - if (adminCount <= 1) return { error: 'Cannot remove the last admin', status: 400 }; + if (adminCount <= 1) throw new DomainError(400, 'Cannot remove the last admin'); } } @@ -291,7 +292,7 @@ export class AdminService { } }); } catch (err) { - if (err instanceof UserIdentityTakenError) return { error: identityTakenMessage(err), status: 409 }; + if (err instanceof UserIdentityTakenError) throw new DomainError(409, identityTakenMessage(err)); throw err; } @@ -318,11 +319,11 @@ export class AdminService { // SECURITY: plain JS comparison, deliberately kept exactly here — never // folded into a repository method's WHERE clause (R4). if (Number.parseInt(id) === currentUserId) { - return { error: 'Cannot delete own account', status: 400 }; + throw new DomainError(400, 'Cannot delete own account'); } const userToDel = await this.users.findIdAndEmail(Number(id)); - if (!userToDel) return { error: 'User not found', status: 404 }; + if (!userToDel) throw new DomainError(404, 'User not found'); await this.userCleanup.deleteUserCompletely(userToDel.id); await emitUserDeleted(userToDel.id); // let plugins erase their own per-user data @@ -344,15 +345,15 @@ export class AdminService { * making that reachable from here would turn a stolen admin session into a * way to strip the second factor off the very account it came from. */ - async resetUserMfa(id: string, actingUserId: number): Promise<{ error?: string; status?: number; success?: boolean; email?: string }> { + async resetUserMfa(id: string, actingUserId: number): Promise<{ success?: boolean; email?: string }> { const targetId = Number(id); // SECURITY: plain JS comparison, deliberately kept exactly here — never // folded into a repository method's WHERE clause (R4). if (targetId === actingUserId) { - return { error: 'Use Settings to change your own two-factor setup', status: 400 }; + throw new DomainError(400, 'Use Settings to change your own two-factor setup'); } const target = await this.users.findIdEmailMfaEnabled(targetId); - if (!target) return { error: 'User not found', status: 404 }; + if (!target) throw new DomainError(404, 'User not found'); // Same three columns disableMfa clears, so an admin reset and a self-service // disable leave the account in exactly one state rather than two; the @@ -426,9 +427,9 @@ export class AdminService { // ── Demo Baseline ────────────────────────────────────────────────────────── - async saveDemoBaseline(): Promise<{ error?: string; status?: number; message?: string }> { + async saveDemoBaseline(): Promise<{ message?: string }> { if (!readEnv().demo.enabled) { - return { error: 'Not found', status: 404 }; + throw new DomainError(404, 'Not found'); } try { // Lazy require: demo-reset is a demo-only module. @@ -437,7 +438,7 @@ export class AdminService { return { message: 'Demo baseline saved. Hourly resets will restore to this state.' }; } catch (err: unknown) { console.error(err); - return { error: 'Failed to save baseline', status: 500 }; + throw new DomainError(500, 'Failed to save baseline'); } } @@ -676,28 +677,28 @@ export class AdminService { const addon = await this.addonsRepo.findById(id); const provider = await this.photoProviders.findById(id); const docProvider = await this.documentProviders.findById(id); - if (!addon && !provider && !docProvider) return { error: 'Addon not found', status: 404 }; + if (!addon && !provider && !docProvider) throw new DomainError(404, 'Addon not found'); // The whole addon, not just its config: on a centrally administered install // the operator owns the endpoint, the model and the per-document cost, so // there is nothing here for an instance admin to set — including whether it // runs at all. listAddons hides the row; this closes the route behind it. if (readEnv().managed.enabled && id === ADDON_IDS.LLM_PARSING) { - return { error: MANAGED_FORBIDDEN_ERROR.error, status: 403 }; + throw new DomainError(403, MANAGED_FORBIDDEN_ERROR.error); } // Photo providers are Journey's shelf rows — their whole UI lives inside // journeys, so enabling one under a disabled journey addon would only // advertise an integration nothing can reach. if (provider && data.enabled === true && !(await this.addons.isAddonEnabled(ADDON_IDS.JOURNEY))) { - return { error: 'Enable the Journey addon first', status: 409 }; + throw new DomainError(409, 'Enable the Journey addon first'); } // Same rule one shelf down: a document provider only exists to serve the // file manager, so switching one on under a disabled Documents addon would // advertise a sync nothing can reach. if (docProvider && data.enabled === true && !(await this.addons.isAddonEnabled(ADDON_IDS.DOCUMENTS))) { - return { error: 'Enable the Documents addon first', status: 409 }; + throw new DomainError(409, 'Enable the Documents addon first'); } await this.uow.transactional(async () => { @@ -784,17 +785,16 @@ export class AdminService { // ── JWT Rotation ─────────────────────────────────────────────────────────── - rotateJwtSecret(): { error?: string; status?: number } { + rotateJwtSecret(): void { const newSecret = crypto.randomBytes(32).toString('hex'); const { dataDir, jwtSecretFile: secretFile } = this.dataPaths; try { if (!fs.existsSync(dataDir)) fs.mkdirSync(dataDir, { recursive: true }); fs.writeFileSync(secretFile, newSecret, { mode: 0o600 }); } catch { - return { error: 'Failed to persist new JWT secret to disk', status: 500 }; + throw new DomainError(500, 'Failed to persist new JWT secret to disk'); } updateJwtSecret(newSecret); - return {}; } invalidateMcpSessions() { invalidateMcpSessions(); } diff --git a/server/src/nest/auth/auth-public.controller.ts b/server/src/nest/auth/auth-public.controller.ts index 45e380e27..72e674826 100644 --- a/server/src/nest/auth/auth-public.controller.ts +++ b/server/src/nest/auth/auth-public.controller.ts @@ -12,6 +12,7 @@ import { Public } from '../auth-core/public.decorator'; import { MfaExempt } from '../auth-core/mfa-policy.guard'; import { extractToken, verifiedSessionClaims } from '../auth-core/jwt-verify'; import { SessionsService, sessionClientFrom } from '../sessions/sessions.service'; +import { catchDomainError, DomainError } from '../common/domain-error'; const WINDOW = 15 * 60 * 1000; const LOGIN_MIN_LATENCY_MS = 350; @@ -60,9 +61,6 @@ export class AuthPublicController { @HttpCode(200) async demoLogin(@Req() req: Request, @Res({ passthrough: true }) res: Response) { const result = await this.auth.demoLogin(sessionClientFrom(req)); - if (result.error) { - throw new HttpException({ error: result.error }, result.status!); - } this.auth.setAuthCookie(res, result.token!, req); return { token: result.token, user: result.user }; } @@ -72,9 +70,6 @@ export class AuthPublicController { async invite(@Param('token') token: string, @Req() req: Request) { await this.limit('login', req, 10); const result = await this.auth.validateInviteToken(token); - if (result.error) { - throw new HttpException({ error: result.error }, result.status!); - } return { valid: result.valid, max_uses: result.max_uses, used_count: result.used_count, expires_at: result.expires_at }; } @@ -84,9 +79,6 @@ export class AuthPublicController { async register(@Body() body: RegisterDto, @Req() req: Request, @Res({ passthrough: true }) res: Response) { await this.limit('login', req, 10); const result = await this.auth.registerUser(body, sessionClientFrom(req)); - if (result.error) { - throw new HttpException({ error: result.error }, result.status!); - } await this.audit.writeAudit({ userId: result.auditUserId!, action: 'user.register', ip: getClientIp(req), details: result.auditDetails }); this.auth.setAuthCookie(res, result.token!, req); return { token: result.token, user: result.user }; @@ -156,10 +148,10 @@ export class AuthPublicController { // a dedicated bucket) — without it reset tokens could be guessed unthrottled. await this.limit('reset', req, 5); const ip = getClientIp(req); - const result = await this.auth.resetPassword(body); - if (result.error) { - await this.audit.writeAudit({ userId: null, action: 'user.password_reset_fail', ip, details: { reason: result.error } }); - throw new HttpException({ error: result.error }, result.status!); + const result = await catchDomainError(() => this.auth.resetPassword(body)); + if (result instanceof DomainError) { + await this.audit.writeAudit({ userId: null, action: 'user.password_reset_fail', ip, details: { reason: result.publicMessage } }); + throw result; } if (result.mfa_required) { return { mfa_required: true }; @@ -174,9 +166,6 @@ export class AuthPublicController { async verifyMfaLogin(@Body() body: MfaVerifyLoginDto, @Req() req: Request, @Res({ passthrough: true }) res: Response) { await this.limit('mfa', req, 5); const result = await this.auth.verifyMfaLogin(body, sessionClientFrom(req)); - if (result.error) { - throw new HttpException({ error: result.error }, result.status!); - } await this.audit.writeAudit({ userId: result.auditUserId!, action: 'user.login', ip: getClientIp(req), details: { mfa: true } }); this.auth.setAuthCookie(res, result.token!, req, result.remember); return { token: result.token, user: result.user }; diff --git a/server/src/nest/auth/auth.controller.ts b/server/src/nest/auth/auth.controller.ts index d6ab99b71..df74d49f4 100644 --- a/server/src/nest/auth/auth.controller.ts +++ b/server/src/nest/auth/auth.controller.ts @@ -119,9 +119,6 @@ export class AuthController { const remember = decodeSessionClaims(cookie)?.remember; const cookieSession = typeof cookie === 'string' && cookie.length > 0; const result = await this.auth.changePassword(user.id, user.email, body, remember, sessionClientFrom(req), cookieSession); - if (result.error) { - throw new HttpException({ error: result.error }, result.status!); - } // Refresh this device's cookie with the new password_version so the user // stays logged in here while all other sessions are invalidated. if (result.token) this.auth.setAuthCookie(res, result.token, req, remember); @@ -131,10 +128,7 @@ export class AuthController { @Delete('me') async deleteAccount(@CurrentUser() user: User, @Req() req: Request) { - const result = await this.auth.deleteAccount(user.id, user.email, user.role); - if (result.error) { - throw new HttpException({ error: result.error }, result.status!); - } + await this.auth.deleteAccount(user.id, user.email, user.role); await this.audit.writeAudit({ userId: user.id, action: 'user.account_delete', ip: getClientIp(req) }); return { success: true }; } @@ -180,9 +174,6 @@ export class AuthController { @Put('me/settings') async updateSettings(@CurrentUser() user: User, @Body() body: SettingsUpdateDto, @Req() req: Request) { const result = await this.profile.updateSettings(user.id, body); - if (result.error) { - throw new HttpException({ error: result.error }, result.status!); - } await this.auditApiKeys(user.id, result.changedKeys ?? [], req); return { success: result.success, user: result.user }; } @@ -190,9 +181,6 @@ export class AuthController { @Get('me/settings') async getSettings(@CurrentUser() user: User) { const result = await this.profile.getSettings(user.id); - if (result.error) { - throw new HttpException({ error: result.error }, result.status!); - } return { settings: result.settings }; } @@ -226,9 +214,6 @@ export class AuthController { @Get('validate-keys') async validateKeys(@CurrentUser() user: User) { const result = await this.profile.validateKeys(user.id); - if (result.error) { - throw new HttpException({ error: result.error }, result.status!); - } return { maps: result.maps, weather: result.weather, maps_details: result.maps_details }; } @@ -236,9 +221,6 @@ export class AuthController { @MfaExempt('the setup screen reads the policy it is asking the user to satisfy') async getAppSettings(@CurrentUser() user: User) { const result = await this.auth.getAppSettings(user.id); - if (result.error) { - throw new HttpException({ error: result.error }, result.status!); - } return result.data; } @@ -246,9 +228,6 @@ export class AuthController { @MfaExempt('an admin locked out by their own policy must still be able to lift it') async updateAppSettings(@CurrentUser() user: User, @Body() body: AppSettingsUpdateDto, @Req() req: Request) { const result = await this.auth.updateAppSettings(user.id, body); - if (result.error) { - throw new HttpException({ error: result.error }, result.status!); - } await this.audit.writeAudit({ userId: user.id, action: 'settings.app_update', ip: getClientIp(req), details: result.auditSummary, debugDetails: result.auditDebugDetails }); // Named so the settings tab can say which fields the operator holds rather // than showing a saved value that silently did not save. @@ -264,9 +243,6 @@ export class AuthController { @HttpCode(200) async mfaSetup(@CurrentUser() user: User) { const result = await this.auth.setupMfa(user.id, user.email); - if (result.error) { - throw new HttpException({ error: result.error }, result.status!); - } try { const qr_svg = await result.qrPromise!; return { secret: result.secret, otpauth_url: result.otpauth_url, qr_svg }; @@ -282,9 +258,6 @@ export class AuthController { async mfaEnable(@CurrentUser() user: User, @Body() body: MfaEnableDto, @Req() req: Request) { await this.limit('mfa', req, 5); const result = await this.auth.enableMfa(user.id, body.code); - if (result.error) { - throw new HttpException({ error: result.error }, result.status!); - } await this.audit.writeAudit({ userId: user.id, action: 'user.mfa_enable', ip: getClientIp(req) }); return { success: true, mfa_enabled: result.mfa_enabled, backup_codes: result.backup_codes }; } @@ -294,9 +267,6 @@ export class AuthController { async mfaDisable(@CurrentUser() user: User, @Body() body: MfaDisableDto, @Req() req: Request) { await this.limit('login', req, 5); const result = await this.auth.disableMfa(user.id, user.email, body, currentSessionId(req)); - if (result.error) { - throw new HttpException({ error: result.error }, result.status!); - } await this.audit.writeAudit({ userId: user.id, action: 'user.mfa_disable', ip: getClientIp(req) }); return { success: true, mfa_enabled: result.mfa_enabled }; } @@ -312,18 +282,12 @@ export class AuthController { async createMcpToken(@CurrentUser() user: User, @Body() body: McpTokenCreateDto, @Req() req: Request) { await this.limit('login', req, 5); const result = await this.tokens.createMcpToken(user.id, body.name); - if (result.error) { - throw new HttpException({ error: result.error }, result.status!); - } return { token: result.token }; } @Delete('mcp-tokens/:id') async deleteMcpToken(@CurrentUser() user: User, @Param('id') id: string) { - const result = await this.tokens.deleteMcpToken(user.id, id); - if (result.error) { - throw new HttpException({ error: result.error }, result.status!); - } + await this.tokens.deleteMcpToken(user.id, id); return { success: true }; } @@ -352,18 +316,12 @@ export class AuthController { // before this field existed does. Narrowing stays opt-in so the change // cannot break an integration that is already running. const result = await this.tokens.createApiToken(user.id, body.name, body.scopes); - if (result.error) { - throw new HttpException({ error: result.error }, result.status!); - } return { token: result.token }; } @Delete('api-tokens/:id') async deleteApiToken(@CurrentUser() user: User, @Param('id') id: string) { - const result = await this.tokens.deleteApiToken(user.id, id); - if (result.error) { - throw new HttpException({ error: result.error }, result.status!); - } + await this.tokens.deleteApiToken(user.id, id); return { success: true }; } @@ -377,9 +335,6 @@ export class AuthController { // 503-ing every other user's ws and download tokens. await this.limitUser('ws_token', user.id, 120); const result = await this.tokens.createWsToken(user.id); - if (result.error) { - throw new HttpException({ error: result.error }, result.status!); - } return { token: result.token }; } diff --git a/server/src/nest/auth/auth.service.ts b/server/src/nest/auth/auth.service.ts index 42c7fcc2e..cabb5447c 100644 --- a/server/src/nest/auth/auth.service.ts +++ b/server/src/nest/auth/auth.service.ts @@ -1,3 +1,4 @@ +import { DomainError } from '../common/domain-error'; import { Injectable, OnModuleInit, OnModuleDestroy } from '@nestjs/common'; import bcrypt from 'bcryptjs'; import jwt from 'jsonwebtoken'; @@ -129,7 +130,6 @@ setInterval(() => { }, 5 * 60 * 1000).unref?.(); export interface ResetPasswordOutcome { - error?: string; status?: number; success?: boolean; /** When true the client must collect a TOTP/backup code and call again. */ @@ -410,28 +410,28 @@ export class AuthService { // Auth: register, login, demo // ------------------------------------------------------------------------- - async demoLogin(client?: SessionClient): Promise<{ error?: string; status?: number; token?: string; user?: Record }> { + async demoLogin(client?: SessionClient): Promise<{ token?: string; user?: Record }> { if (!readEnv().demo.enabled) { - return { error: 'Not found', status: 404 }; + throw new DomainError(404, 'Not found'); } const user = await this.usersRepo.findByEmailExact(DEMO_EMAIL_PRIMARY); - if (!user) return { error: 'Demo user not found', status: 500 }; + if (!user) throw new DomainError(500, 'Demo user not found'); const token = await this.generateToken(user, undefined, client); const safe = stripUserForClient(toClientUser(user)) as Record; return { token, user: { ...safe, avatar_url: avatarUrl(user) } }; } - async validateInviteToken(token: string): Promise<{ error?: string; status?: number; valid?: boolean; max_uses?: number; used_count?: number; expires_at?: string | null }> { + async validateInviteToken(token: string): Promise<{ valid?: boolean; max_uses?: number; used_count?: number; expires_at?: string | null }> { const invite = await this.inviteTokens.findByToken(token); - if (!invite) return { error: 'Invalid invite link', status: 404 }; - if (invite.max_uses > 0 && invite.used_count >= invite.max_uses) return { error: 'Invite link has been fully used', status: 410 }; - if (invite.expires_at && new Date(invite.expires_at) < new Date()) return { error: 'Invite link has expired', status: 410 }; + if (!invite) throw new DomainError(404, 'Invalid invite link'); + if (invite.max_uses > 0 && invite.used_count >= invite.max_uses) throw new DomainError(410, 'Invite link has been fully used'); + if (invite.expires_at && new Date(invite.expires_at) < new Date()) throw new DomainError(410, 'Invite link has expired'); // A nullable column stays null on the wire: `?? undefined` would make // JSON.stringify drop the key for a never-expiring invite. return { valid: true, max_uses: invite.max_uses, used_count: invite.used_count, expires_at: invite.expires_at }; } - async registerUser(rawBody: unknown, client?: SessionClient): Promise<{ error?: string; status?: number; token?: string; user?: Record; auditUserId?: number; auditDetails?: Record }> { + async registerUser(rawBody: unknown, client?: SessionClient): Promise<{ token?: string; user?: Record; auditUserId?: number; auditDetails?: Record }> { const body = rawBody as { username?: string; email?: string; password?: string; invite_token?: string }; const username = typeof body.username === 'string' ? body.username.trim() : ''; const email = typeof body.email === 'string' ? body.email.trim() : ''; @@ -442,27 +442,27 @@ export class AuthService { let validInvite: Awaited> = null; if (invite_token) { validInvite = await this.inviteTokens.findByToken(invite_token); - if (!validInvite) return { error: 'Invalid invite link', status: 400 }; - if (validInvite.max_uses > 0 && validInvite.used_count >= validInvite.max_uses) return { error: 'Invite link has been fully used', status: 410 }; - if (validInvite.expires_at && new Date(validInvite.expires_at) < new Date()) return { error: 'Invite link has expired', status: 410 }; + if (!validInvite) throw new DomainError(400, 'Invalid invite link'); + if (validInvite.max_uses > 0 && validInvite.used_count >= validInvite.max_uses) throw new DomainError(410, 'Invite link has been fully used'); + if (validInvite.expires_at && new Date(validInvite.expires_at) < new Date()) throw new DomainError(410, 'Invite link has expired'); } if (userCount > 0 && !validInvite) { const toggles = await this.resolveAuthToggles(); if (!toggles.password_registration) { - return { error: 'Password registration is disabled. Contact your administrator.', status: 403 }; + throw new DomainError(403, 'Password registration is disabled. Contact your administrator.'); } } if (!username || !email || !password) { - return { error: 'Username, email and password are required', status: 400 }; + throw new DomainError(400, 'Username, email and password are required'); } const pwCheck = validatePassword(password); - if (!pwCheck.ok) return { error: pwCheck.reason, status: 400 }; + if (!pwCheck.ok) throw new DomainError(400, pwCheck.reason); if (!EMAIL_REGEX.test(email)) { - return { error: 'Invalid email format', status: 400 }; + throw new DomainError(400, 'Invalid email format'); } const password_hash = bcrypt.hashSync(password, BCRYPT_COST); @@ -476,7 +476,7 @@ export class AuthService { // Ignore guests (#1362): their synthetic username/email must never block a real signup. const existingUserId = await this.usersRepo.findIdByEmailOrUsernameCI(email, username); if (existingUserId !== null) { - return { error: 'Registration failed. Please try different credentials.', status: 409 }; + throw new DomainError(409, 'Registration failed. Please try different credentials.'); } const isFirstUser = (await this.usersRepo.countNonGuest()) === 0; const role = isFirstUser ? 'admin' : 'user'; @@ -509,10 +509,11 @@ export class AuthService { } catch (err) { // The check above runs in the transaction, but a concurrent signup can // still win the insert; the database's unique indexes refuse the second. + if (err instanceof DomainError) throw err; if (err instanceof UserIdentityTakenError) { - return { error: 'Registration failed. Please try different credentials.', status: 409 }; + throw new DomainError(409, 'Registration failed. Please try different credentials.'); } - return { error: 'Error creating user', status: 500 }; + throw new DomainError(500, 'Error creating user'); } } @@ -623,25 +624,25 @@ export class AuthService { remember?: boolean, client?: SessionClient, issueSession = true, - ): Promise<{ error?: string; status?: number; success?: boolean; token?: string }> { + ): Promise<{ success?: boolean; token?: string }> { const body = rawBody as { current_password?: string; new_password?: string }; if (await this.isOidcOnlyMode()) { - return { error: 'Password authentication is disabled.', status: 403 }; + throw new DomainError(403, 'Password authentication is disabled.'); } if (readEnv().demo.enabled && isDemoEmail(userEmail)) { - return { error: 'Password change is disabled in demo mode.', status: 403 }; + throw new DomainError(403, 'Password change is disabled in demo mode.'); } const { current_password, new_password } = body; - if (!current_password) return { error: 'Current password is required', status: 400 }; - if (!new_password) return { error: 'New password is required', status: 400 }; + if (!current_password) throw new DomainError(400, 'Current password is required'); + if (!new_password) throw new DomainError(400, 'New password is required'); const pwCheck = validatePassword(new_password); - if (!pwCheck.ok) return { error: pwCheck.reason, status: 400 }; + if (!pwCheck.ok) throw new DomainError(400, pwCheck.reason); const user = await this.usersRepo.getPasswordHashAndVersion(userId); if (!user || !bcrypt.compareSync(current_password, user.password_hash)) { - return { error: 'Current password is incorrect', status: 401 }; + throw new DomainError(401, 'Current password is incorrect'); } const hash = bcrypt.hashSync(new_password, BCRYPT_COST); @@ -671,14 +672,14 @@ export class AuthService { return { success: true, token }; } - async deleteAccount(userId: number, userEmail: string, userRole: string): Promise<{ error?: string; status?: number; success?: boolean }> { + async deleteAccount(userId: number, userEmail: string, userRole: string): Promise<{ success?: boolean }> { if (readEnv().demo.enabled && isDemoEmail(userEmail)) { - return { error: 'Account deletion is disabled in demo mode.', status: 403 }; + throw new DomainError(403, 'Account deletion is disabled in demo mode.'); } if (userRole === 'admin') { const adminCount = await this.usersRepo.countAdmins(); if (adminCount <= 1) { - return { error: 'Cannot delete the last admin account', status: 400 }; + throw new DomainError(400, 'Cannot delete the last admin account'); } } await this.userCleanup.deleteUserCompletely(userId); @@ -700,9 +701,9 @@ export class AuthService { // work, not with a move. // ------------------------------------------------------------------------- - async getAppSettings(userId: number): Promise<{ error?: string; status?: number; data?: Record }> { + async getAppSettings(userId: number): Promise<{ data?: Record }> { const role = await this.usersRepo.getRole(userId); - if (role !== 'admin') return { error: 'Admin access required', status: 403 }; + if (role !== 'admin') throw new DomainError(403, 'Admin access required'); const result: Record = {}; for (const key of ADMIN_FORM_SETTING_KEYS) { @@ -716,8 +717,6 @@ export class AuthService { userId: number, rawBody: unknown ): Promise<{ - error?: string; - status?: number; success?: boolean; auditSummary?: Record; auditDebugDetails?: Record; @@ -726,7 +725,7 @@ export class AuthService { }> { const body = rawBody as Record; const role = await this.usersRepo.getRole(userId); - if (role !== 'admin') return { error: 'Admin access required', status: 403 }; + if (role !== 'admin') throw new DomainError(403, 'Admin access required'); const { require_mfa } = body; if (require_mfa === true || require_mfa === 'true') { @@ -735,10 +734,7 @@ export class AuthService { // their own account with a passkey may enable it too (not only TOTP). const adminHasPasskey = await this.webauthnCredentials.hasAny(userId); if (!(adminMfa?.mfa_enabled === 1) && !adminHasPasskey) { - return { - error: 'Secure your own account with two-factor authentication or a passkey before requiring it for all users.', - status: 400, - }; + throw new DomainError(400, 'Secure your own account with two-factor authentication or a passkey before requiring it for all users.'); } } @@ -749,7 +745,7 @@ export class AuthService { const nextPasswordLogin = body.password_login !== undefined ? (String(body.password_login) === 'true') : current.password_login; const nextOidcLogin = body.oidc_login !== undefined ? (String(body.oidc_login) === 'true') : current.oidc_login; if (!nextPasswordLogin && (!nextOidcLogin || !oidcConfigured)) { - return { error: 'Cannot disable all login methods. At least one must remain enabled.', status: 400 }; + throw new DomainError(400, 'Cannot disable all login methods. At least one must remain enabled.'); } } @@ -807,13 +803,13 @@ export class AuthService { // MFA // ------------------------------------------------------------------------- - async setupMfa(userId: number, userEmail: string): Promise<{ error?: string; status?: number; secret?: string; otpauth_url?: string; qrPromise?: Promise }> { + async setupMfa(userId: number, userEmail: string): Promise<{ secret?: string; otpauth_url?: string; qrPromise?: Promise }> { if (readEnv().demo.enabled && isDemoEmail(userEmail)) { - return { error: 'MFA is not available in demo mode.', status: 403 }; + throw new DomainError(403, 'MFA is not available in demo mode.'); } const row = await this.usersRepo.getMfaEnabled(userId); if (row?.mfa_enabled) { - return { error: 'MFA is already enabled', status: 400 }; + throw new DomainError(400, 'MFA is already enabled'); } let secret: string, otpauth_url: string; try { @@ -822,24 +818,24 @@ export class AuthService { otpauth_url = authenticator.keyuri(userEmail, 'TREK', secret); } catch (err) { console.error('[MFA] Setup error:', err); - return { error: 'MFA setup failed', status: 500 }; + throw new DomainError(500, 'MFA setup failed'); } return { secret, otpauth_url, qrPromise: QRCode.toString(otpauth_url, { type: 'svg', width: 250 }) }; } - async enableMfa(userId: number, rawCode: unknown): Promise<{ error?: string; status?: number; success?: boolean; mfa_enabled?: boolean; backup_codes?: string[] }> { + async enableMfa(userId: number, rawCode: unknown): Promise<{ success?: boolean; mfa_enabled?: boolean; backup_codes?: string[] }> { const code = rawCode as string | undefined; if (!code) { - return { error: 'Verification code is required', status: 400 }; + throw new DomainError(400, 'Verification code is required'); } const pending = this.getPendingMfaSecret(userId); if (!pending) { - return { error: 'No MFA setup in progress. Start the setup again.', status: 400 }; + throw new DomainError(400, 'No MFA setup in progress. Start the setup again.'); } const tokenStr = String(code).replace(/\s/g, ''); const ok = authenticator.verify({ token: tokenStr, secret: pending }); if (!ok) { - return { error: 'Invalid verification code', status: 401 }; + throw new DomainError(401, 'Invalid verification code'); } const backupCodes = generateBackupCodes(); const backupHashes = backupCodes.map(hashBackupCodeBcrypt); @@ -854,31 +850,31 @@ export class AuthService { userEmail: string, rawBody: unknown, currentSessionId?: string, - ): Promise<{ error?: string; status?: number; success?: boolean; mfa_enabled?: boolean }> { + ): Promise<{ success?: boolean; mfa_enabled?: boolean }> { const body = rawBody as { password?: string; code?: string }; if (readEnv().demo.enabled && isDemoEmail(userEmail)) { - return { error: 'MFA cannot be changed in demo mode.', status: 403 }; + throw new DomainError(403, 'MFA cannot be changed in demo mode.'); } const policy = await readAppSetting(this.appSettings, 'require_mfa'); if (policy === 'true') { - return { error: 'Two-factor authentication cannot be disabled while it is required for all users.', status: 403 }; + throw new DomainError(403, 'Two-factor authentication cannot be disabled while it is required for all users.'); } const { password, code } = body; if (!password || !code) { - return { error: 'Password and authenticator code are required', status: 400 }; + throw new DomainError(400, 'Password and authenticator code are required'); } const user = await this.usersRepo.findById(userId); if (!user?.mfa_enabled || !user.mfa_secret) { - return { error: 'MFA is not enabled', status: 400 }; + throw new DomainError(400, 'MFA is not enabled'); } if (!user.password_hash || !bcrypt.compareSync(password, user.password_hash)) { - return { error: 'Incorrect password', status: 401 }; + throw new DomainError(401, 'Incorrect password'); } const secret = decryptMfaSecret(user.mfa_secret); const tokenStr = String(code).replace(/\s/g, ''); const ok = authenticator.verify({ token: tokenStr, secret }); if (!ok) { - return { error: 'Invalid verification code', status: 401 }; + throw new DomainError(401, 'Invalid verification code'); } // Every other session ends with the second factor; this one just proved both. await this.uow.transactional(async () => { @@ -890,8 +886,6 @@ export class AuthService { } async verifyMfaLogin(rawBody: unknown, client?: SessionClient): Promise<{ - error?: string; - status?: number; token?: string; user?: Record; remember?: boolean; @@ -901,16 +895,16 @@ export class AuthService { const { mfa_token, code, remember_me } = body; const remember = remember_me === true; if (!mfa_token || !code) { - return { error: 'Verification token and code are required', status: 400 }; + throw new DomainError(400, 'Verification token and code are required'); } try { const decoded = jwt.verify(mfa_token, JWT_SECRET, { algorithms: ['HS256'] }) as { id: number; purpose?: string }; if (decoded.purpose !== 'mfa_login') { - return { error: 'Invalid verification token', status: 401 }; + throw new DomainError(401, 'Invalid verification token'); } const user = await this.usersRepo.findById(decoded.id); if (!user || user.mfa_enabled !== 1 || !user.mfa_secret) { - return { error: 'Invalid session', status: 401 }; + throw new DomainError(401, 'Invalid session'); } const secret = decryptMfaSecret(user.mfa_secret); const tokenStr = String(code).trim(); @@ -921,7 +915,7 @@ export class AuthService { // any store older than the bcrypt migration keeps working. const idx = hashes.findIndex((h) => matchBackupCode(tokenStr, h)); if (idx === -1) { - return { error: 'Invalid verification code', status: 401 }; + throw new DomainError(401, 'Invalid verification code'); } hashes.splice(idx, 1); // Consume the backup code and record the login atomically — the code @@ -941,8 +935,9 @@ export class AuthService { remember, auditUserId: Number(user.id), }; - } catch { - return { error: 'Invalid or expired verification token', status: 401 }; + } catch (err) { + if (err instanceof DomainError) throw err; + throw new DomainError(401, 'Invalid or expired verification token'); } } @@ -1021,28 +1016,28 @@ export class AuthService { const body = rawBody as { token?: string; new_password?: string; mfa_code?: string }; const { token, new_password, mfa_code } = body; if (!token || typeof token !== 'string') { - return { error: 'Reset token is required', status: 400 }; + throw new DomainError(400, 'Reset token is required'); } if (!new_password || typeof new_password !== 'string') { - return { error: 'New password is required', status: 400 }; + throw new DomainError(400, 'New password is required'); } // Check the policy BEFORE touching the token so an invalid password // does not burn the user's one-time link. const pwCheck = validatePassword(new_password); - if (!pwCheck.ok) return { error: pwCheck.reason!, status: 400 }; + if (!pwCheck.ok) throw new DomainError(400, pwCheck.reason!); const tokenHash = hashResetToken(token); const row = await this.passwordResetTokens.findByTokenHash(tokenHash); - if (!row) return { error: 'Invalid or expired reset link', status: 400 }; - if (row.consumed_at) return { error: 'This reset link has already been used', status: 400 }; + if (!row) throw new DomainError(400, 'Invalid or expired reset link'); + if (row.consumed_at) throw new DomainError(400, 'This reset link has already been used'); if ((parseDbTimestamp(row.expires_at)?.getTime() ?? Number.NaN) < Date.now()) { - return { error: 'Reset link has expired. Please request a new one.', status: 400 }; + throw new DomainError(400, 'Reset link has expired. Please request a new one.'); } const user = await this.usersRepo.findResetTarget(row.user_id); - if (!user) return { error: 'Invalid or expired reset link', status: 400 }; + if (!user) throw new DomainError(400, 'Invalid or expired reset link'); // MFA gate. If enabled, require a valid TOTP or backup code. const mfaOn = user.mfa_enabled === 1; @@ -1050,7 +1045,7 @@ export class AuthService { if (mfaOn) { if (!user.mfa_secret) { // Data inconsistency — fail closed. - return { error: 'MFA is enabled but not configured. Contact your administrator.', status: 500 }; + throw new DomainError(500, 'MFA is enabled but not configured. Contact your administrator.'); } const supplied = typeof mfa_code === 'string' ? mfa_code.trim() : ''; if (!supplied) return { mfa_required: true, status: 200 }; @@ -1060,7 +1055,7 @@ export class AuthService { if (!okTotp) { const hashes = parseBackupCodeHashes(user.mfa_backup_codes); const idx = hashes.findIndex((h) => matchBackupCode(supplied, h)); - if (idx === -1) return { error: 'Invalid MFA code', status: 401 }; + if (idx === -1) throw new DomainError(401, 'Invalid MFA code'); backupCodeConsumedIndex = idx; } } diff --git a/server/src/nest/auth/passkey.controller.ts b/server/src/nest/auth/passkey.controller.ts index 48d5a5489..17c3e1530 100644 --- a/server/src/nest/auth/passkey.controller.ts +++ b/server/src/nest/auth/passkey.controller.ts @@ -53,7 +53,6 @@ export class PasskeyController { // The Origin header is only a pre-ceremony sanity input (see PasskeyService) // — the RP ID itself is never derived from request headers. const result = await this.passkeys.passkeyRegisterOptions(user.id, body?.password, req.headers.origin); - if (result.error) throw new HttpException({ error: result.error }, result.status!); return result.options; } @@ -63,7 +62,6 @@ export class PasskeyController { @UseGuards(PasskeyEnabledGuard, JwtAuthGuard) async registerVerify(@CurrentUser() user: User, @Body() body: PasskeyRegisterVerifyDto, @Req() req: Request) { const result = await this.passkeys.passkeyRegisterVerify(user.id, body); - if (result.error) throw new HttpException({ error: result.error }, result.status!); await this.audit.writeAudit({ userId: user.id, action: 'user.passkey_register', ip: getClientIp(req) }); return { success: true, credential: result.credential }; } @@ -76,7 +74,6 @@ export class PasskeyController { async loginOptions(@Req() req: Request) { await this.limit('login', req, 10); const result = await this.passkeys.passkeyLoginOptions(req.headers.origin); - if (result.error) throw new HttpException({ error: result.error }, result.status!); return result.options; } @@ -112,8 +109,7 @@ export class PasskeyController { @Patch('credentials/:id') @UseGuards(JwtAuthGuard) async rename(@CurrentUser() user: User, @Param('id') id: string, @Body() body: PasskeyRenameDto) { - const result = await this.passkeys.renamePasskey(user.id, id, body?.name); - if (result.error) throw new HttpException({ error: result.error }, result.status!); + await this.passkeys.renamePasskey(user.id, id, body?.name); return { success: true }; } @@ -121,8 +117,7 @@ export class PasskeyController { @UseGuards(JwtAuthGuard) async remove(@CurrentUser() user: User, @Param('id') id: string, @Body() body: PasskeyDeleteDto, @Req() req: Request) { await this.limit('login', req, 5); - const result = await this.passkeys.deletePasskey(user.id, id, body?.password); - if (result.error) throw new HttpException({ error: result.error }, result.status!); + await this.passkeys.deletePasskey(user.id, id, body?.password); await this.audit.writeAudit({ userId: user.id, action: 'user.passkey_delete', resource: String(id), ip: getClientIp(req) }); return { success: true }; } diff --git a/server/src/nest/auth/passkey.service.ts b/server/src/nest/auth/passkey.service.ts index 1ece6f7e5..0a13fab74 100644 --- a/server/src/nest/auth/passkey.service.ts +++ b/server/src/nest/auth/passkey.service.ts @@ -1,3 +1,4 @@ +import { DomainError } from '../common/domain-error'; import { Injectable, Logger } from '@nestjs/common'; import { InjectRepository } from '@mikro-orm/nestjs'; import bcrypt from 'bcryptjs'; @@ -41,6 +42,9 @@ const NOT_CONFIGURED = { error: 'Passkey login is not configured for this server // used to tell "no such credential" apart from "bad signature" (CWE-203). const AUTH_FAILED = { error: 'Authentication failed', status: 401 } as const; +/** One of the canned `{ error, status }` pairs above, raised. */ +const refusal = (r: { error: string; status: number }) => new DomainError(r.status, r.error); + // Reference-compared sentinel (oidc invite_exhausted precedent): thrown inside // the register transaction to keep the duplicate 409 distinct from the generic // insert-failure 400 without string-matching SQLite errors. @@ -224,19 +228,19 @@ export class PasskeyService { userId: number, password: string | undefined, requestOrigin?: string, - ): Promise<{ error?: string; status?: number; options?: Awaited> }> { + ): Promise<{ options?: Awaited> }> { const cfg = await this.webauthn.resolve(); - if (!cfg) return { ...NOT_CONFIGURED }; - if (this.originCannotVerify(cfg, requestOrigin)) return { ...NOT_CONFIGURED }; + if (!cfg) throw refusal(NOT_CONFIGURED); + if (this.originCannotVerify(cfg, requestOrigin)) throw refusal(NOT_CONFIGURED); const user = await this.users.findById(userId); - if (!user) return { error: 'User not found', status: 404 }; + if (!user) throw new DomainError(404, 'User not found'); // Re-authentication: a hijacked session must not be able to silently plant an // attacker-controlled passkey. Require the current password (parity with the // change-password / disable-MFA step-up). if (!password || !user.password_hash || !bcrypt.compareSync(password, user.password_hash)) { - return { error: 'Incorrect password', status: 401 }; + throw new DomainError(401, 'Incorrect password'); } const existing = await this.webauthnCredentials.listExcludeCredentials(userId); @@ -264,20 +268,20 @@ export class PasskeyService { async passkeyRegisterVerify( userId: number, body: { attestationResponse?: unknown; name?: unknown }, - ): Promise<{ error?: string; status?: number; success?: boolean; credential?: unknown }> { + ): Promise<{ success?: boolean; credential?: unknown }> { const cfg = await this.webauthn.resolve(); - if (!cfg) return { ...NOT_CONFIGURED }; + if (!cfg) throw refusal(NOT_CONFIGURED); const resp = body?.attestationResponse; - if (!resp) return { error: 'Invalid registration response', status: 400 }; + if (!resp) throw new DomainError(400, 'Invalid registration response'); const challenge = challengeFromResponse(resp); - if (!challenge) return { error: 'Invalid registration response', status: 400 }; + if (!challenge) throw new DomainError(400, 'Invalid registration response'); const now = Date.now(); const claimed = await this.claimChallenge(challenge, 'registration', now); if (!claimed || claimed.user_id !== userId) { - return { error: 'Registration challenge expired. Please try again.', status: 400 }; + throw new DomainError(400, 'Registration challenge expired. Please try again.'); } const expectedOrigin = this.expectedOrigins(cfg, resp); @@ -296,11 +300,11 @@ export class PasskeyService { this.logger.warn( `Passkey registration rejected (expectedRPID=${cfg.rpID}, expectedOrigin=[${expectedOrigin.join(', ')}]): ${err instanceof Error ? err.message : String(err)}`, ); - return { error: 'Could not register this passkey.', status: 400 }; + throw new DomainError(400, 'Could not register this passkey.'); } if (!verification.verified || !verification.registrationInfo) { - return { error: 'Could not register this passkey.', status: 400 }; + throw new DomainError(400, 'Could not register this passkey.'); } // Persist ONLY the values the verifier vouches for — never anything parsed @@ -329,9 +333,9 @@ export class PasskeyService { }); } catch (err) { if (err === DUPLICATE_CREDENTIAL) { - return { error: 'This passkey is already registered.', status: 409 }; + throw new DomainError(409, 'This passkey is already registered.'); } - return { error: 'Could not register this passkey.', status: 400 }; + throw new DomainError(400, 'Could not register this passkey.'); } const created = (await this.webauthnCredentials.findCreatedCredential(credential.id))!; @@ -343,13 +347,11 @@ export class PasskeyService { // ------------------------------------------------------------------------- async passkeyLoginOptions(requestOrigin?: string): Promise<{ - error?: string; - status?: number; options?: Awaited>; }> { const cfg = await this.webauthn.resolve(); - if (!cfg) return { ...NOT_CONFIGURED }; - if (this.originCannotVerify(cfg, requestOrigin)) return { ...NOT_CONFIGURED }; + if (!cfg) throw refusal(NOT_CONFIGURED); + if (this.originCannotVerify(cfg, requestOrigin)) throw refusal(NOT_CONFIGURED); const now = Date.now(); await this.purgeExpiredChallenges(now); @@ -452,9 +454,9 @@ export class PasskeyService { return rows.map((r) => ({ ...r, backed_up: r.backed_up === 1 })); } - async renamePasskey(userId: number, id: string, name: unknown): Promise<{ error?: string; status?: number; success?: boolean }> { + async renamePasskey(userId: number, id: string, name: unknown): Promise<{ success?: boolean }> { const cleanName = sanitizeName(name); - if (!cleanName) return { error: 'Name is required', status: 400 }; + if (!cleanName) throw new DomainError(400, 'Name is required'); // Convert, VALIDATE, and answer the legacy not-found before the // repository call (program rule 15): the legacy `UPDATE ... WHERE id = ? // AND user_id = ?` bound `Number(id)` as a plain parameter — a @@ -466,10 +468,10 @@ export class PasskeyService { // reference and throws — a 500 where the legacy 404'd (Plan 3b Task 3 // review, F1). const rowId = toRowId(id); - if (rowId === null) return { error: 'Passkey not found', status: 404 }; + if (rowId === null) throw new DomainError(404, 'Passkey not found'); // Ownership enforced in SQL (404 on miss, never a 403 that leaks existence). const changes = await this.webauthnCredentials.renameOwned(rowId, userId, cleanName); - if (changes === 0) return { error: 'Passkey not found', status: 404 }; + if (changes === 0) throw new DomainError(404, 'Passkey not found'); return { success: true }; } @@ -477,27 +479,27 @@ export class PasskeyService { userId: number, id: string, password: string | undefined, - ): Promise<{ error?: string; status?: number; success?: boolean }> { + ): Promise<{ success?: boolean }> { // Re-auth before removing a credential (a hijacked session must not be able to // strip the victim's passkeys). Deleting is always allowed because every // account keeps a usable password as recovery fallback — losing all passkeys // can never lock anyone out. const passwordHash = await this.users.getPasswordHash(userId); if (!passwordHash || !password || !bcrypt.compareSync(password, passwordHash)) { - return { error: 'Incorrect password', status: 401 }; + throw new DomainError(401, 'Incorrect password'); } // Same guard as `renamePasskey` (F1) — placed after the password check // to match the legacy statement order exactly (a wrong password still // answers 401 before a bad id is ever considered). const rowId = toRowId(id); - if (rowId === null) return { error: 'Passkey not found', status: 404 }; + if (rowId === null) throw new DomainError(404, 'Passkey not found'); const changes = await this.webauthnCredentials.deleteOwned(rowId, userId); - if (changes === 0) return { error: 'Passkey not found', status: 404 }; + if (changes === 0) throw new DomainError(404, 'Passkey not found'); return { success: true }; } /** Admin: clear all of a user's passkeys (e.g. on suspected compromise). */ - async adminResetPasskeys(targetUserId: number): Promise<{ error?: string; status?: number; success?: boolean; deleted?: number; email?: string }> { + async adminResetPasskeys(targetUserId: number): Promise<{ success?: boolean; deleted?: number; email?: string }> { // `AdminService.resetUserPasskeys` converts the route param with a bare // `Number(id)` before calling in — a non-numeric id arrives here as // `NaN`, still typed `number` at the JS level. Same F1 guard: validate @@ -505,9 +507,9 @@ export class PasskeyService { // `SELECT id, email FROM users WHERE id = ?` bound `Number(id)` too and // simply matched no row). const rowId = toRowId(targetUserId); - if (rowId === null) return { error: 'User not found', status: 404 }; + if (rowId === null) throw new DomainError(404, 'User not found'); const target = await this.users.findIdAndEmail(rowId); - if (!target) return { error: 'User not found', status: 404 }; + if (!target) throw new DomainError(404, 'User not found'); const deleted = await this.webauthnCredentials.deleteAllForUser(rowId); return { success: true, deleted, email: target.email }; } diff --git a/server/src/nest/auth/registration-invites.service.ts b/server/src/nest/auth/registration-invites.service.ts index 6957fcc53..b4dc5f597 100644 --- a/server/src/nest/auth/registration-invites.service.ts +++ b/server/src/nest/auth/registration-invites.service.ts @@ -1,3 +1,4 @@ +import { DomainError } from '../common/domain-error'; import { Injectable } from '@nestjs/common'; import { InjectRepository } from '@mikro-orm/nestjs'; import crypto from 'crypto'; @@ -101,7 +102,7 @@ export class RegistrationInvitesService { const numericId = toRowId(id); // RI6 — the 404 check. if (numericId === null || (await this.inviteTokens.findIdById(numericId)) === null) { - return { error: 'Invite not found', status: 404 }; + throw new DomainError(404, 'Invite not found'); } // RI7. await this.inviteTokens.deleteById(numericId); diff --git a/server/src/nest/auth/user-profile.service.ts b/server/src/nest/auth/user-profile.service.ts index 034b63803..705ce3a89 100644 --- a/server/src/nest/auth/user-profile.service.ts +++ b/server/src/nest/auth/user-profile.service.ts @@ -1,3 +1,4 @@ +import { DomainError } from '../common/domain-error'; import { Injectable } from '@nestjs/common'; import { InjectRepository } from '@mikro-orm/nestjs'; import { readEnv, getAppUrl } from '../../app-config'; @@ -184,29 +185,29 @@ export class UserProfileService { async updateSettings( userId: number, rawBody: unknown - ): Promise<{ error?: string; status?: number; success?: boolean; user?: Record; changedKeys?: string[] }> { + ): Promise<{ success?: boolean; user?: Record; changedKeys?: string[] }> { const body = rawBody as { maps_api_key?: string; openweather_api_key?: string; unsplash_api_key?: string; amap_api_key?: string; username?: string; email?: string }; const { maps_api_key, openweather_api_key, unsplash_api_key, amap_api_key, username, email } = body; if (username !== undefined) { const trimmed = username.trim(); if (!trimmed || trimmed.length < 2 || trimmed.length > 50) { - return { error: 'Username must be between 2 and 50 characters', status: 400 }; + throw new DomainError(400, 'Username must be between 2 and 50 characters'); } if (!/^[a-zA-Z0-9_.-]+$/.test(trimmed)) { - return { error: 'Username can only contain letters, numbers, underscores, dots and hyphens', status: 400 }; + throw new DomainError(400, 'Username can only contain letters, numbers, underscores, dots and hyphens'); } const conflict = await this.usersRepo.findIdByUsernameCI(trimmed, userId); - if (conflict) return { error: 'Username already taken', status: 409 }; + if (conflict) throw new DomainError(409, 'Username already taken'); } if (email !== undefined) { const trimmed = email.trim(); if (!trimmed || !EMAIL_REGEX.test(trimmed)) { - return { error: 'Invalid email format', status: 400 }; + throw new DomainError(400, 'Invalid email format'); } const conflict = await this.usersRepo.findIdByEmailCI(trimmed, userId); - if (conflict) return { error: 'Email already taken', status: 409 }; + if (conflict) throw new DomainError(409, 'Email already taken'); } // The name and email half of this body stays the user's own in every mode; @@ -241,7 +242,7 @@ export class UserProfileService { } catch (err) { // Another write took the name or address between the check above and this one. if (err instanceof UserIdentityTakenError) { - return { error: err.field === 'email' ? 'Email already taken' : 'Username already taken', status: 409 }; + throw new DomainError(409, err.field === 'email' ? 'Email already taken' : 'Username already taken'); } throw err; } @@ -258,9 +259,9 @@ export class UserProfileService { }; } - async getSettings(userId: number): Promise<{ error?: string; status?: number; settings?: Record }> { + async getSettings(userId: number): Promise<{ settings?: Record }> { const user = await this.usersRepo.getApiKeyColumns(userId); - if (user?.role !== 'admin') return { error: 'Admin access required', status: 403 }; + if (user?.role !== 'admin') throw new DomainError(403, 'Admin access required'); // The one endpoint in the codebase that hands back a stored key in the // clear. That is fine when the admin pasted it in themselves and wrong when @@ -348,9 +349,9 @@ export class UserProfileService { // Key validation // ------------------------------------------------------------------------- - async validateKeys(userId: number): Promise<{ error?: string; status?: number; maps: boolean; weather: boolean; maps_details: null | { ok: boolean; status: number | null; status_text: string | null; error_message: string | null; error_status: string | null; error_raw: string | null } }> { + async validateKeys(userId: number): Promise<{ maps: boolean; weather: boolean; maps_details: null | { ok: boolean; status: number | null; status_text: string | null; error_message: string | null; error_status: string | null; error_raw: string | null } }> { const user = await this.usersRepo.getRoleAndWeatherKey(userId); - if (user?.role !== 'admin') return { error: 'Admin access required', status: 403, maps: false, weather: false, maps_details: null }; + if (user?.role !== 'admin') throw new DomainError(403, 'Admin access required'); const result: { maps: boolean; diff --git a/server/src/nest/common/domain-error.ts b/server/src/nest/common/domain-error.ts index 096a4a8f7..aab3764c9 100644 --- a/server/src/nest/common/domain-error.ts +++ b/server/src/nest/common/domain-error.ts @@ -79,14 +79,13 @@ export const conflict = (message: string, details?: Record) => * Runs `fn` and returns the DomainError it raised instead of throwing it; any * other error propagates. For the few callers that must act on a refusal before * passing it on (an audit row, a constant-time pad) and for service tests. + * Test the result with `instanceof DomainError`. */ -export async function catchDomainError( - fn: () => Promise | T, -): Promise<{ ok: true; value: T } | { ok: false; error: DomainError }> { +export async function catchDomainError(fn: () => Promise | T): Promise { try { - return { ok: true, value: await fn() }; + return await fn(); } catch (err) { - if (err instanceof DomainError) return { ok: false, error: err }; + if (err instanceof DomainError) return err; throw err; } } diff --git a/server/src/nest/oauth/oauth.service.ts b/server/src/nest/oauth/oauth.service.ts index 56dcf1411..2fbf25401 100644 --- a/server/src/nest/oauth/oauth.service.ts +++ b/server/src/nest/oauth/oauth.service.ts @@ -1,3 +1,4 @@ +import { DomainError } from '../common/domain-error'; import crypto, { randomBytes, randomUUID } from 'crypto'; import { Injectable } from '@nestjs/common'; import { InjectRepository } from '@mikro-orm/nestjs'; @@ -745,10 +746,10 @@ export class OauthService { async adminRevokeOAuthSession(id: string) { const tokenId = toRowId(id); - if (tokenId === null) return { error: 'Session not found', status: 404 }; + if (tokenId === null) throw new DomainError(404, 'Session not found'); const row = await this.tokens.findById(tokenId); - if (!row) return { error: 'Session not found', status: 404 }; + if (!row) throw new DomainError(404, 'Session not found'); await this.tokens.revokeById(tokenId); revokeUserSessionsForClient(row.user_id, row.client_id); return {}; diff --git a/server/src/nest/tokens/token.service.ts b/server/src/nest/tokens/token.service.ts index fa5d5e214..4e42be531 100644 --- a/server/src/nest/tokens/token.service.ts +++ b/server/src/nest/tokens/token.service.ts @@ -1,3 +1,4 @@ +import { DomainError } from '../common/domain-error'; import { Injectable } from '@nestjs/common'; import { InjectRepository } from '@mikro-orm/nestjs'; import { randomBytes, createHash } from 'crypto'; @@ -108,13 +109,13 @@ export class TokenService { return await this.createToken(userId, rawName, 'api', scopes); } - private async createToken(userId: number, rawName: unknown, kind: TokenKind, scopes?: readonly string[]): Promise<{ error?: string; status?: number; token?: Record }> { + private async createToken(userId: number, rawName: unknown, kind: TokenKind, scopes?: readonly string[]): Promise<{ token?: Record }> { const name = rawName as string | undefined; - if (!name?.trim()) return { error: 'Token name is required', status: 400 }; - if (name.trim().length > 100) return { error: 'Token name must be 100 characters or less', status: 400 }; + if (!name?.trim()) throw new DomainError(400, 'Token name is required'); + if (name.trim().length > 100) throw new DomainError(400, 'Token name must be 100 characters or less'); const tokenCount = await this.tokens.countByUserAndKind(userId, kind); - if (tokenCount >= 10) return { error: 'Maximum of 10 tokens per user reached', status: 400 }; + if (tokenCount >= 10) throw new DomainError(400, 'Maximum of 10 tokens per user reached'); const rawToken = 'trek_' + randomBytes(24).toString('hex'); const tokenHash = createHash('sha256').update(rawToken).digest('hex'); @@ -162,7 +163,7 @@ export class TokenService { * happily delete a token the MCP panel manages, and the user would find a key * missing from a screen they never opened. */ - private async deleteToken(userId: number, tokenId: string, kind: TokenKind): Promise<{ error?: string; status?: number; success?: boolean }> { + private async deleteToken(userId: number, tokenId: string, kind: TokenKind): Promise<{ success?: boolean }> { // Convert, VALIDATE, and answer the legacy not-found before any // repository call (program rule 15): the legacy statement bound // `tokenId` straight into `WHERE id = ?` and let SQLite's affinity rules @@ -170,9 +171,9 @@ export class TokenService { // leniency, so a bare `Number()` turned a 404 into a 500 (Plan 3b Task 2 // review, F1). const id = toRowId(tokenId); - if (id === null) return { error: 'Token not found', status: 404 }; + if (id === null) throw new DomainError(404, 'Token not found'); const token = await this.tokens.findOwnedByKind(id, userId, kind); - if (!token) return { error: 'Token not found', status: 404 }; + if (!token) throw new DomainError(404, 'Token not found'); await this.tokens.deleteById(id); // Best-effort, like the changePassword/resetPassword revocations: a session // sweep failure must not turn a successful token delete into a 500. @@ -184,12 +185,12 @@ export class TokenService { // Ephemeral tokens // ------------------------------------------------------------------------- - async createWsToken(userId: number): Promise<{ error?: string; status?: number; token?: string }> { + async createWsToken(userId: number): Promise<{ token?: string }> { // Bind the ws-token to the user's current password_version so a token minted // before a password reset is rejected on connect (defence-in-depth session gate). const pv = (await this.users.getPasswordVersion(userId)) ?? 0; const token = this.ephemeral.create(userId, 'ws', { pv }); - if (!token) return { error: 'Service unavailable', status: 503 }; + if (!token) throw new DomainError(503, 'Service unavailable'); return { token }; } @@ -222,9 +223,9 @@ export class TokenService { // Same guard as `deleteToken` above — convert, VALIDATE, answer the // legacy 404 before any repository call (F1). const numericId = toRowId(id); - if (numericId === null) return { error: 'Token not found', status: 404 }; + if (numericId === null) throw new DomainError(404, 'Token not found'); const token = await this.tokens.findBasic(numericId); - if (!token) return { error: 'Token not found', status: 404 }; + if (!token) throw new DomainError(404, 'Token not found'); await this.tokens.deleteById(numericId); revokeUserSessions(token.user_id); return {}; diff --git a/server/tests/helpers/domain-error.ts b/server/tests/helpers/domain-error.ts new file mode 100644 index 000000000..5da09a72a --- /dev/null +++ b/server/tests/helpers/domain-error.ts @@ -0,0 +1,20 @@ +import { DomainError } from '../../src/nest/common/domain-error'; + +/** + * Awaits a service call and hands a `DomainError` refusal back as the + * `{ error, status }` result the service used to return, so a test can assert a + * refusal and a success through one shape. Any other rejection propagates. + */ +export async function asLegacyResult(call: Promise | T): Promise { + try { + return (await call) as T & { error?: string; status?: number }; + } catch (err) { + if (err instanceof DomainError) { + return { ...err.details, error: err.publicMessage, status: err.getStatus() } as T & { + error?: string; + status?: number; + }; + } + throw err; + } +} diff --git a/server/tests/unit/nest/admin.controller.test.ts b/server/tests/unit/nest/admin.controller.test.ts index fced5bf23..f3c26cbb2 100644 --- a/server/tests/unit/nest/admin.controller.test.ts +++ b/server/tests/unit/nest/admin.controller.test.ts @@ -1,3 +1,4 @@ +import { DomainError } from '../../../src/nest/common/domain-error'; import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest'; import { HttpException, NotFoundException } from '@nestjs/common'; import type { Request } from 'express'; @@ -88,7 +89,7 @@ afterEach(() => { delete process.env.NODE_ENV; }); describe('AdminController users', () => { it('lists, creates (201 + audit), maps an error', async () => { expect(await adminCtl(svc({ listUsers: vi.fn().mockReturnValue([{ id: 1 }]) } as Partial)).listUsers()).toEqual({ users: [{ id: 1 }] }); - expect(await thrown(() => adminCtl(svc({ createUser: vi.fn().mockReturnValue({ error: 'Email taken', status: 409 }) } as Partial)).createUser(user, {}, req))).toEqual({ status: 409, body: { error: 'Email taken' } }); + expect(await thrown(() => adminCtl(svc({ createUser: vi.fn().mockRejectedValue(new DomainError(409, 'Email taken')) } as Partial)).createUser(user, {}, req))).toEqual({ status: 409, body: { error: 'Email taken' } }); const c = adminCtl(svc({ createUser: vi.fn().mockReturnValue({ user: { id: 2 }, insertedId: 2, auditDetails: {} }) } as Partial)); expect(await c.createUser(user, { email: 'a@b.c' }, req)).toEqual({ user: { id: 2 } }); expect(writeAudit).toHaveBeenCalledWith(expect.objectContaining({ action: 'admin.user_create' })); @@ -96,7 +97,7 @@ describe('AdminController users', () => { it('update + delete audit and map errors', async () => { expect(await adminCtl(svc({ updateUser: vi.fn().mockReturnValue({ user: { id: 2 }, previousEmail: 'a@b.c', changed: ['role'] }) } as Partial)).updateUser(user, '2', {}, req)).toEqual({ user: { id: 2 } }); - expect(await thrown(() => adminCtl(svc({ deleteUser: vi.fn().mockReturnValue({ error: 'Cannot delete self', status: 400 }) } as Partial)).deleteUser(user, '1', req))).toEqual({ status: 400, body: { error: 'Cannot delete self' } }); + expect(await thrown(() => adminCtl(svc({ deleteUser: vi.fn().mockRejectedValue(new DomainError(400, 'Cannot delete self')) } as Partial)).deleteUser(user, '1', req))).toEqual({ status: 400, body: { error: 'Cannot delete self' } }); expect(await adminCtl(svc({ deleteUser: vi.fn().mockReturnValue({ email: 'a@b.c' }) } as Partial)).deleteUser(user, '2', req)).toEqual({ success: true }); }); }); @@ -114,7 +115,7 @@ describe('AdminController permissions + oidc + misc', () => { it('save-demo-baseline maps error, else returns message', async () => { - expect(await thrown(() => adminCtl(svc({ saveDemoBaseline: vi.fn().mockReturnValue({ error: 'not demo', status: 400 }) } as Partial)).saveDemoBaseline(user, req))).toEqual({ status: 400, body: { error: 'not demo' } }); + expect(await thrown(() => adminCtl(svc({ saveDemoBaseline: vi.fn().mockRejectedValue(new DomainError(400, 'not demo')) } as Partial)).saveDemoBaseline(user, req))).toEqual({ status: 400, body: { error: 'not demo' } }); expect(await adminCtl(svc({ saveDemoBaseline: vi.fn().mockReturnValue({ message: 'saved' }) } as Partial)).saveDemoBaseline(user, req)).toEqual({ success: true, message: 'saved' }); }); }); @@ -127,7 +128,7 @@ describe('AdminController invites', () => { const c = adminCtl(svc(), undefined, undefined, {}, { createInvite: vi.fn().mockReturnValue({ invite: { id: 5 }, inviteId: 5, uses: 1, expiresInDays: 7 }) }); expect(await c.createInvite(user, {}, req)).toEqual({ invite: { id: 5 } }); expect(writeAudit).toHaveBeenCalledWith(expect.objectContaining({ action: 'admin.invite_create' })); - expect(await thrown(() => adminCtl(svc(), undefined, undefined, {}, { deleteInvite: vi.fn().mockReturnValue({ error: 'not found', status: 404 }) }).deleteInvite(user, '5', req))).toEqual({ status: 404, body: { error: 'not found' } }); + expect(await thrown(() => adminCtl(svc(), undefined, undefined, {}, { deleteInvite: vi.fn().mockRejectedValue(new DomainError(404, 'not found')) }).deleteInvite(user, '5', req))).toEqual({ status: 404, body: { error: 'not found' } }); }); @@ -157,7 +158,7 @@ describe('AdminController addons + sessions + jwt + defaults', () => { }); it('addon update maps the journey-dependency refusal to its status, before any audit', async () => { - const c = adminCtl(svc({ updateAddon: vi.fn().mockReturnValue({ error: 'Enable the Journey addon first', status: 409 }) } as Partial)); + const c = adminCtl(svc({ updateAddon: vi.fn().mockRejectedValue(new DomainError(409, 'Enable the Journey addon first')) } as Partial)); const err = await c.updateAddon(user, 'immich', { enabled: true }, req).then( () => null, (e: unknown) => e as HttpException, @@ -170,20 +171,12 @@ describe('AdminController addons + sessions + jwt + defaults', () => { it('oauth-sessions revoke audits; rotate-jwt maps error', async () => { expect(await adminCtl(svc(), undefined, undefined, {}, {}, { adminRevokeOAuthSession: vi.fn().mockReturnValue({}) }).revokeOAuthSession(user, '3', req)).toEqual({ success: true }); - expect(await thrown(() => adminCtl(svc({ rotateJwtSecret: vi.fn().mockReturnValue({ error: 'locked', status: 409 }) } as Partial)).rotateJwtSecret(user, req))).toEqual({ status: 409, body: { error: 'locked' } }); + expect(await thrown(() => adminCtl(svc({ rotateJwtSecret: vi.fn(() => { throw new DomainError(409, 'locked'); }) } as Partial)).rotateJwtSecret(user, req))).toEqual({ status: 409, body: { error: 'locked' } }); expect(await adminCtl(svc({ rotateJwtSecret: vi.fn().mockReturnValue({}) } as Partial)).rotateJwtSecret(user, req)).toEqual({ success: true }); }); }); -describe('AdminController error envelope fallbacks', () => { - it('ok() defaults to 400 when the error envelope omits a status', async () => { - expect(await thrown(() => adminCtl(svc({ createUser: vi.fn().mockReturnValue({ error: 'boom' }) } as Partial)).createUser(user, {}, req))).toEqual({ status: 400, body: { error: 'boom' } }); - }); - - -}); - describe('AdminController read-only getters', () => { it('return service values verbatim', async () => { expect(await adminCtl(svc({ resetUserPasskeys: vi.fn().mockReturnValue({ email: 'a@b.c', deleted: 2 }) } as Partial)).resetUserPasskeys(user, '4', req)).toEqual({ success: true, deleted: 2 }); @@ -213,8 +206,8 @@ describe('AdminController read-only getters', () => { describe('AdminController tokens + sessions', () => { it('mcp token + oauth session deletes return success and map errors', async () => { expect(await adminCtl(svc(), undefined, undefined, { adminDeleteMcpToken: vi.fn().mockReturnValue({}) }).deleteMcpToken(user, '2', req)).toEqual({ success: true }); - expect(await thrown(() => adminCtl(svc(), undefined, undefined, { adminDeleteMcpToken: vi.fn().mockReturnValue({ error: 'no token', status: 404 }) }).deleteMcpToken(user, '9', req))).toEqual({ status: 404, body: { error: 'no token' } }); - expect(await thrown(() => adminCtl(svc(), undefined, undefined, {}, {}, { adminRevokeOAuthSession: vi.fn().mockReturnValue({ error: 'no session', status: 404 }) }).revokeOAuthSession(user, '9', req))).toEqual({ status: 404, body: { error: 'no session' } }); + expect(await thrown(() => adminCtl(svc(), undefined, undefined, { adminDeleteMcpToken: vi.fn().mockRejectedValue(new DomainError(404, 'no token')) }).deleteMcpToken(user, '9', req))).toEqual({ status: 404, body: { error: 'no token' } }); + expect(await thrown(() => adminCtl(svc(), undefined, undefined, {}, {}, { adminRevokeOAuthSession: vi.fn().mockRejectedValue(new DomainError(404, 'no session')) }).revokeOAuthSession(user, '9', req))).toEqual({ status: 404, body: { error: 'no session' } }); }); }); diff --git a/server/tests/unit/nest/admin.service.test.ts b/server/tests/unit/nest/admin.service.test.ts index 43a8e7133..1f011747d 100644 --- a/server/tests/unit/nest/admin.service.test.ts +++ b/server/tests/unit/nest/admin.service.test.ts @@ -9,6 +9,7 @@ * integration tests don't exercise. VCJOB-001 pins the version-check cron path * (it replaced ADMIN-BR-001 when the old admin bridge died with the cron move). */ +import { asLegacyResult } from '../../helpers/domain-error'; import { describe, it, expect, vi, beforeAll, beforeEach, afterEach, afterAll } from 'vitest'; import { UsersRepository } from '../../../src/db/repositories/Users.repository'; import { ADDON_IDS, MCP_GATED_ADDON_IDS } from '../../../src/addons'; @@ -196,18 +197,18 @@ beforeAll(async () => { // Legacy free-function names bound to the service, so the moved cases below read // exactly as they did before the fold. const listUsers = () => svc.listUsers(); -const svcCreateUser = (d: Parameters[0]) => svc.createUser(d); -const updateUser = (id: string, d: Parameters[1]) => svc.updateUser(id, d); -const deleteUser = (id: string, actingId: number) => svc.deleteUser(id, actingId); +const svcCreateUser = (d: Parameters[0]) => asLegacyResult(svc.createUser(d)); +const updateUser = (id: string, d: Parameters[1]) => asLegacyResult(svc.updateUser(id, d)); +const deleteUser = (id: string, actingId: number) => asLegacyResult(svc.deleteUser(id, actingId)); const getStats = () => svc.getStats(); const getPermissions = () => svc.getPermissions(); const savePermissions = (p: Record) => svc.savePermissions(p); const getAuditLog = (q: { limit?: string; offset?: string }) => svc.getAuditLog(q); -const saveDemoBaseline = () => svc.saveDemoBaseline(); +const saveDemoBaseline = () => asLegacyResult(svc.saveDemoBaseline()); const getGithubReleases = (perPage?: string, page?: string) => svc.getGithubReleases(perPage, page); const checkVersion = () => svc.checkVersion(); const listAddons = () => svc.listAddons(); -const updateAddon = (id: string, d: Parameters[1]) => svc.updateAddon(id, d); +const updateAddon = (id: string, d: Parameters[1]) => asLegacyResult(svc.updateAddon(id, d)); beforeEach(() => { resetTestDb(testDb); @@ -831,7 +832,7 @@ describe('resetUserMfa', () => { await auth.generateToken({ id: user.id }); await auth.generateToken({ id: admin.user.id }); - expect((await svc.resetUserMfa(String(user.id), admin.user.id)) as { success?: boolean }).toMatchObject({ success: true }); + expect((await asLegacyResult(svc.resetUserMfa(String(user.id), admin.user.id))) as { success?: boolean }).toMatchObject({ success: true }); expect(await liveSessions(user.id)).toBe(0); expect(await liveSessions(admin.user.id)).toBe(1); }); @@ -843,7 +844,7 @@ describe('resetUserMfa', () => { const admin = createAdmin(testDb); const { user } = createUserWithMfa(testDb); - const result = (await svc.resetUserMfa(String(user.id), admin.user.id)) as { success?: boolean; email?: string }; + const result = (await asLegacyResult(svc.resetUserMfa(String(user.id), admin.user.id))) as { success?: boolean; email?: string }; expect(result.success).toBe(true); expect(result.email).toBe(user.email); @@ -859,7 +860,7 @@ describe('resetUserMfa', () => { // self-service path in Settings asks for the current password. const admin = createAdmin(testDb); - const result = (await svc.resetUserMfa(String(admin.user.id), admin.user.id)) as { error?: string; status?: number }; + const result = (await asLegacyResult(svc.resetUserMfa(String(admin.user.id), admin.user.id))) as { error?: string; status?: number }; expect(result.status).toBe(400); expect(result.error).toMatch(/your own/i); @@ -868,7 +869,7 @@ describe('resetUserMfa', () => { it('ADMIN-SVC-085 — 404 for a user that is not there', async () => { const admin = createAdmin(testDb); - expect((await svc.resetUserMfa('99999', admin.user.id)) as { status?: number }).toMatchObject({ status: 404 }); + expect((await asLegacyResult(svc.resetUserMfa('99999', admin.user.id))) as { status?: number }).toMatchObject({ status: 404 }); }); }); diff --git a/server/tests/unit/nest/auth-core/auth-guard.test.ts b/server/tests/unit/nest/auth-core/auth-guard.test.ts index c9c641c56..ecd7c9b2b 100644 --- a/server/tests/unit/nest/auth-core/auth-guard.test.ts +++ b/server/tests/unit/nest/auth-core/auth-guard.test.ts @@ -10,6 +10,7 @@ import { PasskeyEnabledGuard } from '../../../../src/nest/auth/passkey-enabled.g import { PasskeyController } from '../../../../src/nest/auth/passkey.controller'; import type { PasskeyService } from '../../../../src/nest/auth/passkey.service'; import { setAuthCookie } from '../../../../src/nest/common/cookie'; +import { DomainError } from '../../../../src/nest/common/domain-error'; import { RateLimitService } from '../../../../src/nest/common/rate-limit.service'; import type { User } from '../../../../src/types'; import type { EntityManager } from '@mikro-orm/core'; @@ -240,7 +241,7 @@ describe('PasskeyController', () => { } it('register/options maps a service error, else returns the options', async () => { - passkey.passkeyRegisterOptions.mockResolvedValue({ error: 'Incorrect password', status: 401 }); + passkey.passkeyRegisterOptions.mockRejectedValue(new DomainError(401, 'Incorrect password')); expect(await thrownAsync(() => pc(rl()).registerOptions(user, { password: 'x' }, req))).toEqual({ status: 401, body: { error: 'Incorrect password' }, @@ -250,7 +251,7 @@ describe('PasskeyController', () => { }); it('register/verify maps a service error, else audits and returns the credential', async () => { - passkey.passkeyRegisterVerify.mockResolvedValue({ error: 'Verification failed', status: 400 } as never); + passkey.passkeyRegisterVerify.mockRejectedValue(new DomainError(400, 'Verification failed')); expect(await thrownAsync(() => pc(rl()).registerVerify(user, {}, req))).toEqual({ status: 400, body: { error: 'Verification failed' }, @@ -261,7 +262,7 @@ describe('PasskeyController', () => { }); it('login/options maps a service error, else returns the options', async () => { - passkey.passkeyLoginOptions.mockResolvedValue({ error: 'Not configured', status: 503 } as never); + passkey.passkeyLoginOptions.mockRejectedValue(new DomainError(503, 'Not configured')); expect(await thrownAsync(() => pc(rl()).loginOptions(req))).toEqual({ status: 503, body: { error: 'Not configured' }, @@ -297,7 +298,7 @@ describe('PasskeyController', () => { passkey.listPasskeys.mockResolvedValue([{ id: 'a' }]); expect(await pc(rl()).list(user)).toEqual({ credentials: [{ id: 'a' }] }); - passkey.renamePasskey.mockResolvedValue({ error: 'Not found', status: 404 }); + passkey.renamePasskey.mockRejectedValue(new DomainError(404, 'Not found')); expect(await thrownAsync(() => pc(rl()).rename(user, 'cid', { name: 'x' }))).toEqual({ status: 404, body: { error: 'Not found' }, @@ -305,7 +306,7 @@ describe('PasskeyController', () => { passkey.renamePasskey.mockResolvedValue({ success: true }); expect(await pc(rl()).rename(user, 'cid', { name: 'x' })).toEqual({ success: true }); - passkey.deletePasskey.mockResolvedValue({ error: 'Incorrect password', status: 401 }); + passkey.deletePasskey.mockRejectedValue(new DomainError(401, 'Incorrect password')); expect(await thrownAsync(() => pc(rl()).remove(user, 'cid', { password: 'x' }, req))).toEqual({ status: 401, body: { error: 'Incorrect password' }, diff --git a/server/tests/unit/nest/auth.controller.test.ts b/server/tests/unit/nest/auth.controller.test.ts index 5d60d96dd..4acef1477 100644 --- a/server/tests/unit/nest/auth.controller.test.ts +++ b/server/tests/unit/nest/auth.controller.test.ts @@ -1,3 +1,4 @@ +import { DomainError } from '../../../src/nest/common/domain-error'; import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest'; import { RuntimeEnvService } from '../../../src/nest/app-config/runtime-env.service'; import { HttpException } from '@nestjs/common'; @@ -90,7 +91,7 @@ describe('RateLimitService', () => { describe('AuthPublicController', () => { it('demo-login maps error, else sets the cookie + returns token/user', async () => { - expect(await thrownAsync(() => apc(asvc({ demoLogin: vi.fn().mockReturnValue({ error: 'Demo disabled', status: 403 }) } as Partial), rl()).demoLogin(req, res))).toEqual({ status: 403, body: { error: 'Demo disabled' } }); + expect(await thrownAsync(() => apc(asvc({ demoLogin: vi.fn().mockRejectedValue(new DomainError(403, 'Demo disabled')) } as Partial), rl()).demoLogin(req, res))).toEqual({ status: 403, body: { error: 'Demo disabled' } }); const setAuthCookie = vi.fn(); const demoLogin = vi.fn().mockReturnValue({ token: 'tk', user }); const c = apc(asvc({ demoLogin, setAuthCookie } as Partial), rl()); @@ -101,7 +102,7 @@ describe('AuthPublicController', () => { }); it('register audits + sets cookie; maps error', async () => { - expect(await thrownAsync(() => apc(asvc({ registerUser: vi.fn().mockReturnValue({ error: 'Email taken', status: 409 }) } as Partial), rl()).register(anyBody(), req, res))).toEqual({ status: 409, body: { error: 'Email taken' } }); + expect(await thrownAsync(() => apc(asvc({ registerUser: vi.fn().mockRejectedValue(new DomainError(409, 'Email taken')) } as Partial), rl()).register(anyBody(), req, res))).toEqual({ status: 409, body: { error: 'Email taken' } }); const setAuthCookie = vi.fn(); const c = apc(asvc({ registerUser: vi.fn().mockReturnValue({ token: 'tk', user, auditUserId: 1, auditDetails: {} }), setAuthCookie } as Partial), rl()); expect(await c.register({ email: 'a@b.c', password: 'p' }, req, res)).toEqual({ token: 'tk', user }); @@ -136,7 +137,7 @@ describe('AuthPublicController', () => { }, 10000); it('reset-password: error audits a fail, mfa branch, success', async () => { - expect(await thrownAsync(() => apc(asvc({ resetPassword: vi.fn().mockReturnValue({ error: 'Invalid token', status: 400 }) } as Partial), rl()).resetPassword(anyBody(), req))).toEqual({ status: 400, body: { error: 'Invalid token' } }); + expect(await thrownAsync(() => apc(asvc({ resetPassword: vi.fn().mockRejectedValue(new DomainError(400, 'Invalid token')) } as Partial), rl()).resetPassword(anyBody(), req))).toEqual({ status: 400, body: { error: 'Invalid token' } }); expect(writeAudit).toHaveBeenCalledWith(expect.objectContaining({ action: 'user.password_reset_fail' })); expect(await apc(asvc({ resetPassword: vi.fn().mockReturnValue({ mfa_required: true }) } as Partial), rl()).resetPassword(anyBody(), req)).toEqual({ mfa_required: true }); expect(await apc(asvc({ resetPassword: vi.fn().mockReturnValue({ userId: 1 }) } as Partial), rl()).resetPassword(anyBody(), req)).toEqual({ success: true }); @@ -152,7 +153,7 @@ describe('AuthPublicController', () => { }); it('invite maps a service error', async () => { - const c = apc(asvc({ validateInviteToken: vi.fn().mockReturnValue({ error: 'Expired', status: 410 }) } as Partial), rl()); + const c = apc(asvc({ validateInviteToken: vi.fn().mockRejectedValue(new DomainError(410, 'Expired')) } as Partial), rl()); expect(await thrownAsync(() => c.invite('tok', req))).toEqual({ status: 410, body: { error: 'Expired' } }); }); @@ -194,7 +195,7 @@ describe('AuthPublicController', () => { }); it('mfa/verify-login maps a service error', async () => { - const c = apc(asvc({ verifyMfaLogin: vi.fn().mockReturnValue({ error: 'Bad code', status: 401 }) } as Partial), rl()); + const c = apc(asvc({ verifyMfaLogin: vi.fn().mockRejectedValue(new DomainError(401, 'Bad code')) } as Partial), rl()); expect(await thrownAsync(() => c.verifyMfaLogin(anyBody(), req, res))).toEqual({ status: 401, body: { error: 'Bad code' } }); }); @@ -259,7 +260,7 @@ describe('AuthController (authenticated)', () => { }); it('change-password maps error, else audits', async () => { - expect(await thrownAsync(() => ac(asvc({ changePassword: vi.fn().mockReturnValue({ error: 'Wrong', status: 400 }) } as Partial), rl()).changePassword(user, anyBody(), req, res))).toEqual({ status: 400, body: { error: 'Wrong' } }); + expect(await thrownAsync(() => ac(asvc({ changePassword: vi.fn().mockRejectedValue(new DomainError(400, 'Wrong')) } as Partial), rl()).changePassword(user, anyBody(), req, res))).toEqual({ status: 400, body: { error: 'Wrong' } }); expect(await ac(asvc({ changePassword: vi.fn().mockReturnValue({}) } as Partial), rl()).changePassword(user, anyBody(), req, res)).toEqual({ success: true }); expect(writeAudit).toHaveBeenCalledWith(expect.objectContaining({ action: 'user.password_change' })); }); @@ -367,7 +368,7 @@ describe('AuthController (authenticated)', () => { }); it('delete-account maps error, else audits and succeeds', async () => { - expect(await thrownAsync(() => ac(asvc({ deleteAccount: vi.fn().mockReturnValue({ error: 'Last admin', status: 403 }) } as Partial), rl()).deleteAccount(user, req))).toEqual({ status: 403, body: { error: 'Last admin' } }); + expect(await thrownAsync(() => ac(asvc({ deleteAccount: vi.fn().mockRejectedValue(new DomainError(403, 'Last admin')) } as Partial), rl()).deleteAccount(user, req))).toEqual({ status: 403, body: { error: 'Last admin' } }); expect(await ac(asvc({ deleteAccount: vi.fn().mockReturnValue({}) } as Partial), rl()).deleteAccount(user, req)).toEqual({ success: true }); expect(writeAudit).toHaveBeenCalledWith(expect.objectContaining({ action: 'user.account_delete' })); }); @@ -418,9 +419,9 @@ describe('AuthController (authenticated)', () => { }); it('update-settings + get-settings map errors, else return their payloads', async () => { - expect(await thrownAsync(() => ac(asvc({}), rl(), {}, { updateSettings: vi.fn().mockReturnValue({ error: 'Bad', status: 400 }) }).updateSettings(user, {}, req))).toEqual({ status: 400, body: { error: 'Bad' } }); + expect(await thrownAsync(() => ac(asvc({}), rl(), {}, { updateSettings: vi.fn().mockRejectedValue(new DomainError(400, 'Bad')) }).updateSettings(user, {}, req))).toEqual({ status: 400, body: { error: 'Bad' } }); expect(await ac(asvc({}), rl(), {}, { updateSettings: vi.fn().mockReturnValue({ success: true, user: { id: 1 } }) }).updateSettings(user, {}, req)).toEqual({ success: true, user: { id: 1 } }); - expect(await thrownAsync(() => ac(asvc({}), rl(), {}, { getSettings: vi.fn().mockReturnValue({ error: 'Nope', status: 404 }) }).getSettings(user))).toEqual({ status: 404, body: { error: 'Nope' } }); + expect(await thrownAsync(() => ac(asvc({}), rl(), {}, { getSettings: vi.fn().mockRejectedValue(new DomainError(404, 'Nope')) }).getSettings(user))).toEqual({ status: 404, body: { error: 'Nope' } }); expect(await ac(asvc({}), rl(), {}, { getSettings: vi.fn().mockReturnValue({ settings: { theme: 'dark' } }) }).getSettings(user)).toEqual({ settings: { theme: 'dark' } }); }); @@ -435,27 +436,27 @@ describe('AuthController (authenticated)', () => { }); it('validate-keys maps error, else returns the maps/weather payload', async () => { - expect(await thrownAsync(() => ac(asvc({}), rl(), {}, { validateKeys: vi.fn().mockResolvedValue({ error: 'fail', status: 502 }) }).validateKeys(user))).toEqual({ status: 502, body: { error: 'fail' } }); + expect(await thrownAsync(() => ac(asvc({}), rl(), {}, { validateKeys: vi.fn().mockRejectedValue(new DomainError(502, 'fail')) }).validateKeys(user))).toEqual({ status: 502, body: { error: 'fail' } }); const ok = ac(asvc({}), rl(), {}, { validateKeys: vi.fn().mockResolvedValue({ maps: true, weather: false, maps_details: { ok: 1 } }) }); expect(await ok.validateKeys(user)).toEqual({ maps: true, weather: false, maps_details: { ok: 1 } }); }); it('app-settings get maps error, else returns data; put maps error, else audits', async () => { - expect(await thrownAsync(() => ac(asvc({ getAppSettings: vi.fn().mockReturnValue({ error: 'denied', status: 403 }) } as Partial), rl()).getAppSettings(user))).toEqual({ status: 403, body: { error: 'denied' } }); + expect(await thrownAsync(() => ac(asvc({ getAppSettings: vi.fn().mockRejectedValue(new DomainError(403, 'denied')) } as Partial), rl()).getAppSettings(user))).toEqual({ status: 403, body: { error: 'denied' } }); expect(await ac(asvc({ getAppSettings: vi.fn().mockReturnValue({ data: { x: 1 } }) } as Partial), rl()).getAppSettings(user)).toEqual({ x: 1 }); - expect(await thrownAsync(() => ac(asvc({ updateAppSettings: vi.fn().mockReturnValue({ error: 'bad', status: 400 }) } as Partial), rl()).updateAppSettings(user, {}, req))).toEqual({ status: 400, body: { error: 'bad' } }); + expect(await thrownAsync(() => ac(asvc({ updateAppSettings: vi.fn().mockRejectedValue(new DomainError(400, 'bad')) } as Partial), rl()).updateAppSettings(user, {}, req))).toEqual({ status: 400, body: { error: 'bad' } }); expect(await ac(asvc({ updateAppSettings: vi.fn().mockReturnValue({ auditSummary: 's', auditDebugDetails: 'd' }) } as Partial), rl()).updateAppSettings(user, {}, req)).toEqual({ success: true }); expect(writeAudit).toHaveBeenCalledWith(expect.objectContaining({ action: 'settings.app_update' })); }); it('mfa/setup maps a service error before ever awaiting the QR promise', async () => { - const c = ac(asvc({ setupMfa: vi.fn().mockReturnValue({ error: 'already on', status: 409 }) } as Partial), rl()); + const c = ac(asvc({ setupMfa: vi.fn().mockRejectedValue(new DomainError(409, 'already on')) } as Partial), rl()); expect(await thrownAsync(() => c.mfaSetup(user))).toEqual({ status: 409, body: { error: 'already on' } }); }); it('mfa/enable + mfa/disable map errors', async () => { - expect(await thrownAsync(() => ac(asvc({ enableMfa: vi.fn().mockReturnValue({ error: 'Invalid code', status: 400 }) } as Partial), rl()).mfaEnable(user, { code: 'x' }, req))).toEqual({ status: 400, body: { error: 'Invalid code' } }); - expect(await thrownAsync(() => ac(asvc({ disableMfa: vi.fn().mockReturnValue({ error: 'Wrong', status: 401 }) } as Partial), rl()).mfaDisable(user, anyBody(), req))).toEqual({ status: 401, body: { error: 'Wrong' } }); + expect(await thrownAsync(() => ac(asvc({ enableMfa: vi.fn().mockRejectedValue(new DomainError(400, 'Invalid code')) } as Partial), rl()).mfaEnable(user, { code: 'x' }, req))).toEqual({ status: 400, body: { error: 'Invalid code' } }); + expect(await thrownAsync(() => ac(asvc({ disableMfa: vi.fn().mockRejectedValue(new DomainError(401, 'Wrong')) } as Partial), rl()).mfaDisable(user, anyBody(), req))).toEqual({ status: 401, body: { error: 'Wrong' } }); const disableMfa = vi.fn().mockReturnValue({ mfa_enabled: false }); const ok = ac(asvc({ disableMfa } as Partial), rl()); expect(await ok.mfaDisable(user, anyBody(), req)).toEqual({ success: true, mfa_enabled: false }); @@ -470,8 +471,8 @@ describe('AuthController (authenticated)', () => { it('mcp-tokens list + create error + delete error/success', async () => { expect(await ac(asvc({}), rl(), { listMcpTokens: vi.fn().mockReturnValue([{ id: 't' }]) }).listMcpTokens(user)).toEqual({ tokens: [{ id: 't' }] }); - expect(await thrownAsync(() => ac(asvc({}), rl(), { createMcpToken: vi.fn().mockReturnValue({ error: 'Name taken', status: 409 }) }).createMcpToken(user, { name: 'x' }, req))).toEqual({ status: 409, body: { error: 'Name taken' } }); - expect(await thrownAsync(() => ac(asvc({}), rl(), { deleteMcpToken: vi.fn().mockReturnValue({ error: 'Not found', status: 404 }) }).deleteMcpToken(user, 'tid'))).toEqual({ status: 404, body: { error: 'Not found' } }); + expect(await thrownAsync(() => ac(asvc({}), rl(), { createMcpToken: vi.fn().mockRejectedValue(new DomainError(409, 'Name taken')) }).createMcpToken(user, { name: 'x' }, req))).toEqual({ status: 409, body: { error: 'Name taken' } }); + expect(await thrownAsync(() => ac(asvc({}), rl(), { deleteMcpToken: vi.fn().mockRejectedValue(new DomainError(404, 'Not found')) }).deleteMcpToken(user, 'tid'))).toEqual({ status: 404, body: { error: 'Not found' } }); expect(await ac(asvc({}), rl(), { deleteMcpToken: vi.fn().mockReturnValue({}) }).deleteMcpToken(user, 'tid')).toEqual({ success: true }); }); @@ -481,8 +482,8 @@ describe('AuthController (authenticated)', () => { it('api-tokens list + create success/error + delete error/success', async () => { expect(await ac(asvc({}), rl(), { listApiTokens: vi.fn().mockReturnValue([{ id: 'a' }]) }).listApiTokens(user)).toEqual({ tokens: [{ id: 'a' }] }); expect(await ac(asvc({}), rl(), { createApiToken: vi.fn().mockReturnValue({ token: 'trek_x' }) }).createApiToken(user, { name: 'Homepage' }, req)).toEqual({ token: 'trek_x' }); - expect(await thrownAsync(() => ac(asvc({}), rl(), { createApiToken: vi.fn().mockReturnValue({ error: 'Name taken', status: 409 }) }).createApiToken(user, { name: 'x' }, req))).toEqual({ status: 409, body: { error: 'Name taken' } }); - expect(await thrownAsync(() => ac(asvc({}), rl(), { deleteApiToken: vi.fn().mockReturnValue({ error: 'Not found', status: 404 }) }).deleteApiToken(user, 'tid'))).toEqual({ status: 404, body: { error: 'Not found' } }); + expect(await thrownAsync(() => ac(asvc({}), rl(), { createApiToken: vi.fn().mockRejectedValue(new DomainError(409, 'Name taken')) }).createApiToken(user, { name: 'x' }, req))).toEqual({ status: 409, body: { error: 'Name taken' } }); + expect(await thrownAsync(() => ac(asvc({}), rl(), { deleteApiToken: vi.fn().mockRejectedValue(new DomainError(404, 'Not found')) }).deleteApiToken(user, 'tid'))).toEqual({ status: 404, body: { error: 'Not found' } }); expect(await ac(asvc({}), rl(), { deleteApiToken: vi.fn().mockReturnValue({}) }).deleteApiToken(user, 'tid')).toEqual({ success: true }); }); @@ -499,7 +500,7 @@ describe('AuthController (authenticated)', () => { }); it('ws-token maps error, else returns the token', async () => { - expect(await thrownAsync(() => ac(asvc({}), rl(), { createWsToken: vi.fn().mockReturnValue({ error: 'down', status: 503 }) }).wsToken(user))).toEqual({ status: 503, body: { error: 'down' } }); + expect(await thrownAsync(() => ac(asvc({}), rl(), { createWsToken: vi.fn().mockRejectedValue(new DomainError(503, 'down')) }).wsToken(user))).toEqual({ status: 503, body: { error: 'down' } }); expect(await ac(asvc({}), rl(), { createWsToken: vi.fn().mockReturnValue({ token: 'ws' }) }).wsToken(user)).toEqual({ token: 'ws' }); }); diff --git a/server/tests/unit/nest/auth.service.test.ts b/server/tests/unit/nest/auth.service.test.ts index d60bf321b..35c290819 100644 --- a/server/tests/unit/nest/auth.service.test.ts +++ b/server/tests/unit/nest/auth.service.test.ts @@ -50,6 +50,7 @@ vi.mock('../../../src/mcp/sessionManager', () => ({ revokeUserSessions: vi.fn() // Imports (after mocks) // --------------------------------------------------------------------------- +import { asLegacyResult } from '../../helpers/domain-error'; import { db as testDb } from '../../../src/db/database'; import { describe, it, expect, beforeAll, beforeEach, afterEach, afterAll, vi } from 'vitest'; import { UsersRepository } from '../../../src/db/repositories/Users.repository'; @@ -205,7 +206,7 @@ describe('requestPasswordReset — OIDC/SSO accounts', () => { describe('getAppSettings', () => { it('AUTH-DB-013: returns 403 for non-admin', async () => { const { user } = createUser(testDb); - const result = await svc.getAppSettings(user.id); + const result = await asLegacyResult(svc.getAppSettings(user.id)); expect(result.status).toBe(403); expect(result.error).toMatch(/admin/i); }); @@ -213,7 +214,7 @@ describe('getAppSettings', () => { it('AUTH-DB-014: returns settings object for admin with known key allow_registration', async () => { const { user } = createAdmin(testDb); await appSetting('allow_registration', 'true'); - const result = await svc.getAppSettings(user.id); + const result = await asLegacyResult(svc.getAppSettings(user.id)); expect(result.status).toBeUndefined(); expect(result.data).toBeDefined(); expect(result.data).toHaveProperty('allow_registration', 'true'); @@ -332,7 +333,7 @@ describe('setupMfa', () => { it('AUTH-DB-022: returns 403 in demo mode for demo@nomad.app', async () => { vi.stubEnv('DEMO_MODE', 'true'); const { user } = createUser(testDb, { email: 'demo@nomad.app' }); - const result = await svc.setupMfa(user.id, 'demo@nomad.app'); + const result = await asLegacyResult(svc.setupMfa(user.id, 'demo@nomad.app')); expect(result.status).toBe(403); expect(result.error).toMatch(/demo mode/i); vi.unstubAllEnvs(); @@ -341,14 +342,14 @@ describe('setupMfa', () => { it('AUTH-DB-023: returns 400 when MFA is already enabled', async () => { const { user } = createUser(testDb); await updateRows(orm, Users, { id: user.id }, { mfa_enabled: 1 }); - const result = await svc.setupMfa(user.id, user.email); + const result = await asLegacyResult(svc.setupMfa(user.id, user.email)); expect(result.status).toBe(400); expect(result.error).toMatch(/already enabled/i); }); it('AUTH-DB-024: returns secret and otpauth_url when MFA setup starts successfully', async () => { const { user } = createUser(testDb); - const result = await svc.setupMfa(user.id, user.email); + const result = await asLegacyResult(svc.setupMfa(user.id, user.email)); expect(result.error).toBeUndefined(); expect(typeof result.secret).toBe('string'); expect(result.secret!.length).toBeGreaterThan(0); @@ -365,7 +366,7 @@ describe('setupMfa', () => { describe('enableMfa', () => { it('AUTH-DB-025: returns 400 when no verification code is provided', async () => { const { user } = createUser(testDb); - const result = await svc.enableMfa(user.id, undefined); + const result = await asLegacyResult(svc.enableMfa(user.id, undefined)); expect(result.status).toBe(400); expect(result.error).toMatch(/code is required/i); }); @@ -373,7 +374,7 @@ describe('enableMfa', () => { it('AUTH-DB-026: returns 400 when there is no pending MFA setup', async () => { const { user } = createUser(testDb); // No setupMfa called first, so no pending entry exists - const result = await svc.enableMfa(user.id, '123456'); + const result = await asLegacyResult(svc.enableMfa(user.id, '123456')); expect(result.status).toBe(400); expect(result.error).toMatch(/no mfa setup in progress/i); }); @@ -387,10 +388,10 @@ describe('disableMfa', () => { it('AUTH-DB-027: returns 403 in demo mode for demo@nomad.app', async () => { vi.stubEnv('DEMO_MODE', 'true'); const { user } = createUser(testDb, { email: 'demo@nomad.app' }); - const result = await svc.disableMfa(user.id, 'demo@nomad.app', { + const result = await asLegacyResult(svc.disableMfa(user.id, 'demo@nomad.app', { password: 'password123', code: '000000', - }); + })); expect(result.status).toBe(403); expect(result.error).toMatch(/demo mode/i); vi.unstubAllEnvs(); @@ -399,11 +400,11 @@ describe('disableMfa', () => { it('AUTH-DB-028: returns 400 when password or code is missing', async () => { const { user } = createUser(testDb); - const missingCode = await svc.disableMfa(user.id, user.email, { password: 'pass', code: undefined }); + const missingCode = await asLegacyResult(svc.disableMfa(user.id, user.email, { password: 'pass', code: undefined })); expect(missingCode.status).toBe(400); expect(missingCode.error).toMatch(/password and authenticator code/i); - const missingPassword = await svc.disableMfa(user.id, user.email, { password: undefined, code: '123456' }); + const missingPassword = await asLegacyResult(svc.disableMfa(user.id, user.email, { password: undefined, code: '123456' })); expect(missingPassword.status).toBe(400); expect(missingPassword.error).toMatch(/password and authenticator code/i); }); @@ -411,7 +412,7 @@ describe('disableMfa', () => { it('AUTH-DB-029: returns 400 when MFA is not enabled on the account', async () => { const { user } = createUser(testDb); // mfa_enabled defaults to 0 / not set - const result = await svc.disableMfa(user.id, user.email, { password: 'password123', code: '000000' }); + const result = await asLegacyResult(svc.disableMfa(user.id, user.email, { password: 'password123', code: '000000' })); expect(result.status).toBe(400); expect(result.error).toMatch(/not enabled/i); }); @@ -423,7 +424,7 @@ describe('disableMfa', () => { describe('validateInviteToken', () => { it('AUTH-DB-030: returns 404 for unknown token', async () => { - const result = await svc.validateInviteToken('no-such-token'); + const result = await asLegacyResult(svc.validateInviteToken('no-such-token')); expect(result.status).toBe(404); }); @@ -432,13 +433,13 @@ describe('validateInviteToken', () => { const invite = createInviteToken(testDb, { max_uses: 1 }); // manually set used_count = 1 to simulate exhaustion await updateRows(orm, InviteTokens, { id: invite.id }, { used_count: 1 }); - const result = await svc.validateInviteToken(invite.token); + const result = await asLegacyResult(svc.validateInviteToken(invite.token)); expect(result.status).toBe(410); }); it('AUTH-DB-032: returns 410 when expired', async () => { const invite = createInviteToken(testDb, { expires_at: '2000-01-01T00:00:00.000Z' }); - const result = await svc.validateInviteToken(invite.token); + const result = await asLegacyResult(svc.validateInviteToken(invite.token)); expect(result.status).toBe(410); }); }); @@ -454,7 +455,7 @@ describe('registerUser — OIDC-only / registration-disabled', () => { await insertRow(orm, AppSettings, { key: 'oidc_issuer', value: 'https://x' }); await insertRow(orm, AppSettings, { key: 'oidc_client_id', value: 'id' }); - const result = await svc.registerUser({ username: 'u', email: 'new@x.com', password: 'Secure123!' }); + const result = await asLegacyResult(svc.registerUser({ username: 'u', email: 'new@x.com', password: 'Secure123!' })); expect(result.status).toBe(403); expect(result.error).toMatch(/password registration is disabled/i); }); @@ -463,7 +464,7 @@ describe('registerUser — OIDC-only / registration-disabled', () => { createUser(testDb); // ensure userCount > 0 await appSetting('allow_registration', 'false'); - const result = await svc.registerUser({ username: 'u2', email: 'n2@x.com', password: 'Secure123!' }); + const result = await asLegacyResult(svc.registerUser({ username: 'u2', email: 'n2@x.com', password: 'Secure123!' })); expect(result.status).toBe(403); }); }); @@ -495,7 +496,7 @@ describe('changePassword — OIDC-only mode', () => { await appSetting('oidc_issuer', 'https://x'); await appSetting('oidc_client_id', 'id'); - const result = await svc.changePassword(user.id, user.email, { current_password: password, new_password: 'New1234!' }); + const result = await asLegacyResult(svc.changePassword(user.id, user.email, { current_password: password, new_password: 'New1234!' })); expect(result.status).toBe(403); }); }); @@ -511,7 +512,7 @@ describe('changePassword — session invalidation', () => { expect(await pvOf(user.id)).toBe(0); expect(await mcpCount(user.id)).toBe(1); - const result = await svc.changePassword(user.id, user.email, { current_password: password, new_password: 'New1234!' }); + const result = await asLegacyResult(svc.changePassword(user.id, user.email, { current_password: password, new_password: 'New1234!' })); expect(result.success).toBe(true); expect(typeof result.token).toBe('string'); // fresh session for the current device @@ -525,7 +526,7 @@ describe('changePassword — session invalidation', () => { expect(await verifyJwtAndLoadUser(stolen, await createTestUsersRepo(testDb), await createTestUserSessionsRepo(testDb))).not.toBeNull(); - await svc.changePassword(user.id, user.email, { current_password: password, new_password: 'New1234!' }); + await asLegacyResult(svc.changePassword(user.id, user.email, { current_password: password, new_password: 'New1234!' })); expect(await verifyJwtAndLoadUser(stolen, await createTestUsersRepo(testDb), await createTestUserSessionsRepo(testDb))).toBeNull(); // invalidated by the pv bump }); @@ -534,7 +535,7 @@ describe('changePassword — session invalidation', () => { const { user, password } = createUser(testDb); await svc.generateToken({ id: user.id }); - const result = await svc.changePassword(user.id, user.email, { current_password: password, new_password: 'New1234!' }, undefined, {}, false); + const result = await asLegacyResult(svc.changePassword(user.id, user.email, { current_password: password, new_password: 'New1234!' }, undefined, {}, false)); expect(result).toEqual({ success: true }); expect(await pvOf(user.id)).toBe(1); @@ -548,7 +549,7 @@ describe('changePassword — session invalidation', () => { const jwt = require('jsonwebtoken'); const { user, password } = createUser(testDb); - const result = await svc.changePassword(user.id, user.email, { current_password: password, new_password: 'New1234!' }, true); + const result = await asLegacyResult(svc.changePassword(user.id, user.email, { current_password: password, new_password: 'New1234!' }, true)); expect(result.success).toBe(true); const decoded = jwt.decode(result.token!) as { remember?: boolean; iat: number; exp: number }; expect(decoded.remember).toBe(true); @@ -580,7 +581,7 @@ describe('changePassword — session invalidation', () => { await addPushDevice(user.id, 'https://web.push.apple.com/mine-2'); await addPushDevice(other.id, 'https://fcm.googleapis.com/fcm/send/theirs'); - const result = await svc.changePassword(user.id, user.email, { current_password: password, new_password: 'New1234!' }); + const result = await asLegacyResult(svc.changePassword(user.id, user.email, { current_password: password, new_password: 'New1234!' })); expect(result.success).toBe(true); expect(await pushDeviceCount(user.id)).toBe(0); @@ -591,7 +592,7 @@ describe('changePassword — session invalidation', () => { const { user } = createUser(testDb); await addPushDevice(user.id, 'https://fcm.googleapis.com/fcm/send/mine'); - const result = await svc.changePassword(user.id, user.email, { current_password: 'wrong', new_password: 'New1234!' }); + const result = await asLegacyResult(svc.changePassword(user.id, user.email, { current_password: 'wrong', new_password: 'New1234!' })); expect(result.status).toBe(401); expect(await pushDeviceCount(user.id)).toBe(1); @@ -614,7 +615,7 @@ describe('changePassword — session invalidation', () => { expect(await mcpCount(user.id)).toBe(1); expect(await pushDeviceCount(user.id)).toBe(1); // The new hash rolled back as well, so the old password is still the one that counts. - expect((await svc.changePassword(user.id, user.email, { current_password: password, new_password: 'New1234!' })).success) + expect((await asLegacyResult(svc.changePassword(user.id, user.email, { current_password: password, new_password: 'New1234!' }))).success) .toBe(true); }); }); @@ -628,7 +629,7 @@ describe('disableMfa — require_mfa policy', () => { const { user } = createUser(testDb); await appSetting('require_mfa', 'true'); - const result = await svc.disableMfa(user.id, user.email, { password: 'pass', code: '123456' }); + const result = await asLegacyResult(svc.disableMfa(user.id, user.email, { password: 'pass', code: '123456' })); expect(result.status).toBe(403); expect(result.error).toMatch(/cannot be disabled/i); }); @@ -640,7 +641,7 @@ describe('disableMfa — require_mfa policy', () => { describe('verifyMfaLogin — validation', () => { it('AUTH-DB-038: returns 400 when mfa_token or code is missing', async () => { - const result = await svc.verifyMfaLogin({ mfa_token: undefined, code: undefined }); + const result = await asLegacyResult(svc.verifyMfaLogin({ mfa_token: undefined, code: undefined })); expect(result.status).toBe(400); }); @@ -648,7 +649,7 @@ describe('verifyMfaLogin — validation', () => { // eslint-disable-next-line @typescript-eslint/no-require-imports const jwt = require('jsonwebtoken'); const tok = jwt.sign({ id: 1, purpose: 'wrong' }, 'test-secret', { expiresIn: '5m', algorithm: 'HS256' }); - const result = await svc.verifyMfaLogin({ mfa_token: tok, code: '123456' }); + const result = await asLegacyResult(svc.verifyMfaLogin({ mfa_token: tok, code: '123456' })); expect(result.status).toBe(401); expect(result.error).toMatch(/invalid/i); }); @@ -657,7 +658,7 @@ describe('verifyMfaLogin — validation', () => { // eslint-disable-next-line @typescript-eslint/no-require-imports const jwt = require('jsonwebtoken'); const tok = jwt.sign({ id: 99999, purpose: 'mfa_login' }, 'test-secret', { expiresIn: '5m', algorithm: 'HS256' }); - const result = await svc.verifyMfaLogin({ mfa_token: tok, code: '123456' }); + const result = await asLegacyResult(svc.verifyMfaLogin({ mfa_token: tok, code: '123456' })); expect(result.status).toBe(401); }); }); @@ -789,16 +790,16 @@ describe('getAppConfig', () => { describe('demoLogin', () => { it('AUTH-DB-053: 404 outside demo mode', async () => { vi.stubEnv('DEMO_MODE', ''); - expect(await svc.demoLogin()).toEqual({ error: 'Not found', status: 404 }); + expect(await asLegacyResult(svc.demoLogin())).toEqual({ error: 'Not found', status: 404 }); vi.unstubAllEnvs(); }); it('AUTH-DB-054: 500 when the demo user row is missing; token + safe user when present', async () => { vi.stubEnv('DEMO_MODE', 'true'); - expect(await svc.demoLogin()).toEqual({ error: 'Demo user not found', status: 500 }); + expect(await asLegacyResult(svc.demoLogin())).toEqual({ error: 'Demo user not found', status: 500 }); // demoLogin looks up DEMO_EMAIL_PRIMARY specifically (not any demo alias). const { user } = createUser(testDb, { email: 'demo@trek.app' }); - const result = await svc.demoLogin({ userAgent: 'Demo visitor' }); + const result = await asLegacyResult(svc.demoLogin({ userAgent: 'Demo visitor' })); expect(typeof result.token).toBe('string'); expect(result.user).not.toHaveProperty('password_hash'); expect((await sessionRows(testDb, user.id)).map((r) => r.user_agent)).toEqual(['Demo visitor']); @@ -809,7 +810,7 @@ describe('demoLogin', () => { describe('validateInviteToken — valid path', () => { it('AUTH-DB-055: returns valid with usage metadata', async () => { const invite = createInviteToken(testDb, { max_uses: 5 }); - const result = await svc.validateInviteToken(invite.token); + const result = await asLegacyResult(svc.validateInviteToken(invite.token)); expect(result.valid).toBe(true); expect(result.max_uses).toBe(5); expect(result.used_count).toBe(0); @@ -818,7 +819,7 @@ describe('validateInviteToken — valid path', () => { describe('registerUser — success paths', () => { it('AUTH-DB-056: first user becomes admin and gets a token', async () => { - const result = await svc.registerUser({ username: 'first', email: 'first@x.com', password: 'Secure123!' }); + const result = await asLegacyResult(svc.registerUser({ username: 'first', email: 'first@x.com', password: 'Secure123!' })); expect(result.error).toBeUndefined(); expect(typeof result.token).toBe('string'); expect((result.user as { role: string }).role).toBe('admin'); @@ -827,11 +828,11 @@ describe('registerUser — success paths', () => { it('AUTH-DB-057: missing fields / bad email / duplicate answer their bespoke 400/409s', async () => { createUser(testDb, { username: 'taken', email: 'taken@x.com' }); - expect(await svc.registerUser({ username: '', email: 'a@x.com', password: 'Secure123!' })) + expect(await asLegacyResult(svc.registerUser({ username: '', email: 'a@x.com', password: 'Secure123!' }))) .toEqual({ error: 'Username, email and password are required', status: 400 }); - expect(await svc.registerUser({ username: 'u', email: 'not-an-email', password: 'Secure123!' })) + expect(await asLegacyResult(svc.registerUser({ username: 'u', email: 'not-an-email', password: 'Secure123!' }))) .toEqual({ error: 'Invalid email format', status: 400 }); - expect(await svc.registerUser({ username: 'TAKEN', email: 'other@x.com', password: 'Secure123!' })) + expect(await asLegacyResult(svc.registerUser({ username: 'TAKEN', email: 'other@x.com', password: 'Secure123!' }))) .toEqual({ error: 'Registration failed. Please try different credentials.', status: 409 }); }); @@ -839,7 +840,7 @@ describe('registerUser — success paths', () => { createUser(testDb); await appSetting('allow_registration', 'false'); const invite = createInviteToken(testDb, { max_uses: 2 }); - const result = await svc.registerUser({ username: 'invited', email: 'invited@x.com', password: 'Secure123!', invite_token: invite.token }); + const result = await asLegacyResult(svc.registerUser({ username: 'invited', email: 'invited@x.com', password: 'Secure123!', invite_token: invite.token })); expect(result.error).toBeUndefined(); const { used_count } = (await findRow(orm, InviteTokens, { id: invite.id })) ?? {}; expect(used_count).toBe(1); @@ -911,20 +912,20 @@ describe('getCurrentUser', () => { describe('deleteAccount', () => { it('AUTH-DB-065: refuses to delete the last admin', async () => { const { user } = createAdmin(testDb); - expect(await svc.deleteAccount(user.id, user.email, 'admin')) + expect(await asLegacyResult(svc.deleteAccount(user.id, user.email, 'admin'))) .toEqual({ error: 'Cannot delete the last admin account', status: 400 }); }); it('AUTH-DB-066: demo mode blocks deletion', async () => { vi.stubEnv('DEMO_MODE', 'true'); - expect(await svc.deleteAccount(1, 'demo@nomad.app', 'user')) + expect(await asLegacyResult(svc.deleteAccount(1, 'demo@nomad.app', 'user'))) .toEqual({ error: 'Account deletion is disabled in demo mode.', status: 403 }); vi.unstubAllEnvs(); }); it('AUTH-DB-067: deletes a regular user row', async () => { const { user } = createUser(testDb); - expect(await svc.deleteAccount(user.id, user.email, 'user')).toEqual({ success: true }); + expect(await asLegacyResult(svc.deleteAccount(user.id, user.email, 'user'))).toEqual({ success: true }); expect(await findRow(orm, Users, { id: user.id })).toBeNull(); }); }); @@ -932,12 +933,12 @@ describe('deleteAccount', () => { describe('updateAppSettings', () => { it('AUTH-DB-071: 403 for non-admin', async () => { const { user } = createUser(testDb); - expect((await svc.updateAppSettings(user.id, {})).status).toBe(403); + expect((await asLegacyResult(svc.updateAppSettings(user.id, {}))).status).toBe(403); }); it('AUTH-DB-072: require_mfa=true refuses when the admin has neither MFA nor a passkey', async () => { const { user } = createAdmin(testDb); - const result = await svc.updateAppSettings(user.id, { require_mfa: true }); + const result = await asLegacyResult(svc.updateAppSettings(user.id, { require_mfa: true })); expect(result.status).toBe(400); expect(result.error).toMatch(/secure your own account/i); }); @@ -945,7 +946,7 @@ describe('updateAppSettings', () => { it('AUTH-DB-073: require_mfa=true is allowed once the admin has MFA enabled', async () => { const { user } = createAdmin(testDb); await updateRows(orm, Users, { id: user.id }, { mfa_enabled: 1 }); - const result = await svc.updateAppSettings(user.id, { require_mfa: true }); + const result = await asLegacyResult(svc.updateAppSettings(user.id, { require_mfa: true })); expect(result.success).toBe(true); expect(result.auditSummary).toMatchObject({ require_mfa: true }); await deleteRows(orm, AppSettings, { key: 'require_mfa' }); @@ -953,7 +954,7 @@ describe('updateAppSettings', () => { it('AUTH-DB-074: lockout prevention refuses disabling every login method', async () => { const { user } = createAdmin(testDb); - const result = await svc.updateAppSettings(user.id, { password_login: 'false', oidc_login: 'false' }); + const result = await asLegacyResult(svc.updateAppSettings(user.id, { password_login: 'false', oidc_login: 'false' })); expect(result).toEqual({ error: 'Cannot disable all login methods. At least one must remain enabled.', status: 400 }); }); @@ -962,7 +963,7 @@ describe('updateAppSettings', () => { // longer flags (or needs) a scheduler restart. const { user } = createAdmin(testDb); await appSetting('smtp_pass', 'stored'); - const result = await svc.updateAppSettings(user.id, { smtp_pass: '••••••••', notification_channels: 'email' }); + const result = await asLegacyResult(svc.updateAppSettings(user.id, { smtp_pass: '••••••••', notification_channels: 'email' })); expect(result.success).toBe(true); const value = await readAppSetting(orm, 'smtp_pass'); expect(value).toBe('stored'); // sentinel never overwrites the secret @@ -974,9 +975,9 @@ describe('updateAppSettings', () => { describe('MFA success flows', () => { it('AUTH-DB-076: setup → enable with a valid TOTP code returns backup codes and persists the encrypted secret', async () => { const { user } = createUser(testDb); - const setup = await svc.setupMfa(user.id, user.email); + const setup = await asLegacyResult(svc.setupMfa(user.id, user.email)); const code = authenticator.generate(setup.secret!); - const result = await svc.enableMfa(user.id, code); + const result = await asLegacyResult(svc.enableMfa(user.id, code)); expect(result.success).toBe(true); expect(result.backup_codes).toHaveLength(10); const row = await readUser(orm, user.id); @@ -986,15 +987,15 @@ describe('MFA success flows', () => { it('AUTH-DB-077: enable with a wrong code answers 401 and keeps the pending secret', async () => { const { user } = createUser(testDb); - await svc.setupMfa(user.id, user.email); - expect(await svc.enableMfa(user.id, '000000')).toEqual({ error: 'Invalid verification code', status: 401 }); + await asLegacyResult(svc.setupMfa(user.id, user.email)); + expect(await asLegacyResult(svc.enableMfa(user.id, '000000'))).toEqual({ error: 'Invalid verification code', status: 401 }); }); it('AUTH-DB-078: disableMfa succeeds with the right password + TOTP code', async () => { const { user, password } = createUser(testDb); const secret = authenticator.generateSecret(); await updateRows(orm, Users, { id: user.id }, { mfa_enabled: 1, mfa_secret: 'enc:' + secret }); - const result = await svc.disableMfa(user.id, user.email, { password, code: authenticator.generate(secret) }); + const result = await asLegacyResult(svc.disableMfa(user.id, user.email, { password, code: authenticator.generate(secret) })); expect(result).toEqual({ success: true, mfa_enabled: false }); const row = await readUser(orm, user.id); expect(row.mfa_enabled).toBe(0); @@ -1005,7 +1006,7 @@ describe('MFA success flows', () => { const { user } = createUser(testDb); const secret = authenticator.generateSecret(); await updateRows(orm, Users, { id: user.id }, { mfa_enabled: 1, mfa_secret: 'enc:' + secret }); - expect(await svc.disableMfa(user.id, user.email, { password: 'wrong', code: authenticator.generate(secret) })) + expect(await asLegacyResult(svc.disableMfa(user.id, user.email, { password: 'wrong', code: authenticator.generate(secret) }))) .toEqual({ error: 'Incorrect password', status: 401 }); }); @@ -1015,7 +1016,7 @@ describe('MFA success flows', () => { await updateRows(orm, Users, { id: user.id }, { mfa_enabled: 1, mfa_secret: 'enc:' + secret }); const interstitial = await svc.loginUser({ email: user.email, password }); expect(interstitial.mfa_required).toBe(true); - const result = await svc.verifyMfaLogin({ mfa_token: interstitial.mfa_token, code: authenticator.generate(secret) }); + const result = await asLegacyResult(svc.verifyMfaLogin({ mfa_token: interstitial.mfa_token, code: authenticator.generate(secret) })); expect(typeof result.token).toBe('string'); expect(result.auditUserId).toBe(user.id); }); @@ -1029,7 +1030,7 @@ describe('MFA success flows', () => { mfa_enabled: 1, mfa_secret: 'enc:' + secret, mfa_backup_codes: JSON.stringify(codes.map(hashBackupCode)), updated_at: '2000-01-01T00:00:00.000Z', }); const interstitial = await svc.loginUser({ email: user.email, password }); - const result = await svc.verifyMfaLogin({ mfa_token: interstitial.mfa_token, code: 'AAAA-1111' }); + const result = await asLegacyResult(svc.verifyMfaLogin({ mfa_token: interstitial.mfa_token, code: 'AAAA-1111' })); expect(typeof result.token).toBe('string'); const row = await readUser(orm, user.id); expect(JSON.parse(row.mfa_backup_codes)).toHaveLength(1); // used code spliced out @@ -1038,7 +1039,7 @@ describe('MFA success flows', () => { expect(row.updated_at).not.toBe('2000-01-01T00:00:00.000Z'); // the spent code no longer verifies const again = await svc.loginUser({ email: user.email, password }); - expect((await svc.verifyMfaLogin({ mfa_token: again.mfa_token, code: 'AAAA-1111' })).status).toBe(401); + expect((await asLegacyResult(svc.verifyMfaLogin({ mfa_token: again.mfa_token, code: 'AAAA-1111' }))).status).toBe(401); }); }); @@ -1047,12 +1048,12 @@ describe('resetPassword', () => { const { user } = createUser(testDb); const issued = await svc.requestPasswordReset(user.email, '1.2.3.4'); expect(issued.reason).toBe('issued'); - const result = await svc.resetPassword({ token: issued.tokenForDelivery!, new_password: 'Fresh123!' }); + const result = await asLegacyResult(svc.resetPassword({ token: issued.tokenForDelivery!, new_password: 'Fresh123!' })); expect(result).toEqual({ success: true, userId: user.id }); const row = await readUser(orm, user.id); expect(row.password_version).toBe(1); // token is burned — a second use answers the bespoke 400 - expect(await svc.resetPassword({ token: issued.tokenForDelivery!, new_password: 'Fresh456!' })) + expect(await asLegacyResult(svc.resetPassword({ token: issued.tokenForDelivery!, new_password: 'Fresh456!' }))) .toEqual({ error: 'This reset link has already been used', status: 400 }); }); @@ -1064,7 +1065,7 @@ describe('resetPassword', () => { await addPushDevice(other.id, 'https://fcm.googleapis.com/fcm/send/theirs'); const issued = await svc.requestPasswordReset(user.email, null); - expect(await svc.resetPassword({ token: issued.tokenForDelivery!, new_password: 'Fresh123!' })) + expect(await asLegacyResult(svc.resetPassword({ token: issued.tokenForDelivery!, new_password: 'Fresh123!' }))) .toEqual({ success: true, userId: user.id }); expect(await countRows(orm, McpTokens, { user: user.id })).toBe(0); @@ -1087,21 +1088,21 @@ describe('resetPassword', () => { const row = await readUser(orm, user.id); expect(row.password_version).toBe(0); // Nothing was burned, so the same link still completes the reset. - expect(await svc.resetPassword({ token: issued.tokenForDelivery!, new_password: 'Fresh123!' })) + expect(await asLegacyResult(svc.resetPassword({ token: issued.tokenForDelivery!, new_password: 'Fresh123!' }))) .toEqual({ success: true, userId: user.id }); expect(await pushDeviceCount(user.id)).toBe(0); }); it('AUTH-DB-083: bespoke 400s for missing/unknown/expired tokens', async () => { - expect(await svc.resetPassword({ new_password: 'Fresh123!' })).toEqual({ error: 'Reset token is required', status: 400 }); - expect(await svc.resetPassword({ token: 't' })).toEqual({ error: 'New password is required', status: 400 }); - expect(await svc.resetPassword({ token: 'unknown-token', new_password: 'Fresh123!' })) + expect(await asLegacyResult(svc.resetPassword({ new_password: 'Fresh123!' }))).toEqual({ error: 'Reset token is required', status: 400 }); + expect(await asLegacyResult(svc.resetPassword({ token: 't' }))).toEqual({ error: 'New password is required', status: 400 }); + expect(await asLegacyResult(svc.resetPassword({ token: 'unknown-token', new_password: 'Fresh123!' }))) .toEqual({ error: 'Invalid or expired reset link', status: 400 }); const { user } = createUser(testDb); const issued = await svc.requestPasswordReset(user.email, null); await updateRows(orm, PasswordResetTokens, { user: user.id }, { expires_at: '2000-01-01T00:00:00.000Z' }); - expect(await svc.resetPassword({ token: issued.tokenForDelivery!, new_password: 'Fresh123!' })) + expect(await asLegacyResult(svc.resetPassword({ token: issued.tokenForDelivery!, new_password: 'Fresh123!' }))) .toEqual({ error: 'Reset link has expired. Please request a new one.', status: 400 }); }); @@ -1113,7 +1114,7 @@ describe('resetPassword', () => { const aMinuteAgo = new Date(Date.now() - 60_000).toISOString().slice(0, 19).replace('T', ' '); await updateRows(orm, PasswordResetTokens, { user: user.id }, { expires_at: aMinuteAgo }); - expect(await svc.resetPassword({ token: issued.tokenForDelivery ?? '', new_password: 'Fresh123!' })) + expect(await asLegacyResult(svc.resetPassword({ token: issued.tokenForDelivery ?? '', new_password: 'Fresh123!' }))) .toEqual({ error: 'Reset link has expired. Please request a new one.', status: 400 }); }); @@ -1123,10 +1124,10 @@ describe('resetPassword', () => { await updateRows(orm, Users, { id: user.id }, { mfa_enabled: 1, mfa_secret: 'enc:' + secret, mfa_backup_codes: JSON.stringify([hashBackupCode('CCCC-3333')]) }); const issued = await svc.requestPasswordReset(user.email, null); // no code → mfa_required interstitial, token NOT burned - expect(await svc.resetPassword({ token: issued.tokenForDelivery!, new_password: 'Fresh123!' })) + expect(await asLegacyResult(svc.resetPassword({ token: issued.tokenForDelivery!, new_password: 'Fresh123!' }))) .toEqual({ mfa_required: true, status: 200 }); // wrong code → 401 - expect(await svc.resetPassword({ token: issued.tokenForDelivery!, new_password: 'Fresh123!', mfa_code: '000000' })) + expect(await asLegacyResult(svc.resetPassword({ token: issued.tokenForDelivery!, new_password: 'Fresh123!', mfa_code: '000000' }))) .toEqual({ error: 'Invalid MFA code', status: 401 }); // AU44 vs AU33 (F3, task-5-review-security.md): resetPassword's backup-code @@ -1141,7 +1142,7 @@ describe('resetPassword', () => { const setBackupCodesAndTouchSpy = vi.spyOn(usersRepo, 'setBackupCodesAndTouch'); try { // backup code → success + code consumed - expect(await svc.resetPassword({ token: issued.tokenForDelivery!, new_password: 'Fresh123!', mfa_code: 'CCCC-3333' })) + expect(await asLegacyResult(svc.resetPassword({ token: issued.tokenForDelivery!, new_password: 'Fresh123!', mfa_code: 'CCCC-3333' }))) .toEqual({ success: true, userId: user.id }); expect(setBackupCodesSpy).toHaveBeenCalledTimes(1); expect(setBackupCodesAndTouchSpy).not.toHaveBeenCalled(); @@ -1203,7 +1204,7 @@ describe('auth quirk fixes', () => { await updateRows(orm, InviteTokens, { id: invite.id }, { trip: trip.id }); joinTripAsMember.mockImplementationOnce(() => { throw new Error('boom'); }); - const result = await svc.registerUser({ username: 'rollback', email: 'rollback@x.com', password: 'Secure123!', invite_token: invite.token }); + const result = await asLegacyResult(svc.registerUser({ username: 'rollback', email: 'rollback@x.com', password: 'Secure123!', invite_token: invite.token })); expect(result).toEqual({ error: 'Error creating user', status: 500 }); expect(await findRow(orm, Users, { email: 'rollback@x.com' })).toBeNull(); @@ -1217,7 +1218,7 @@ describe('auth quirk fixes', () => { await updateRows(orm, Users, { id: user.id }, { mfa_enabled: 1, mfa_secret: 'enc:' + secret, mfa_backup_codes: JSON.stringify([hashBackupCode('DDDD-4444')]) }); const interstitial = await svc.loginUser({ email: user.email, password }); - const result = await svc.verifyMfaLogin({ mfa_token: interstitial.mfa_token, code: 'DDDD-4444' }); + const result = await asLegacyResult(svc.verifyMfaLogin({ mfa_token: interstitial.mfa_token, code: 'DDDD-4444' })); expect(typeof result.token).toBe('string'); const row = await readUser(orm, user.id); @@ -1281,7 +1282,7 @@ describe('registerUser loses the race for an email', () => { it('AUTH-IDENT-001: a signup the database refuses after the check passed answers the same 409, not a 500', async () => { createUser(testDb, { username: 'anna', email: 'anna@x.com' }); vi.spyOn(UsersRepository.prototype, 'findIdByEmailOrUsernameCI').mockResolvedValue(null); - expect(await svc.registerUser({ username: 'anna2', email: 'ANNA@x.com', password: 'Secure123!' })).toEqual({ + expect(await asLegacyResult(svc.registerUser({ username: 'anna2', email: 'ANNA@x.com', password: 'Secure123!' }))).toEqual({ error: 'Registration failed. Please try different credentials.', status: 409, }); @@ -1325,7 +1326,7 @@ describe('session tracking', () => { const laptop = await svc.generateToken({ id: user.id }); const phone = await svc.generateToken({ id: user.id }); - const result = await svc.changePassword(user.id, user.email, { current_password: password, new_password: 'New1234!' }, undefined, { userAgent: 'Safari' }); + const result = await asLegacyResult(svc.changePassword(user.id, user.email, { current_password: password, new_password: 'New1234!' }, undefined, { userAgent: 'Safari' })); expect(await svc.verifyJwtToken(laptop)).toBeNull(); expect(await svc.verifyJwtToken(phone)).toBeNull(); @@ -1338,7 +1339,7 @@ describe('session tracking', () => { const token = await svc.generateToken({ id: user.id }); const issued = await svc.requestPasswordReset(user.email, null); - expect(await svc.resetPassword({ token: issued.tokenForDelivery!, new_password: 'Fresh123!' })).toEqual({ success: true, userId: user.id }); + expect(await asLegacyResult(svc.resetPassword({ token: issued.tokenForDelivery!, new_password: 'Fresh123!' }))).toEqual({ success: true, userId: user.id }); expect(await active(user.id)).toEqual([]); expect(await svc.verifyJwtToken(token)).toBeNull(); }); @@ -1351,7 +1352,7 @@ describe('session tracking', () => { const elsewhere = await svc.generateToken({ id: user.id }); const hereId = (decodeJwt(here) as { jti: string }).jti; - expect(await svc.disableMfa(user.id, user.email, { password, code: authenticator.generate(secret) }, hereId)).toEqual({ success: true, mfa_enabled: false }); + expect(await asLegacyResult(svc.disableMfa(user.id, user.email, { password, code: authenticator.generate(secret) }, hereId))).toEqual({ success: true, mfa_enabled: false }); expect(await svc.verifyJwtToken(here)).not.toBeNull(); expect(await svc.verifyJwtToken(elsewhere)).toBeNull(); }); @@ -1362,7 +1363,7 @@ describe('session tracking', () => { await updateRows(orm, Users, { id: user.id }, { mfa_enabled: 1, mfa_secret: 'enc:' + secret }); const token = await svc.generateToken({ id: user.id }); - expect((await svc.disableMfa(user.id, user.email, { password: 'wrong', code: authenticator.generate(secret) })).status).toBe(401); + expect((await asLegacyResult(svc.disableMfa(user.id, user.email, { password: 'wrong', code: authenticator.generate(secret) }))).status).toBe(401); expect(await svc.verifyJwtToken(token)).not.toBeNull(); }); @@ -1372,13 +1373,13 @@ describe('session tracking', () => { await updateRows(orm, Users, { id: user.id }, { mfa_enabled: 1, mfa_secret: 'enc:' + secret }); const interstitial = await svc.loginUser({ email: user.email, password }); - const result = await svc.verifyMfaLogin({ mfa_token: interstitial.mfa_token, code: authenticator.generate(secret) }, { userAgent: 'Edge' }); + const result = await asLegacyResult(svc.verifyMfaLogin({ mfa_token: interstitial.mfa_token, code: authenticator.generate(secret) }, { userAgent: 'Edge' })); expect(await svc.verifyJwtToken(result.token!)).not.toBeNull(); expect((await sessionRows(testDb, user.id)).map((row) => row.user_agent)).toEqual(['Edge']); }); it('AUTH-SESS-009: register records the first session with its device', async () => { - const result = await svc.registerUser({ username: 'sess-reg', email: 'sess-reg@example.test', password: 'Secure123!' }, { userAgent: 'Chrome' }); + const result = await asLegacyResult(svc.registerUser({ username: 'sess-reg', email: 'sess-reg@example.test', password: 'Secure123!' }, { userAgent: 'Chrome' })); const userId = (decodeJwt(result.token!) as { id: number }).id; expect((await sessionRows(testDb, userId)).map((row) => row.user_agent)).toEqual(['Chrome']); }); diff --git a/server/tests/unit/nest/domain-error.test.ts b/server/tests/unit/nest/domain-error.test.ts index b2b926a9f..5f3c2a909 100644 --- a/server/tests/unit/nest/domain-error.test.ts +++ b/server/tests/unit/nest/domain-error.test.ts @@ -64,7 +64,7 @@ describe('DomainError', () => { describe('catchDomainError', () => { it('returns the value of a call that succeeds', async () => { - expect(await catchDomainError(() => 5)).toEqual({ ok: true, value: 5 }); + expect(await catchDomainError(() => 5)).toBe(5); }); it('returns the DomainError a call raised', async () => { @@ -73,7 +73,7 @@ describe('catchDomainError', () => { await catchDomainError(async () => { throw err; }), - ).toEqual({ ok: false, error: err }); + ).toBe(err); }); it('rethrows any other error', async () => { diff --git a/server/tests/unit/nest/oidc.service.test.ts b/server/tests/unit/nest/oidc.service.test.ts index cb9b55cec..508f58922 100644 --- a/server/tests/unit/nest/oidc.service.test.ts +++ b/server/tests/unit/nest/oidc.service.test.ts @@ -9,6 +9,7 @@ * ReDoS-sensitive issuer trailing-slash regex. Constructed directly (no * TestingModule, repo convention) over a real in-memory SQLite database. */ +import { asLegacyResult } from '../../helpers/domain-error'; import { describe, it, expect, vi, beforeAll, beforeEach, afterAll, afterEach } from 'vitest'; import { generateKeyPairSync } from 'crypto'; import jwtLib from 'jsonwebtoken'; @@ -1487,7 +1488,7 @@ describe('OIDC configured — the four checks', () => { const configSaysSo = (await auth.getAppConfig(null)).oidc_configured; // 3: turning password login off is refused when OIDC is not configured (oidc_login is on by default). const admin = createAdmin(testDb).user; - const save = await auth.updateAppSettings(admin.id, { password_login: false }); + const save = await asLegacyResult(auth.updateAppSettings(admin.id, { password_login: false })); const lockoutSaysSo = save.error === undefined; await deleteRows(o, AppSettings, { key: { $in: ['password_login'] } }); // 4: the provider config. diff --git a/server/tests/unit/nest/passkey.service.test.ts b/server/tests/unit/nest/passkey.service.test.ts index e5f81f871..3cc26af76 100644 --- a/server/tests/unit/nest/passkey.service.test.ts +++ b/server/tests/unit/nest/passkey.service.test.ts @@ -13,6 +13,7 @@ * crypto is not under test, the service's handling of its verdicts is. * Constructed directly (no TestingModule, repo convention). */ +import { asLegacyResult } from '../../helpers/domain-error'; import { describe, it, expect, vi, beforeAll, beforeEach, afterAll } from 'vitest'; // ── DB setup ────────────────────────────────────────────────────────────────── @@ -255,17 +256,17 @@ function registrationVerdict(overrides: Record = {}, credential describe('passkeyRegisterOptions', () => { it('PASSKEY-SVC-001: returns the not-configured 400 when no RP config resolves', async () => { resolveWebauthnConfigMock.mockReturnValue(null); - expect(await svc.passkeyRegisterOptions(1, 'pw')).toEqual({ error: NOT_CONFIGURED_ERROR, status: 400 }); + expect(await asLegacyResult(svc.passkeyRegisterOptions(1, 'pw'))).toEqual({ error: NOT_CONFIGURED_ERROR, status: 400 }); }); it('PASSKEY-SVC-002: 404s for an unknown user', async () => { - expect(await svc.passkeyRegisterOptions(999_999, 'pw')).toEqual({ error: 'User not found', status: 404 }); + expect(await asLegacyResult(svc.passkeyRegisterOptions(999_999, 'pw'))).toEqual({ error: 'User not found', status: 404 }); }); it('PASSKEY-SVC-003: requires the current password (missing and wrong both 401)', async () => { const { user, password } = createUser(testDb); - expect(await svc.passkeyRegisterOptions(user.id, undefined)).toEqual({ error: 'Incorrect password', status: 401 }); - expect(await svc.passkeyRegisterOptions(user.id, `${password}x`)).toEqual({ error: 'Incorrect password', status: 401 }); + expect(await asLegacyResult(svc.passkeyRegisterOptions(user.id, undefined))).toEqual({ error: 'Incorrect password', status: 401 }); + expect(await asLegacyResult(svc.passkeyRegisterOptions(user.id, `${password}x`))).toEqual({ error: 'Incorrect password', status: 401 }); }); it('PASSKEY-SVC-004: generates options excluding existing credentials and stores the challenge', async () => { @@ -275,7 +276,7 @@ describe('passkeyRegisterOptions', () => { await seedChallenge('stale', user.id, 'registration', Date.now() - 1000); // purged on the way in swMock.generateRegistrationOptions.mockResolvedValue({ challenge: 'reg-chal' }); - const result = await svc.passkeyRegisterOptions(user.id, password); + const result = await asLegacyResult(svc.passkeyRegisterOptions(user.id, password)); expect(result).toEqual({ options: { challenge: 'reg-chal' } }); const call = swMock.generateRegistrationOptions.mock.calls[0][0]; @@ -302,7 +303,7 @@ describe('passkeyRegisterOptions', () => { // not-configured 400 comes BEFORE the ceremony, not as a cryptic verify 400. resolveWebauthnConfigMock.mockReturnValue(LOCALHOST_CFG); const { user, password } = createUser(testDb); - expect(await svc.passkeyRegisterOptions(user.id, password, 'https://trip.example.org')) + expect(await asLegacyResult(svc.passkeyRegisterOptions(user.id, password, 'https://trip.example.org'))) .toEqual({ error: NOT_CONFIGURED_ERROR, status: 400 }); expect(swMock.generateRegistrationOptions).not.toHaveBeenCalled(); expect(await challengeCount()).toBe(0); @@ -315,9 +316,9 @@ describe('passkeyRegisterOptions', () => { swMock.generateRegistrationOptions.mockImplementation(async () => ({ challenge: `reg-chal-${++seq}` })); // Listed origin, in-scope origin on another port, no header (curl) — all pass. - expect(await svc.passkeyRegisterOptions(user.id, password, 'http://localhost:5173')).toEqual({ options: { challenge: 'reg-chal-1' } }); - expect(await svc.passkeyRegisterOptions(user.id, password, 'http://localhost:8080')).toEqual({ options: { challenge: 'reg-chal-2' } }); - expect(await svc.passkeyRegisterOptions(user.id, password, undefined)).toEqual({ options: { challenge: 'reg-chal-3' } }); + expect(await asLegacyResult(svc.passkeyRegisterOptions(user.id, password, 'http://localhost:5173'))).toEqual({ options: { challenge: 'reg-chal-1' } }); + expect(await asLegacyResult(svc.passkeyRegisterOptions(user.id, password, 'http://localhost:8080'))).toEqual({ options: { challenge: 'reg-chal-2' } }); + expect(await asLegacyResult(svc.passkeyRegisterOptions(user.id, password, undefined))).toEqual({ options: { challenge: 'reg-chal-3' } }); }); it('PASSKEY-SVC-035: a mismatching origin never blocks a real or explicit config (proxy tolerance)', async () => { @@ -329,10 +330,10 @@ describe('passkeyRegisterOptions', () => { swMock.generateRegistrationOptions.mockImplementation(async () => ({ challenge: `reg-chal-${++seq}` })); resolveWebauthnConfigMock.mockReturnValue(CFG); // derived from a real APP_URL - expect(await svc.passkeyRegisterOptions(user.id, password, 'http://10.0.0.5:3001')).toEqual({ options: { challenge: 'reg-chal-1' } }); + expect(await asLegacyResult(svc.passkeyRegisterOptions(user.id, password, 'http://10.0.0.5:3001'))).toEqual({ options: { challenge: 'reg-chal-1' } }); resolveWebauthnConfigMock.mockReturnValue({ ...LOCALHOST_CFG, explicitOrigins: true }); // operator contract - expect(await svc.passkeyRegisterOptions(user.id, password, 'https://trip.example.org')).toEqual({ options: { challenge: 'reg-chal-2' } }); + expect(await asLegacyResult(svc.passkeyRegisterOptions(user.id, password, 'https://trip.example.org'))).toEqual({ options: { challenge: 'reg-chal-2' } }); }); }); @@ -341,21 +342,21 @@ describe('passkeyRegisterOptions', () => { describe('passkeyRegisterVerify', () => { it('PASSKEY-SVC-005: returns the not-configured 400 when no RP config resolves', async () => { resolveWebauthnConfigMock.mockReturnValue(null); - expect(await svc.passkeyRegisterVerify(1, { attestationResponse: {} })).toEqual({ error: NOT_CONFIGURED_ERROR, status: 400 }); + expect(await asLegacyResult(svc.passkeyRegisterVerify(1, { attestationResponse: {} }))).toEqual({ error: NOT_CONFIGURED_ERROR, status: 400 }); }); it('PASSKEY-SVC-006: 400s on a missing or undecodable attestation response', async () => { - expect(await svc.passkeyRegisterVerify(1, {})).toEqual({ error: 'Invalid registration response', status: 400 }); - expect(await svc.passkeyRegisterVerify(1, { attestationResponse: { response: { clientDataJSON: '!!!not-base64-json' } } })) + expect(await asLegacyResult(svc.passkeyRegisterVerify(1, {}))).toEqual({ error: 'Invalid registration response', status: 400 }); + expect(await asLegacyResult(svc.passkeyRegisterVerify(1, { attestationResponse: { response: { clientDataJSON: '!!!not-base64-json' } } }))) .toEqual({ error: 'Invalid registration response', status: 400 }); }); it('PASSKEY-SVC-007: 400s on an unknown or expired challenge', async () => { const { user } = createUser(testDb); - expect(await svc.passkeyRegisterVerify(user.id, { attestationResponse: ceremonyResponse('never-stored') })) + expect(await asLegacyResult(svc.passkeyRegisterVerify(user.id, { attestationResponse: ceremonyResponse('never-stored') }))) .toEqual({ error: 'Registration challenge expired. Please try again.', status: 400 }); await seedChallenge('expired', user.id, 'registration', Date.now() - 1); - expect(await svc.passkeyRegisterVerify(user.id, { attestationResponse: ceremonyResponse('expired') })) + expect(await asLegacyResult(svc.passkeyRegisterVerify(user.id, { attestationResponse: ceremonyResponse('expired') }))) .toEqual({ error: 'Registration challenge expired. Please try again.', status: 400 }); }); @@ -363,11 +364,11 @@ describe('passkeyRegisterVerify', () => { const { user: alice } = createUser(testDb); const { user: bob } = createUser(testDb); await seedChallenge('cross', alice.id, 'registration'); - expect(await svc.passkeyRegisterVerify(bob.id, { attestationResponse: ceremonyResponse('cross') })) + expect(await asLegacyResult(svc.passkeyRegisterVerify(bob.id, { attestationResponse: ceremonyResponse('cross') }))) .toEqual({ error: 'Registration challenge expired. Please try again.', status: 400 }); // The mismatched claim still consumed the row: the rightful owner can't use it either. expect(await challengeCount()).toBe(0); - expect(await svc.passkeyRegisterVerify(alice.id, { attestationResponse: ceremonyResponse('cross') })) + expect(await asLegacyResult(svc.passkeyRegisterVerify(alice.id, { attestationResponse: ceremonyResponse('cross') }))) .toEqual({ error: 'Registration challenge expired. Please try again.', status: 400 }); }); @@ -375,12 +376,12 @@ describe('passkeyRegisterVerify', () => { const { user } = createUser(testDb); await seedChallenge('c1', user.id, 'registration'); swMock.verifyRegistrationResponse.mockRejectedValue(new Error('boom')); - expect(await svc.passkeyRegisterVerify(user.id, { attestationResponse: ceremonyResponse('c1') })) + expect(await asLegacyResult(svc.passkeyRegisterVerify(user.id, { attestationResponse: ceremonyResponse('c1') }))) .toEqual({ error: 'Could not register this passkey.', status: 400 }); await seedChallenge('c2', user.id, 'registration'); swMock.verifyRegistrationResponse.mockResolvedValue({ verified: false }); - expect(await svc.passkeyRegisterVerify(user.id, { attestationResponse: ceremonyResponse('c2') })) + expect(await asLegacyResult(svc.passkeyRegisterVerify(user.id, { attestationResponse: ceremonyResponse('c2') }))) .toEqual({ error: 'Could not register this passkey.', status: 400 }); }); @@ -389,7 +390,7 @@ describe('passkeyRegisterVerify', () => { await insertCredential(user.id, { credential_id: 'dup-cred' }); await seedChallenge('c3', user.id, 'registration'); swMock.verifyRegistrationResponse.mockResolvedValue(registrationVerdict({}, { id: 'dup-cred' })); - expect(await svc.passkeyRegisterVerify(user.id, { attestationResponse: ceremonyResponse('c3') })) + expect(await asLegacyResult(svc.passkeyRegisterVerify(user.id, { attestationResponse: ceremonyResponse('c3') }))) .toEqual({ error: 'This passkey is already registered.', status: 409 }); }); @@ -398,7 +399,7 @@ describe('passkeyRegisterVerify', () => { await seedChallenge('c4', user.id, 'registration'); swMock.verifyRegistrationResponse.mockResolvedValue(registrationVerdict({}, { counter: 7 })); - const result = await svc.passkeyRegisterVerify(user.id, { attestationResponse: ceremonyResponse('c4'), name: ' My Key ' }); + const result = await asLegacyResult(svc.passkeyRegisterVerify(user.id, { attestationResponse: ceremonyResponse('c4'), name: ' My Key ' })); expect(result.success).toBe(true); expect(result.credential).toMatchObject({ name: 'My Key', device_type: 'singleDevice', backed_up: false, last_used_at: null }); @@ -412,12 +413,12 @@ describe('passkeyRegisterVerify', () => { await seedChallenge('c5', user.id, 'registration'); swMock.verifyRegistrationResponse.mockResolvedValue(registrationVerdict({ credentialDeviceType: 'multiDevice', credentialBackedUp: true }, { id: 'synced-cred' })); - const synced = await svc.passkeyRegisterVerify(user.id, { attestationResponse: ceremonyResponse('c5'), name: ' ' }); + const synced = await asLegacyResult(svc.passkeyRegisterVerify(user.id, { attestationResponse: ceremonyResponse('c5'), name: ' ' })); expect(synced.credential).toMatchObject({ name: 'Passkey (synced)', backed_up: true }); await seedChallenge('c6', user.id, 'registration'); swMock.verifyRegistrationResponse.mockResolvedValue(registrationVerdict({}, { id: 'named-cred' })); - const long = await svc.passkeyRegisterVerify(user.id, { attestationResponse: ceremonyResponse('c6'), name: 'x'.repeat(80) }); + const long = await asLegacyResult(svc.passkeyRegisterVerify(user.id, { attestationResponse: ceremonyResponse('c6'), name: 'x'.repeat(80) })); expect(long.credential).toMatchObject({ name: 'x'.repeat(60) }); }); @@ -428,7 +429,7 @@ describe('passkeyRegisterVerify', () => { testDb.exec('PRAGMA foreign_keys = OFF'); await seedChallenge('c7', 999_999, 'registration'); testDb.exec('PRAGMA foreign_keys = ON'); - expect(await svc.passkeyRegisterVerify(999_999, { attestationResponse: ceremonyResponse('c7') })) + expect(await asLegacyResult(svc.passkeyRegisterVerify(999_999, { attestationResponse: ceremonyResponse('c7') }))) .toEqual({ error: 'Could not register this passkey.', status: 400 }); }); @@ -439,7 +440,7 @@ describe('passkeyRegisterVerify', () => { testDb.exec('PRAGMA foreign_keys = OFF'); await seedChallenge('c8', 999_999, 'registration'); testDb.exec('PRAGMA foreign_keys = ON'); - await svc.passkeyRegisterVerify(999_999, { attestationResponse: ceremonyResponse('c8') }); + await asLegacyResult(svc.passkeyRegisterVerify(999_999, { attestationResponse: ceremonyResponse('c8') })); expect(await countRows(orm, WebauthnCredentials)).toBe(0); expect(await challengeCount()).toBe(0); // the claim stays spent — the challenge is not resurrected }); @@ -449,7 +450,7 @@ describe('passkeyRegisterVerify', () => { await insertCredential(user.id, { credential_id: 'dup-cred-2', name: 'Original' }); await seedChallenge('c9', user.id, 'registration'); swMock.verifyRegistrationResponse.mockResolvedValue(registrationVerdict({}, { id: 'dup-cred-2' })); - expect(await svc.passkeyRegisterVerify(user.id, { attestationResponse: ceremonyResponse('c9'), name: 'Impostor' })) + expect(await asLegacyResult(svc.passkeyRegisterVerify(user.id, { attestationResponse: ceremonyResponse('c9'), name: 'Impostor' }))) .toEqual({ error: 'This passkey is already registered.', status: 409 }); const row = (await findRow(orm, WebauthnCredentials, { credential_id: 'dup-cred-2' })); expect(row).toMatchObject({ name: 'Original', user_id: user.id }); @@ -466,9 +467,9 @@ describe('passkeyRegisterVerify', () => { await seedChallenge('w1', user.id, 'registration'); swMock.verifyRegistrationResponse.mockResolvedValue(registrationVerdict()); - const result = await svc.passkeyRegisterVerify(user.id, { + const result = await asLegacyResult(svc.passkeyRegisterVerify(user.id, { attestationResponse: ceremonyResponse('w1', {}, { origin: 'https://trek.example.org' }), - }); + })); expect(result.success).toBe(true); expect(swMock.verifyRegistrationResponse.mock.calls[0][0].expectedOrigin) .toEqual(['http://trek.example.org', 'https://trek.example.org']); @@ -482,13 +483,13 @@ describe('passkeyRegisterVerify', () => { resolveWebauthnConfigMock.mockReturnValue({ ...CFG, origins: ['https://trek.example.com'], explicitOrigins: true }); await seedChallenge('w2', user.id, 'registration'); swMock.verifyRegistrationResponse.mockResolvedValue(registrationVerdict()); - await svc.passkeyRegisterVerify(user.id, { attestationResponse: ceremonyResponse('w2', {}, { origin: 'https://app.trek.example.com' }) }); + await asLegacyResult(svc.passkeyRegisterVerify(user.id, { attestationResponse: ceremonyResponse('w2', {}, { origin: 'https://app.trek.example.com' }) })); expect(swMock.verifyRegistrationResponse.mock.calls[0][0].expectedOrigin).toEqual(['https://trek.example.com']); // Derived config, but the asserted origin is outside the RP scope. resolveWebauthnConfigMock.mockReturnValue(CFG); await seedChallenge('w3', user.id, 'registration'); - await svc.passkeyRegisterVerify(user.id, { attestationResponse: ceremonyResponse('w3', {}, { origin: 'https://evil.example.net' }) }); + await asLegacyResult(svc.passkeyRegisterVerify(user.id, { attestationResponse: ceremonyResponse('w3', {}, { origin: 'https://evil.example.net' }) })); expect(swMock.verifyRegistrationResponse.mock.calls[1][0].expectedOrigin).toEqual(['https://trek.example.com']); }); @@ -502,12 +503,12 @@ describe('passkeyRegisterVerify', () => { await seedChallenge('w4', user.id, 'registration'); swMock.verifyRegistrationResponse.mockResolvedValueOnce(registrationVerdict({}, { id: 'sibling-cred' })); - await svc.passkeyRegisterVerify(user.id, { attestationResponse: ceremonyResponse('w4', {}, { origin: 'https://blog.example.com' }) }); + await asLegacyResult(svc.passkeyRegisterVerify(user.id, { attestationResponse: ceremonyResponse('w4', {}, { origin: 'https://blog.example.com' }) })); expect(swMock.verifyRegistrationResponse.mock.calls[0][0].expectedOrigin).toEqual(['https://example.com']); await seedChallenge('w5', user.id, 'registration'); swMock.verifyRegistrationResponse.mockResolvedValueOnce(registrationVerdict({}, { id: 'port-cred' })); - await svc.passkeyRegisterVerify(user.id, { attestationResponse: ceremonyResponse('w5', {}, { origin: 'https://example.com:8443' }) }); + await asLegacyResult(svc.passkeyRegisterVerify(user.id, { attestationResponse: ceremonyResponse('w5', {}, { origin: 'https://example.com:8443' }) })); expect(swMock.verifyRegistrationResponse.mock.calls[1][0].expectedOrigin).toEqual(['https://example.com', 'https://example.com:8443']); }); }); @@ -517,14 +518,14 @@ describe('passkeyRegisterVerify', () => { describe('passkeyLoginOptions', () => { it('PASSKEY-SVC-014: returns the not-configured 400 when no RP config resolves', async () => { resolveWebauthnConfigMock.mockReturnValue(null); - expect(await svc.passkeyLoginOptions()).toEqual({ error: NOT_CONFIGURED_ERROR, status: 400 }); + expect(await asLegacyResult(svc.passkeyLoginOptions())).toEqual({ error: NOT_CONFIGURED_ERROR, status: 400 }); }); it('PASSKEY-SVC-015: stores an anonymous authentication challenge (discoverable flow)', async () => { await seedChallenge('stale', null, 'authentication', Date.now() - 1000); swMock.generateAuthenticationOptions.mockResolvedValue({ challenge: 'auth-chal' }); - expect(await svc.passkeyLoginOptions()).toEqual({ options: { challenge: 'auth-chal' } }); + expect(await asLegacyResult(svc.passkeyLoginOptions())).toEqual({ options: { challenge: 'auth-chal' } }); // No allowCredentials → the endpoint can't enumerate accounts. expect(swMock.generateAuthenticationOptions.mock.calls[0][0]).toEqual({ rpID: CFG.rpID, userVerification: 'required' }); @@ -535,12 +536,12 @@ describe('passkeyLoginOptions', () => { it('PASSKEY-SVC-038: the derived localhost fallback fails fast for a foreign request origin (#2147)', async () => { resolveWebauthnConfigMock.mockReturnValue(LOCALHOST_CFG); - expect(await svc.passkeyLoginOptions('https://trip.example.org')).toEqual({ error: NOT_CONFIGURED_ERROR, status: 400 }); + expect(await asLegacyResult(svc.passkeyLoginOptions('https://trip.example.org'))).toEqual({ error: NOT_CONFIGURED_ERROR, status: 400 }); expect(swMock.generateAuthenticationOptions).not.toHaveBeenCalled(); // No Origin header (non-browser client) → unchanged legacy behavior. swMock.generateAuthenticationOptions.mockResolvedValue({ challenge: 'auth-chal' }); - expect(await svc.passkeyLoginOptions()).toEqual({ options: { challenge: 'auth-chal' } }); + expect(await asLegacyResult(svc.passkeyLoginOptions())).toEqual({ options: { challenge: 'auth-chal' } }); }); }); @@ -738,9 +739,9 @@ describe('renamePasskey', () => { it('PASSKEY-SVC-026: rejects a missing, non-string or whitespace-only name', async () => { const { user } = createUser(testDb); const cred = await insertCredential(user.id); - expect(await svc.renamePasskey(user.id, String(cred.id), undefined)).toEqual({ error: 'Name is required', status: 400 }); - expect(await svc.renamePasskey(user.id, String(cred.id), 42)).toEqual({ error: 'Name is required', status: 400 }); - expect(await svc.renamePasskey(user.id, String(cred.id), ' ')).toEqual({ error: 'Name is required', status: 400 }); + expect(await asLegacyResult(svc.renamePasskey(user.id, String(cred.id), undefined))).toEqual({ error: 'Name is required', status: 400 }); + expect(await asLegacyResult(svc.renamePasskey(user.id, String(cred.id), 42))).toEqual({ error: 'Name is required', status: 400 }); + expect(await asLegacyResult(svc.renamePasskey(user.id, String(cred.id), ' '))).toEqual({ error: 'Name is required', status: 400 }); }); it('PASSKEY-SVC-027: renames (trimmed, capped at 60) and 404s on foreign or unknown ids', async () => { @@ -748,18 +749,18 @@ describe('renamePasskey', () => { const { user: other } = createUser(testDb); const cred = await insertCredential(user.id); - expect(await svc.renamePasskey(user.id, String(cred.id), ` ${'n'.repeat(80)} `)).toEqual({ success: true }); + expect(await asLegacyResult(svc.renamePasskey(user.id, String(cred.id), ` ${'n'.repeat(80)} `))).toEqual({ success: true }); const row = (await findRow(orm, WebauthnCredentials, { id: cred.id })); expect(row?.name).toBe('n'.repeat(60)); - expect(await svc.renamePasskey(other.id, String(cred.id), 'Steal')).toEqual({ error: 'Passkey not found', status: 404 }); - expect(await svc.renamePasskey(user.id, '999999', 'Ghost')).toEqual({ error: 'Passkey not found', status: 404 }); + expect(await asLegacyResult(svc.renamePasskey(other.id, String(cred.id), 'Steal'))).toEqual({ error: 'Passkey not found', status: 404 }); + expect(await asLegacyResult(svc.renamePasskey(user.id, '999999', 'Ghost'))).toEqual({ error: 'Passkey not found', status: 404 }); }); it('PASSKEY-SVC-042: 404s (not a 500) on a non-numeric id — Plan 3b Task 3 review, F1', async () => { const { user } = createUser(testDb); - expect(await svc.renamePasskey(user.id, 'abc', 'Ghost')).toEqual({ error: 'Passkey not found', status: 404 }); - expect(await svc.renamePasskey(user.id, '1abc', 'Ghost')).toEqual({ error: 'Passkey not found', status: 404 }); + expect(await asLegacyResult(svc.renamePasskey(user.id, 'abc', 'Ghost'))).toEqual({ error: 'Passkey not found', status: 404 }); + expect(await asLegacyResult(svc.renamePasskey(user.id, '1abc', 'Ghost'))).toEqual({ error: 'Passkey not found', status: 404 }); }); // task-3-rereview.md R1: the legacy `renamePasskey` bound `Number(id)` @@ -769,7 +770,7 @@ describe('renamePasskey', () => { // (accepted deviation, not parity), not "the legacy 404". it('PASSKEY-SVC-042B: refuses a prefixed-numeric id (0x10) via the toRowId narrowing', async () => { const { user } = createUser(testDb); - expect(await svc.renamePasskey(user.id, '0x10', 'Ghost')).toEqual({ error: 'Passkey not found', status: 404 }); + expect(await asLegacyResult(svc.renamePasskey(user.id, '0x10', 'Ghost'))).toEqual({ error: 'Passkey not found', status: 404 }); }); }); @@ -779,10 +780,10 @@ describe('deletePasskey', () => { it('PASSKEY-SVC-028: requires the current password (missing, wrong, or no hash all 401)', async () => { const { user, password } = createUser(testDb); const cred = await insertCredential(user.id); - expect(await svc.deletePasskey(user.id, String(cred.id), undefined)).toEqual({ error: 'Incorrect password', status: 401 }); - expect(await svc.deletePasskey(user.id, String(cred.id), `${password}x`)).toEqual({ error: 'Incorrect password', status: 401 }); + expect(await asLegacyResult(svc.deletePasskey(user.id, String(cred.id), undefined))).toEqual({ error: 'Incorrect password', status: 401 }); + expect(await asLegacyResult(svc.deletePasskey(user.id, String(cred.id), `${password}x`))).toEqual({ error: 'Incorrect password', status: 401 }); await updateRows(orm, Users, { id: user.id }, { password_hash: '' }); - expect(await svc.deletePasskey(user.id, String(cred.id), password)).toEqual({ error: 'Incorrect password', status: 401 }); + expect(await asLegacyResult(svc.deletePasskey(user.id, String(cred.id), password))).toEqual({ error: 'Incorrect password', status: 401 }); }); it('PASSKEY-SVC-029: deletes own credentials only — foreign ids 404 without leaking', async () => { @@ -790,14 +791,14 @@ describe('deletePasskey', () => { const { user: other, password: otherPassword } = createUser(testDb); const cred = await insertCredential(user.id); - expect(await svc.deletePasskey(other.id, String(cred.id), otherPassword)).toEqual({ error: 'Passkey not found', status: 404 }); - expect(await svc.deletePasskey(user.id, String(cred.id), password)).toEqual({ success: true }); + expect(await asLegacyResult(svc.deletePasskey(other.id, String(cred.id), otherPassword))).toEqual({ error: 'Passkey not found', status: 404 }); + expect(await asLegacyResult(svc.deletePasskey(user.id, String(cred.id), password))).toEqual({ success: true }); expect(await countRows(orm, WebauthnCredentials)).toBe(0); }); it('PASSKEY-SVC-043: 404s (not a 500) on a non-numeric id once the password check passes — Plan 3b Task 3 review, F1', async () => { const { user, password } = createUser(testDb); - expect(await svc.deletePasskey(user.id, 'abc', password)).toEqual({ error: 'Passkey not found', status: 404 }); + expect(await asLegacyResult(svc.deletePasskey(user.id, 'abc', password))).toEqual({ error: 'Passkey not found', status: 404 }); }); // task-3-rereview.md R1: same distinction as PASSKEY-SVC-042B — @@ -805,12 +806,12 @@ describe('deletePasskey', () => { // acted on credential 16, not 404'd. This pins the `toRowId` narrowing. it('PASSKEY-SVC-043B: refuses a prefixed-numeric id (0x10) via the toRowId narrowing, once the password check passes', async () => { const { user, password } = createUser(testDb); - expect(await svc.deletePasskey(user.id, '0x10', password)).toEqual({ error: 'Passkey not found', status: 404 }); + expect(await asLegacyResult(svc.deletePasskey(user.id, '0x10', password))).toEqual({ error: 'Passkey not found', status: 404 }); }); it('PASSKEY-SVC-044: a wrong password still wins over a non-numeric id — 401, not 404 (legacy statement order)', async () => { const { user, password } = createUser(testDb); - expect(await svc.deletePasskey(user.id, 'abc', `${password}x`)).toEqual({ error: 'Incorrect password', status: 401 }); + expect(await asLegacyResult(svc.deletePasskey(user.id, 'abc', `${password}x`))).toEqual({ error: 'Incorrect password', status: 401 }); }); }); @@ -818,7 +819,7 @@ describe('deletePasskey', () => { describe('adminResetPasskeys', () => { it('PASSKEY-SVC-030: 404s on an unknown user, else clears all credentials and reports the count', async () => { - expect(await svc.adminResetPasskeys(999_999)).toEqual({ error: 'User not found', status: 404 }); + expect(await asLegacyResult(svc.adminResetPasskeys(999_999))).toEqual({ error: 'User not found', status: 404 }); const { user } = createUser(testDb); const { user: other } = createUser(testDb); @@ -826,12 +827,12 @@ describe('adminResetPasskeys', () => { await insertCredential(user.id); const kept = await insertCredential(other.id); - expect(await svc.adminResetPasskeys(user.id)).toEqual({ success: true, deleted: 2, email: user.email }); - expect(await svc.adminResetPasskeys(user.id)).toEqual({ success: true, deleted: 0, email: user.email }); + expect(await asLegacyResult(svc.adminResetPasskeys(user.id))).toEqual({ success: true, deleted: 2, email: user.email }); + expect(await asLegacyResult(svc.adminResetPasskeys(user.id))).toEqual({ success: true, deleted: 0, email: user.email }); expect((await findRows(orm, WebauthnCredentials)).map((r) => ({ id: r.id }))).toEqual([{ id: kept.id }]); }); it('PASSKEY-SVC-045: 404s (not a 500) when the caller hands in NaN — AdminService.resetUserPasskeys converts a non-numeric route id with a bare Number(id) before calling in (Plan 3b Task 3 review, F1)', async () => { - expect(await svc.adminResetPasskeys(Number('abc'))).toEqual({ error: 'User not found', status: 404 }); + expect(await asLegacyResult(svc.adminResetPasskeys(Number('abc')))).toEqual({ error: 'User not found', status: 404 }); }); }); diff --git a/server/tests/unit/nest/registration-invites.service.test.ts b/server/tests/unit/nest/registration-invites.service.test.ts index 21672720a..3af28c3c7 100644 --- a/server/tests/unit/nest/registration-invites.service.test.ts +++ b/server/tests/unit/nest/registration-invites.service.test.ts @@ -7,6 +7,7 @@ * stays greppable. Constructed directly (no TestingModule, repo convention). */ +import { asLegacyResult } from '../../helpers/domain-error'; import { describe, it, expect, beforeAll, beforeEach, afterAll, vi } from 'vitest'; vi.mock('../../../src/db/database', async () => { @@ -58,14 +59,14 @@ describe('Invites', () => { it('ADMIN-SVC-027 — deleteInvite removes invite', async () => { const { user: admin } = createAdmin(testDb); const invite = createInviteToken(testDb, { created_by: admin.id }) as any; - const result = await svc.deleteInvite(String(invite.id)) as any; + const result = await asLegacyResult(svc.deleteInvite(String(invite.id))) as any; expect(result.error).toBeUndefined(); const check = await findRow(await sharedTestOrm(testDb), InviteTokens, { id: invite.id }); expect(check).toBeNull(); }); it('ADMIN-SVC-028 — deleteInvite returns 404 for non-existent invite', async () => { - const result = await svc.deleteInvite('99999') as any; + const result = await asLegacyResult(svc.deleteInvite('99999')) as any; expect(result.status).toBe(404); }); @@ -73,7 +74,7 @@ describe('Invites', () => { const { user: admin } = createAdmin(testDb); await insertRow(await sharedTestOrm(testDb), InviteTokens, { id: 16, token: 'hex-survivor', max_uses: 1, used_count: 0, expires_at: null, createdByRef: admin.id }); - const result = await svc.deleteInvite('0x10'); + const result = await asLegacyResult(svc.deleteInvite('0x10')); expect(result).toEqual({ error: 'Invite not found', status: 404 }); expect(await findRow(await sharedTestOrm(testDb), InviteTokens, { id: 16 })).not.toBeNull(); diff --git a/server/tests/unit/nest/token.service.test.ts b/server/tests/unit/nest/token.service.test.ts index bbe6fff74..242fe86cf 100644 --- a/server/tests/unit/nest/token.service.test.ts +++ b/server/tests/unit/nest/token.service.test.ts @@ -33,6 +33,7 @@ vi.mock('../../../src/mcp/sessionManager', () => ({ revokeUserSessions: vi.fn() // Imports (after mocks) // --------------------------------------------------------------------------- +import { asLegacyResult } from '../../helpers/domain-error'; import { db as testDb } from '../../../src/db/database'; import { describe, it, expect, beforeAll, beforeEach, afterAll, vi } from 'vitest'; import { resetTestDb } from '../../helpers/test-db'; @@ -79,19 +80,19 @@ afterAll(() => { describe('MCP token service', () => { it('AUTH-DB-041: createMcpToken returns 400 when name is missing', async () => { const { user } = createUser(testDb); - const result = await svc.createMcpToken(user.id, undefined); + const result = await asLegacyResult(svc.createMcpToken(user.id, undefined)); expect(result.status).toBe(400); }); it('AUTH-DB-042: createMcpToken returns 400 when name exceeds 100 chars', async () => { const { user } = createUser(testDb); - const result = await svc.createMcpToken(user.id, 'a'.repeat(101)); + const result = await asLegacyResult(svc.createMcpToken(user.id, 'a'.repeat(101))); expect(result.status).toBe(400); }); it('AUTH-DB-043: createMcpToken creates token and returns raw_token', async () => { const { user } = createUser(testDb); - const result = await svc.createMcpToken(user.id, 'My Token'); + const result = await asLegacyResult(svc.createMcpToken(user.id, 'My Token')); expect(result.token).toBeDefined(); expect((result.token as any).raw_token).toMatch(/^trek_/); }); @@ -101,22 +102,22 @@ describe('MCP token service', () => { for (let i = 0; i < 10; i++) { await insertRow(await sharedTestOrm(testDb), McpTokens, { user: user.id, name: `Token ${i}`, token_hash: `hash${i}`, token_prefix: `trek_prefix${i}` }); } - const result = await svc.createMcpToken(user.id, 'One More'); + const result = await asLegacyResult(svc.createMcpToken(user.id, 'One More')); expect(result.status).toBe(400); }); it('AUTH-DB-045: deleteMcpToken returns 404 for non-existent token', async () => { const { user } = createUser(testDb); - const result = await svc.deleteMcpToken(user.id, '99999'); + const result = await asLegacyResult(svc.deleteMcpToken(user.id, '99999')); expect(result.status).toBe(404); }); it('AUTH-DB-046: deleteMcpToken deletes the token and returns success', async () => { const { user } = createUser(testDb); - const created = await svc.createMcpToken(user.id, 'Deletable Token'); + const created = await asLegacyResult(svc.createMcpToken(user.id, 'Deletable Token')); const tokenId = String((created.token as any).id); - const result = await svc.deleteMcpToken(user.id, tokenId); + const result = await asLegacyResult(svc.deleteMcpToken(user.id, tokenId)); expect(result).toEqual({ success: true }); const row = await tokenRow(tokenId); @@ -125,19 +126,19 @@ describe('MCP token service', () => { it('AUTH-DB-092: deleteMcpToken succeeds even when the session sweep throws (best-effort)', async () => { const { user } = createUser(testDb); - const created = await svc.createMcpToken(user.id, 'sweep-down'); + const created = await asLegacyResult(svc.createMcpToken(user.id, 'sweep-down')); const tokenId = String((created.token as { id: number }).id); vi.mocked(revokeUserSessions).mockImplementationOnce(() => { throw new Error('sweep down'); }); - expect(await svc.deleteMcpToken(user.id, tokenId)).toEqual({ success: true }); + expect(await asLegacyResult(svc.deleteMcpToken(user.id, tokenId))).toEqual({ success: true }); expect(await tokenRow(tokenId)).toBeNull(); }); it('TOKEN-001: listMcpTokens is scoped to the caller and never exposes the hash', async () => { const { user } = createUser(testDb); const { user: other } = createUser(testDb); - await svc.createMcpToken(user.id, 'mine'); - await svc.createMcpToken(other.id, 'theirs'); + await asLegacyResult(svc.createMcpToken(user.id, 'mine')); + await asLegacyResult(svc.createMcpToken(other.id, 'theirs')); const mine = await svc.listMcpTokens(user.id) as Record[]; expect(mine).toHaveLength(1); @@ -148,21 +149,21 @@ describe('MCP token service', () => { it('TOKEN-002: deleteMcpToken refuses a token that belongs to someone else', async () => { const { user } = createUser(testDb); const { user: other } = createUser(testDb); - const created = await svc.createMcpToken(other.id, 'not-yours'); + const created = await asLegacyResult(svc.createMcpToken(other.id, 'not-yours')); const tokenId = String((created.token as { id: number }).id); - expect(await svc.deleteMcpToken(user.id, tokenId)).toEqual({ error: 'Token not found', status: 404 }); + expect(await asLegacyResult(svc.deleteMcpToken(user.id, tokenId))).toEqual({ error: 'Token not found', status: 404 }); expect(await tokenRow(tokenId)).not.toBeNull(); }); it('TOKEN-017: deleteMcpToken 404s (not 500) on a non-numeric id — Plan 3b Task 2 review, F1', async () => { const { user } = createUser(testDb); - expect(await svc.deleteMcpToken(user.id, 'abc')).toEqual({ error: 'Token not found', status: 404 }); + expect(await asLegacyResult(svc.deleteMcpToken(user.id, 'abc'))).toEqual({ error: 'Token not found', status: 404 }); }); it('TOKEN-018: deleteMcpToken 404s on a hex-shaped id — Number("0x10") is 16, a safe integer a bare Number() conversion would accept, but SQLite affinity never would (Plan 3b Task 2 review, F1)', async () => { const { user } = createUser(testDb); - expect(await svc.deleteMcpToken(user.id, '0x10')).toEqual({ error: 'Token not found', status: 404 }); + expect(await asLegacyResult(svc.deleteMcpToken(user.id, '0x10'))).toEqual({ error: 'Token not found', status: 404 }); }); }); @@ -177,7 +178,7 @@ describe('MCP token service', () => { describe('API key service', () => { it('TOKEN-010: an API key does not verify as an MCP token', async () => { const { user } = createUser(testDb); - const created = await svc.createApiToken(user.id, 'dawarich'); + const created = await asLegacyResult(svc.createApiToken(user.id, 'dawarich')); const raw = (created.token as { raw_token: string }).raw_token; expect((await svc.verifyApiToken(raw))?.id).toBe(user.id); @@ -186,7 +187,7 @@ describe('API key service', () => { it('TOKEN-011: an MCP token does not verify as an API key', async () => { const { user } = createUser(testDb); - const created = await svc.createMcpToken(user.id, 'claude'); + const created = await asLegacyResult(svc.createMcpToken(user.id, 'claude')); const raw = (created.token as { raw_token: string }).raw_token; expect((await svc.verifyMcpToken(raw))?.id).toBe(user.id); @@ -195,8 +196,8 @@ describe('API key service', () => { it('TOKEN-012: each list shows only its own kind', async () => { const { user } = createUser(testDb); - await svc.createMcpToken(user.id, 'claude'); - await svc.createApiToken(user.id, 'dawarich'); + await asLegacyResult(svc.createMcpToken(user.id, 'claude')); + await asLegacyResult(svc.createApiToken(user.id, 'dawarich')); const mcp = await svc.listMcpTokens(user.id) as Record[]; const api = await svc.listApiTokens(user.id) as Record[]; @@ -206,11 +207,11 @@ describe('API key service', () => { it('TOKEN-013: deleting across kinds 404s, identically to an unknown id', async () => { const { user } = createUser(testDb); - const mcpId = String((await svc.createMcpToken(user.id, 'claude')).token!.id as number); - const apiId = String((await svc.createApiToken(user.id, 'dawarich')).token!.id as number); + const mcpId = String((await asLegacyResult(svc.createMcpToken(user.id, 'claude'))).token!.id as number); + const apiId = String((await asLegacyResult(svc.createApiToken(user.id, 'dawarich'))).token!.id as number); - expect(await svc.deleteApiToken(user.id, mcpId)).toEqual({ error: 'Token not found', status: 404 }); - expect(await svc.deleteMcpToken(user.id, apiId)).toEqual({ error: 'Token not found', status: 404 }); + expect(await asLegacyResult(svc.deleteApiToken(user.id, mcpId))).toEqual({ error: 'Token not found', status: 404 }); + expect(await asLegacyResult(svc.deleteMcpToken(user.id, apiId))).toEqual({ error: 'Token not found', status: 404 }); // Neither row was touched: a wrong-kind delete must not be a way to revoke // someone's assistant access from the API-key screen. expect(await tokenRow(mcpId)).not.toBeNull(); @@ -219,16 +220,16 @@ describe('API key service', () => { it('TOKEN-014: the ten-token ceiling counts each kind on its own', async () => { const { user } = createUser(testDb); - for (let i = 0; i < 10; i += 1) await svc.createMcpToken(user.id, `mcp-${i}`); + for (let i = 0; i < 10; i += 1) await asLegacyResult(svc.createMcpToken(user.id, `mcp-${i}`)); - expect((await svc.createMcpToken(user.id, 'one-too-many')).status).toBe(400); + expect((await asLegacyResult(svc.createMcpToken(user.id, 'one-too-many'))).status).toBe(400); // A full MCP shelf must not lock the user out of minting an API key. - expect((await svc.createApiToken(user.id, 'dawarich')).status).toBeUndefined(); + expect((await asLegacyResult(svc.createApiToken(user.id, 'dawarich'))).status).toBeUndefined(); }); it('TOKEN-015: an API key is stored hashed, with only a prefix in the clear', async () => { const { user } = createUser(testDb); - const created = await svc.createApiToken(user.id, 'dawarich'); + const created = await asLegacyResult(svc.createApiToken(user.id, 'dawarich')); const raw = (created.token as { raw_token: string }).raw_token; const row = (await findRow(await sharedTestOrm(testDb), McpTokens, { user: user.id }))!; @@ -239,7 +240,7 @@ describe('API key service', () => { it('TOKEN-016: verifying an API key records last_used_at, so a stale key is visible', async () => { const { user } = createUser(testDb); - const raw = (await svc.createApiToken(user.id, 'dawarich')).token!.raw_token as string; + const raw = (await asLegacyResult(svc.createApiToken(user.id, 'dawarich'))).token!.raw_token as string; const before = await svc.listApiTokens(user.id) as Record[]; expect(before[0].last_used_at).toBeNull(); @@ -265,7 +266,7 @@ describe('MCP token service (admin view)', () => { }); it('ADMIN-SVC-069 — adminDeleteMcpToken returns 404 for non-existent token', async () => { - const result = await svc.adminDeleteMcpToken('99999') as any; + const result = await asLegacyResult(svc.adminDeleteMcpToken('99999')) as any; expect(result.status).toBe(404); expect(result.error).toBeDefined(); }); @@ -273,8 +274,8 @@ describe('MCP token service (admin view)', () => { it('TOKEN-003: listAllMcpTokens spans users and carries the owner username', async () => { const { user } = createUser(testDb); const { user: other } = createUser(testDb); - await svc.createMcpToken(user.id, 'a'); - await svc.createMcpToken(other.id, 'b'); + await asLegacyResult(svc.createMcpToken(user.id, 'a')); + await asLegacyResult(svc.createMcpToken(other.id, 'b')); // `listAllMcpTokens` returns the repository's own typed // `McpTokenWithUsernameRow[]` now (Plan 3b Task 2) — no cast needed @@ -289,26 +290,26 @@ describe('MCP token service (admin view)', () => { it('TOKEN-004: adminDeleteMcpToken removes any user token and revokes that user', async () => { const { user } = createUser(testDb); - const created = await svc.createMcpToken(user.id, 'admin-killed'); + const created = await asLegacyResult(svc.createMcpToken(user.id, 'admin-killed')); const tokenId = String((created.token as { id: number }).id); - expect(await svc.adminDeleteMcpToken(tokenId)).toEqual({}); + expect(await asLegacyResult(svc.adminDeleteMcpToken(tokenId))).toEqual({}); expect(await tokenRow(tokenId)).toBeNull(); expect(revokeUserSessions).toHaveBeenCalledWith(user.id); }); it('TOKEN-005: adminDeleteMcpToken 404s on an unknown id without revoking anyone', async () => { - expect(await svc.adminDeleteMcpToken('99999')).toEqual({ error: 'Token not found', status: 404 }); + expect(await asLegacyResult(svc.adminDeleteMcpToken('99999'))).toEqual({ error: 'Token not found', status: 404 }); expect(revokeUserSessions).not.toHaveBeenCalled(); }); it('TOKEN-019: adminDeleteMcpToken 404s (not 500) on a non-numeric id, without revoking anyone — Plan 3b Task 2 review, F1', async () => { - expect(await svc.adminDeleteMcpToken('abc')).toEqual({ error: 'Token not found', status: 404 }); + expect(await asLegacyResult(svc.adminDeleteMcpToken('abc'))).toEqual({ error: 'Token not found', status: 404 }); expect(revokeUserSessions).not.toHaveBeenCalled(); }); it('TOKEN-020: adminDeleteMcpToken 404s on a hex-shaped id (Plan 3b Task 2 review, F1)', async () => { - expect(await svc.adminDeleteMcpToken('0x10')).toEqual({ error: 'Token not found', status: 404 }); + expect(await asLegacyResult(svc.adminDeleteMcpToken('0x10'))).toEqual({ error: 'Token not found', status: 404 }); expect(revokeUserSessions).not.toHaveBeenCalled(); }); }); @@ -329,7 +330,7 @@ describe('ephemeral tokens', () => { it('AUTH-DB-087: createWsToken returns the ephemeral token when the store answers', async () => { const { user } = createUser(testDb); vi.mocked(createEphemeralToken).mockReturnValueOnce('ws-tok'); - expect(await svc.createWsToken(user.id)).toEqual({ token: 'ws-tok' }); + expect(await asLegacyResult(svc.createWsToken(user.id))).toEqual({ token: 'ws-tok' }); }); it('TOKEN-006: createWsToken binds the caller password_version, so a pre-reset token is rejected on connect', async () => { @@ -337,20 +338,20 @@ describe('ephemeral tokens', () => { await updateRows(await sharedTestOrm(testDb), Users, { id: user.id }, { password_version: 7 }); vi.mocked(createEphemeralToken).mockReturnValueOnce('ws-tok'); - await svc.createWsToken(user.id); + await asLegacyResult(svc.createWsToken(user.id)); expect(createEphemeralToken).toHaveBeenCalledWith(user.id, 'ws', { pv: 7 }); }); it('TOKEN-007: createWsToken falls back to pv 0 for a user row without one', async () => { vi.mocked(createEphemeralToken).mockReturnValueOnce('ws-tok'); - await svc.createWsToken(99999); + await asLegacyResult(svc.createWsToken(99999)); expect(createEphemeralToken).toHaveBeenCalledWith(99999, 'ws', { pv: 0 }); }); it('TOKEN-008: createWsToken reports 503 when the store refuses', async () => { const { user } = createUser(testDb); vi.mocked(createEphemeralToken).mockReturnValueOnce(null as unknown as string); - expect(await svc.createWsToken(user.id)).toEqual({ error: 'Service unavailable', status: 503 }); + expect(await asLegacyResult(svc.createWsToken(user.id))).toEqual({ error: 'Service unavailable', status: 503 }); }); }); @@ -361,7 +362,7 @@ describe('ephemeral tokens', () => { describe('verifyMcpToken', () => { it('AUTH-BR-002: verifyMcpToken resolves a freshly created token to its user', async () => { const { user } = createUser(testDb); - const created = await svc.createMcpToken(user.id, 'bridge-case'); + const created = await asLegacyResult(svc.createMcpToken(user.id, 'bridge-case')); const raw = (created.token as { raw_token: string }).raw_token; const resolved = await svc.verifyMcpToken(raw); @@ -371,7 +372,7 @@ describe('verifyMcpToken', () => { it('TOKEN-009: a successful verify stamps last_used_at, a failed one changes nothing', async () => { const { user } = createUser(testDb); - const created = await svc.createMcpToken(user.id, 'stamped'); + const created = await asLegacyResult(svc.createMcpToken(user.id, 'stamped')); const raw = (created.token as { raw_token: string }).raw_token; const id = (created.token as { id: number }).id; expect((await tokenRow(id))!.last_used_at).toBeNull(); @@ -385,7 +386,7 @@ describe('verifyMcpToken', () => { it('TOKEN-010: verifyMcpToken returns identity columns only, never the password hash', async () => { const { user } = createUser(testDb); - const created = await svc.createMcpToken(user.id, 'lean'); + const created = await asLegacyResult(svc.createMcpToken(user.id, 'lean')); const raw = (created.token as { raw_token: string }).raw_token; const resolved = await svc.verifyMcpToken(raw) as unknown as Record; diff --git a/server/tests/unit/nest/user-profile.service.test.ts b/server/tests/unit/nest/user-profile.service.test.ts index 04f80f3f2..b8159f8e6 100644 --- a/server/tests/unit/nest/user-profile.service.test.ts +++ b/server/tests/unit/nest/user-profile.service.test.ts @@ -31,6 +31,7 @@ vi.mock('../../../src/nest/common/crypto/apiKeyCrypto', () => ({ // Imports (after mocks) // --------------------------------------------------------------------------- +import { asLegacyResult } from '../../helpers/domain-error'; import { db as testDb } from '../../../src/db/database'; import { describe, it, expect, beforeAll, beforeEach, afterAll, vi } from 'vitest'; import { resetTestDb } from '../../helpers/test-db'; @@ -83,21 +84,21 @@ afterAll(() => { describe('updateSettings', () => { it('AUTH-DB-001: updates username successfully', async () => { const { user } = createUser(testDb); - const result = await profile.updateSettings(user.id, { username: 'newname' }); + const result = await asLegacyResult(profile.updateSettings(user.id, { username: 'newname' })); expect(result.success).toBe(true); expect(result.user?.username).toBe('newname'); }); it('AUTH-DB-002: returns 400 when username is too short (< 2 chars)', async () => { const { user } = createUser(testDb); - const result = await profile.updateSettings(user.id, { username: 'x' }); + const result = await asLegacyResult(profile.updateSettings(user.id, { username: 'x' })); expect(result.status).toBe(400); expect(result.error).toMatch(/between 2 and 50/i); }); it('AUTH-DB-003: returns 400 when username has invalid characters (spaces)', async () => { const { user } = createUser(testDb); - const result = await profile.updateSettings(user.id, { username: 'bad name' }); + const result = await asLegacyResult(profile.updateSettings(user.id, { username: 'bad name' })); expect(result.status).toBe(400); expect(result.error).toMatch(/only contain/i); }); @@ -105,21 +106,21 @@ describe('updateSettings', () => { it('AUTH-DB-004: returns 409 when username is already taken by another user', async () => { const { user: user1 } = createUser(testDb, { username: 'alice' }); const { user: user2 } = createUser(testDb, { username: 'bob' }); - const result = await profile.updateSettings(user2.id, { username: user1.username }); + const result = await asLegacyResult(profile.updateSettings(user2.id, { username: user1.username })); expect(result.status).toBe(409); expect(result.error).toMatch(/already taken/i); }); it('AUTH-DB-005: updates email successfully', async () => { const { user } = createUser(testDb); - const result = await profile.updateSettings(user.id, { email: 'new@example.com' }); + const result = await asLegacyResult(profile.updateSettings(user.id, { email: 'new@example.com' })); expect(result.success).toBe(true); expect(result.user?.email).toBe('new@example.com'); }); it('AUTH-DB-006: returns 400 for invalid email format', async () => { const { user } = createUser(testDb); - const result = await profile.updateSettings(user.id, { email: 'not-an-email' }); + const result = await asLegacyResult(profile.updateSettings(user.id, { email: 'not-an-email' })); expect(result.status).toBe(400); expect(result.error).toMatch(/invalid email/i); }); @@ -127,14 +128,14 @@ describe('updateSettings', () => { it('AUTH-DB-007: returns 409 when email is already taken by another user', async () => { const { user: user1 } = createUser(testDb, { email: 'taken@example.com' }); const { user: user2 } = createUser(testDb); - const result = await profile.updateSettings(user2.id, { email: user1.email }); + const result = await asLegacyResult(profile.updateSettings(user2.id, { email: user1.email })); expect(result.status).toBe(409); expect(result.error).toMatch(/already taken/i); }); it('AUTH-DB-008: returns success with no field changes when empty body is passed', async () => { const { user } = createUser(testDb); - const result = await profile.updateSettings(user.id, {}); + const result = await asLegacyResult(profile.updateSettings(user.id, {})); expect(result.success).toBe(true); }); }); @@ -146,7 +147,7 @@ describe('updateSettings', () => { describe('getSettings', () => { it('AUTH-DB-009: returns 403 for non-admin user', async () => { const { user } = createUser(testDb); - const result = await profile.getSettings(user.id); + const result = await asLegacyResult(profile.getSettings(user.id)); expect(result.status).toBe(403); expect(result.error).toMatch(/admin/i); }); @@ -154,7 +155,7 @@ describe('getSettings', () => { it('AUTH-DB-010: returns maps_api_key and openweather_api_key for admin', async () => { const { user } = createAdmin(testDb); await setUserColumns(user.id, { maps_api_key: 'maps-key-value', openweather_api_key: 'weather-key-value' }); - const result = await profile.getSettings(user.id); + const result = await asLegacyResult(profile.getSettings(user.id)); expect(result.status).toBeUndefined(); expect(result.settings).toBeDefined(); expect(result.settings).toHaveProperty('maps_api_key'); @@ -167,7 +168,7 @@ describe('getSettings', () => { // Returned to the client masked, never in plaintext. expect(result.user.unsplash_api_key).toBe('-----key'); // getSettings returns the stored key to the admin. - expect((await profile.getSettings(user.id)).settings?.unsplash_api_key).toBe('unsplash-secret-key'); + expect((await asLegacyResult(profile.getSettings(user.id))).settings?.unsplash_api_key).toBe('unsplash-secret-key'); }); it('AUTH-DB-010c: a key set in the environment comes back as its variable name, not a value (#1881)', async () => { @@ -176,7 +177,7 @@ describe('getSettings', () => { try { const { user } = createAdmin(testDb); await profile.updateApiKeys(user.id, { maps_api_key: 'stored-but-overridden', unsplash_api_key: 'stored-unsplash' }); - const settings = (await profile.getSettings(user.id)).settings; + const settings = (await asLegacyResult(profile.getSettings(user.id))).settings; // Neither the stored value nobody searches with nor the operator's own. expect(settings?.maps_api_key).toBeNull(); expect(settings?.env_keys).toEqual({ maps_api_key: 'PLACES_API_KEY' }); @@ -191,7 +192,7 @@ describe('getSettings', () => { it('AUTH-DB-010d: without a variable env_keys is empty', async () => { const { user } = createAdmin(testDb); - expect((await profile.getSettings(user.id)).settings?.env_keys).toEqual({}); + expect((await asLegacyResult(profile.getSettings(user.id))).settings?.env_keys).toEqual({}); }); }); @@ -226,16 +227,14 @@ describe('listUsers', () => { describe('validateKeys', () => { it('AUTH-DB-015: returns 403 for non-admin', async () => { const { user } = createUser(testDb); - const result = await profile.validateKeys(user.id); + const result = await asLegacyResult(profile.validateKeys(user.id)); expect(result.status).toBe(403); expect(result.error).toMatch(/admin/i); - expect(result.maps).toBe(false); - expect(result.weather).toBe(false); }); it('AUTH-DB-016: returns { maps: false, weather: false } when no API keys are stored', async () => { const { user } = createAdmin(testDb); - const result = await profile.validateKeys(user.id); + const result = await asLegacyResult(profile.validateKeys(user.id)); expect(result.maps).toBe(false); expect(result.weather).toBe(false); expect(result.maps_details).toBeNull(); @@ -251,7 +250,7 @@ describe('validateKeys', () => { text: async () => '', } as Response); - const result = await profile.validateKeys(user.id); + const result = await asLegacyResult(profile.validateKeys(user.id)); expect(result.maps).toBe(true); expect(result.maps_details?.ok).toBe(true); @@ -266,7 +265,7 @@ describe('validateKeys', () => { .spyOn(global, 'fetch') .mockRejectedValueOnce(new Error('Network failure')); - const result = await profile.validateKeys(user.id); + const result = await asLegacyResult(profile.validateKeys(user.id)); expect(result.maps).toBe(false); expect(result.maps_details?.error_status).toBe('FETCH_ERROR'); expect(result.maps_details?.error_message).toBe('Network failure'); @@ -286,7 +285,7 @@ describe('validateKeys', () => { text: async () => '', } as Response); - await profile.validateKeys(user.id); + await asLegacyResult(profile.validateKeys(user.id)); const headers = (fetchSpy.mock.calls[0]?.[1]?.headers ?? {}) as Record; expect(headers.Referer).toBe('https://trek.example.com'); @@ -309,7 +308,7 @@ describe('validateKeys', () => { // key belongs to whoever runs the box, and the panel used to answer // "no maps key" while every search on that same install worked. const { user } = createAdmin(testDb); - const result = await profile.validateKeys(user.id); + const result = await asLegacyResult(profile.validateKeys(user.id)); const headers = (fetchSpy.mock.calls[0]?.[1]?.headers ?? {}) as Record; expect(headers['X-Goog-Api-Key']).toBe('operator-key'); expect(result.maps).toBe(true); @@ -334,7 +333,7 @@ describe('validateKeys', () => { text: async () => '', } as Response); - await profile.validateKeys(user.id); + await asLegacyResult(profile.validateKeys(user.id)); const headers = (fetchSpy.mock.calls[0]?.[1]?.headers ?? {}) as Record; expect(headers['X-Goog-Api-Key']).toBe('live-instance-key'); // And it asks for the fields the real search asks for, so a key restricted @@ -454,9 +453,9 @@ describe('instance-wide API keys', () => { const { user } = createAdmin(testDb); await setUserColumns(user.id, { maps_api_key: 'stale-personal-key' }); await setAppSetting(await sharedTestOrm(testDb), 'maps_api_key', 'live-instance-key'); - expect((await profile.getSettings(user.id)).settings?.maps_api_key).toBe('live-instance-key'); + expect((await asLegacyResult(profile.getSettings(user.id))).settings?.maps_api_key).toBe('live-instance-key'); // openweather is per-user and unaffected. - expect((await profile.getSettings(user.id)).settings?.openweather_api_key).toBeNull(); + expect((await asLegacyResult(profile.getSettings(user.id))).settings?.openweather_api_key).toBeNull(); }); it('AUTH-DB-106: updateMapsKey mirrors for an admin and stays personal for a member', async () => { @@ -470,7 +469,7 @@ describe('instance-wide API keys', () => { it('AUTH-DB-107: updateSettings mirrors the key half without touching name/email handling', async () => { const { user } = createAdmin(testDb); - const result = await profile.updateSettings(user.id, { maps_api_key: 'from-settings-route', username: 'renamed' }); + const result = await asLegacyResult(profile.updateSettings(user.id, { maps_api_key: 'from-settings-route', username: 'renamed' })); expect(result.success).toBe(true); expect(result.user?.username).toBe('renamed'); expect(await instanceRow('maps_api_key')).toBe('from-settings-route'); @@ -508,7 +507,7 @@ describe('changedKeys', () => { expect(result.changedKeys).toEqual([]); expect(await instanceRow('maps_api_key')).toBeUndefined(); expect((await profile.updateMapsKey(user.id, 'operator-owns-this')).changedKeys).toEqual([]); - expect((await profile.updateSettings(user.id, { maps_api_key: 'operator-owns-this' })).changedKeys).toEqual([]); + expect((await asLegacyResult(profile.updateSettings(user.id, { maps_api_key: 'operator-owns-this' }))).changedKeys).toEqual([]); } finally { if (prev === undefined) delete process.env.TREK_MANAGED; else process.env.TREK_MANAGED = prev; @@ -522,7 +521,7 @@ describe('updateSettings loses the race for an email', () => { const { user } = createUser(testDb, { email: 'carl@example.com' }); const check = vi.spyOn(UsersRepository.prototype, 'findIdByEmailCI').mockResolvedValue(null); try { - const result = await profile.updateSettings(user.id, { email: 'ANNA@example.com' }); + const result = await asLegacyResult(profile.updateSettings(user.id, { email: 'ANNA@example.com' })); expect(result).toEqual({ error: 'Email already taken', status: 409 }); } finally { check.mockRestore();