From b72645272b64e8f90dd57784dfd3fb256e745756 Mon Sep 17 00:00:00 2001 From: jubnl Date: Sun, 16 Aug 2026 09:49:46 +0200 Subject: [PATCH] fix(server): consolidate allowed_file_types onto AllowedFileTypesService --- server/src/nest/auth/auth.module.ts | 2 ++ server/src/nest/auth/auth.service.ts | 8 +++++++- server/src/nest/files/files.service.ts | 13 +++---------- server/tests/helpers/mcp-test-controllers.ts | 2 ++ server/tests/unit/nest/admin.service.test.ts | 3 ++- .../unit/nest/admin.version-notification.test.ts | 3 ++- server/tests/unit/nest/auth.service.test.ts | 14 ++++++++++++++ server/tests/unit/nest/files.service.test.ts | 16 +++++++++------- server/tests/unit/nest/oidc.service.test.ts | 2 ++ server/tests/unit/nest/passkey.service.test.ts | 2 ++ 10 files changed, 45 insertions(+), 20 deletions(-) diff --git a/server/src/nest/auth/auth.module.ts b/server/src/nest/auth/auth.module.ts index 25344f8d0..2542227b8 100644 --- a/server/src/nest/auth/auth.module.ts +++ b/server/src/nest/auth/auth.module.ts @@ -23,6 +23,7 @@ import { StorageModule } from '../storage/storage.module'; import { StorageService } from '../storage/storage.service'; import { buildStorageUploadOptions } from '../storage/storage-upload.factory'; import { AVATAR_FILE_FILTER, MAX_AVATAR_SIZE } from './auth.controller'; +import { AllowedFileTypesModule } from '../files/allowed-file-types.module'; /** * Auth module — public flows (login/register/reset/mfa-verify/logout) and the @@ -60,6 +61,7 @@ import { AVATAR_FILE_FILTER, MAX_AVATAR_SIZE } from './auth.controller'; }), }), StorageModule, + AllowedFileTypesModule, EphemeralTokenModule, RateLimitModule, AuditModule, PermissionsModule, TripMembershipModule, MailerModule, AppConfigModule, TokensModule, BudgetModule], controllers: [AuthPublicController, AuthController, PasskeyController], providers: [AuthService, UserProfileService, RegistrationInvitesService, PasskeyService, UserCleanupService, WebauthnConfigService, AuthMcp], diff --git a/server/src/nest/auth/auth.service.ts b/server/src/nest/auth/auth.service.ts index 3cb0c91c6..e09f1a768 100644 --- a/server/src/nest/auth/auth.service.ts +++ b/server/src/nest/auth/auth.service.ts @@ -30,6 +30,7 @@ import { TripMembershipService } from '../trip-membership/trip-membership.servic import { WebauthnConfigService } from './webauthn-config.service'; import { setAuthCookie, clearAuthCookie } from '../common/cookie'; import { MailerService } from '../notifications/mailer/mailer.service'; +import { AllowedFileTypesService } from '../files/allowed-file-types.service'; import { getAppUrl } from '../../app-config'; import { ADMIN_SETTINGS_KEYS, @@ -127,6 +128,7 @@ export class AuthService { private readonly userCleanup: UserCleanupService, private readonly mailer: MailerService, private readonly tokens: EphemeralTokenService, + private readonly allowedFileTypes: AllowedFileTypesService, ) {} // Cookie @@ -290,7 +292,11 @@ export class AuthService { oidc_configured: oidcConfigured, oidc_display_name: oidcConfigured ? (oidcDisplayName || 'SSO') : undefined, require_mfa: requireMfaRow?.value === 'true', - allowed_file_types: this.db.get<{ value: string }>("SELECT value FROM app_settings WHERE key = 'allowed_file_types'")?.value || 'jpg,jpeg,png,gif,webp,heic,pdf,doc,docx,xls,xlsx,txt,csv', + // The canonical live-read: same query + DEFAULT_ALLOWED_EXTENSIONS + // fallback the upload filters use, so the client's picker and the + // server's acceptance can never drift (the historical inline copy here + // dropped pkpass, pkpasses, md and markdown). + allowed_file_types: this.allowedFileTypes.get(), // Whether the configuration belongs to whoever operates this install // rather than to its admin. The client uses it to stop offering settings // the server would refuse anyway — it is an honesty flag for the UI, never diff --git a/server/src/nest/files/files.service.ts b/server/src/nest/files/files.service.ts index 33b52768b..2a02649bc 100644 --- a/server/src/nest/files/files.service.ts +++ b/server/src/nest/files/files.service.ts @@ -45,9 +45,9 @@ export interface FileLink { * DatabaseService's canAccessTrip (the legacy verifyTripAccess re-export was a * plain wrapper over the same helper); the file_* permissions, path-resolution * guard, download-token auth and WebSocket broadcasts are unchanged. The - * load-time constants live in files.constants.ts; the one out-of-DI consumer - * path (module-scope multer configs) reaches getAllowedExtensions through - * files.bridge.ts. + * load-time constants live in files.constants.ts; the admin allowed-types + * live-read is AllowedFileTypesService (the single query owner since the + * storage slice-2 consolidation). * * Two deliberate post-migration fixes over the legacy behavior: createFileLink * no longer swallows insert errors, and updateFile coerces an empty-string @@ -74,13 +74,6 @@ export class FilesService { this.realtime.broadcast(tripId, event, payload, socketId); } - getAllowedExtensions(): string { - try { - const row = this.db.get<{ value: string }>("SELECT value FROM app_settings WHERE key = 'allowed_file_types'"); - return row?.value || DEFAULT_ALLOWED_EXTENSIONS; - } catch { return DEFAULT_ALLOWED_EXTENSIONS; } - } - // --------------------------------------------------------------------------- // Token-based download auth & safe path resolution (used by the unguarded // download route) diff --git a/server/tests/helpers/mcp-test-controllers.ts b/server/tests/helpers/mcp-test-controllers.ts index ba4d6434e..77a1de006 100644 --- a/server/tests/helpers/mcp-test-controllers.ts +++ b/server/tests/helpers/mcp-test-controllers.ts @@ -74,6 +74,7 @@ import { makeNotificationsService, makeNotificationPreferencesService } from './ import { AddonsService } from '../../src/nest/addons/addons.service'; import { notificationsStub } from './notifications'; import { EphemeralTokenService } from '../../src/nest/auth/ephemeral-token.service'; +import { AllowedFileTypesService } from '../../src/nest/files/allowed-file-types.service'; /** * Hand-wired counterpart of the boot-time discovery in McpRegistryService, @@ -102,6 +103,7 @@ export function createMcpTestRegistry(): McpRegistry { new UserCleanupService(dbService, budgetService), new MailerService(dbService), new EphemeralTokenService(), + new AllowedFileTypesService(dbService), ); const queryHelpersService = new QueryHelpersService(dbService); const daysService = new DaysService(dbService, permissionsService, realtimeService, queryHelpersService); diff --git a/server/tests/unit/nest/admin.service.test.ts b/server/tests/unit/nest/admin.service.test.ts index e0e2139cf..fe84dd6d6 100644 --- a/server/tests/unit/nest/admin.service.test.ts +++ b/server/tests/unit/nest/admin.service.test.ts @@ -80,6 +80,7 @@ import type { RuntimeEnvService } from '../../../src/nest/app-config/runtime-env import { __clearVersionCacheForTests } from '../../../src/nest/admin/admin.helpers'; import { makeNotificationsService, makeNotificationPreferencesService } from '../../helpers/notifications'; import { EphemeralTokenService } from '../../../src/nest/auth/ephemeral-token.service'; +import { AllowedFileTypesService } from '../../../src/nest/files/allowed-file-types.service'; const dbs = new DatabaseService(testDb); const realtime = new RealtimeService(); @@ -90,7 +91,7 @@ const userCleanup = new UserCleanupService(dbs, new BudgetService(dbs, permissio // and the mailer was missing entirely, so `auth` was built with its last four // collaborators shifted by one. Nothing failed, because none of the cases below // reach a path that uses them. -const auth = new AuthService(dbs, permissions, new TripMembershipService(dbs), webauthn, userCleanup, new MailerService(dbs), new EphemeralTokenService()); +const auth = new AuthService(dbs, permissions, new TripMembershipService(dbs), webauthn, userCleanup, new MailerService(dbs), new EphemeralTokenService(), new AllowedFileTypesService(dbs)); const svc = new AdminService( dbs, new AddonsService(dbs), diff --git a/server/tests/unit/nest/admin.version-notification.test.ts b/server/tests/unit/nest/admin.version-notification.test.ts index 9710104ae..fc514e145 100644 --- a/server/tests/unit/nest/admin.version-notification.test.ts +++ b/server/tests/unit/nest/admin.version-notification.test.ts @@ -59,6 +59,7 @@ import { NotificationsService } from '../../../src/nest/notifications/notificati import { AdminService } from '../../../src/nest/admin/admin.service'; import { makeNotificationsService, makeNotificationPreferencesService } from '../../helpers/notifications'; import { EphemeralTokenService } from '../../../src/nest/auth/ephemeral-token.service'; +import { AllowedFileTypesService } from '../../../src/nest/files/allowed-file-types.service'; const dbs = new DatabaseService(testDb); const realtime = new RealtimeService(); @@ -69,7 +70,7 @@ const userCleanup = new UserCleanupService(dbs, new BudgetService(dbs, permissio // and the mailer was missing entirely, so `auth` was built with its last four // collaborators shifted by one. Nothing failed, because the version-check path // below never reaches them. -const auth = new AuthService(dbs, permissions, new TripMembershipService(dbs), webauthn, userCleanup, new MailerService(dbs), new EphemeralTokenService()); +const auth = new AuthService(dbs, permissions, new TripMembershipService(dbs), webauthn, userCleanup, new MailerService(dbs), new EphemeralTokenService(), new AllowedFileTypesService(dbs)); const svc = new AdminService( dbs, new AddonsService(dbs), diff --git a/server/tests/unit/nest/auth.service.test.ts b/server/tests/unit/nest/auth.service.test.ts index 8588ea1a8..0f0c4c5e5 100644 --- a/server/tests/unit/nest/auth.service.test.ts +++ b/server/tests/unit/nest/auth.service.test.ts @@ -82,6 +82,8 @@ import { WebauthnConfigService } from '../../../src/nest/auth/webauthn-config.se import { revokeUserSessions } from '../../../src/mcp/sessionManager'; import { MailerService } from '../../../src/nest/notifications/mailer/mailer.service'; import { EphemeralTokenService } from '../../../src/nest/auth/ephemeral-token.service'; +import { AllowedFileTypesService } from '../../../src/nest/files/allowed-file-types.service'; +import { DEFAULT_ALLOWED_EXTENSIONS } from '../../../src/nest/files/files.constants'; // MailerService is injected since the notifications fold — a stub instead of a // module mock. sendPasswordResetEmail is the only thing auth reaches for. @@ -103,6 +105,7 @@ const svc = new AuthService( new UserCleanupService(new DatabaseService(testDb), new BudgetService(new DatabaseService(testDb), new PermissionsService(new DatabaseService(testDb)), new ExchangeRatesService(), new RealtimeService())), mailerStub, new EphemeralTokenService(), + new AllowedFileTypesService(new DatabaseService(testDb)), ); // --------------------------------------------------------------------------- @@ -559,6 +562,17 @@ describe('getAppConfig', () => { vi.unstubAllEnvs(); }); + it('AUTH-DB-050b: fresh-install fallback matches DEFAULT_ALLOWED_EXTENSIONS (pkpass/md included)', () => { + // No allowed_file_types row: the config payload must advertise the same + // default list the upload filters actually enforce — the historical + // hardcoded copy dropped pkpass, pkpasses, md and markdown, so the client + // greyed out types the server accepts. + createUser(testDb); + testDb.prepare("DELETE FROM app_settings WHERE key = 'allowed_file_types'").run(); + const cfg = svc.getAppConfig(null); + expect(cfg.allowed_file_types).toBe(DEFAULT_ALLOWED_EXTENSIONS); + }); + it('AUTH-DB-051: authenticated caller gets the permissions block; app_settings rows flow through', () => { const { user } = createUser(testDb); testDb.prepare("INSERT OR REPLACE INTO app_settings (key, value) VALUES ('require_mfa', 'true')").run(); diff --git a/server/tests/unit/nest/files.service.test.ts b/server/tests/unit/nest/files.service.test.ts index 184d3dd7c..f636c2005 100644 --- a/server/tests/unit/nest/files.service.test.ts +++ b/server/tests/unit/nest/files.service.test.ts @@ -166,26 +166,28 @@ describe('files.constants', () => { }); }); -// ── getAllowedExtensions ────────────────────────────────────────────────────── +// ── allowed extensions (canonical owner: AllowedFileTypesService) ───────────── +// FilesService.getAllowedExtensions was deleted in the storage slice-2 +// consolidation (it had no callers); the same four behaviors are pinned +// against the single remaining query owner. -describe('getAllowedExtensions', () => { +describe('AllowedFileTypesService.get', () => { it('FILE-SVC-006: returns the app_settings value when set', () => { setAppSetting(testDb, 'allowed_file_types', 'pdf,txt'); - expect(svc.getAllowedExtensions()).toBe('pdf,txt'); + expect(new AllowedFileTypesService(new DatabaseService(testDb)).get()).toBe('pdf,txt'); }); it('FILE-SVC-007: returns the default when the row is absent', () => { - expect(svc.getAllowedExtensions()).toBe(DEFAULT_ALLOWED_EXTENSIONS); + expect(new AllowedFileTypesService(new DatabaseService(testDb)).get()).toBe(DEFAULT_ALLOWED_EXTENSIONS); }); it('FILE-SVC-008: returns the default for an empty value (|| coercion, not ??)', () => { setAppSetting(testDb, 'allowed_file_types', ''); - expect(svc.getAllowedExtensions()).toBe(DEFAULT_ALLOWED_EXTENSIONS); + expect(new AllowedFileTypesService(new DatabaseService(testDb)).get()).toBe(DEFAULT_ALLOWED_EXTENSIONS); }); it('FILE-SVC-009: returns the default when the query throws (no app_settings table)', () => { - const bareSvc = new FilesService(new DatabaseService(bareDb), permissionsStub, new RealtimeService(), new EphemeralTokenService()); - expect(bareSvc.getAllowedExtensions()).toBe(DEFAULT_ALLOWED_EXTENSIONS); + expect(new AllowedFileTypesService(new DatabaseService(bareDb)).get()).toBe(DEFAULT_ALLOWED_EXTENSIONS); }); }); diff --git a/server/tests/unit/nest/oidc.service.test.ts b/server/tests/unit/nest/oidc.service.test.ts index d707d954e..30f8dc056 100644 --- a/server/tests/unit/nest/oidc.service.test.ts +++ b/server/tests/unit/nest/oidc.service.test.ts @@ -103,6 +103,7 @@ import { AuthService } from '../../../src/nest/auth/auth.service'; import { WebauthnConfigService } from '../../../src/nest/auth/webauthn-config.service'; import { UserCleanupService } from '../../../src/nest/auth/user-cleanup.service'; import { EphemeralTokenService } from '../../../src/nest/auth/ephemeral-token.service'; +import { AllowedFileTypesService } from '../../../src/nest/files/allowed-file-types.service'; import { OidcService } from '../../../src/nest/oidc/oidc.service'; import { MailerService } from '../../../src/nest/notifications/mailer/mailer.service'; @@ -123,6 +124,7 @@ const auth = new AuthService( new UserCleanupService(new DatabaseService(testDb), new BudgetService(new DatabaseService(testDb), new PermissionsService(new DatabaseService(testDb)), new ExchangeRatesService(), new RealtimeService())), mailerStub, new EphemeralTokenService(), + new AllowedFileTypesService(new DatabaseService(testDb)), ); const svc = new OidcService(new DatabaseService(testDb), auth, membership); diff --git a/server/tests/unit/nest/passkey.service.test.ts b/server/tests/unit/nest/passkey.service.test.ts index f84ed289e..f330b68e6 100644 --- a/server/tests/unit/nest/passkey.service.test.ts +++ b/server/tests/unit/nest/passkey.service.test.ts @@ -84,6 +84,7 @@ import { PasskeyService } from '../../../src/nest/auth/passkey.service'; import { WebauthnConfigService } from '../../../src/nest/auth/webauthn-config.service'; import { UserCleanupService } from '../../../src/nest/auth/user-cleanup.service'; import { EphemeralTokenService } from '../../../src/nest/auth/ephemeral-token.service'; +import { AllowedFileTypesService } from '../../../src/nest/files/allowed-file-types.service'; import { MailerService } from '../../../src/nest/notifications/mailer/mailer.service'; // MailerService is injected since the notifications fold — a stub instead of a @@ -113,6 +114,7 @@ const auth = new AuthService( new UserCleanupService(new DatabaseService(testDb), new BudgetService(new DatabaseService(testDb), new PermissionsService(new DatabaseService(testDb)), new ExchangeRatesService(), new RealtimeService())), mailerStub, new EphemeralTokenService(), + new AllowedFileTypesService(new DatabaseService(testDb)), ); const svc = new PasskeyService(new DatabaseService(testDb), auth, webauthn);