From a40b418038a1d3e61d152a3d3869557aecfbbce6 Mon Sep 17 00:00:00 2001 From: NoiceHax Date: Thu, 13 Aug 2026 22:39:31 +0530 Subject: [PATCH] fix(2fa): gate two-factor auth when the encryption key is missing TWO_FACTOR_ENCRYPTION_KEY has no default, so instances upgrading to 3.3 without setting it hit an unhandled throw from encryptSecret: the setup dialog only reported "Something went wrong", and where 2FA was required the blocking dialog made the whole app unusable. The status endpoint now reports 2FA as not required while the key is missing or invalid, and setup initiation returns 503 with a two-factor-error-not-configured code that the setup dialog renders as a readable message. Also documents the variable in the README and the example compose file. Closes #4443 --- README.md | 4 + docker-compose.yml | 2 + public/intl/messages/en-US.json | 1 + src/app/api/2fa/setup/initiate/route.test.ts | 20 +++++ src/app/api/2fa/setup/initiate/route.ts | 12 ++- src/app/api/2fa/status/route.test.ts | 89 +++++++++++++++++++ src/app/api/2fa/status/route.ts | 6 ++ src/components/messages.ts | 1 + src/components/modals/TwoFactorSetupModal.tsx | 6 +- src/lib/response.ts | 9 ++ src/lib/two-factor/crypto.test.ts | 28 ++++++ src/lib/two-factor/crypto.ts | 12 ++- 12 files changed, 182 insertions(+), 8 deletions(-) create mode 100644 src/app/api/2fa/status/route.test.ts diff --git a/README.md b/README.md index 248347927..e0ebef01b 100644 --- a/README.md +++ b/README.md @@ -50,6 +50,10 @@ Optional: set `API_URL` to change the base URL used by internal UI API calls. Relative paths are served under `BASE_PATH`; absolute URLs are proxied through the local `/api` route. For example, `API_URL=/internal-api` or `API_URL=https://api.example.com/api`. +Optional: set `TWO_FACTOR_ENCRYPTION_KEY` to a 64-character hex string to enable two-factor +authentication. Generate one with `openssl rand -hex 32`. Two-factor authentication is unavailable +and cannot be required until this key is set. + The connection URL format: ```bash diff --git a/docker-compose.yml b/docker-compose.yml index 348c294ca..53f73b8de 100644 --- a/docker-compose.yml +++ b/docker-compose.yml @@ -7,6 +7,8 @@ services: environment: DATABASE_URL: postgresql://umami:umami@db:5432/umami APP_SECRET: replace-me-with-a-random-string + # Required for two-factor authentication, generate with: openssl rand -hex 32 + TWO_FACTOR_ENCRYPTION_KEY: replace-me-with-a-64-character-hex-string depends_on: db: condition: service_healthy diff --git a/public/intl/messages/en-US.json b/public/intl/messages/en-US.json index 552381ef1..0ed5102da 100644 --- a/public/intl/messages/en-US.json +++ b/public/intl/messages/en-US.json @@ -505,6 +505,7 @@ "two-factor-error-already-enabled": "Two-factor authentication is already enabled.", "two-factor-error-not-enabled": "Two-factor authentication is not enabled.", "two-factor-error-no-pending-setup": "No pending two-factor authentication setup found.", + "two-factor-error-not-configured": "Two-factor authentication is not available because the server is missing a valid TWO_FACTOR_ENCRYPTION_KEY. Contact your administrator for help.", "two-factor-locked-message": "Too many failed attempts. Try again at {time}.", "two-factor-active-description": "Your account is protected with two-factor authentication. Each login requires a code from your authenticator app." } diff --git a/src/app/api/2fa/setup/initiate/route.test.ts b/src/app/api/2fa/setup/initiate/route.test.ts index 04d605e2b..055cb8d3f 100644 --- a/src/app/api/2fa/setup/initiate/route.test.ts +++ b/src/app/api/2fa/setup/initiate/route.test.ts @@ -8,6 +8,7 @@ const mocks = vi.hoisted(() => ({ upsert: vi.fn(), generateTotpSecret: vi.fn(), encryptSecret: vi.fn(), + isTwoFactorConfigured: vi.fn(), generateOtpAuthUri: vi.fn(), generateQrCodeDataUrl: vi.fn(), })); @@ -33,6 +34,7 @@ vi.mock('@/lib/prisma', () => ({ vi.mock('@/lib/two-factor/crypto', () => ({ encryptSecret: mocks.encryptSecret, + isTwoFactorConfigured: mocks.isTwoFactorConfigured, })); vi.mock('@/lib/two-factor/totp', () => ({ @@ -48,9 +50,11 @@ beforeEach(() => { mocks.upsert.mockReset(); mocks.generateTotpSecret.mockReset(); mocks.encryptSecret.mockReset(); + mocks.isTwoFactorConfigured.mockReset(); mocks.generateOtpAuthUri.mockReset(); mocks.generateQrCodeDataUrl.mockReset(); + mocks.isTwoFactorConfigured.mockReturnValue(true); mocks.parseRequest.mockResolvedValue({ auth: { user: { id: 'user-1' } }, error: undefined, @@ -82,6 +86,22 @@ test('POST creates a pending 2FA setup and returns the manual key and QR data', expect(response.status).toBe(200); }); +test('POST reports a configuration error when the encryption key is missing', async () => { + mocks.isTwoFactorConfigured.mockReturnValue(false); + + const response = await POST( + new Request('http://localhost/api/2fa/setup/initiate', { method: 'POST' }), + ); + + expect(mocks.upsert).not.toHaveBeenCalled(); + await expect(response.json()).resolves.toMatchObject({ + error: { + code: 'two-factor-error-not-configured', + }, + }); + expect(response.status).toBe(503); +}); + test('POST rejects setup when 2FA is already enabled for the user', async () => { mocks.findUnique.mockResolvedValue({ userId: 'user-1', isEnabled: true }); diff --git a/src/app/api/2fa/setup/initiate/route.ts b/src/app/api/2fa/setup/initiate/route.ts index 01288601e..2ad68da61 100644 --- a/src/app/api/2fa/setup/initiate/route.ts +++ b/src/app/api/2fa/setup/initiate/route.ts @@ -1,6 +1,6 @@ import { parseRequest } from '@/lib/request'; -import { badRequest, json, notFound } from '@/lib/response'; -import { encryptSecret } from '@/lib/two-factor/crypto'; +import { badRequest, json, notFound, serviceUnavailable } from '@/lib/response'; +import { encryptSecret, isTwoFactorConfigured } from '@/lib/two-factor/crypto'; import { generateOtpAuthUri, generateQrCodeDataUrl, @@ -20,6 +20,14 @@ export async function POST(request: Request) { return error(); } + // Secrets cannot be stored without an encryption key + if (!isTwoFactorConfigured()) { + return serviceUnavailable({ + code: 'two-factor-error-not-configured', + message: 'TWO_FACTOR_ENCRYPTION_KEY is missing or invalid', + }); + } + const userId = auth.user.id; const user = await getUser(userId); diff --git a/src/app/api/2fa/status/route.test.ts b/src/app/api/2fa/status/route.test.ts new file mode 100644 index 000000000..c1e4f090d --- /dev/null +++ b/src/app/api/2fa/status/route.test.ts @@ -0,0 +1,89 @@ +import { beforeEach, expect, test, vi } from 'vitest'; +import prisma from '@/lib/prisma'; +import { parseRequest } from '@/lib/request'; +import { GET } from './route'; + +const KEY = 'a'.repeat(64); + +vi.mock('@/lib/request', () => ({ + parseRequest: vi.fn(), +})); + +vi.mock('@/lib/prisma', () => ({ + default: { + client: { + twoFactorAuth: { + findUnique: vi.fn(), + }, + appSetting: { + findUnique: vi.fn(), + }, + user: { + findUnique: vi.fn(), + }, + teamUser: { + findMany: vi.fn(), + }, + team: { + findMany: vi.fn(), + }, + }, + }, +})); + +const parseRequestMock = vi.mocked(parseRequest); +const prismaMock = vi.mocked(prisma, true); + +beforeEach(() => { + vi.unstubAllEnvs(); + parseRequestMock.mockReset(); + prismaMock.client.twoFactorAuth.findUnique.mockReset(); + prismaMock.client.appSetting.findUnique.mockReset(); + prismaMock.client.user.findUnique.mockReset(); + prismaMock.client.teamUser.findMany.mockReset(); + prismaMock.client.team.findMany.mockReset(); + + parseRequestMock.mockResolvedValue({ + auth: { + user: { + id: 'user-1', + }, + }, + error: undefined, + }); + prismaMock.client.twoFactorAuth.findUnique.mockResolvedValue(null as any); + prismaMock.client.appSetting.findUnique.mockResolvedValue({ + key: 'twoFactorRequiredGlobal', + value: 'true', + } as any); + prismaMock.client.user.findUnique.mockResolvedValue({ twoFactorRequired: false } as any); + prismaMock.client.teamUser.findMany.mockResolvedValue([] as any); + prismaMock.client.team.findMany.mockResolvedValue([] as any); +}); + +test('GET requires 2FA when it is globally enabled', async () => { + vi.stubEnv('TWO_FACTOR_ENCRYPTION_KEY', KEY); + + const response = await GET(new Request('http://localhost/api/2fa/status')); + + await expect(response.json()).resolves.toEqual({ + isEnabled: false, + isRequired: true, + requiredReason: 'global', + }); + expect(response.status).toBe(200); +}); + +test('GET does not require 2FA when the encryption key is missing', async () => { + vi.stubEnv('TWO_FACTOR_ENCRYPTION_KEY', ''); + + const response = await GET(new Request('http://localhost/api/2fa/status')); + + expect(prismaMock.client.appSetting.findUnique).not.toHaveBeenCalled(); + await expect(response.json()).resolves.toEqual({ + isEnabled: false, + isRequired: false, + requiredReason: null, + }); + expect(response.status).toBe(200); +}); diff --git a/src/app/api/2fa/status/route.ts b/src/app/api/2fa/status/route.ts index d7dc9a44f..57aa781bf 100644 --- a/src/app/api/2fa/status/route.ts +++ b/src/app/api/2fa/status/route.ts @@ -1,6 +1,7 @@ import prisma from '@/lib/prisma'; import { parseRequest } from '@/lib/request'; import { json } from '@/lib/response'; +import { isTwoFactorConfigured } from '@/lib/two-factor/crypto'; export async function GET(request: Request) { const { auth, error } = await parseRequest(request); @@ -18,6 +19,11 @@ export async function GET(request: Request) { const twoFactor = await prisma.client.twoFactorAuth.findUnique({ where: { userId } }); const isEnabled = twoFactor?.isEnabled ?? false; + // 2FA cannot be set up without an encryption key, so it is never required + if (!isTwoFactorConfigured()) { + return json({ isEnabled, isRequired: false, requiredReason: null }); + } + // Globally required const globalSetting = await prisma.client.appSetting.findUnique({ where: { key: 'twoFactorRequiredGlobal' }, diff --git a/src/components/messages.ts b/src/components/messages.ts index cc0d69d4b..b717200d4 100644 --- a/src/components/messages.ts +++ b/src/components/messages.ts @@ -506,5 +506,6 @@ export const messages: Record = { twoFactorErrorAlreadyEnabled: 'message.two-factor-error-already-enabled', twoFactorErrorNotEnabled: 'message.two-factor-error-not-enabled', twoFactorErrorNoPendingSetup: 'message.two-factor-error-no-pending-setup', + twoFactorErrorNotConfigured: 'message.two-factor-error-not-configured', replayMinDurationDescription: 'message.replay-min-duration-description', }; diff --git a/src/components/modals/TwoFactorSetupModal.tsx b/src/components/modals/TwoFactorSetupModal.tsx index 75bd01f19..4277b18f2 100644 --- a/src/components/modals/TwoFactorSetupModal.tsx +++ b/src/components/modals/TwoFactorSetupModal.tsx @@ -58,7 +58,7 @@ interface TwoFactorSetupModalProps { } export function TwoFactorSetupModal({ required, onClose }: TwoFactorSetupModalProps) { - const { t, labels, messages } = useMessages(); + const { t, labels, messages, getErrorMessage } = useMessages(); const [qrCodeDataUrl, setQrCodeDataUrl] = useState(null); const [manualKey, setManualKey] = useState(null); const [otpValue, setOtpValue] = useState(''); @@ -79,7 +79,7 @@ export function TwoFactorSetupModal({ required, onClose }: TwoFactorSetupModalPr setQrCodeDataUrl(data.qrCodeDataUrl); setManualKey(data.manualKey); }, - onError: () => setError(t(messages.error)), + onError: (err: any) => setError(getErrorMessage(err) || t(messages.error)), }, ); }, []); @@ -92,7 +92,7 @@ export function TwoFactorSetupModal({ required, onClose }: TwoFactorSetupModalPr const data: any = await confirm({ token }); setBackupCodes(data.backupCodes); } catch (err: any) { - setError(err.message || t(messages.error)); + setError(getErrorMessage(err) || t(messages.error)); } }; diff --git a/src/lib/response.ts b/src/lib/response.ts index 4c3f071a7..c3e2dbd68 100644 --- a/src/lib/response.ts +++ b/src/lib/response.ts @@ -54,6 +54,15 @@ export function notFound(error?: Record) { ); } +export function serviceUnavailable(error?: Record) { + return Response.json( + { + error: { message: 'Service unavailable', code: 'service-unavailable', status: 503, ...error }, + }, + { status: 503 }, + ); +} + export function serverError(error?: unknown) { if (error && typeof error !== 'string') { // eslint-disable-next-line no-console diff --git a/src/lib/two-factor/crypto.test.ts b/src/lib/two-factor/crypto.test.ts index 94f74ba7c..9f6f4d87e 100644 --- a/src/lib/two-factor/crypto.test.ts +++ b/src/lib/two-factor/crypto.test.ts @@ -51,3 +51,31 @@ describe('encryptSecret/decryptSecret', () => { vi.unstubAllEnvs(); }); }); + +describe('isTwoFactorConfigured', () => { + test('returns true with a valid key', async () => { + vi.stubEnv('TWO_FACTOR_ENCRYPTION_KEY', KEY); + vi.resetModules(); + const { isTwoFactorConfigured } = await import('./crypto'); + + expect(isTwoFactorConfigured()).toBe(true); + + vi.unstubAllEnvs(); + }); + + test('returns false when the key is missing or invalid', async () => { + vi.stubEnv('TWO_FACTOR_ENCRYPTION_KEY', ''); + vi.resetModules(); + const { isTwoFactorConfigured } = await import('./crypto'); + + expect(isTwoFactorConfigured()).toBe(false); + + vi.stubEnv('TWO_FACTOR_ENCRYPTION_KEY', 'too-short'); + expect(isTwoFactorConfigured()).toBe(false); + + vi.stubEnv('TWO_FACTOR_ENCRYPTION_KEY', 'z'.repeat(64)); + expect(isTwoFactorConfigured()).toBe(false); + + vi.unstubAllEnvs(); + }); +}); diff --git a/src/lib/two-factor/crypto.ts b/src/lib/two-factor/crypto.ts index 3901351e4..097c8b26a 100644 --- a/src/lib/two-factor/crypto.ts +++ b/src/lib/two-factor/crypto.ts @@ -1,14 +1,20 @@ import { createCipheriv, createDecipheriv, randomBytes } from 'crypto'; const ALGORITHM = 'aes-256-gcm'; +const KEY_REGEX = /^[0-9a-fA-F]{64}$/; + +/** Checks that TWO_FACTOR_ENCRYPTION_KEY holds a valid 256-bit key, without throwing. */ +export function isTwoFactorConfigured(): boolean { + const hex = process.env.TWO_FACTOR_ENCRYPTION_KEY; + return !!hex && KEY_REGEX.test(hex); +} /** Loads and validates the 256-bit encryption key from TWO_FACTOR_ENCRYPTION_KEY. */ function getKey(): Buffer { - const hex = process.env.TWO_FACTOR_ENCRYPTION_KEY; - if (!hex || hex.length !== 64 || !/^[0-9a-fA-F]{64}$/.test(hex)) { + if (!isTwoFactorConfigured()) { throw new Error('TWO_FACTOR_ENCRYPTION_KEY is missing or invalid'); } - return Buffer.from(hex, 'hex'); + return Buffer.from(process.env.TWO_FACTOR_ENCRYPTION_KEY, 'hex'); } /** Encrypts a TOTP secret using AES-256-GCM. Returns `ciphertext:iv:authTag` as hex. */