From 45611e2514c4c2957fada8e300f7c5535253ac89 Mon Sep 17 00:00:00 2001 From: Maurice Date: Sun, 9 Aug 2026 21:48:49 +0200 Subject: [PATCH] Separate the profile a user administers from the identity they log in with The second cut into AuthService. Display settings, avatar, the third-party API keys a user pastes in and the member directory are not identity: no password is checked, no session is issued, no MFA secret is touched. UserProfileService takes those eight methods verbatim, with DatabaseService as its only injected dependency. AuthService keeps register, login, JWT, MFA, password reset and account deletion, and is now roughly 900 lines lighter than it started this branch. Not exported from AuthModule: only its own controller needs it, and an export nobody imports is noise. The controller test harness builds AuthController positionally, so the two new parameters landed in optional slots and the stubs for the moved methods moved with them. --- server/src/nest/auth/auth.controller.ts | 19 +- server/src/nest/auth/auth.module.ts | 3 +- server/src/nest/auth/auth.service.ts | 237 ---------------- server/src/nest/auth/user-profile.service.ts | 260 +++++++++++++++++ .../tests/unit/nest/auth.controller.test.ts | 29 +- server/tests/unit/nest/auth.service.test.ts | 205 -------------- .../unit/nest/user-profile.service.test.ts | 266 ++++++++++++++++++ 7 files changed, 553 insertions(+), 466 deletions(-) create mode 100644 server/src/nest/auth/user-profile.service.ts create mode 100644 server/tests/unit/nest/user-profile.service.test.ts diff --git a/server/src/nest/auth/auth.controller.ts b/server/src/nest/auth/auth.controller.ts index d37b572a0..2ab443b23 100644 --- a/server/src/nest/auth/auth.controller.ts +++ b/server/src/nest/auth/auth.controller.ts @@ -24,6 +24,7 @@ import fs from 'fs'; import { v4 as uuid } from 'uuid'; import { AuthService } from './auth.service'; import { TokenService } from '../tokens/token.service'; +import { UserProfileService } from './user-profile.service'; import { avatarDir } from './auth.helpers'; import { ChangePasswordDto, @@ -73,7 +74,7 @@ const AVATAR_UPLOAD = { @Controller('api/auth') @UseGuards(JwtAuthGuard) export class AuthController { - constructor(private readonly auth: AuthService, private readonly tokens: TokenService, private readonly rl: RateLimitService, private readonly audit: AuditService, private readonly env: RuntimeEnvService) {} + constructor(private readonly auth: AuthService, private readonly profile: UserProfileService, private readonly tokens: TokenService, private readonly rl: RateLimitService, private readonly audit: AuditService, private readonly env: RuntimeEnvService) {} private limit(bucket: string, req: Request, max: number): void { if (!this.rl.check(bucket, req.ip || 'unknown', max, WINDOW, Date.now())) { @@ -116,17 +117,17 @@ export class AuthController { @Put('me/maps-key') mapsKey(@CurrentUser() user: User, @Body() body: MapsKeyUpdateDto) { - return this.auth.updateMapsKey(user.id, body.maps_api_key); + return this.profile.updateMapsKey(user.id, body.maps_api_key); } @Put('me/api-keys') apiKeys(@CurrentUser() user: User, @Body() body: ApiKeysUpdateDto) { - return this.auth.updateApiKeys(user.id, body); + return this.profile.updateApiKeys(user.id, body); } @Put('me/settings') updateSettings(@CurrentUser() user: User, @Body() body: SettingsUpdateDto) { - const result = this.auth.updateSettings(user.id, body); + const result = this.profile.updateSettings(user.id, body); if (result.error) { throw new HttpException({ error: result.error }, result.status!); } @@ -135,7 +136,7 @@ export class AuthController { @Get('me/settings') getSettings(@CurrentUser() user: User) { - const result = this.auth.getSettings(user.id); + const result = this.profile.getSettings(user.id); if (result.error) { throw new HttpException({ error: result.error }, result.status!); } @@ -152,22 +153,22 @@ export class AuthController { if (!file) { throw new HttpException({ error: 'No image uploaded' }, 400); } - return this.auth.saveAvatar(user.id, file.filename); + return this.profile.saveAvatar(user.id, file.filename); } @Delete('avatar') async deleteAvatar(@CurrentUser() user: User) { - return this.auth.deleteAvatar(user.id); + return this.profile.deleteAvatar(user.id); } @Get('users') users(@CurrentUser() user: User) { - return { users: this.auth.listUsers(user.id) }; + return { users: this.profile.listUsers(user.id) }; } @Get('validate-keys') async validateKeys(@CurrentUser() user: User) { - const result = await this.auth.validateKeys(user.id); + const result = await this.profile.validateKeys(user.id); if (result.error) { throw new HttpException({ error: result.error }, result.status!); } diff --git a/server/src/nest/auth/auth.module.ts b/server/src/nest/auth/auth.module.ts index 762e43025..f17332f3a 100644 --- a/server/src/nest/auth/auth.module.ts +++ b/server/src/nest/auth/auth.module.ts @@ -5,6 +5,7 @@ import { AuthPublicController } from './auth-public.controller'; import { AuthController } from './auth.controller'; import { PasskeyController } from './passkey.controller'; import { AuthService } from './auth.service'; +import { UserProfileService } from './user-profile.service'; import { PasskeyService } from './passkey.service'; import { AuthMcp } from './auth.mcp'; import { UserCleanupService } from './user-cleanup.service'; @@ -38,7 +39,7 @@ import { TripMembershipModule } from '../trip-membership/trip-membership.module' @Module({ imports: [RateLimitModule, AuditModule, PermissionsModule, TripMembershipModule, MailerModule, AppConfigModule, TokensModule], controllers: [AuthPublicController, AuthController, PasskeyController], - providers: [AuthService, PasskeyService, UserCleanupService, WebauthnConfigService, AuthMcp], + providers: [AuthService, UserProfileService, PasskeyService, UserCleanupService, WebauthnConfigService, AuthMcp], exports: [AuthService, PasskeyService, UserCleanupService], }) export class AuthModule {} diff --git a/server/src/nest/auth/auth.service.ts b/server/src/nest/auth/auth.service.ts index 0e7fa6f57..c89e2ced1 100644 --- a/server/src/nest/auth/auth.service.ts +++ b/server/src/nest/auth/auth.service.ts @@ -572,243 +572,6 @@ export class AuthService implements OnModuleInit, OnModuleDestroy { return { success: true }; } - // ------------------------------------------------------------------------- - // API keys - // ------------------------------------------------------------------------- - - updateMapsKey(userId: number, key: unknown) { - const maps_api_key = key as string | null | undefined; - this.db.run( - 'UPDATE users SET maps_api_key = ?, updated_at = CURRENT_TIMESTAMP WHERE id = ?', - maybe_encrypt_api_key(maps_api_key), userId - ); - return { success: true, maps_api_key: mask_stored_api_key(maps_api_key) }; - } - - updateApiKeys(userId: number, rawBody: unknown) { - const body = rawBody as { maps_api_key?: string; openweather_api_key?: string; unsplash_api_key?: string }; - const current = this.db.get>('SELECT maps_api_key, openweather_api_key, unsplash_api_key FROM users WHERE id = ?', userId); - - // `?? null` instead of the former non-null assertions: a user row deleted - // mid-request must degrade to a 0-row UPDATE, not a TypeError/500. - this.db.run( - 'UPDATE users SET maps_api_key = ?, openweather_api_key = ?, unsplash_api_key = ?, updated_at = CURRENT_TIMESTAMP WHERE id = ?', - body.maps_api_key !== undefined ? maybe_encrypt_api_key(body.maps_api_key) : current?.maps_api_key ?? null, - body.openweather_api_key !== undefined ? maybe_encrypt_api_key(body.openweather_api_key) : current?.openweather_api_key ?? null, - body.unsplash_api_key !== undefined ? maybe_encrypt_api_key(body.unsplash_api_key) : current?.unsplash_api_key ?? null, - userId - ); - - const updated = this.db.get>( - 'SELECT id, username, email, role, maps_api_key, openweather_api_key, unsplash_api_key, avatar, mfa_enabled FROM users WHERE id = ?', - userId - ); - - const u = updated ? { ...updated, mfa_enabled: !!(updated.mfa_enabled === 1 || updated.mfa_enabled === true) } : undefined; - return { - success: true, - user: { ...u, maps_api_key: mask_stored_api_key(u?.maps_api_key), openweather_api_key: mask_stored_api_key(u?.openweather_api_key), unsplash_api_key: mask_stored_api_key(u?.unsplash_api_key), avatar_url: avatarUrl(updated || {}) }, - }; - } - - updateSettings( - userId: number, - rawBody: unknown - ): { error?: string; status?: number; success?: boolean; user?: Record } { - const body = rawBody as { maps_api_key?: string; openweather_api_key?: string; unsplash_api_key?: string; username?: string; email?: string }; - const { maps_api_key, openweather_api_key, unsplash_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 }; - } - if (!/^[a-zA-Z0-9_.-]+$/.test(trimmed)) { - return { error: 'Username can only contain letters, numbers, underscores, dots and hyphens', status: 400 }; - } - const conflict = this.db.get('SELECT id FROM users WHERE LOWER(username) = LOWER(?) AND id != ? AND COALESCE(is_guest, 0) = 0', trimmed, userId); - if (conflict) return { error: 'Username already taken', status: 409 }; - } - - if (email !== undefined) { - const trimmed = email.trim(); - if (!trimmed || !EMAIL_REGEX.test(trimmed)) { - return { error: 'Invalid email format', status: 400 }; - } - const conflict = this.db.get('SELECT id FROM users WHERE LOWER(email) = LOWER(?) AND id != ? AND COALESCE(is_guest, 0) = 0', trimmed, userId); - if (conflict) return { error: 'Email already taken', status: 409 }; - } - - const updates: string[] = []; - const params: (string | number | null)[] = []; - - if (maps_api_key !== undefined) { updates.push('maps_api_key = ?'); params.push(maybe_encrypt_api_key(maps_api_key)); } - if (openweather_api_key !== undefined) { updates.push('openweather_api_key = ?'); params.push(maybe_encrypt_api_key(openweather_api_key)); } - if (unsplash_api_key !== undefined) { updates.push('unsplash_api_key = ?'); params.push(maybe_encrypt_api_key(unsplash_api_key)); } - if (username !== undefined) { updates.push('username = ?'); params.push(username.trim()); } - if (email !== undefined) { updates.push('email = ?'); params.push(email.trim()); } - - if (updates.length > 0) { - updates.push('updated_at = CURRENT_TIMESTAMP'); - params.push(userId); - this.db.run(`UPDATE users SET ${updates.join(', ')} WHERE id = ?`, ...params); - } - - const updated = this.db.get>( - 'SELECT id, username, email, role, maps_api_key, openweather_api_key, unsplash_api_key, avatar, mfa_enabled FROM users WHERE id = ?', - userId - ); - - const u = updated ? { ...updated, mfa_enabled: !!(updated.mfa_enabled === 1 || updated.mfa_enabled === true) } : undefined; - return { - success: true, - user: { ...u, maps_api_key: mask_stored_api_key(u?.maps_api_key), openweather_api_key: mask_stored_api_key(u?.openweather_api_key), unsplash_api_key: mask_stored_api_key(u?.unsplash_api_key), avatar_url: avatarUrl(updated || {}) }, - }; - } - - getSettings(userId: number): { error?: string; status?: number; settings?: Record } { - const user = this.db.get>( - 'SELECT role, maps_api_key, openweather_api_key, unsplash_api_key FROM users WHERE id = ?', - userId - ); - if (user?.role !== 'admin') return { error: 'Admin access required', status: 403 }; - - return { - settings: { - maps_api_key: decrypt_api_key(user.maps_api_key), - openweather_api_key: decrypt_api_key(user.openweather_api_key), - unsplash_api_key: decrypt_api_key(user.unsplash_api_key), - }, - }; - } - - // ------------------------------------------------------------------------- - // Avatar - // ------------------------------------------------------------------------- - - async saveAvatar(userId: number, filename: string) { - const current = this.db.get<{ avatar: string | null }>('SELECT avatar FROM users WHERE id = ?', userId); - // Only a locally uploaded file has something to clean up. An OIDC picture URL - // (#1399) has no file on disk, so skip the rm — path.join on a URL is meaningless. - if (current?.avatar && !/^https:\/\//i.test(current.avatar)) { - // Fire-and-forget: leftover files are harmless; the DB update is - // the source of truth for which avatar is current. - const oldPath = path.join(avatarDir, current.avatar); - await fs.promises.rm(oldPath, { force: true }).catch(() => {}); - } - - this.db.run('UPDATE users SET avatar = ?, updated_at = CURRENT_TIMESTAMP WHERE id = ?', filename, userId); - - const updated = this.db.get>('SELECT id, username, email, role, avatar FROM users WHERE id = ?', userId); - return { success: true, avatar_url: avatarUrl(updated || {}) }; - } - - async deleteAvatar(userId: number) { - const current = this.db.get<{ avatar: string | null }>('SELECT avatar FROM users WHERE id = ?', userId); - // An OIDC picture URL (#1399) has no local file — only rm an uploaded one. - if (current?.avatar && !/^https:\/\//i.test(current.avatar)) { - const filePath = path.join(avatarDir, current.avatar); - await fs.promises.rm(filePath, { force: true }).catch(() => {}); - } - this.db.run('UPDATE users SET avatar = NULL, updated_at = CURRENT_TIMESTAMP WHERE id = ?', userId); - return { success: true }; - } - - // ------------------------------------------------------------------------- - // User directory - // ------------------------------------------------------------------------- - - listUsers(excludeUserId: number) { - // The global user directory feeds the trip member-add / contributor pickers — - // guests (#1362) are trip-scoped and must never be selectable here. - const users = this.db.all>( - 'SELECT id, username, avatar FROM users WHERE id != ? AND COALESCE(is_guest, 0) = 0 ORDER BY username ASC', - excludeUserId - ); - return users.map(u => ({ ...u, avatar_url: avatarUrl(u) })); - } - - // ------------------------------------------------------------------------- - // 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 } }> { - const user = this.db.get>('SELECT role, maps_api_key, openweather_api_key FROM users WHERE id = ?', userId); - if (user?.role !== 'admin') return { error: 'Admin access required', status: 403, maps: false, weather: false, maps_details: null }; - - const result: { - 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; - }; - } = { maps: false, weather: false, maps_details: null }; - - const maps_api_key = decrypt_api_key(user.maps_api_key); - if (maps_api_key) { - try { - const mapsRes = await fetch( - `https://places.googleapis.com/v1/places:searchText`, - { - method: 'POST', - headers: { - 'Content-Type': 'application/json', - 'X-Goog-Api-Key': maps_api_key, - 'X-Goog-FieldMask': 'places.displayName', - }, - body: JSON.stringify({ textQuery: 'test' }), - } - ); - result.maps = mapsRes.status === 200; - let error_text: string | null = null; - let error_json: any = null; - if (!result.maps) { - try { - error_text = await mapsRes.text(); - try { error_json = JSON.parse(error_text); } catch { error_json = null; } - } catch { error_text = null; error_json = null; } - } - result.maps_details = { - ok: result.maps, - status: mapsRes.status, - status_text: mapsRes.statusText || null, - error_message: error_json?.error?.message || null, - error_status: error_json?.error?.status || null, - error_raw: error_text, - }; - } catch (err: unknown) { - result.maps = false; - result.maps_details = { - ok: false, - status: null, - status_text: null, - error_message: err instanceof Error ? err.message : 'Request failed', - error_status: 'FETCH_ERROR', - error_raw: null, - }; - } - } - - const openweather_api_key = decrypt_api_key(user.openweather_api_key); - if (openweather_api_key) { - try { - const weatherRes = await fetch( - `https://api.openweathermap.org/data/2.5/weather?q=London&appid=${openweather_api_key}` - ); - result.weather = weatherRes.status === 200; - } catch { - result.weather = false; - } - } - - return result; - } - // ------------------------------------------------------------------------- // Admin settings // ------------------------------------------------------------------------- diff --git a/server/src/nest/auth/user-profile.service.ts b/server/src/nest/auth/user-profile.service.ts new file mode 100644 index 000000000..fd5247c48 --- /dev/null +++ b/server/src/nest/auth/user-profile.service.ts @@ -0,0 +1,260 @@ +import { Injectable } from '@nestjs/common'; +import path from 'path'; +import fs from 'fs'; +import { DatabaseService } from '../database/database.service'; +import { decrypt_api_key, maybe_encrypt_api_key } from '../common/crypto/apiKeyCrypto'; +import { avatarUrl } from '../common/avatarUrl'; +import { EMAIL_REGEX, avatarDir, mask_stored_api_key } from './auth.helpers'; +import { User } from '../../types'; + +/** + * The account a user administers about themselves: display settings, avatar, + * the third-party API keys they paste in, and the directory listing other + * members are picked from. + * + * Split out of AuthService, which had grown to 1471 lines by owning identity, + * profile, settings and tokens together. None of this is identity: no password + * is checked here, no session is issued, no MFA secret is touched. Everything + * moved verbatim — same SQL, same validation order, same masking, same error + * strings and status codes. + * + * DatabaseService is the only injected dependency, which is what made this the + * second-cheapest cut after tokens. + */ +@Injectable() +export class UserProfileService { + constructor(private readonly db: DatabaseService) {} + + updateMapsKey(userId: number, key: unknown) { + const maps_api_key = key as string | null | undefined; + this.db.run( + 'UPDATE users SET maps_api_key = ?, updated_at = CURRENT_TIMESTAMP WHERE id = ?', + maybe_encrypt_api_key(maps_api_key), userId + ); + return { success: true, maps_api_key: mask_stored_api_key(maps_api_key) }; + } + + updateApiKeys(userId: number, rawBody: unknown) { + const body = rawBody as { maps_api_key?: string; openweather_api_key?: string; unsplash_api_key?: string }; + const current = this.db.get>('SELECT maps_api_key, openweather_api_key, unsplash_api_key FROM users WHERE id = ?', userId); + + // `?? null` instead of the former non-null assertions: a user row deleted + // mid-request must degrade to a 0-row UPDATE, not a TypeError/500. + this.db.run( + 'UPDATE users SET maps_api_key = ?, openweather_api_key = ?, unsplash_api_key = ?, updated_at = CURRENT_TIMESTAMP WHERE id = ?', + body.maps_api_key !== undefined ? maybe_encrypt_api_key(body.maps_api_key) : current?.maps_api_key ?? null, + body.openweather_api_key !== undefined ? maybe_encrypt_api_key(body.openweather_api_key) : current?.openweather_api_key ?? null, + body.unsplash_api_key !== undefined ? maybe_encrypt_api_key(body.unsplash_api_key) : current?.unsplash_api_key ?? null, + userId + ); + + const updated = this.db.get>( + 'SELECT id, username, email, role, maps_api_key, openweather_api_key, unsplash_api_key, avatar, mfa_enabled FROM users WHERE id = ?', + userId + ); + + const u = updated ? { ...updated, mfa_enabled: !!(updated.mfa_enabled === 1 || updated.mfa_enabled === true) } : undefined; + return { + success: true, + user: { ...u, maps_api_key: mask_stored_api_key(u?.maps_api_key), openweather_api_key: mask_stored_api_key(u?.openweather_api_key), unsplash_api_key: mask_stored_api_key(u?.unsplash_api_key), avatar_url: avatarUrl(updated || {}) }, + }; + } + + updateSettings( + userId: number, + rawBody: unknown + ): { error?: string; status?: number; success?: boolean; user?: Record } { + const body = rawBody as { maps_api_key?: string; openweather_api_key?: string; unsplash_api_key?: string; username?: string; email?: string }; + const { maps_api_key, openweather_api_key, unsplash_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 }; + } + if (!/^[a-zA-Z0-9_.-]+$/.test(trimmed)) { + return { error: 'Username can only contain letters, numbers, underscores, dots and hyphens', status: 400 }; + } + const conflict = this.db.get('SELECT id FROM users WHERE LOWER(username) = LOWER(?) AND id != ? AND COALESCE(is_guest, 0) = 0', trimmed, userId); + if (conflict) return { error: 'Username already taken', status: 409 }; + } + + if (email !== undefined) { + const trimmed = email.trim(); + if (!trimmed || !EMAIL_REGEX.test(trimmed)) { + return { error: 'Invalid email format', status: 400 }; + } + const conflict = this.db.get('SELECT id FROM users WHERE LOWER(email) = LOWER(?) AND id != ? AND COALESCE(is_guest, 0) = 0', trimmed, userId); + if (conflict) return { error: 'Email already taken', status: 409 }; + } + + const updates: string[] = []; + const params: (string | number | null)[] = []; + + if (maps_api_key !== undefined) { updates.push('maps_api_key = ?'); params.push(maybe_encrypt_api_key(maps_api_key)); } + if (openweather_api_key !== undefined) { updates.push('openweather_api_key = ?'); params.push(maybe_encrypt_api_key(openweather_api_key)); } + if (unsplash_api_key !== undefined) { updates.push('unsplash_api_key = ?'); params.push(maybe_encrypt_api_key(unsplash_api_key)); } + if (username !== undefined) { updates.push('username = ?'); params.push(username.trim()); } + if (email !== undefined) { updates.push('email = ?'); params.push(email.trim()); } + + if (updates.length > 0) { + updates.push('updated_at = CURRENT_TIMESTAMP'); + params.push(userId); + this.db.run(`UPDATE users SET ${updates.join(', ')} WHERE id = ?`, ...params); + } + + const updated = this.db.get>( + 'SELECT id, username, email, role, maps_api_key, openweather_api_key, unsplash_api_key, avatar, mfa_enabled FROM users WHERE id = ?', + userId + ); + + const u = updated ? { ...updated, mfa_enabled: !!(updated.mfa_enabled === 1 || updated.mfa_enabled === true) } : undefined; + return { + success: true, + user: { ...u, maps_api_key: mask_stored_api_key(u?.maps_api_key), openweather_api_key: mask_stored_api_key(u?.openweather_api_key), unsplash_api_key: mask_stored_api_key(u?.unsplash_api_key), avatar_url: avatarUrl(updated || {}) }, + }; + } + + getSettings(userId: number): { error?: string; status?: number; settings?: Record } { + const user = this.db.get>( + 'SELECT role, maps_api_key, openweather_api_key, unsplash_api_key FROM users WHERE id = ?', + userId + ); + if (user?.role !== 'admin') return { error: 'Admin access required', status: 403 }; + + return { + settings: { + maps_api_key: decrypt_api_key(user.maps_api_key), + openweather_api_key: decrypt_api_key(user.openweather_api_key), + unsplash_api_key: decrypt_api_key(user.unsplash_api_key), + }, + }; + } + + // ------------------------------------------------------------------------- + // Avatar + // ------------------------------------------------------------------------- + + async saveAvatar(userId: number, filename: string) { + const current = this.db.get<{ avatar: string | null }>('SELECT avatar FROM users WHERE id = ?', userId); + // Only a locally uploaded file has something to clean up. An OIDC picture URL + // (#1399) has no file on disk, so skip the rm — path.join on a URL is meaningless. + if (current?.avatar && !/^https:\/\//i.test(current.avatar)) { + // Fire-and-forget: leftover files are harmless; the DB update is + // the source of truth for which avatar is current. + const oldPath = path.join(avatarDir, current.avatar); + await fs.promises.rm(oldPath, { force: true }).catch(() => {}); + } + + this.db.run('UPDATE users SET avatar = ?, updated_at = CURRENT_TIMESTAMP WHERE id = ?', filename, userId); + + const updated = this.db.get>('SELECT id, username, email, role, avatar FROM users WHERE id = ?', userId); + return { success: true, avatar_url: avatarUrl(updated || {}) }; + } + + async deleteAvatar(userId: number) { + const current = this.db.get<{ avatar: string | null }>('SELECT avatar FROM users WHERE id = ?', userId); + // An OIDC picture URL (#1399) has no local file — only rm an uploaded one. + if (current?.avatar && !/^https:\/\//i.test(current.avatar)) { + const filePath = path.join(avatarDir, current.avatar); + await fs.promises.rm(filePath, { force: true }).catch(() => {}); + } + this.db.run('UPDATE users SET avatar = NULL, updated_at = CURRENT_TIMESTAMP WHERE id = ?', userId); + return { success: true }; + } + + // ------------------------------------------------------------------------- + // User directory + // ------------------------------------------------------------------------- + + listUsers(excludeUserId: number) { + // The global user directory feeds the trip member-add / contributor pickers — + // guests (#1362) are trip-scoped and must never be selectable here. + const users = this.db.all>( + 'SELECT id, username, avatar FROM users WHERE id != ? AND COALESCE(is_guest, 0) = 0 ORDER BY username ASC', + excludeUserId + ); + return users.map(u => ({ ...u, avatar_url: avatarUrl(u) })); + } + + // ------------------------------------------------------------------------- + // 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 } }> { + const user = this.db.get>('SELECT role, maps_api_key, openweather_api_key FROM users WHERE id = ?', userId); + if (user?.role !== 'admin') return { error: 'Admin access required', status: 403, maps: false, weather: false, maps_details: null }; + + const result: { + 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; + }; + } = { maps: false, weather: false, maps_details: null }; + + const maps_api_key = decrypt_api_key(user.maps_api_key); + if (maps_api_key) { + try { + const mapsRes = await fetch( + `https://places.googleapis.com/v1/places:searchText`, + { + method: 'POST', + headers: { + 'Content-Type': 'application/json', + 'X-Goog-Api-Key': maps_api_key, + 'X-Goog-FieldMask': 'places.displayName', + }, + body: JSON.stringify({ textQuery: 'test' }), + } + ); + result.maps = mapsRes.status === 200; + let error_text: string | null = null; + let error_json: any = null; + if (!result.maps) { + try { + error_text = await mapsRes.text(); + try { error_json = JSON.parse(error_text); } catch { error_json = null; } + } catch { error_text = null; error_json = null; } + } + result.maps_details = { + ok: result.maps, + status: mapsRes.status, + status_text: mapsRes.statusText || null, + error_message: error_json?.error?.message || null, + error_status: error_json?.error?.status || null, + error_raw: error_text, + }; + } catch (err: unknown) { + result.maps = false; + result.maps_details = { + ok: false, + status: null, + status_text: null, + error_message: err instanceof Error ? err.message : 'Request failed', + error_status: 'FETCH_ERROR', + error_raw: null, + }; + } + } + + const openweather_api_key = decrypt_api_key(user.openweather_api_key); + if (openweather_api_key) { + try { + const weatherRes = await fetch( + `https://api.openweathermap.org/data/2.5/weather?q=London&appid=${openweather_api_key}` + ); + result.weather = weatherRes.status === 200; + } catch { + result.weather = false; + } + } + + return result; + } +} diff --git a/server/tests/unit/nest/auth.controller.test.ts b/server/tests/unit/nest/auth.controller.test.ts index fc0b1c488..4ede80832 100644 --- a/server/tests/unit/nest/auth.controller.test.ts +++ b/server/tests/unit/nest/auth.controller.test.ts @@ -10,6 +10,7 @@ vi.mock('../../../src/nest/common/demo', () => ({ isDemoEmail: vi.fn(() => false import { AuthPublicController } from '../../../src/nest/auth/auth-public.controller'; import { AuthController } from '../../../src/nest/auth/auth.controller'; import type { TokenService } from '../../../src/nest/tokens/token.service'; +import type { UserProfileService } from '../../../src/nest/auth/user-profile.service'; import { RateLimitService } from '../../../src/nest/common/rate-limit.service'; import type { AuthService } from '../../../src/nest/auth/auth.service'; import type { AuditService } from '../../../src/nest/audit/audit.service'; @@ -32,8 +33,8 @@ const audit = { writeAudit } as unknown as AuditService; const apc = (a: AuthService, limiter: RateLimitService) => new AuthPublicController(a, limiter, audit); // Tokens moved to TokenService; the controller takes it second. Stubbed via a // third, optional argument so every non-token call site stays as it was. -const ac = (a: AuthService, limiter: RateLimitService, t: Partial = {}) => - new AuthController(a, t as TokenService, limiter, audit, new RuntimeEnvService()); +const ac = (a: AuthService, limiter: RateLimitService, t: Partial = {}, pr: Partial = {}) => + new AuthController(a, pr as UserProfileService, t as TokenService, limiter, audit, new RuntimeEnvService()); function thrown(fn: () => unknown): { status: number; body: unknown } { try { fn(); } catch (err) { @@ -225,7 +226,7 @@ describe('AuthController (authenticated)', () => { delete process.env.DEMO_MODE; expect(await thrownAsync(() => ac(asvc(), rl()).avatar(user, undefined))).toEqual({ status: 400, body: { error: 'No image uploaded' } }); const saveAvatar = vi.fn().mockResolvedValue({ avatar: '/a.jpg' }); - expect(await ac(asvc({ saveAvatar } as Partial), rl()).avatar(user, { filename: 'a.jpg' } as Express.Multer.File)).toEqual({ avatar: '/a.jpg' }); + expect(await ac(asvc({}), rl(), {}, { saveAvatar }).avatar(user, { filename: 'a.jpg' } as Express.Multer.File)).toEqual({ avatar: '/a.jpg' }); }); it('mfa/setup awaits the QR promise, maps a generation failure to 500', async () => { @@ -273,32 +274,32 @@ describe('AuthController (authenticated)', () => { it('maps-key + api-keys pass straight through to the service', () => { const updateMapsKey = vi.fn().mockReturnValue({ success: true }); - expect(ac(asvc({ updateMapsKey } as Partial), rl()).mapsKey(user, { maps_api_key: 'k' })).toEqual({ success: true }); + expect(ac(asvc({}), rl(), {}, { updateMapsKey }).mapsKey(user, { maps_api_key: 'k' })).toEqual({ success: true }); expect(updateMapsKey).toHaveBeenCalledWith(1, 'k'); const updateApiKeys = vi.fn().mockReturnValue({ ok: 1 }); - expect(ac(asvc({ updateApiKeys } as Partial), rl()).apiKeys(user, { a: 1 })).toEqual({ ok: 1 }); + expect(ac(asvc({}), rl(), {}, { updateApiKeys }).apiKeys(user, { a: 1 })).toEqual({ ok: 1 }); }); it('update-settings + get-settings map errors, else return their payloads', () => { - expect(thrown(() => ac(asvc({ updateSettings: vi.fn().mockReturnValue({ error: 'Bad', status: 400 }) } as Partial), rl()).updateSettings(user, {}))).toEqual({ status: 400, body: { error: 'Bad' } }); - expect(ac(asvc({ updateSettings: vi.fn().mockReturnValue({ success: true, user: { id: 1 } }) } as Partial), rl()).updateSettings(user, {})).toEqual({ success: true, user: { id: 1 } }); - expect(thrown(() => ac(asvc({ getSettings: vi.fn().mockReturnValue({ error: 'Nope', status: 404 }) } as Partial), rl()).getSettings(user))).toEqual({ status: 404, body: { error: 'Nope' } }); - expect(ac(asvc({ getSettings: vi.fn().mockReturnValue({ settings: { theme: 'dark' } }) } as Partial), rl()).getSettings(user)).toEqual({ settings: { theme: 'dark' } }); + expect(thrown(() => ac(asvc({}), rl(), {}, { updateSettings: vi.fn().mockReturnValue({ error: 'Bad', status: 400 }) }).updateSettings(user, {}))).toEqual({ status: 400, body: { error: 'Bad' } }); + expect(ac(asvc({}), rl(), {}, { updateSettings: vi.fn().mockReturnValue({ success: true, user: { id: 1 } }) }).updateSettings(user, {})).toEqual({ success: true, user: { id: 1 } }); + expect(thrown(() => ac(asvc({}), rl(), {}, { getSettings: vi.fn().mockReturnValue({ error: 'Nope', status: 404 }) }).getSettings(user))).toEqual({ status: 404, body: { error: 'Nope' } }); + expect(ac(asvc({}), rl(), {}, { getSettings: vi.fn().mockReturnValue({ settings: { theme: 'dark' } }) }).getSettings(user)).toEqual({ settings: { theme: 'dark' } }); }); // travel-stats left with getTravelStats; it is covered by // atlas.controller.test.ts (TravelStatsController). it('delete-avatar + users delegate to the service', async () => { const deleteAvatar = vi.fn().mockResolvedValue({ removed: true }); - expect(await ac(asvc({ deleteAvatar } as Partial), rl()).deleteAvatar(user)).toEqual({ removed: true }); + expect(await ac(asvc({}), rl(), {}, { deleteAvatar }).deleteAvatar(user)).toEqual({ removed: true }); const listUsers = vi.fn().mockReturnValue([{ id: 1 }]); - expect(ac(asvc({ listUsers } as Partial), rl()).users(user)).toEqual({ users: [{ id: 1 }] }); + expect(ac(asvc({}), rl(), {}, { listUsers }).users(user)).toEqual({ users: [{ id: 1 }] }); expect(listUsers).toHaveBeenCalledWith(1); }); it('validate-keys maps error, else returns the maps/weather payload', async () => { - expect(await thrownAsync(() => ac(asvc({ validateKeys: vi.fn().mockResolvedValue({ error: 'fail', status: 502 }) } as Partial), rl()).validateKeys(user))).toEqual({ status: 502, body: { error: 'fail' } }); - const ok = ac(asvc({ validateKeys: vi.fn().mockResolvedValue({ maps: true, weather: false, maps_details: { ok: 1 } }) } as Partial), rl()); + expect(await thrownAsync(() => ac(asvc({}), rl(), {}, { validateKeys: vi.fn().mockResolvedValue({ error: 'fail', status: 502 }) }).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 } }); }); @@ -339,6 +340,6 @@ describe('AuthController (authenticated)', () => { process.env.DEMO_MODE = 'true'; vi.mocked(isDemoEmail).mockReturnValue(false); const saveAvatar = vi.fn().mockResolvedValue({ avatar: '/b.png' }); - expect(await ac(asvc({ saveAvatar } as Partial), rl()).avatar(user, { filename: 'b.png' } as Express.Multer.File)).toEqual({ avatar: '/b.png' }); + expect(await ac(asvc({}), rl(), {}, { saveAvatar }).avatar(user, { filename: 'b.png' } as Express.Multer.File)).toEqual({ avatar: '/b.png' }); }); }); diff --git a/server/tests/unit/nest/auth.service.test.ts b/server/tests/unit/nest/auth.service.test.ts index 7fdaea004..1bcb3302a 100644 --- a/server/tests/unit/nest/auth.service.test.ts +++ b/server/tests/unit/nest/auth.service.test.ts @@ -149,127 +149,6 @@ describe('requestPasswordReset — OIDC/SSO accounts', () => { }); }); -// --------------------------------------------------------------------------- -// updateSettings -// --------------------------------------------------------------------------- - -describe('updateSettings', () => { - it('AUTH-DB-001: updates username successfully', () => { - const { user } = createUser(testDb); - const result = svc.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)', () => { - const { user } = createUser(testDb); - const result = svc.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)', () => { - const { user } = createUser(testDb); - const result = svc.updateSettings(user.id, { username: 'bad name' }); - expect(result.status).toBe(400); - expect(result.error).toMatch(/only contain/i); - }); - - it('AUTH-DB-004: returns 409 when username is already taken by another user', () => { - const { user: user1 } = createUser(testDb, { username: 'alice' }); - const { user: user2 } = createUser(testDb, { username: 'bob' }); - const result = svc.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', () => { - const { user } = createUser(testDb); - const result = svc.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', () => { - const { user } = createUser(testDb); - const result = svc.updateSettings(user.id, { email: 'not-an-email' }); - expect(result.status).toBe(400); - expect(result.error).toMatch(/invalid email/i); - }); - - it('AUTH-DB-007: returns 409 when email is already taken by another user', () => { - const { user: user1 } = createUser(testDb, { email: 'taken@example.com' }); - const { user: user2 } = createUser(testDb); - const result = svc.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', () => { - const { user } = createUser(testDb); - const result = svc.updateSettings(user.id, {}); - expect(result.success).toBe(true); - }); -}); - -// --------------------------------------------------------------------------- -// getSettings -// --------------------------------------------------------------------------- - -describe('getSettings', () => { - it('AUTH-DB-009: returns 403 for non-admin user', () => { - const { user } = createUser(testDb); - const result = svc.getSettings(user.id); - expect(result.status).toBe(403); - expect(result.error).toMatch(/admin/i); - }); - - it('AUTH-DB-010: returns maps_api_key and openweather_api_key for admin', () => { - const { user } = createAdmin(testDb); - testDb - .prepare('UPDATE users SET maps_api_key = ?, openweather_api_key = ? WHERE id = ?') - .run('maps-key-value', 'weather-key-value', user.id); - const result = svc.getSettings(user.id); - expect(result.status).toBeUndefined(); - expect(result.settings).toBeDefined(); - expect(result.settings).toHaveProperty('maps_api_key'); - expect(result.settings).toHaveProperty('openweather_api_key'); - }); - - it('AUTH-DB-010b: round-trips unsplash_api_key through updateApiKeys — masked to the client, readable via getSettings', () => { - const { user } = createAdmin(testDb); - const result = svc.updateApiKeys(user.id, { unsplash_api_key: 'unsplash-secret-key' }); - // 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(svc.getSettings(user.id).settings?.unsplash_api_key).toBe('unsplash-secret-key'); - }); -}); - -// --------------------------------------------------------------------------- -// listUsers -// --------------------------------------------------------------------------- - -describe('listUsers', () => { - it('AUTH-DB-011: returns all users except self, sorted by username', () => { - const { user: self } = createUser(testDb, { username: 'zzself' }); - createUser(testDb, { username: 'alice' }); - createUser(testDb, { username: 'charlie' }); - createUser(testDb, { username: 'bob' }); - const result = svc.listUsers(self.id); - expect(result).toHaveLength(3); - const names = result.map((u) => u.username); - expect(names).toEqual([...names].sort()); - expect(names).not.toContain('zzself'); - }); - - it('AUTH-DB-012: returns empty array when only one user exists', () => { - const { user } = createUser(testDb); - const result = svc.listUsers(user.id); - expect(result).toHaveLength(0); - }); -}); - // --------------------------------------------------------------------------- // getAppSettings // --------------------------------------------------------------------------- @@ -294,62 +173,6 @@ describe('getAppSettings', () => { }); }); -// --------------------------------------------------------------------------- -// validateKeys -// --------------------------------------------------------------------------- - -describe('validateKeys', () => { - it('AUTH-DB-015: returns 403 for non-admin', async () => { - const { user } = createUser(testDb); - const result = await svc.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 svc.validateKeys(user.id); - expect(result.maps).toBe(false); - expect(result.weather).toBe(false); - expect(result.maps_details).toBeNull(); - }); - - it('AUTH-DB-017: returns { maps: true } when fetch returns 200', async () => { - const { user } = createAdmin(testDb); - testDb.prepare('UPDATE users SET maps_api_key = ? WHERE id = ?').run('test-key', user.id); - - const fetchSpy = vi.spyOn(global, 'fetch').mockResolvedValueOnce({ - status: 200, - statusText: 'OK', - text: async () => '', - } as Response); - - const result = await svc.validateKeys(user.id); - expect(result.maps).toBe(true); - expect(result.maps_details?.ok).toBe(true); - - fetchSpy.mockRestore(); - }); - - it('AUTH-DB-018: returns { maps: false } when fetch throws a network error', async () => { - const { user } = createAdmin(testDb); - testDb.prepare('UPDATE users SET maps_api_key = ? WHERE id = ?').run('test-key', user.id); - - const fetchSpy = vi - .spyOn(global, 'fetch') - .mockRejectedValueOnce(new Error('Network failure')); - - const result = await svc.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'); - - fetchSpy.mockRestore(); - }); -}); - // --------------------------------------------------------------------------- // isOidcOnlyMode // --------------------------------------------------------------------------- @@ -883,29 +706,6 @@ describe('deleteAccount', () => { }); }); -describe('updateMapsKey / avatar', () => { - it('AUTH-DB-068: updateMapsKey stores and answers the masked key', () => { - const { user } = createUser(testDb); - const result = svc.updateMapsKey(user.id, 'maps-key-123456'); - expect(result.success).toBe(true); - expect(result.maps_api_key).toBe('----3456'); - }); - - it('AUTH-DB-069: saveAvatar updates the row and answers the public url', async () => { - const { user } = createUser(testDb); - const result = await svc.saveAvatar(user.id, 'new.png'); - expect(result).toEqual({ success: true, avatar_url: '/uploads/avatars/new.png' }); - }); - - it('AUTH-DB-070: deleteAvatar nulls the column; an OIDC https avatar skips the file rm', async () => { - const { user } = createUser(testDb); - testDb.prepare('UPDATE users SET avatar = ? WHERE id = ?').run('https://idp/pic.jpg', user.id); - expect(await svc.deleteAvatar(user.id)).toEqual({ success: true }); - const row = testDb.prepare('SELECT avatar FROM users WHERE id = ?').get(user.id) as { avatar: string | null }; - expect(row.avatar).toBeNull(); - }); -}); - describe('updateAppSettings', () => { it('AUTH-DB-071: 403 for non-admin', () => { const { user } = createUser(testDb); @@ -1125,11 +925,6 @@ describe('auth quirk fixes', () => { expect(row.last_login).not.toBeNull(); }); - it('AUTH-DB-093: updateApiKeys degrades gracefully when the user row is gone (no TypeError/500)', () => { - expect(() => svc.updateApiKeys(999999, { maps_api_key: 'k' })).not.toThrow(); - const result = svc.updateApiKeys(999999, { openweather_api_key: 'w' }); - expect(result.success).toBe(true); - }); }); // --------------------------------------------------------------------------- diff --git a/server/tests/unit/nest/user-profile.service.test.ts b/server/tests/unit/nest/user-profile.service.test.ts new file mode 100644 index 000000000..45288129f --- /dev/null +++ b/server/tests/unit/nest/user-profile.service.test.ts @@ -0,0 +1,266 @@ +/** + * user-profile.service.test.ts + * + * DB-centric unit tests for UserProfileService against a real in-memory SQLite + * database. The cases moved here with the methods, out of auth.service.test.ts; + * the AUTH-DB-* case IDs are preserved so the history stays greppable. + * Constructed directly (no TestingModule, repo convention). + */ + +// --------------------------------------------------------------------------- +// vi.hoisted: build the real in-memory DB and the module mock before any import +// --------------------------------------------------------------------------- + +const { testDb, dbMock } = vi.hoisted(() => { + const Database = require('better-sqlite3'); + const db = new Database(':memory:'); + db.exec('PRAGMA journal_mode = WAL'); + db.exec('PRAGMA foreign_keys = ON'); + db.exec('PRAGMA busy_timeout = 5000'); + const mock = { db, closeDb: () => {}, reinitialize: () => {}, canAccessTrip: () => undefined, isOwner: () => false }; + return { testDb: db, dbMock: mock }; +}); + +vi.mock('../../../src/db/database', () => dbMock); +vi.mock('../../../src/nest/common/crypto/apiKeyCrypto', () => ({ + decrypt_api_key: vi.fn((v) => v), + maybe_encrypt_api_key: vi.fn((v) => v), + mask_stored_api_key: vi.fn((v: string | null | undefined) => (v ? '••••••••' : null)), + encrypt_api_key: vi.fn((v) => v), +})); + +// --------------------------------------------------------------------------- +// Imports (after mocks) +// --------------------------------------------------------------------------- + +import { describe, it, expect, beforeAll, beforeEach, afterAll, vi } from 'vitest'; +import { createTables } from '../../../src/db/schema'; +import { runMigrations } from '../../../src/db/migrations'; +import { resetTestDb } from '../../helpers/test-db'; +import { createUser, createAdmin } from '../../helpers/factories'; +import { UserProfileService } from '../../../src/nest/auth/user-profile.service'; +import { DatabaseService } from '../../../src/nest/database/database.service'; + +const profile = new UserProfileService(new DatabaseService(testDb)); + +beforeAll(() => { + createTables(testDb); + runMigrations(testDb); +}); + +beforeEach(() => { + resetTestDb(testDb); + vi.clearAllMocks(); +}); + +afterAll(() => { + testDb.close(); +}); + +// --------------------------------------------------------------------------- +// updateSettings +// --------------------------------------------------------------------------- + +describe('updateSettings', () => { + it('AUTH-DB-001: updates username successfully', () => { + const { user } = createUser(testDb); + const result = 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)', () => { + const { user } = createUser(testDb); + const result = 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)', () => { + const { user } = createUser(testDb); + const result = profile.updateSettings(user.id, { username: 'bad name' }); + expect(result.status).toBe(400); + expect(result.error).toMatch(/only contain/i); + }); + + it('AUTH-DB-004: returns 409 when username is already taken by another user', () => { + const { user: user1 } = createUser(testDb, { username: 'alice' }); + const { user: user2 } = createUser(testDb, { username: 'bob' }); + const result = 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', () => { + const { user } = createUser(testDb); + const result = 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', () => { + const { user } = createUser(testDb); + const result = profile.updateSettings(user.id, { email: 'not-an-email' }); + expect(result.status).toBe(400); + expect(result.error).toMatch(/invalid email/i); + }); + + it('AUTH-DB-007: returns 409 when email is already taken by another user', () => { + const { user: user1 } = createUser(testDb, { email: 'taken@example.com' }); + const { user: user2 } = createUser(testDb); + const result = 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', () => { + const { user } = createUser(testDb); + const result = profile.updateSettings(user.id, {}); + expect(result.success).toBe(true); + }); +}); + +// --------------------------------------------------------------------------- +// getSettings +// --------------------------------------------------------------------------- + +describe('getSettings', () => { + it('AUTH-DB-009: returns 403 for non-admin user', () => { + const { user } = createUser(testDb); + const result = profile.getSettings(user.id); + expect(result.status).toBe(403); + expect(result.error).toMatch(/admin/i); + }); + + it('AUTH-DB-010: returns maps_api_key and openweather_api_key for admin', () => { + const { user } = createAdmin(testDb); + testDb + .prepare('UPDATE users SET maps_api_key = ?, openweather_api_key = ? WHERE id = ?') + .run('maps-key-value', 'weather-key-value', user.id); + const result = profile.getSettings(user.id); + expect(result.status).toBeUndefined(); + expect(result.settings).toBeDefined(); + expect(result.settings).toHaveProperty('maps_api_key'); + expect(result.settings).toHaveProperty('openweather_api_key'); + }); + + it('AUTH-DB-010b: round-trips unsplash_api_key through updateApiKeys — masked to the client, readable via getSettings', () => { + const { user } = createAdmin(testDb); + const result = profile.updateApiKeys(user.id, { unsplash_api_key: 'unsplash-secret-key' }); + // 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(profile.getSettings(user.id).settings?.unsplash_api_key).toBe('unsplash-secret-key'); + }); +}); + +// --------------------------------------------------------------------------- +// listUsers +// --------------------------------------------------------------------------- + +describe('listUsers', () => { + it('AUTH-DB-011: returns all users except self, sorted by username', () => { + const { user: self } = createUser(testDb, { username: 'zzself' }); + createUser(testDb, { username: 'alice' }); + createUser(testDb, { username: 'charlie' }); + createUser(testDb, { username: 'bob' }); + const result = profile.listUsers(self.id); + expect(result).toHaveLength(3); + const names = result.map((u) => u.username); + expect(names).toEqual([...names].sort()); + expect(names).not.toContain('zzself'); + }); + + it('AUTH-DB-012: returns empty array when only one user exists', () => { + const { user } = createUser(testDb); + const result = profile.listUsers(user.id); + expect(result).toHaveLength(0); + }); +}); + +// --------------------------------------------------------------------------- +// validateKeys +// --------------------------------------------------------------------------- + +describe('validateKeys', () => { + it('AUTH-DB-015: returns 403 for non-admin', async () => { + const { user } = createUser(testDb); + const result = await 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); + expect(result.maps).toBe(false); + expect(result.weather).toBe(false); + expect(result.maps_details).toBeNull(); + }); + + it('AUTH-DB-017: returns { maps: true } when fetch returns 200', async () => { + const { user } = createAdmin(testDb); + testDb.prepare('UPDATE users SET maps_api_key = ? WHERE id = ?').run('test-key', user.id); + + const fetchSpy = vi.spyOn(global, 'fetch').mockResolvedValueOnce({ + status: 200, + statusText: 'OK', + text: async () => '', + } as Response); + + const result = await profile.validateKeys(user.id); + expect(result.maps).toBe(true); + expect(result.maps_details?.ok).toBe(true); + + fetchSpy.mockRestore(); + }); + + it('AUTH-DB-018: returns { maps: false } when fetch throws a network error', async () => { + const { user } = createAdmin(testDb); + testDb.prepare('UPDATE users SET maps_api_key = ? WHERE id = ?').run('test-key', user.id); + + const fetchSpy = vi + .spyOn(global, 'fetch') + .mockRejectedValueOnce(new Error('Network failure')); + + const result = await 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'); + + fetchSpy.mockRestore(); + }); +}); + +describe('updateMapsKey / avatar', () => { + it('AUTH-DB-068: updateMapsKey stores and answers the masked key', () => { + const { user } = createUser(testDb); + const result = profile.updateMapsKey(user.id, 'maps-key-123456'); + expect(result.success).toBe(true); + expect(result.maps_api_key).toBe('----3456'); + }); + + it('AUTH-DB-069: saveAvatar updates the row and answers the public url', async () => { + const { user } = createUser(testDb); + const result = await profile.saveAvatar(user.id, 'new.png'); + expect(result).toEqual({ success: true, avatar_url: '/uploads/avatars/new.png' }); + }); + + it('AUTH-DB-070: deleteAvatar nulls the column; an OIDC https avatar skips the file rm', async () => { + const { user } = createUser(testDb); + testDb.prepare('UPDATE users SET avatar = ? WHERE id = ?').run('https://idp/pic.jpg', user.id); + expect(await profile.deleteAvatar(user.id)).toEqual({ success: true }); + const row = testDb.prepare('SELECT avatar FROM users WHERE id = ?').get(user.id) as { avatar: string | null }; + expect(row.avatar).toBeNull(); + }); +}); + +describe('profile quirk fixes', () => { + it('AUTH-DB-093: updateApiKeys degrades gracefully when the user row is gone (no TypeError/500)', () => { + expect(() => profile.updateApiKeys(999999, { maps_api_key: 'k' })).not.toThrow(); + const result = profile.updateApiKeys(999999, { openweather_api_key: 'w' }); + expect(result.success).toBe(true); + }); +});