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
This commit is contained in:
1 parent
de474a1da9
commit
a40b418038
12 files changed
+182
-8
No files matched your search
@@ -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.
|
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`.
|
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:
|
The connection URL format:
|
||||||
|
|
||||||
```bash
|
```bash
|
||||||
|
|||||||
@@ -7,6 +7,8 @@ services:
|
|||||||
environment:
|
environment:
|
||||||
DATABASE_URL: postgresql://umami:umami@db:5432/umami
|
DATABASE_URL: postgresql://umami:umami@db:5432/umami
|
||||||
APP_SECRET: replace-me-with-a-random-string
|
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:
|
depends_on:
|
||||||
db:
|
db:
|
||||||
condition: service_healthy
|
condition: service_healthy
|
||||||
|
|||||||
@@ -505,6 +505,7 @@
|
|||||||
"two-factor-error-already-enabled": "Two-factor authentication is already enabled.",
|
"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-not-enabled": "Two-factor authentication is not enabled.",
|
||||||
"two-factor-error-no-pending-setup": "No pending two-factor authentication setup found.",
|
"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-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."
|
"two-factor-active-description": "Your account is protected with two-factor authentication. Each login requires a code from your authenticator app."
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -8,6 +8,7 @@ const mocks = vi.hoisted(() => ({
|
|||||||
upsert: vi.fn(),
|
upsert: vi.fn(),
|
||||||
generateTotpSecret: vi.fn(),
|
generateTotpSecret: vi.fn(),
|
||||||
encryptSecret: vi.fn(),
|
encryptSecret: vi.fn(),
|
||||||
|
isTwoFactorConfigured: vi.fn(),
|
||||||
generateOtpAuthUri: vi.fn(),
|
generateOtpAuthUri: vi.fn(),
|
||||||
generateQrCodeDataUrl: vi.fn(),
|
generateQrCodeDataUrl: vi.fn(),
|
||||||
}));
|
}));
|
||||||
@@ -33,6 +34,7 @@ vi.mock('@/lib/prisma', () => ({
|
|||||||
|
|
||||||
vi.mock('@/lib/two-factor/crypto', () => ({
|
vi.mock('@/lib/two-factor/crypto', () => ({
|
||||||
encryptSecret: mocks.encryptSecret,
|
encryptSecret: mocks.encryptSecret,
|
||||||
|
isTwoFactorConfigured: mocks.isTwoFactorConfigured,
|
||||||
}));
|
}));
|
||||||
|
|
||||||
vi.mock('@/lib/two-factor/totp', () => ({
|
vi.mock('@/lib/two-factor/totp', () => ({
|
||||||
@@ -48,9 +50,11 @@ beforeEach(() => {
|
|||||||
mocks.upsert.mockReset();
|
mocks.upsert.mockReset();
|
||||||
mocks.generateTotpSecret.mockReset();
|
mocks.generateTotpSecret.mockReset();
|
||||||
mocks.encryptSecret.mockReset();
|
mocks.encryptSecret.mockReset();
|
||||||
|
mocks.isTwoFactorConfigured.mockReset();
|
||||||
mocks.generateOtpAuthUri.mockReset();
|
mocks.generateOtpAuthUri.mockReset();
|
||||||
mocks.generateQrCodeDataUrl.mockReset();
|
mocks.generateQrCodeDataUrl.mockReset();
|
||||||
|
|
||||||
|
mocks.isTwoFactorConfigured.mockReturnValue(true);
|
||||||
mocks.parseRequest.mockResolvedValue({
|
mocks.parseRequest.mockResolvedValue({
|
||||||
auth: { user: { id: 'user-1' } },
|
auth: { user: { id: 'user-1' } },
|
||||||
error: undefined,
|
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);
|
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 () => {
|
test('POST rejects setup when 2FA is already enabled for the user', async () => {
|
||||||
mocks.findUnique.mockResolvedValue({ userId: 'user-1', isEnabled: true });
|
mocks.findUnique.mockResolvedValue({ userId: 'user-1', isEnabled: true });
|
||||||
|
|
||||||
|
|||||||
@@ -1,6 +1,6 @@
|
|||||||
import { parseRequest } from '@/lib/request';
|
import { parseRequest } from '@/lib/request';
|
||||||
import { badRequest, json, notFound } from '@/lib/response';
|
import { badRequest, json, notFound, serviceUnavailable } from '@/lib/response';
|
||||||
import { encryptSecret } from '@/lib/two-factor/crypto';
|
import { encryptSecret, isTwoFactorConfigured } from '@/lib/two-factor/crypto';
|
||||||
import {
|
import {
|
||||||
generateOtpAuthUri,
|
generateOtpAuthUri,
|
||||||
generateQrCodeDataUrl,
|
generateQrCodeDataUrl,
|
||||||
@@ -20,6 +20,14 @@ export async function POST(request: Request) {
|
|||||||
return error();
|
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 userId = auth.user.id;
|
||||||
const user = await getUser(userId);
|
const user = await getUser(userId);
|
||||||
|
|
||||||
|
|||||||
@@ -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);
|
||||||
|
});
|
||||||
@@ -1,6 +1,7 @@
|
|||||||
import prisma from '@/lib/prisma';
|
import prisma from '@/lib/prisma';
|
||||||
import { parseRequest } from '@/lib/request';
|
import { parseRequest } from '@/lib/request';
|
||||||
import { json } from '@/lib/response';
|
import { json } from '@/lib/response';
|
||||||
|
import { isTwoFactorConfigured } from '@/lib/two-factor/crypto';
|
||||||
|
|
||||||
export async function GET(request: Request) {
|
export async function GET(request: Request) {
|
||||||
const { auth, error } = await parseRequest(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 twoFactor = await prisma.client.twoFactorAuth.findUnique({ where: { userId } });
|
||||||
const isEnabled = twoFactor?.isEnabled ?? false;
|
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
|
// Globally required
|
||||||
const globalSetting = await prisma.client.appSetting.findUnique({
|
const globalSetting = await prisma.client.appSetting.findUnique({
|
||||||
where: { key: 'twoFactorRequiredGlobal' },
|
where: { key: 'twoFactorRequiredGlobal' },
|
||||||
|
|||||||
@@ -506,5 +506,6 @@ export const messages: Record<string, string> = {
|
|||||||
twoFactorErrorAlreadyEnabled: 'message.two-factor-error-already-enabled',
|
twoFactorErrorAlreadyEnabled: 'message.two-factor-error-already-enabled',
|
||||||
twoFactorErrorNotEnabled: 'message.two-factor-error-not-enabled',
|
twoFactorErrorNotEnabled: 'message.two-factor-error-not-enabled',
|
||||||
twoFactorErrorNoPendingSetup: 'message.two-factor-error-no-pending-setup',
|
twoFactorErrorNoPendingSetup: 'message.two-factor-error-no-pending-setup',
|
||||||
|
twoFactorErrorNotConfigured: 'message.two-factor-error-not-configured',
|
||||||
replayMinDurationDescription: 'message.replay-min-duration-description',
|
replayMinDurationDescription: 'message.replay-min-duration-description',
|
||||||
};
|
};
|
||||||
@@ -58,7 +58,7 @@ interface TwoFactorSetupModalProps {
|
|||||||
}
|
}
|
||||||
|
|
||||||
export function TwoFactorSetupModal({ required, onClose }: TwoFactorSetupModalProps) {
|
export function TwoFactorSetupModal({ required, onClose }: TwoFactorSetupModalProps) {
|
||||||
const { t, labels, messages } = useMessages();
|
const { t, labels, messages, getErrorMessage } = useMessages();
|
||||||
const [qrCodeDataUrl, setQrCodeDataUrl] = useState<string | null>(null);
|
const [qrCodeDataUrl, setQrCodeDataUrl] = useState<string | null>(null);
|
||||||
const [manualKey, setManualKey] = useState<string | null>(null);
|
const [manualKey, setManualKey] = useState<string | null>(null);
|
||||||
const [otpValue, setOtpValue] = useState('');
|
const [otpValue, setOtpValue] = useState('');
|
||||||
@@ -79,7 +79,7 @@ export function TwoFactorSetupModal({ required, onClose }: TwoFactorSetupModalPr
|
|||||||
setQrCodeDataUrl(data.qrCodeDataUrl);
|
setQrCodeDataUrl(data.qrCodeDataUrl);
|
||||||
setManualKey(data.manualKey);
|
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 });
|
const data: any = await confirm({ token });
|
||||||
setBackupCodes(data.backupCodes);
|
setBackupCodes(data.backupCodes);
|
||||||
} catch (err: any) {
|
} catch (err: any) {
|
||||||
setError(err.message || t(messages.error));
|
setError(getErrorMessage(err) || t(messages.error));
|
||||||
}
|
}
|
||||||
};
|
};
|
||||||
|
|
||||||
|
|||||||
@@ -54,6 +54,15 @@ export function notFound(error?: Record<string, any>) {
|
|||||||
);
|
);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
export function serviceUnavailable(error?: Record<string, any>) {
|
||||||
|
return Response.json(
|
||||||
|
{
|
||||||
|
error: { message: 'Service unavailable', code: 'service-unavailable', status: 503, ...error },
|
||||||
|
},
|
||||||
|
{ status: 503 },
|
||||||
|
);
|
||||||
|
}
|
||||||
|
|
||||||
export function serverError(error?: unknown) {
|
export function serverError(error?: unknown) {
|
||||||
if (error && typeof error !== 'string') {
|
if (error && typeof error !== 'string') {
|
||||||
// eslint-disable-next-line no-console
|
// eslint-disable-next-line no-console
|
||||||
|
|||||||
@@ -51,3 +51,31 @@ describe('encryptSecret/decryptSecret', () => {
|
|||||||
vi.unstubAllEnvs();
|
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();
|
||||||
|
});
|
||||||
|
});
|
||||||
@@ -1,14 +1,20 @@
|
|||||||
import { createCipheriv, createDecipheriv, randomBytes } from 'crypto';
|
import { createCipheriv, createDecipheriv, randomBytes } from 'crypto';
|
||||||
|
|
||||||
const ALGORITHM = 'aes-256-gcm';
|
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. */
|
/** Loads and validates the 256-bit encryption key from TWO_FACTOR_ENCRYPTION_KEY. */
|
||||||
function getKey(): Buffer {
|
function getKey(): Buffer {
|
||||||
const hex = process.env.TWO_FACTOR_ENCRYPTION_KEY;
|
if (!isTwoFactorConfigured()) {
|
||||||
if (!hex || hex.length !== 64 || !/^[0-9a-fA-F]{64}$/.test(hex)) {
|
|
||||||
throw new Error('TWO_FACTOR_ENCRYPTION_KEY is missing or invalid');
|
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. */
|
/** Encrypts a TOTP secret using AES-256-GCM. Returns `ciphertext:iv:authTag` as hex. */
|
||||||
|
|||||||
Reference in new issue
Block a user