diff --git a/server/src/nest/admin/admin.bridge.ts b/server/src/nest/admin/admin.bridge.ts index 9c0a0f23a..490a20d91 100644 --- a/server/src/nest/admin/admin.bridge.ts +++ b/server/src/nest/admin/admin.bridge.ts @@ -44,14 +44,12 @@ const notifPrefs = new NotificationPreferencesService(dbs, mailer); const auth = new AuthService(dbs, permissions, new AtlasService(dbs), new TripMembershipService(dbs), webauthn, userCleanup, mailer); const admin = new AdminService( dbs, - new SettingsService(dbs), new AddonsService(dbs), new PasskeyService(dbs, auth, webauthn), auth, permissions, new NotificationsService(dbs, realtime, mailer, new WebhookService(dbs), new NtfyService(dbs), notifPrefs), userCleanup, - notifPrefs, ); export function checkAndNotifyVersion(): Promise { diff --git a/server/src/nest/admin/admin.controller.ts b/server/src/nest/admin/admin.controller.ts index 022e01257..270e45609 100644 --- a/server/src/nest/admin/admin.controller.ts +++ b/server/src/nest/admin/admin.controller.ts @@ -115,20 +115,6 @@ export class AdminController { @Get('audit-log') auditLog(@Query() query: { limit?: string; offset?: string }) { return this.admin.getAuditLog(query); } - // ── OIDC ── - @Get('oidc') - getOidc() { return this.admin.getOidcSettings(); } - - @Put('oidc') - updateOidc(@CurrentUser() user: User, @Body() body: AdminOidcUpdateDto, @Req() req: Request) { - const result = this.admin.updateOidcSettings(body); - if (result.error) { - throw new HttpException({ error: result.error }, result.status || 400); - } - this.audit.writeAudit({ userId: user.id, action: 'admin.oidc_update', ip: getClientIp(req), details: { issuer_set: !!body.issuer } }); - return { success: true }; - } - @Post('save-demo-baseline') @HttpCode(200) saveDemoBaseline(@CurrentUser() user: User, @Req() req: Request) { @@ -149,16 +135,6 @@ export class AdminController { @Get('version-check') async versionCheck() { return this.admin.checkVersion(); } - // ── Admin notification preferences ── - @Get('notification-preferences') - getNotificationPrefs(@CurrentUser() user: User) { return this.admin.getPreferencesMatrix(user.id, user.role); } - - @Put('notification-preferences') - setNotificationPrefs(@CurrentUser() user: User, @Body() body: AdminNotificationPreferencesDto) { - this.admin.setAdminPreferences(user.id, body); - return this.admin.getPreferencesMatrix(user.id, user.role); - } - // ── Invites ── @Get('invites') listInvites() { return { invites: this.admin.listInvites() }; } @@ -182,7 +158,6 @@ export class AdminController { return { success: true }; } - // ── Feature toggles ── // ── Feature toggles ── // Straight to AddonsService, which owns every one of these flags. They used to go // through eleven-plus pass-through methods on AdminService, and the three places @@ -300,20 +275,6 @@ export class AdminController { } // ── Default user settings ── - @Get('default-user-settings') - getDefaultUserSettings() { return this.admin.getAdminUserDefaults(); } - - @Put('default-user-settings') - setDefaultUserSettings(@CurrentUser() user: User, @Body() body: AdminDefaultUserSettingsDto, @Req() req: Request) { - try { - this.admin.setAdminUserDefaults(body as unknown as Record); - this.audit.writeAudit({ userId: user.id, action: 'admin.default_user_settings_update', ip: getClientIp(req), details: body as Record }); - return this.admin.getAdminUserDefaults(); - } catch (err) { - throw new HttpException({ error: err instanceof Error ? err.message : String(err) }, 400); - } - } - // ── Dev-only: test notification (404 outside development, mirroring the conditional mount) ── @Post('dev/test-notification') @HttpCode(200) diff --git a/server/src/nest/admin/admin.service.ts b/server/src/nest/admin/admin.service.ts index c91e38c50..8d3e0a0ef 100644 --- a/server/src/nest/admin/admin.service.ts +++ b/server/src/nest/admin/admin.service.ts @@ -21,10 +21,8 @@ import { prepareLlmAddonConfigForWrite, maskLlmAddonConfig } from '../llm-parse/ import { getPhotoProviderConfig } from '../memories/memories.helpers'; import { validatePassword } from '../common/passwordPolicy'; import { UserCleanupService } from '../auth/user-cleanup.service'; -import { NotificationPreferencesService } from '../notifications/notification-preferences.service'; import { DatabaseService } from '../database/database.service'; import { AddonsService } from '../addons/addons.service'; -import { SettingsService } from '../settings/settings.service'; import { PasskeyService } from '../auth/passkey.service'; import { AuthService } from '../auth/auth.service'; import { PermissionsService } from '../permissions/permissions.service'; @@ -69,14 +67,12 @@ const VERSION_FAILURE_TTL = 60_000; export class AdminService { constructor( private readonly db: DatabaseService, - private readonly settings: SettingsService, private readonly addons: AddonsService, private readonly passkeys: PasskeyService, private readonly auth: AuthService, private readonly permissions: PermissionsService, private readonly notifications: NotificationsService, private readonly userCleanup: UserCleanupService, - private readonly notifPrefs: NotificationPreferencesService, ) {} // ── User CRUD ────────────────────────────────────────────────────────────── @@ -318,50 +314,6 @@ export class AdminService { return { entries, total, limit, offset }; } - // ── OIDC Settings ────────────────────────────────────────────────────────── - - getOidcSettings() { - const get = (key: string) => - this.db.get<{ value: string }>('SELECT value FROM app_settings WHERE key = ?', key)?.value || ''; - const secret = decrypt_api_key(get('oidc_client_secret')); - return { - issuer: get('oidc_issuer'), - client_id: get('oidc_client_id'), - client_secret_set: !!secret, - display_name: get('oidc_display_name'), - oidc_only: get('oidc_only') === 'true', - discovery_url: get('oidc_discovery_url'), - }; - } - - updateOidcSettings(data: { - issuer?: string; - client_id?: string; - client_secret?: string; - display_name?: string; - discovery_url?: string; - }): { error?: string; status?: number; success?: boolean } { - // Lockout prevention: can't remove OIDC config when password login is disabled - if ((data.issuer === '' || data.client_id === '') && !this.auth.resolveAuthToggles().password_login) { - return { - error: 'Cannot remove SSO configuration while password login is disabled. Enable password login first.', - status: 400, - }; - } - - const set = (key: string, val: string) => - this.db.run('INSERT OR REPLACE INTO app_settings (key, value) VALUES (?, ?)', key, val || ''); - // All five writes are one SSO config — a partial apply would leave the - // instance with an issuer but no client id (or vice versa). - this.db.transaction(() => { - set('oidc_issuer', data.issuer ?? ''); - set('oidc_client_id', data.client_id ?? ''); - if (data.client_secret !== undefined) set('oidc_client_secret', maybe_encrypt_api_key(data.client_secret) ?? ''); - set('oidc_display_name', data.display_name ?? ''); - set('oidc_discovery_url', data.discovery_url ?? ''); - }); - return { success: true }; - } // ── Demo Baseline ────────────────────────────────────────────────────────── @@ -790,8 +742,4 @@ export class AdminService { // ── Settings + notification preference helpers (non-admin-service modules) ── - getAdminUserDefaults() { return this.settings.getAdminUserDefaults(); } - setAdminUserDefaults(body: Record) { return this.settings.setAdminUserDefaults(body); } - getPreferencesMatrix(userId: number, role: string) { return this.notifPrefs.getPreferencesMatrix(userId, role, 'admin'); } - setAdminPreferences(userId: number, body: unknown) { return this.notifPrefs.setAdminPreferences(userId, body as Parameters[1]); } } diff --git a/server/src/nest/notifications/notifications.controller.ts b/server/src/nest/notifications/notifications.controller.ts index 63a48790e..4c5942dac 100644 --- a/server/src/nest/notifications/notifications.controller.ts +++ b/server/src/nest/notifications/notifications.controller.ts @@ -22,6 +22,9 @@ import { NotificationRespondDto, } from './notifications.dto'; import { JwtAuthGuard } from '../auth/jwt-auth.guard'; +import { AdminGuard } from '../auth/admin.guard'; +import { NotificationPreferencesService } from './notification-preferences.service'; +import { AdminNotificationPreferencesDto } from '../admin/admin.dto'; import { CurrentUser } from '../auth/current-user.decorator'; // The masked placeholder the client sends instead of a stored secret (8× U+2022). @@ -199,3 +202,30 @@ export class NotificationsController { return id; } } + +/** + * /api/admin/notification-preferences — the admin-scope row of the preference matrix. + * + * Two routes that sat on AdminController and reached NotificationPreferencesService + * through a pair of pass-through methods on AdminService. The owner is here; the + * 'admin' scope argument they always passed is now written once, at the only place + * that uses it. + */ +@Controller('api/admin/notification-preferences') +@UseGuards(JwtAuthGuard, AdminGuard) +export class AdminNotificationPreferencesController { + constructor(private readonly prefs: NotificationPreferencesService) {} + + @Get() + get(@CurrentUser() user: User) { + return this.prefs.getPreferencesMatrix(user.id, user.role, 'admin'); + } + + @Put() + set(@CurrentUser() user: User, @Body() body: AdminNotificationPreferencesDto) { + this.prefs.setAdminPreferences(user.id, body); + // Answer with the refreshed matrix rather than the raw write result — the admin + // panel renders straight from this response. + return this.prefs.getPreferencesMatrix(user.id, user.role, 'admin'); + } +} diff --git a/server/src/nest/notifications/notifications.module.ts b/server/src/nest/notifications/notifications.module.ts index 4dffa21ed..a6b6e00fb 100644 --- a/server/src/nest/notifications/notifications.module.ts +++ b/server/src/nest/notifications/notifications.module.ts @@ -1,5 +1,5 @@ import { Module } from '@nestjs/common'; -import { NotificationsController } from './notifications.controller'; +import { AdminNotificationPreferencesController, NotificationsController } from './notifications.controller'; import { NotificationsMcp } from './notifications.mcp'; import { NotificationsService } from './notifications.service'; import { NotificationPreferencesService } from './notification-preferences.service'; @@ -17,7 +17,7 @@ import { AuthModule } from '../auth/auth.module'; * the plugin RPC surface, HostSurfaceRpc). */ @Module({ imports: [AuthModule, MailerModule], - controllers: [NotificationsController], + controllers: [NotificationsController, AdminNotificationPreferencesController], providers: [NotificationsService, NotificationPreferencesService, WebhookService, NtfyService, NotificationsMcp], exports: [NotificationsService, NotificationPreferencesService], }) diff --git a/server/src/nest/oidc/oidc.controller.ts b/server/src/nest/oidc/oidc.controller.ts index bae630917..0d9dd09f0 100644 --- a/server/src/nest/oidc/oidc.controller.ts +++ b/server/src/nest/oidc/oidc.controller.ts @@ -1,8 +1,15 @@ -import { Controller, Get, Query, Req, Res } from '@nestjs/common'; +import { Body, Controller, Get, HttpException, Put, Query, Req, Res, UseGuards } from '@nestjs/common'; import type { Request, Response } from 'express'; import { readEnv } from '../../app-config'; import { OidcService, OIDC_STATE_TTL_MS } from './oidc.service'; import { cookieOptions } from '../common/cookie'; +import { AdminGuard } from '../auth/admin.guard'; +import { JwtAuthGuard } from '../auth/jwt-auth.guard'; +import { CurrentUser } from '../auth/current-user.decorator'; +import type { User } from '../../types'; +import { AuditService } from '../audit/audit.service'; +import { getClientIp } from '../audit/client-ip'; +import { AdminOidcUpdateDto } from '../admin/admin.dto'; const OIDC_STATE_COOKIE = 'trek_oidc_state'; @@ -165,3 +172,44 @@ export class OidcController { res.json({ token: result.token }); } } + +/** + * /api/admin/oidc — the SSO configuration. + * + * Two routes that sat on AdminController with their SQL in AdminService, for a domain + * that already had a module. They read and write the same five app_settings keys the + * login flow next door consumes, so they belong here; the lockout guard that refuses + * to remove the config while password login is off lives with them in OidcService. + * + * The path, the {error,status} envelope and the audit action are unchanged. + */ +@Controller('api/admin/oidc') +@UseGuards(JwtAuthGuard, AdminGuard) +export class AdminOidcController { + constructor( + private readonly oidc: OidcService, + private readonly audit: AuditService, + ) {} + + @Get() + get() { + return this.oidc.getOidcSettings(); + } + + @Put() + update(@CurrentUser() user: User, @Body() body: AdminOidcUpdateDto, @Req() req: Request) { + const result = this.oidc.updateOidcSettings(body); + if (result.error) { + throw new HttpException({ error: result.error }, result.status || 400); + } + // Only whether an issuer was set, never the value: the details column is readable + // by every admin and the issuer identifies the customer's IdP tenant. + this.audit.writeAudit({ + userId: user.id, + action: 'admin.oidc_update', + ip: getClientIp(req), + details: { issuer_set: !!body.issuer }, + }); + return { success: true }; + } +} diff --git a/server/src/nest/oidc/oidc.module.ts b/server/src/nest/oidc/oidc.module.ts index ee939bf06..e8d28fa2b 100644 --- a/server/src/nest/oidc/oidc.module.ts +++ b/server/src/nest/oidc/oidc.module.ts @@ -1,12 +1,13 @@ import { Module } from '@nestjs/common'; -import { OidcController } from './oidc.controller'; +import { AdminOidcController, OidcController } from './oidc.controller'; +import { AuditModule } from '../audit/audit.module'; import { OidcService } from './oidc.service'; import { AuthModule } from '../auth/auth.module'; import { TripMembershipModule } from '../trip-membership/trip-membership.module'; @Module({ - imports: [AuthModule, TripMembershipModule], - controllers: [OidcController], + imports: [AuthModule, TripMembershipModule, AuditModule], + controllers: [OidcController, AdminOidcController], providers: [OidcService], }) export class OidcModule {} diff --git a/server/src/nest/oidc/oidc.service.ts b/server/src/nest/oidc/oidc.service.ts index 0fbf606e1..b1f0d4550 100644 --- a/server/src/nest/oidc/oidc.service.ts +++ b/server/src/nest/oidc/oidc.service.ts @@ -8,7 +8,7 @@ import type { Request, Response } from 'express'; import { readEnv, getAppUrl } from '../../app-config'; import { JWT_SECRET, SESSION_DURATION_SECONDS } from '../../config'; import { User } from '../../types'; -import { decrypt_api_key } from '../common/crypto/apiKeyCrypto'; +import { decrypt_api_key, maybe_encrypt_api_key } from '../common/crypto/apiKeyCrypto'; import { TripMembershipService } from '../trip-membership/trip-membership.service'; import { setAuthCookie } from '../common/cookie'; import { AuthService } from '../auth/auth.service'; @@ -620,4 +620,53 @@ export class OidcService implements OnModuleDestroy { touchLastLogin(userId: number): void { this.db.prepare('UPDATE users SET last_login = CURRENT_TIMESTAMP, login_count = login_count + 1 WHERE id = ?').run(userId); } + + // ── OIDC settings ────────────────────────────────────────────────────────── + // Moved here from AdminService, which held the SQL for a domain that already had a + // module of its own. The lockout guard below is the reason this cannot be a plain + // settings write: removing the SSO config while password login is off would lock + // every user out of the instance. + + getOidcSettings() { + const get = (key: string) => + this.db.get<{ value: string }>('SELECT value FROM app_settings WHERE key = ?', key)?.value || ''; + const secret = decrypt_api_key(get('oidc_client_secret')); + return { + issuer: get('oidc_issuer'), + client_id: get('oidc_client_id'), + client_secret_set: !!secret, + display_name: get('oidc_display_name'), + oidc_only: get('oidc_only') === 'true', + discovery_url: get('oidc_discovery_url'), + }; + } + + updateOidcSettings(data: { + issuer?: string; + client_id?: string; + client_secret?: string; + display_name?: string; + discovery_url?: string; + }): { error?: string; status?: number; success?: boolean } { + // Lockout prevention: can't remove OIDC config when password login is disabled + if ((data.issuer === '' || data.client_id === '') && !this.auth.resolveAuthToggles().password_login) { + return { + error: 'Cannot remove SSO configuration while password login is disabled. Enable password login first.', + status: 400, + }; + } + + const set = (key: string, val: string) => + this.db.run('INSERT OR REPLACE INTO app_settings (key, value) VALUES (?, ?)', key, val || ''); + // All five writes are one SSO config — a partial apply would leave the + // instance with an issuer but no client id (or vice versa). + this.db.transaction(() => { + set('oidc_issuer', data.issuer ?? ''); + set('oidc_client_id', data.client_id ?? ''); + if (data.client_secret !== undefined) set('oidc_client_secret', maybe_encrypt_api_key(data.client_secret) ?? ''); + set('oidc_display_name', data.display_name ?? ''); + set('oidc_discovery_url', data.discovery_url ?? ''); + }); + return { success: true }; + } } diff --git a/server/src/nest/settings/settings.controller.ts b/server/src/nest/settings/settings.controller.ts index 79e6fe537..13c657af8 100644 --- a/server/src/nest/settings/settings.controller.ts +++ b/server/src/nest/settings/settings.controller.ts @@ -1,9 +1,14 @@ -import { Body, Controller, Get, HttpCode, HttpException, Post, Put, UseGuards } from '@nestjs/common'; +import { Body, Controller, Get, HttpCode, HttpException, Post, Put, Req, UseGuards } from '@nestjs/common'; +import type { Request } from 'express'; import { MASKED_SETTING_VALUE } from '@trek/shared'; import type { User } from '../../types'; import { SettingsService } from './settings.service'; import { SettingUpsertDto, SettingsBulkDto } from './settings.dto'; +import { AdminDefaultUserSettingsDto } from '../admin/admin.dto'; import { JwtAuthGuard } from '../auth/jwt-auth.guard'; +import { AdminGuard } from '../auth/admin.guard'; +import { AuditService } from '../audit/audit.service'; +import { getClientIp } from '../audit/client-ip'; import { CurrentUser } from '../auth/current-user.decorator'; /** @@ -46,3 +51,42 @@ export class SettingsController { } } } + +/** + * /api/admin/default-user-settings — the defaults a new account starts with. + * + * These two routes sat on AdminController and reached SettingsService through a pair + * of pass-through methods on AdminService that did nothing but forward. The owner is + * here, so the routes are too; the path and both response shapes are unchanged. + */ +@Controller('api/admin/default-user-settings') +@UseGuards(JwtAuthGuard, AdminGuard) +export class AdminDefaultUserSettingsController { + constructor( + private readonly settings: SettingsService, + private readonly audit: AuditService, + ) {} + + @Get() + get() { + return this.settings.getAdminUserDefaults(); + } + + @Put() + update(@CurrentUser() user: User, @Body() body: AdminDefaultUserSettingsDto, @Req() req: Request) { + try { + this.settings.setAdminUserDefaults(body as unknown as Record); + this.audit.writeAudit({ + userId: user.id, + action: 'admin.default_user_settings_update', + ip: getClientIp(req), + details: body as Record, + }); + // Answer with the stored defaults, not the request body: the service normalises + // and drops unknown keys, and the admin panel renders straight from this. + return this.settings.getAdminUserDefaults(); + } catch (err) { + throw new HttpException({ error: err instanceof Error ? err.message : String(err) }, 400); + } + } +} diff --git a/server/src/nest/settings/settings.module.ts b/server/src/nest/settings/settings.module.ts index bb3e0c688..ce4a92e9e 100644 --- a/server/src/nest/settings/settings.module.ts +++ b/server/src/nest/settings/settings.module.ts @@ -1,10 +1,14 @@ import { Module } from '@nestjs/common'; -import { SettingsController } from './settings.controller'; +import { AdminDefaultUserSettingsController, SettingsController } from './settings.controller'; +import { AuthModule } from '../auth/auth.module'; +import { AuditModule } from '../audit/audit.module'; import { SettingsService } from './settings.service'; /** Exports SettingsService for in-container consumers (admin, share, llm-parse). */ @Module({ - controllers: [SettingsController], + // AuthModule for the admin gate on the defaults routes. + imports: [AuthModule, AuditModule], + controllers: [SettingsController, AdminDefaultUserSettingsController], providers: [SettingsService], exports: [SettingsService], }) diff --git a/server/tests/e2e/admin.e2e.test.ts b/server/tests/e2e/admin.e2e.test.ts index ffd90cf1f..f1c9d6fde 100644 --- a/server/tests/e2e/admin.e2e.test.ts +++ b/server/tests/e2e/admin.e2e.test.ts @@ -85,6 +85,12 @@ vi.mock('../../src/nest/notifications/notification-preferences.service', async ( vi.mock('../../src/nest/notifications/notifications.bridge', () => ({ send: vi.fn().mockResolvedValue(undefined) })); import { AdminModule } from '../../src/nest/admin/admin.module'; +// The admin surface is no longer one module: oidc, the account defaults and the admin +// preference matrix moved to the domains that own them, so the app has to assemble +// them too or those routes 404 here while working in production. +import { OidcModule } from '../../src/nest/oidc/oidc.module'; +import { SettingsModule } from '../../src/nest/settings/settings.module'; +import { NotificationsModule } from '../../src/nest/notifications/notifications.module'; import { TrekExceptionFilter } from '../../src/nest/common/trek-exception.filter'; import { ZodValidationPipe } from '../../src/nest/common/zod-validation.pipe'; @@ -93,7 +99,7 @@ describe('Admin e2e (real auth + admin guard + temp SQLite)', () => { let app: Awaited>; async function build() { - const moduleRef = await Test.createTestingModule({ imports: [DatabaseModule, RealtimeModule, AdminModule] }).compile(); + const moduleRef = await Test.createTestingModule({ imports: [DatabaseModule, RealtimeModule, AdminModule, OidcModule, SettingsModule, NotificationsModule] }).compile(); const nest = moduleRef.createNestApplication(); nest.use(cookieParser()); // Mirror the production APP_PIPE (app.module.ts): DTO-typed bodies validate diff --git a/server/tests/unit/nest/admin-default-user-settings.controller.test.ts b/server/tests/unit/nest/admin-default-user-settings.controller.test.ts new file mode 100644 index 000000000..36afba007 --- /dev/null +++ b/server/tests/unit/nest/admin-default-user-settings.controller.test.ts @@ -0,0 +1,82 @@ +/** + * The account defaults, after the two routes moved off AdminController next to + * SettingsService, which owns them. Same path, same audit action, same 400 envelope on + * a rejected write — these cases came over with the routes. + */ +import { describe, it, expect, vi, beforeEach } from 'vitest'; +import { HttpException } from '@nestjs/common'; +import { AdminDefaultUserSettingsController } from '../../../src/nest/settings/settings.controller'; +import { SettingsModule } from '../../../src/nest/settings/settings.module'; +import type { SettingsService } from '../../../src/nest/settings/settings.service'; +import type { AuditService } from '../../../src/nest/audit/audit.service'; +import type { User } from '../../../src/types'; +import { expectRegisteredController } from '../../helpers/module-providers'; + +const user = { id: 1, role: 'admin' } as User; +const req = { headers: {}, socket: {} } as never; +const writeAudit = vi.fn(); + +function controller(over: Partial = {}) { + const settings = { + getAdminUserDefaults: vi.fn(() => ({ theme: 'dark' })), + setAdminUserDefaults: vi.fn(), + ...over, + } as unknown as SettingsService; + return { c: new AdminDefaultUserSettingsController(settings, { writeAudit } as unknown as AuditService), settings }; +} + +const thrown = (run: () => unknown) => { + try { + run(); + return null; + } catch (e) { + return e instanceof HttpException ? { status: e.getStatus(), body: e.getResponse() } : e; + } +}; + +describe('AdminDefaultUserSettingsController', () => { + beforeEach(() => { + vi.clearAllMocks(); + }); + + it('DEFAULTS-001 GET returns the stored defaults verbatim', () => { + expect(controller().c.get()).toEqual({ theme: 'dark' }); + }); + + it('DEFAULTS-002 PUT writes, audits, and answers with the STORED defaults', () => { + const { c, settings } = controller(); + // Not the request body: the service normalises and drops unknown keys, and the + // admin panel renders straight from this response. + expect(c.update(user, { theme: 'light' } as never, req)).toEqual({ theme: 'dark' }); + expect(settings.setAdminUserDefaults).toHaveBeenCalledWith({ theme: 'light' }); + expect(writeAudit).toHaveBeenCalledWith( + expect.objectContaining({ userId: 1, action: 'admin.default_user_settings_update', details: { theme: 'light' } }), + ); + }); + + it('DEFAULTS-003 a rejected write is a 400 carrying the message, and is not audited', () => { + const { c } = controller({ + setAdminUserDefaults: vi.fn(() => { + throw new Error('unknown setting: nope'); + }), + } as Partial); + expect(thrown(() => c.update(user, { nope: 1 } as never, req))).toEqual({ + status: 400, + body: { error: 'unknown setting: nope' }, + }); + expect(writeAudit).not.toHaveBeenCalled(); + }); + + it('DEFAULTS-004 a non-Error throw is stringified rather than swallowed', () => { + const { c } = controller({ + setAdminUserDefaults: vi.fn(() => { + throw 'plain string'; + }), + } as Partial); + expect(thrown(() => c.update(user, {} as never, req))).toEqual({ status: 400, body: { error: 'plain string' } }); + }); + + it('DEFAULTS-005 the class is listed in its module controllers', () => { + expectRegisteredController(SettingsModule, AdminDefaultUserSettingsController); + }); +}); diff --git a/server/tests/unit/nest/admin-notification-preferences.controller.test.ts b/server/tests/unit/nest/admin-notification-preferences.controller.test.ts new file mode 100644 index 000000000..c7b78a713 --- /dev/null +++ b/server/tests/unit/nest/admin-notification-preferences.controller.test.ts @@ -0,0 +1,52 @@ +/** + * The admin-scope preference matrix, after the two routes moved off AdminController + * next to NotificationPreferencesService, which owns it. The 'admin' scope argument + * they always passed is now written once, at the only place that uses it. + */ +import { describe, it, expect, vi, beforeEach } from 'vitest'; +import { AdminNotificationPreferencesController } from '../../../src/nest/notifications/notifications.controller'; +import { NotificationsModule } from '../../../src/nest/notifications/notifications.module'; +import type { NotificationPreferencesService } from '../../../src/nest/notifications/notification-preferences.service'; +import type { User } from '../../../src/types'; +import { expectRegisteredController } from '../../helpers/module-providers'; + +const admin = { id: 1, role: 'admin' } as User; + +function controller() { + const prefs = { + getPreferencesMatrix: vi.fn(() => ({ rows: [{ event: 'trip_reminder' }] })), + setAdminPreferences: vi.fn(), + } as unknown as NotificationPreferencesService; + return { c: new AdminNotificationPreferencesController(prefs), prefs }; +} + +describe('AdminNotificationPreferencesController', () => { + beforeEach(() => { + vi.clearAllMocks(); + }); + + it('ADMINPREF-001 GET asks for the admin scope, not the user one', () => { + const { c, prefs } = controller(); + expect(c.get(admin)).toEqual({ rows: [{ event: 'trip_reminder' }] }); + expect(prefs.getPreferencesMatrix).toHaveBeenCalledWith(1, 'admin', 'admin'); + }); + + it('ADMINPREF-002 PUT persists, then answers with the REFRESHED matrix', () => { + const { c, prefs } = controller(); + // The admin panel renders straight from this response, so returning the write + // result instead of a fresh read would leave it showing stale rows. + expect(c.set(admin, { trip_reminder: { email: true } } as never)).toEqual({ rows: [{ event: 'trip_reminder' }] }); + expect(prefs.setAdminPreferences).toHaveBeenCalledWith(1, { trip_reminder: { email: true } }); + expect(prefs.getPreferencesMatrix).toHaveBeenCalledWith(1, 'admin', 'admin'); + }); + + it('ADMINPREF-003 the acting user drives the lookup, never a body field', () => { + const { c, prefs } = controller(); + c.get({ id: 7, role: 'admin' } as User); + expect(prefs.getPreferencesMatrix).toHaveBeenCalledWith(7, 'admin', 'admin'); + }); + + it('ADMINPREF-004 the class is listed in its module controllers', () => { + expectRegisteredController(NotificationsModule, AdminNotificationPreferencesController); + }); +}); diff --git a/server/tests/unit/nest/admin.controller.test.ts b/server/tests/unit/nest/admin.controller.test.ts index 00c84284c..44fc48ef5 100644 --- a/server/tests/unit/nest/admin.controller.test.ts +++ b/server/tests/unit/nest/admin.controller.test.ts @@ -84,10 +84,6 @@ describe('AdminController permissions + oidc + misc', () => { expect(c.savePermissions(user, { permissions: {} }, req)).toEqual({ success: true, permissions: {}, skipped: ['bad'] }); }); - it('oidc update maps error, else audits', () => { - expect(thrown(() => adminCtl(svc({ updateOidcSettings: vi.fn().mockReturnValue({ error: 'bad issuer', status: 400 }) } as Partial)).updateOidc(user, {}, req))).toEqual({ status: 400, body: { error: 'bad issuer' } }); - expect(adminCtl(svc({ updateOidcSettings: vi.fn().mockReturnValue({}) } as Partial)).updateOidc(user, { issuer: 'https://idp' }, req)).toEqual({ success: true }); - }); it('save-demo-baseline maps error, else returns message', () => { expect(thrown(() => adminCtl(svc({ saveDemoBaseline: vi.fn().mockReturnValue({ error: 'not demo', status: 400 }) } as Partial)).saveDemoBaseline(user, req))).toEqual({ status: 400, body: { error: 'not demo' } }); @@ -138,12 +134,6 @@ describe('AdminController addons + sessions + jwt + defaults', () => { expect(adminCtl(svc({ rotateJwtSecret: vi.fn().mockReturnValue({}) } as Partial)).rotateJwtSecret(user, req)).toEqual({ success: true }); }); - it('default-user-settings: sets + audits', () => { - const setAdminUserDefaults = vi.fn(); - const c = adminCtl(svc({ setAdminUserDefaults, getAdminUserDefaults: vi.fn().mockReturnValue({ theme: 'dark' }) } as Partial)); - expect(c.setDefaultUserSettings(user, { theme: 'dark' }, req)).toEqual({ theme: 'dark' }); - expect(setAdminUserDefaults).toHaveBeenCalled(); - }); }); describe('AdminController error envelope fallbacks', () => { @@ -151,14 +141,7 @@ describe('AdminController error envelope fallbacks', () => { expect(thrown(() => adminCtl(svc({ createUser: vi.fn().mockReturnValue({ error: 'boom' }) } as Partial)).createUser(user, {}, req))).toEqual({ status: 400, body: { error: 'boom' } }); }); - it('updateOidc defaults to 400 when the service error omits a status', () => { - expect(thrown(() => adminCtl(svc({ updateOidcSettings: vi.fn().mockReturnValue({ error: 'nope' }) } as Partial)).updateOidc(user, {}, req))).toEqual({ status: 400, body: { error: 'nope' } }); - }); - it('updateOidc audits issuer_set=false when no issuer is supplied', () => { - expect(adminCtl(svc({ updateOidcSettings: vi.fn().mockReturnValue({}) } as Partial)).updateOidc(user, {}, req)).toEqual({ success: true }); - expect(writeAudit).toHaveBeenCalledWith(expect.objectContaining({ action: 'admin.oidc_update', details: { issuer_set: false } })); - }); }); describe('AdminController read-only getters', () => { @@ -167,22 +150,13 @@ describe('AdminController read-only getters', () => { expect(adminCtl(svc({ getStats: vi.fn().mockReturnValue({ users: 3 }) } as Partial)).stats()).toEqual({ users: 3 }); expect(adminCtl(svc({ getPermissions: vi.fn().mockReturnValue({ a: 1 }) } as Partial)).permissions()).toEqual({ a: 1 }); expect(adminCtl(svc({ getAuditLog: vi.fn().mockReturnValue({ entries: [] }) } as Partial)).auditLog({})).toEqual({ entries: [] }); - expect(adminCtl(svc({ getOidcSettings: vi.fn().mockReturnValue({ issuer: 'x' }) } as Partial)).getOidc()).toEqual({ issuer: 'x' }); expect(adminCtl(svc({ checkVersion: vi.fn().mockResolvedValue({ current: '1' }) } as Partial)).versionCheck()).resolves.toEqual({ current: '1' }); - expect(adminCtl(svc({ getPreferencesMatrix: vi.fn().mockReturnValue({ rows: [] }) } as Partial)).getNotificationPrefs(user)).toEqual({ rows: [] }); expect(adminCtl(svc({ listInvites: vi.fn().mockReturnValue([{ id: 1 }]) } as Partial)).listInvites()).toEqual({ invites: [{ id: 1 }] }); expect(adminCtl(svc({ listAddons: vi.fn().mockReturnValue([{ id: 'mcp' }]) } as Partial)).listAddons()).toEqual({ addons: [{ id: 'mcp' }] }); expect(adminCtl(svc({ listMcpTokens: vi.fn().mockReturnValue([{ id: 1 }]) } as Partial)).listMcpTokens()).toEqual({ tokens: [{ id: 1 }] }); expect(adminCtl(svc({ listOAuthSessions: vi.fn().mockReturnValue([{ id: 1 }]) } as Partial)).listOAuthSessions()).toEqual({ sessions: [{ id: 1 }] }); - expect(adminCtl(svc({ getAdminUserDefaults: vi.fn().mockReturnValue({ theme: 'dark' }) } as Partial)).getDefaultUserSettings()).toEqual({ theme: 'dark' }); }); - it('setNotificationPrefs persists then returns the refreshed matrix', () => { - const setAdminPreferences = vi.fn(); - const c = adminCtl(svc({ setAdminPreferences, getPreferencesMatrix: vi.fn().mockReturnValue({ rows: [1] }) } as Partial)); - expect(c.setNotificationPrefs(user, { x: 1 })).toEqual({ rows: [1] }); - expect(setAdminPreferences).toHaveBeenCalledWith(user.id, { x: 1 }); - }); it('githubReleases falls back to default paging when no query is given', async () => { const getGithubReleases = vi.fn().mockResolvedValue([{ tag: 'v1' }]); @@ -204,18 +178,6 @@ describe('AdminController tokens + sessions', () => { }); }); -describe('AdminController default-user-settings error path', () => { - it('400 with an Error message when setAdminUserDefaults throws an Error', () => { - const c = adminCtl(svc({ setAdminUserDefaults: vi.fn(() => { throw new Error('bad default'); }) } as Partial)); - expect(thrown(() => c.setDefaultUserSettings(user, { theme: 'x' }, req))).toEqual({ status: 400, body: { error: 'bad default' } }); - }); - - it('400 stringifies a non-Error throw', () => { - const c = adminCtl(svc({ setAdminUserDefaults: vi.fn(() => { throw 'plain string'; }) } as Partial)); - expect(thrown(() => c.setDefaultUserSettings(user, { theme: 'x' }, req))).toEqual({ status: 400, body: { error: 'plain string' } }); - }); - -}); describe('AdminController dev test-notification', () => { it('404 outside development', async () => { diff --git a/server/tests/unit/nest/admin.service.test.ts b/server/tests/unit/nest/admin.service.test.ts index 09fc4803f..0d3edd483 100644 --- a/server/tests/unit/nest/admin.service.test.ts +++ b/server/tests/unit/nest/admin.service.test.ts @@ -82,14 +82,12 @@ const userCleanup = new UserCleanupService(dbs); const auth = new AuthService(dbs, permissions, new AtlasService(dbs), new TripMembershipService(dbs), webauthn, userCleanup); const svc = new AdminService( dbs, - new SettingsService(dbs), new AddonsService(dbs), new PasskeyService(dbs, auth, webauthn), auth, permissions, makeNotificationsService(dbs, realtime), userCleanup, - makeNotificationPreferencesService(dbs), ); // Legacy free-function names bound to the service, so the moved cases below read @@ -372,32 +370,6 @@ describe('getAuditLog — JSON details', () => { // ── OIDC Settings ───────────────────────────────────────────────────────────── -describe('OIDC Settings', () => { - it('ADMIN-SVC-047 — getOidcSettings returns default empty values when no OIDC configured', () => { - const result = getOidcSettings() as any; - expect(result.issuer).toBe(''); - expect(result.client_id).toBe(''); - expect(result.oidc_only).toBe(false); - expect(result.client_secret_set).toBe(false); - expect(result.display_name).toBe(''); - expect(result.discovery_url).toBe(''); - }); - - it('ADMIN-SVC-048 — updateOidcSettings persists issuer and client_id, then getOidcSettings returns them', () => { - updateOidcSettings({ issuer: 'https://auth.example.com', client_id: 'my-client' }); - const result = getOidcSettings() as any; - expect(result.issuer).toBe('https://auth.example.com'); - expect(result.client_id).toBe('my-client'); - }); - - it('ADMIN-SVC-049 — updateOidcSettings does not write oidc_only (replaced by granular toggles)', () => { - updateOidcSettings({ issuer: 'https://auth.example.com', client_id: 'my-client' }); - const result = getOidcSettings() as any; - // oidc_only is no longer managed by updateOidcSettings; use password_login/oidc_login toggles - expect(result.oidc_only).toBe(false); - }); -}); - // ── saveDemoBaseline ────────────────────────────────────────────────────────── describe('saveDemoBaseline', () => { @@ -623,11 +595,4 @@ describe('admin quirk fixes (post-fold)', () => { expect(sessions).toHaveLength(1); expect(sessions[0].scopes).toBeNull(); }); - - it('ADMIN-SVC-075 — updateOidcSettings applies all five writes atomically', () => { - const result = svc.updateOidcSettings({ issuer: 'https://idp', client_id: 'cid', display_name: 'IdP' }) as any; - expect(result.success).toBe(true); - const settings = svc.getOidcSettings(); - expect(settings).toMatchObject({ issuer: 'https://idp', client_id: 'cid', display_name: 'IdP' }); - }); }); diff --git a/server/tests/unit/nest/admin.version-notification.test.ts b/server/tests/unit/nest/admin.version-notification.test.ts index c5a0beb82..4403247ce 100644 --- a/server/tests/unit/nest/admin.version-notification.test.ts +++ b/server/tests/unit/nest/admin.version-notification.test.ts @@ -65,14 +65,12 @@ const userCleanup = new UserCleanupService(dbs); const auth = new AuthService(dbs, permissions, new AtlasService(dbs), new TripMembershipService(dbs), webauthn, userCleanup); const svc = new AdminService( dbs, - new SettingsService(dbs), new AddonsService(dbs), new PasskeyService(dbs, auth, webauthn), auth, permissions, makeNotificationsService(dbs, realtime), userCleanup, - makeNotificationPreferencesService(dbs), ); const checkAndNotifyVersion = () => svc.checkAndNotifyVersion(); diff --git a/server/tests/unit/nest/oidc.service.test.ts b/server/tests/unit/nest/oidc.service.test.ts index 53f73f949..3159f1fc5 100644 --- a/server/tests/unit/nest/oidc.service.test.ts +++ b/server/tests/unit/nest/oidc.service.test.ts @@ -827,3 +827,69 @@ describe('wrapper methods', () => { expect(setAuthCookieMock).toHaveBeenCalledWith(res, 'jwt', req); }); }); + +// The SSO configuration read/write moved here from AdminService, which held the SQL +// for a domain that already had a module. Same cases, same service, new owner. +describe('OIDC settings', () => { + it('ADMIN-SVC-047 — getOidcSettings returns default empty values when no OIDC configured', () => { + const result = svc.getOidcSettings() as any; + expect(result.issuer).toBe(''); + expect(result.client_id).toBe(''); + expect(result.oidc_only).toBe(false); + expect(result.client_secret_set).toBe(false); + expect(result.display_name).toBe(''); + expect(result.discovery_url).toBe(''); + }); + + it('ADMIN-SVC-048 — updateOidcSettings persists issuer and client_id, then getOidcSettings returns them', () => { + svc.updateOidcSettings({ issuer: 'https://auth.example.com', client_id: 'my-client' }); + const result = svc.getOidcSettings() as any; + expect(result.issuer).toBe('https://auth.example.com'); + expect(result.client_id).toBe('my-client'); + }); + + it('ADMIN-SVC-049 — updateOidcSettings does not write oidc_only (replaced by granular toggles)', () => { + svc.updateOidcSettings({ issuer: 'https://auth.example.com', client_id: 'my-client' }); + const result = svc.getOidcSettings() as any; + // oidc_only is no longer managed by updateOidcSettings; use password_login/oidc_login toggles + expect(result.oidc_only).toBe(false); + }); + + it('ADMIN-SVC-075 — updateOidcSettings applies all five writes atomically', () => { + const result = svc.updateOidcSettings({ issuer: 'https://idp', client_id: 'cid', display_name: 'IdP' }) as any; + expect(result.success).toBe(true); + const settings = svc.getOidcSettings(); + expect(settings).toMatchObject({ issuer: 'https://idp', client_id: 'cid', display_name: 'IdP' }); + }); +}); + +describe('OIDC settings — the lockout guard', () => { + it('OIDC-SETTINGS-050 refuses to clear the config while password login is off', () => { + // Clearing the issuer with password login disabled locks every user out of the + // instance: no SSO to log in through, and no password form either. + const toggles = vi.spyOn(auth, 'resolveAuthToggles').mockReturnValue({ password_login: false } as never); + expect(svc.updateOidcSettings({ issuer: '', client_id: 'x' })).toMatchObject({ status: 400 }); + expect(svc.updateOidcSettings({ issuer: 'x', client_id: '' })).toMatchObject({ status: 400 }); + expect((svc.updateOidcSettings({ issuer: '', client_id: '' }) as { error?: string }).error).toMatch(/password login/i); + toggles.mockRestore(); + }); + + it('OIDC-SETTINGS-051 allows the same clear once password login is back on', () => { + const toggles = vi.spyOn(auth, 'resolveAuthToggles').mockReturnValue({ password_login: true } as never); + expect(svc.updateOidcSettings({ issuer: '', client_id: '' })).toEqual({ success: true }); + toggles.mockRestore(); + }); + + it('OIDC-SETTINGS-052 an omitted client_secret keeps the stored one, an empty string clears it', () => { + const toggles = vi.spyOn(auth, 'resolveAuthToggles').mockReturnValue({ password_login: true } as never); + svc.updateOidcSettings({ issuer: 'https://idp', client_id: 'c', client_secret: 'shh' }); + expect(svc.getOidcSettings().client_secret_set).toBe(true); + // Omitted: the write skips the column entirely rather than blanking it, which is + // what lets the admin panel save the form without re-typing the secret. + svc.updateOidcSettings({ issuer: 'https://idp', client_id: 'c' }); + expect(svc.getOidcSettings().client_secret_set).toBe(true); + svc.updateOidcSettings({ issuer: 'https://idp', client_id: 'c', client_secret: '' }); + expect(svc.getOidcSettings().client_secret_set).toBe(false); + toggles.mockRestore(); + }); +});