From 0a77e50da1228ff00c76bc132795bb67144821eb Mon Sep 17 00:00:00 2001 From: Francis Cao Date: Fri, 14 Aug 2026 10:32:09 -0700 Subject: [PATCH] Harden 2FA APi routes and UI when 2FA env var missing. Closes #4443. --- .../admin/security/AdminSecurityPage.tsx | 18 ++- .../users/[userId]/UserTwoFactorSettings.tsx | 40 ++++--- .../settings/security/UserSecurityPage.tsx | 29 ++++- .../teams/[teamId]/TeamTwoFactorSettings.tsx | 40 ++++--- src/app/api/2fa/disable/route.test.ts | 76 ++++++++++++ src/app/api/2fa/disable/route.ts | 8 +- src/app/api/2fa/setup/confirm/route.test.ts | 25 ++++ src/app/api/2fa/setup/confirm/route.ts | 8 +- src/app/api/2fa/setup/initiate/route.test.ts | 4 + src/app/api/2fa/setup/initiate/route.ts | 11 +- src/app/api/2fa/status/route.test.ts | 44 ++++--- src/app/api/2fa/status/route.ts | 63 ++++++++-- src/app/api/2fa/verify/route.test.ts | 31 +++++ src/app/api/2fa/verify/route.ts | 8 +- src/app/api/admin/2fa/global/route.ts | 7 +- src/app/api/admin/teams/[teamId]/2fa/route.ts | 7 +- .../admin/users/[userId]/2fa/route.test.ts | 40 +++++++ src/app/api/admin/users/[userId]/2fa/route.ts | 7 +- src/app/api/auth/login/route.test.ts | 108 ++++++++++++++++++ src/app/api/auth/login/route.ts | 7 +- .../hooks/queries/useTwoFactorStatusQuery.ts | 10 +- src/lib/two-factor/crypto.ts | 10 +- 22 files changed, 511 insertions(+), 90 deletions(-) create mode 100644 src/app/api/2fa/disable/route.test.ts create mode 100644 src/app/api/auth/login/route.test.ts diff --git a/src/app/(main)/admin/security/AdminSecurityPage.tsx b/src/app/(main)/admin/security/AdminSecurityPage.tsx index a962787ae..a243ca4d7 100644 --- a/src/app/(main)/admin/security/AdminSecurityPage.tsx +++ b/src/app/(main)/admin/security/AdminSecurityPage.tsx @@ -11,7 +11,9 @@ export function AdminSecurityPage() { const { mutateAsync: setGlobal } = useUpdateQuery('/admin/2fa/global'); const queryClient = useQueryClient(); - const isGlobalRequired = status?.requiredReason === 'global' || false; + const isConfigured = status?.isConfigured; + const isGlobalRequired = status?.globalRequired ?? false; + const showConfigurationError = isConfigured === false; const handleToggle = async (value: boolean) => { await setGlobal({ required: value }); @@ -27,15 +29,23 @@ export function AdminSecurityPage() { {t(labels.twoFactorAuth)} - {t(messages.twoFactorAdminGlobalDescription)} + + {showConfigurationError + ? t(messages.twoFactorErrorNotConfigured) + : t(messages.twoFactorAdminGlobalDescription)} + - + {t(labels.twoFactorRequireGlobal)} - {isGlobalRequired && ( + {isConfigured === true && isGlobalRequired && ( {t(messages.twoFactorOverrideNote)} diff --git a/src/app/(main)/admin/users/[userId]/UserTwoFactorSettings.tsx b/src/app/(main)/admin/users/[userId]/UserTwoFactorSettings.tsx index 80832d18e..b67ba3418 100644 --- a/src/app/(main)/admin/users/[userId]/UserTwoFactorSettings.tsx +++ b/src/app/(main)/admin/users/[userId]/UserTwoFactorSettings.tsx @@ -32,7 +32,9 @@ export function UserTwoFactorSettings({ userId }: { userId: string }) { const [showReset, setShowReset] = useState(false); const { data: globalStatus } = useTwoFactorStatusQuery(true); - const isGlobalRequired = globalStatus?.requiredReason === 'global'; + const isConfigured = globalStatus?.isConfigured; + const isGlobalRequired = globalStatus?.globalRequired ?? false; + const showConfigurationError = isConfigured === false; const { data: userTfaData } = useTwoFactorUserStatusQuery(userId); const { data: userData } = useUserQuery(userId); @@ -52,6 +54,18 @@ export function UserTwoFactorSettings({ userId }: { userId: string }) { queryClient.invalidateQueries({ queryKey: ['users', { userId }] }); }; + const isToggleDisabled = isGlobalRequired || (!isConfigured && !twoFactorRequired); + const toggle = ( + + + {t(labels.twoFactorRequireUser)} + + ); + const handleReset = async () => { await resetUserTwoFactor(null, { onSuccess: async () => { @@ -75,27 +89,19 @@ export function UserTwoFactorSettings({ userId }: { userId: string }) { {t(messages.twoFactorRequireUserDescription)} + {showConfigurationError && ( + + {t(messages.twoFactorErrorNotConfigured)} + + )} + {isGlobalRequired ? ( - - - {t(labels.twoFactorRequireUser)} - + {toggle} {t(labels.twoFactorGlobalActiveTooltip)} ) : ( - - - {t(labels.twoFactorRequireUser)} - + toggle )} {twoFactorEnabled && ( diff --git a/src/app/(main)/settings/security/UserSecurityPage.tsx b/src/app/(main)/settings/security/UserSecurityPage.tsx index 9d1426a88..9faa8232e 100644 --- a/src/app/(main)/settings/security/UserSecurityPage.tsx +++ b/src/app/(main)/settings/security/UserSecurityPage.tsx @@ -19,6 +19,8 @@ export function UserSecurityPage() { const isEnabled = status?.isEnabled ?? false; const isRequired = status?.isRequired ?? false; + const isConfigured = status?.isConfigured; + const showConfigurationError = isConfigured === false; const handleToggle = (value: boolean) => { if (value) { @@ -55,13 +57,15 @@ export function UserSecurityPage() { )} - {isEnabled ? ( + {showConfigurationError ? ( + {t(messages.twoFactorErrorNotConfigured)} + ) : isEnabled ? ( {t(messages.twoFactorActiveDescription)} ) : ( {t(messages.twoFactorUserDescription)} )} - {isEnabled && isRequired ? ( + {isConfigured === true && isEnabled && isRequired ? ( {t(labels.twoFactorEnable)} - {t(messages.twoFactorRequiredMessage)} + {t(messages.twoFactorRequiredMessage)} - ) : ( + ) : null} + + {isConfigured === true && !isEnabled ? ( {t(labels.twoFactorEnable)} - )} + ) : null} - {isEnabled && isRequired && ( + {isConfigured === true && isEnabled && !isRequired ? ( + + + {t(labels.twoFactorEnable)} + + ) : null} + + {isConfigured === true && isEnabled && isRequired && ( {t(messages.twoFactorRequiredMessage)} diff --git a/src/app/(main)/teams/[teamId]/TeamTwoFactorSettings.tsx b/src/app/(main)/teams/[teamId]/TeamTwoFactorSettings.tsx index b77a4b030..3431431a8 100644 --- a/src/app/(main)/teams/[teamId]/TeamTwoFactorSettings.tsx +++ b/src/app/(main)/teams/[teamId]/TeamTwoFactorSettings.tsx @@ -13,7 +13,9 @@ export function TeamTwoFactorSettings({ teamId }: { teamId: string }) { const queryClient = useQueryClient(); const { data: globalStatus } = useTwoFactorStatusQuery(true); - const isGlobalRequired = globalStatus?.requiredReason === 'global'; + const isConfigured = globalStatus?.isConfigured; + const isGlobalRequired = globalStatus?.globalRequired ?? false; + const showConfigurationError = isConfigured === false; const { data: teamData } = useTeamQuery(teamId); const { mutateAsync: setTeamRequired } = useUpdateQuery(`/admin/teams/${teamId}/2fa`); @@ -25,31 +27,35 @@ export function TeamTwoFactorSettings({ teamId }: { teamId: string }) { queryClient.invalidateQueries({ queryKey: ['teams', { teamId }] }); }; + const isToggleDisabled = isGlobalRequired || (!isConfigured && !twoFactorRequired); + const toggle = ( + + + {t(labels.twoFactorRequireTeam)} + + ); + return ( {t(labels.twoFactorAuth)} + {showConfigurationError && ( + + {t(messages.twoFactorErrorNotConfigured)} + + )} + {isGlobalRequired ? ( - - - {t(labels.twoFactorRequireTeam)} - + {toggle} {t(labels.twoFactorGlobalActiveTooltip)} ) : ( - - - {t(labels.twoFactorRequireTeam)} - + toggle )} diff --git a/src/app/api/2fa/disable/route.test.ts b/src/app/api/2fa/disable/route.test.ts new file mode 100644 index 000000000..b183b1be5 --- /dev/null +++ b/src/app/api/2fa/disable/route.test.ts @@ -0,0 +1,76 @@ +import { beforeEach, expect, test, vi } from 'vitest'; + +const mocks = vi.hoisted(() => ({ + parseRequest: vi.fn(), + isTwoFactorConfigured: vi.fn(), +})); + +vi.mock('@/lib/request', () => ({ + parseRequest: mocks.parseRequest, +})); + +vi.mock('@/lib/prisma', () => ({ + default: { + client: {}, + }, +})); + +vi.mock('@/lib/two-factor/crypto', () => ({ + decryptSecret: vi.fn(), + getTwoFactorConfigurationError: () => ({ + code: 'two-factor-error-not-configured', + message: 'TWO_FACTOR_ENCRYPTION_KEY is missing or invalid', + }), + isTwoFactorConfigured: mocks.isTwoFactorConfigured, +})); + +vi.mock('@/lib/two-factor/rate-limit', () => ({ + checkRateLimit: vi.fn(), + recordFailedAttempt: vi.fn(), + resetRateLimit: vi.fn(), +})); + +vi.mock('@/lib/two-factor/replay-prevention', () => ({ + isOtpReplayed: vi.fn(), + markOtpUsed: vi.fn(), +})); + +vi.mock('@/lib/two-factor/totp', () => ({ + verifyTotp: vi.fn(), +})); + +vi.mock('@/lib/password', () => ({ + checkPassword: vi.fn(), +})); + +vi.mock('@/queries/prisma/user', () => ({ + getUser: vi.fn(), +})); + +import { POST } from './route'; + +beforeEach(() => { + mocks.parseRequest.mockReset(); + mocks.isTwoFactorConfigured.mockReset(); + + mocks.isTwoFactorConfigured.mockReturnValue(true); +}); + +test('POST returns a configuration error after validating the caller', async () => { + mocks.isTwoFactorConfigured.mockReturnValue(false); + mocks.parseRequest.mockResolvedValue({ + auth: { user: { id: 'user-1' } }, + body: { password: 'secret', token: '123456' }, + error: undefined, + }); + + const response = await POST(new Request('http://localhost/api/2fa/disable', { method: 'POST' })); + + expect(mocks.parseRequest).toHaveBeenCalled(); + await expect(response.json()).resolves.toMatchObject({ + error: { + code: 'two-factor-error-not-configured', + }, + }); + expect(response.status).toBe(503); +}); diff --git a/src/app/api/2fa/disable/route.ts b/src/app/api/2fa/disable/route.ts index 27e60ca97..640abdbc0 100644 --- a/src/app/api/2fa/disable/route.ts +++ b/src/app/api/2fa/disable/route.ts @@ -1,8 +1,8 @@ import { z } from 'zod'; import prisma from '@/lib/prisma'; import { parseRequest } from '@/lib/request'; -import { badRequest, forbidden, json, notFound } from '@/lib/response'; -import { decryptSecret } from '@/lib/two-factor/crypto'; +import { badRequest, forbidden, json, notFound, serviceUnavailable } from '@/lib/response'; +import { decryptSecret, getTwoFactorConfigurationError, isTwoFactorConfigured } from '@/lib/two-factor/crypto'; import { checkRateLimit, recordFailedAttempt, resetRateLimit } from '@/lib/two-factor/rate-limit'; import { isOtpReplayed, markOtpUsed } from '@/lib/two-factor/replay-prevention'; import { verifyTotp } from '@/lib/two-factor/totp'; @@ -25,6 +25,10 @@ export async function POST(request: Request) { return error(); } + if (!isTwoFactorConfigured()) { + return serviceUnavailable(getTwoFactorConfigurationError()); + } + const userId = auth.user.id; const { password, token } = body; diff --git a/src/app/api/2fa/setup/confirm/route.test.ts b/src/app/api/2fa/setup/confirm/route.test.ts index 1dfefbfa2..43325dfb2 100644 --- a/src/app/api/2fa/setup/confirm/route.test.ts +++ b/src/app/api/2fa/setup/confirm/route.test.ts @@ -19,6 +19,7 @@ const mocks = vi.hoisted(() => { tx, generateBackupCodes: vi.fn(), decryptSecret: vi.fn(), + isTwoFactorConfigured: vi.fn(), checkRateLimit: vi.fn(), recordFailedAttempt: vi.fn(), resetRateLimit: vi.fn(), @@ -49,6 +50,11 @@ vi.mock('@/lib/two-factor/backup-codes', () => ({ vi.mock('@/lib/two-factor/crypto', () => ({ decryptSecret: mocks.decryptSecret, + getTwoFactorConfigurationError: () => ({ + code: 'two-factor-error-not-configured', + message: 'TWO_FACTOR_ENCRYPTION_KEY is missing or invalid', + }), + isTwoFactorConfigured: mocks.isTwoFactorConfigured, })); vi.mock('@/lib/two-factor/rate-limit', () => ({ @@ -75,6 +81,7 @@ beforeEach(() => { mocks.tx.twoFactorBackupCode.createMany.mockReset(); mocks.generateBackupCodes.mockReset(); mocks.decryptSecret.mockReset(); + mocks.isTwoFactorConfigured.mockReset(); mocks.checkRateLimit.mockReset(); mocks.recordFailedAttempt.mockReset(); mocks.resetRateLimit.mockReset(); @@ -94,6 +101,7 @@ beforeEach(() => { hashed: ['hash-1', 'hash-2'], }); mocks.decryptSecret.mockReturnValue('plain-secret'); + mocks.isTwoFactorConfigured.mockReturnValue(true); mocks.checkRateLimit.mockResolvedValue({ allowed: true }); mocks.recordFailedAttempt.mockResolvedValue({ lockedUntil: undefined }); mocks.resetRateLimit.mockResolvedValue(undefined); @@ -134,6 +142,23 @@ test('POST confirms setup, enables 2FA, stores backup codes, and resets the rate 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/confirm', { method: 'POST' }), + ); + + expect(mocks.checkRateLimit).not.toHaveBeenCalled(); + expect(mocks.decryptSecret).not.toHaveBeenCalled(); + await expect(response.json()).resolves.toMatchObject({ + error: { + code: 'two-factor-error-not-configured', + }, + }); + expect(response.status).toBe(503); +}); + test('POST records a failed attempt and skips writes when the token is invalid', async () => { mocks.verifyTotp.mockResolvedValue(false); diff --git a/src/app/api/2fa/setup/confirm/route.ts b/src/app/api/2fa/setup/confirm/route.ts index ca3a6a6db..3b4987138 100644 --- a/src/app/api/2fa/setup/confirm/route.ts +++ b/src/app/api/2fa/setup/confirm/route.ts @@ -1,9 +1,9 @@ import { z } from 'zod'; import prisma from '@/lib/prisma'; import { parseRequest } from '@/lib/request'; -import { badRequest, json, notFound } from '@/lib/response'; +import { badRequest, json, notFound, serviceUnavailable } from '@/lib/response'; import { generateBackupCodes } from '@/lib/two-factor/backup-codes'; -import { decryptSecret } from '@/lib/two-factor/crypto'; +import { decryptSecret, getTwoFactorConfigurationError, isTwoFactorConfigured } from '@/lib/two-factor/crypto'; import { checkRateLimit, recordFailedAttempt, resetRateLimit } from '@/lib/two-factor/rate-limit'; import { isOtpReplayed, markOtpUsed } from '@/lib/two-factor/replay-prevention'; import { verifyTotp } from '@/lib/two-factor/totp'; @@ -21,6 +21,10 @@ export async function POST(request: Request) { return error(); } + if (!isTwoFactorConfigured()) { + return serviceUnavailable(getTwoFactorConfigurationError()); + } + const userId = auth.user.id; const { token } = body; diff --git a/src/app/api/2fa/setup/initiate/route.test.ts b/src/app/api/2fa/setup/initiate/route.test.ts index 055cb8d3f..018aeacb5 100644 --- a/src/app/api/2fa/setup/initiate/route.test.ts +++ b/src/app/api/2fa/setup/initiate/route.test.ts @@ -34,6 +34,10 @@ vi.mock('@/lib/prisma', () => ({ vi.mock('@/lib/two-factor/crypto', () => ({ encryptSecret: mocks.encryptSecret, + getTwoFactorConfigurationError: () => ({ + code: 'two-factor-error-not-configured', + message: 'TWO_FACTOR_ENCRYPTION_KEY is missing or invalid', + }), isTwoFactorConfigured: mocks.isTwoFactorConfigured, })); diff --git a/src/app/api/2fa/setup/initiate/route.ts b/src/app/api/2fa/setup/initiate/route.ts index 2ad68da61..ed98db21f 100644 --- a/src/app/api/2fa/setup/initiate/route.ts +++ b/src/app/api/2fa/setup/initiate/route.ts @@ -1,6 +1,10 @@ import { parseRequest } from '@/lib/request'; import { badRequest, json, notFound, serviceUnavailable } from '@/lib/response'; -import { encryptSecret, isTwoFactorConfigured } from '@/lib/two-factor/crypto'; +import { + encryptSecret, + getTwoFactorConfigurationError, + isTwoFactorConfigured, +} from '@/lib/two-factor/crypto'; import { generateOtpAuthUri, generateQrCodeDataUrl, @@ -22,10 +26,7 @@ export async function POST(request: Request) { // 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', - }); + return serviceUnavailable(getTwoFactorConfigurationError()); } const userId = auth.user.id; diff --git a/src/app/api/2fa/status/route.test.ts b/src/app/api/2fa/status/route.test.ts index c1e4f090d..42522809b 100644 --- a/src/app/api/2fa/status/route.test.ts +++ b/src/app/api/2fa/status/route.test.ts @@ -1,9 +1,15 @@ 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); +const mocks = vi.hoisted(() => ({ + findTwoFactorAuth: vi.fn(), + findAppSetting: vi.fn(), + findUser: vi.fn(), + findTeamUsers: vi.fn(), + findTeams: vi.fn(), +})); vi.mock('@/lib/request', () => ({ parseRequest: vi.fn(), @@ -13,35 +19,34 @@ vi.mock('@/lib/prisma', () => ({ default: { client: { twoFactorAuth: { - findUnique: vi.fn(), + findUnique: mocks.findTwoFactorAuth, }, appSetting: { - findUnique: vi.fn(), + findUnique: mocks.findAppSetting, }, user: { - findUnique: vi.fn(), + findUnique: mocks.findUser, }, teamUser: { - findMany: vi.fn(), + findMany: mocks.findTeamUsers, }, team: { - findMany: vi.fn(), + findMany: mocks.findTeams, }, }, }, })); 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(); + mocks.findTwoFactorAuth.mockReset(); + mocks.findAppSetting.mockReset(); + mocks.findUser.mockReset(); + mocks.findTeamUsers.mockReset(); + mocks.findTeams.mockReset(); parseRequestMock.mockResolvedValue({ auth: { @@ -51,14 +56,14 @@ beforeEach(() => { }, error: undefined, }); - prismaMock.client.twoFactorAuth.findUnique.mockResolvedValue(null as any); - prismaMock.client.appSetting.findUnique.mockResolvedValue({ + mocks.findTwoFactorAuth.mockResolvedValue(null as any); + mocks.findAppSetting.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); + mocks.findUser.mockResolvedValue({ twoFactorRequired: false } as any); + mocks.findTeamUsers.mockResolvedValue([] as any); + mocks.findTeams.mockResolvedValue([] as any); }); test('GET requires 2FA when it is globally enabled', async () => { @@ -69,6 +74,8 @@ test('GET requires 2FA when it is globally enabled', async () => { await expect(response.json()).resolves.toEqual({ isEnabled: false, isRequired: true, + isConfigured: true, + globalRequired: true, requiredReason: 'global', }); expect(response.status).toBe(200); @@ -79,10 +86,11 @@ test('GET does not require 2FA when the encryption key is missing', async () => 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, + isConfigured: false, + globalRequired: true, 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 57aa781bf..6666d142d 100644 --- a/src/app/api/2fa/status/route.ts +++ b/src/app/api/2fa/status/route.ts @@ -11,7 +11,13 @@ export async function GET(request: Request) { } if (process.env.CLOUD_MODE) { - return json({ isEnabled: false, isRequired: false, requiredReason: null }); + return json({ + isEnabled: false, + isRequired: false, + requiredReason: null, + isConfigured: false, + globalRequired: false, + }); } const userId = auth.user.id; @@ -19,17 +25,30 @@ 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' }, }); const isGlobalRequired = globalSetting?.value === 'true'; - if (isGlobalRequired) return json({ isEnabled, isRequired: true, requiredReason: 'global' }); + + // 2FA cannot be set up without an encryption key, so it is never required + if (!isTwoFactorConfigured()) { + return json({ + isEnabled, + isRequired: false, + requiredReason: null, + isConfigured: false, + globalRequired: isGlobalRequired, + }); + } + if (isGlobalRequired) { + return json({ + isEnabled, + isRequired: true, + requiredReason: 'global', + isConfigured: true, + globalRequired: true, + }); + } // Required for this user const userRecord = await prisma.client.user.findUnique({ @@ -37,7 +56,15 @@ export async function GET(request: Request) { select: { twoFactorRequired: true }, }); const isUserRequired = userRecord?.twoFactorRequired ?? false; - if (isUserRequired) return json({ isEnabled, isRequired: true, requiredReason: 'user' }); + if (isUserRequired) { + return json({ + isEnabled, + isRequired: true, + requiredReason: 'user', + isConfigured: true, + globalRequired: false, + }); + } // Required for this user's teams const userTeams = await prisma.client.teamUser.findMany({ where: { userId } }); @@ -48,7 +75,21 @@ export async function GET(request: Request) { }) : []; const isTeamRequired = teamsWithRequirement.length > 0; - if (isTeamRequired) return json({ isEnabled, isRequired: true, requiredReason: 'team' }); + if (isTeamRequired) { + return json({ + isEnabled, + isRequired: true, + requiredReason: 'team', + isConfigured: true, + globalRequired: false, + }); + } - return json({ isEnabled, isRequired: false, requiredReason: null }); + return json({ + isEnabled, + isRequired: false, + requiredReason: null, + isConfigured: true, + globalRequired: false, + }); } diff --git a/src/app/api/2fa/verify/route.test.ts b/src/app/api/2fa/verify/route.test.ts index c9c63f331..650d98fb9 100644 --- a/src/app/api/2fa/verify/route.test.ts +++ b/src/app/api/2fa/verify/route.test.ts @@ -12,6 +12,7 @@ const mocks = vi.hoisted(() => ({ updateBackupCodes: vi.fn(), verifyBackupCode: vi.fn(), decryptSecret: vi.fn(), + isTwoFactorConfigured: vi.fn(), checkRateLimit: vi.fn(), recordFailedAttempt: vi.fn(), resetRateLimit: vi.fn(), @@ -60,6 +61,11 @@ vi.mock('@/lib/two-factor/backup-codes', () => ({ vi.mock('@/lib/two-factor/crypto', () => ({ decryptSecret: mocks.decryptSecret, + getTwoFactorConfigurationError: () => ({ + code: 'two-factor-error-not-configured', + message: 'TWO_FACTOR_ENCRYPTION_KEY is missing or invalid', + }), + isTwoFactorConfigured: mocks.isTwoFactorConfigured, })); vi.mock('@/lib/two-factor/rate-limit', () => ({ @@ -97,6 +103,7 @@ beforeEach(() => { mocks.updateBackupCodes.mockReset(); mocks.verifyBackupCode.mockReset(); mocks.decryptSecret.mockReset(); + mocks.isTwoFactorConfigured.mockReset(); mocks.checkRateLimit.mockReset(); mocks.recordFailedAttempt.mockReset(); mocks.resetRateLimit.mockReset(); @@ -118,6 +125,7 @@ beforeEach(() => { mocks.findTwoFactorAuth.mockResolvedValue({ userId: 'user-1', isEnabled: true, secret: 'encrypted' }); mocks.createSecureToken.mockReturnValue('full-auth-token'); mocks.decryptSecret.mockReturnValue('plain-secret'); + mocks.isTwoFactorConfigured.mockReturnValue(true); mocks.checkRateLimit.mockResolvedValue({ allowed: true }); mocks.recordFailedAttempt.mockResolvedValue({ lockedUntil: undefined }); mocks.resetRateLimit.mockResolvedValue(undefined); @@ -153,3 +161,26 @@ test('POST accepts a token-only payload and completes 2FA verification', async ( }); expect(response.status).toBe(200); }); + +test('POST returns a configuration error when the encryption key is missing', async () => { + mocks.isTwoFactorConfigured.mockReturnValue(false); + + const response = await POST( + new Request('http://localhost/api/2fa/verify', { + method: 'POST', + headers: { + 'Content-Type': 'application/json', + Authorization: 'Bearer partial-token', + }, + body: JSON.stringify({ token: '123456' }), + }), + ); + + expect(mocks.findTwoFactorAuth).not.toHaveBeenCalled(); + await expect(response.json()).resolves.toMatchObject({ + error: { + code: 'two-factor-error-not-configured', + }, + }); + expect(response.status).toBe(503); +}); diff --git a/src/app/api/2fa/verify/route.ts b/src/app/api/2fa/verify/route.ts index 14f45f7a3..b3c92fad1 100644 --- a/src/app/api/2fa/verify/route.ts +++ b/src/app/api/2fa/verify/route.ts @@ -5,9 +5,9 @@ import { ROLES } from '@/lib/constants'; import { secret } from '@/lib/crypto'; import { createSecureToken, parseSecureToken } from '@/lib/jwt'; import { parseRequest } from '@/lib/request'; -import { badRequest, json, notFound, unauthorized } from '@/lib/response'; +import { badRequest, json, notFound, serviceUnavailable, unauthorized } from '@/lib/response'; import { verifyBackupCode } from '@/lib/two-factor/backup-codes'; -import { decryptSecret } from '@/lib/two-factor/crypto'; +import { decryptSecret, getTwoFactorConfigurationError, isTwoFactorConfigured } from '@/lib/two-factor/crypto'; import { checkRateLimit, recordFailedAttempt, resetRateLimit } from '@/lib/two-factor/rate-limit'; import { isOtpReplayed, markOtpUsed } from '@/lib/two-factor/replay-prevention'; import { verifyTotp } from '@/lib/two-factor/totp'; @@ -34,6 +34,10 @@ export async function POST(request: Request) { return unauthorized({ code: 'two-factor-error-invalid-partial-token' }); } + if (!isTwoFactorConfigured()) { + return serviceUnavailable(getTwoFactorConfigurationError()); + } + const { body, error } = await parseRequest(request, schema, { skipAuth: true }); if (error) { return error(); diff --git a/src/app/api/admin/2fa/global/route.ts b/src/app/api/admin/2fa/global/route.ts index 142b89e6e..96f64be67 100644 --- a/src/app/api/admin/2fa/global/route.ts +++ b/src/app/api/admin/2fa/global/route.ts @@ -1,7 +1,8 @@ import { z } from 'zod'; import prisma from '@/lib/prisma'; import { parseRequest } from '@/lib/request'; -import { json, notFound, unauthorized } from '@/lib/response'; +import { json, notFound, serviceUnavailable, unauthorized } from '@/lib/response'; +import { getTwoFactorConfigurationError, isTwoFactorConfigured } from '@/lib/two-factor/crypto'; import { canEnforceTwoFactorAuthForEveryone } from '@/permissions'; export async function POST(request: Request) { @@ -23,6 +24,10 @@ export async function POST(request: Request) { const { required } = body; + if (required && !isTwoFactorConfigured()) { + return serviceUnavailable(getTwoFactorConfigurationError()); + } + await prisma.client.appSetting.upsert({ where: { key: 'twoFactorRequiredGlobal' }, update: { value: String(required) }, diff --git a/src/app/api/admin/teams/[teamId]/2fa/route.ts b/src/app/api/admin/teams/[teamId]/2fa/route.ts index 58cb2aab1..306c4011c 100644 --- a/src/app/api/admin/teams/[teamId]/2fa/route.ts +++ b/src/app/api/admin/teams/[teamId]/2fa/route.ts @@ -1,7 +1,8 @@ import { z } from 'zod'; import prisma from '@/lib/prisma'; import { parseRequest } from '@/lib/request'; -import { json, notFound, unauthorized } from '@/lib/response'; +import { json, notFound, serviceUnavailable, unauthorized } from '@/lib/response'; +import { getTwoFactorConfigurationError, isTwoFactorConfigured } from '@/lib/two-factor/crypto'; import { canEnforceTwoFactorAuthForTeam } from '@/permissions'; export async function POST(request: Request, { params }: { params: Promise<{ teamId: string }> }) { @@ -24,6 +25,10 @@ export async function POST(request: Request, { params }: { params: Promise<{ tea } const { required } = body; + if (required && !isTwoFactorConfigured()) { + return serviceUnavailable(getTwoFactorConfigurationError()); + } + await prisma.client.team.update({ where: { id: teamId }, data: { twoFactorRequired: required }, diff --git a/src/app/api/admin/users/[userId]/2fa/route.test.ts b/src/app/api/admin/users/[userId]/2fa/route.test.ts index 763e6e0fa..ed70f24e5 100644 --- a/src/app/api/admin/users/[userId]/2fa/route.test.ts +++ b/src/app/api/admin/users/[userId]/2fa/route.test.ts @@ -2,6 +2,7 @@ import { beforeEach, expect, test, vi } from 'vitest'; import { canEnforceTwoFactorAuthForUser } from '@/permissions'; import prisma from '@/lib/prisma'; import { parseRequest } from '@/lib/request'; +import { isTwoFactorConfigured } from '@/lib/two-factor/crypto'; import { updateUser } from '@/queries/prisma/user'; import { DELETE, GET, POST } from './route'; @@ -17,6 +18,14 @@ vi.mock('@/queries/prisma/user', () => ({ updateUser: vi.fn(), })); +vi.mock('@/lib/two-factor/crypto', () => ({ + getTwoFactorConfigurationError: () => ({ + code: 'two-factor-error-not-configured', + message: 'TWO_FACTOR_ENCRYPTION_KEY is missing or invalid', + }), + isTwoFactorConfigured: vi.fn(), +})); + vi.mock('@/lib/prisma', () => ({ default: { client: { @@ -40,12 +49,14 @@ vi.mock('@/lib/prisma', () => ({ const parseRequestMock = vi.mocked(parseRequest); const canEnforceTwoFactorAuthForUserMock = vi.mocked(canEnforceTwoFactorAuthForUser); +const isTwoFactorConfiguredMock = vi.mocked(isTwoFactorConfigured); const updateUserMock = vi.mocked(updateUser); const prismaMock = vi.mocked(prisma, true); beforeEach(() => { parseRequestMock.mockReset(); canEnforceTwoFactorAuthForUserMock.mockReset(); + isTwoFactorConfiguredMock.mockReset(); updateUserMock.mockReset(); prismaMock.client.twoFactorAuth.findUnique.mockReset(); prismaMock.client.twoFactorAuth.deleteMany.mockReset(); @@ -64,6 +75,7 @@ beforeEach(() => { error: undefined, }); canEnforceTwoFactorAuthForUserMock.mockResolvedValue(true); + isTwoFactorConfiguredMock.mockReturnValue(true); }); test('GET returns whether 2FA is enabled for the target user', async () => { @@ -108,6 +120,34 @@ test('POST updates the user-level 2FA requirement flag', async () => { expect(response.status).toBe(200); }); +test('POST rejects enabling a user-level 2FA requirement when the encryption key is missing', async () => { + parseRequestMock.mockResolvedValue({ + auth: { + user: { + id: 'admin-1', + isAdmin: true, + }, + }, + body: { + required: true, + }, + error: undefined, + }); + isTwoFactorConfiguredMock.mockReturnValue(false); + + const response = await POST(new Request('http://localhost/api/admin/users/user-1/2fa', { method: 'POST' }), { + params: Promise.resolve({ userId: 'user-1' }), + }); + + expect(updateUserMock).not.toHaveBeenCalled(); + await expect(response.json()).resolves.toMatchObject({ + error: { + code: 'two-factor-error-not-configured', + }, + }); + expect(response.status).toBe(503); +}); + test('DELETE clears the user 2FA configuration and related support tables', async () => { prismaMock.client.twoFactorAuth.deleteMany.mockResolvedValue({ count: 1 } as any); prismaMock.client.twoFactorBackupCode.deleteMany.mockResolvedValue({ count: 8 } as any); diff --git a/src/app/api/admin/users/[userId]/2fa/route.ts b/src/app/api/admin/users/[userId]/2fa/route.ts index c2565a875..32828d1b4 100644 --- a/src/app/api/admin/users/[userId]/2fa/route.ts +++ b/src/app/api/admin/users/[userId]/2fa/route.ts @@ -1,7 +1,8 @@ import { z } from 'zod'; import prisma from '@/lib/prisma'; import { parseRequest } from '@/lib/request'; -import { json, notFound, unauthorized } from '@/lib/response'; +import { json, notFound, serviceUnavailable, unauthorized } from '@/lib/response'; +import { getTwoFactorConfigurationError, isTwoFactorConfigured } from '@/lib/two-factor/crypto'; import { updateUser } from '@/queries/prisma/user'; import { canEnforceTwoFactorAuthForUser } from '@/permissions'; @@ -47,6 +48,10 @@ export async function POST(request: Request, { params }: { params: Promise<{ use const { userId } = await params; const { required } = body; + if (required && !isTwoFactorConfigured()) { + return serviceUnavailable(getTwoFactorConfigurationError()); + } + const user = await updateUser(userId, { twoFactorRequired: required }); return json({ ok: true, userId: user.id, twoFactorRequired: required }); diff --git a/src/app/api/auth/login/route.test.ts b/src/app/api/auth/login/route.test.ts new file mode 100644 index 000000000..649bcb135 --- /dev/null +++ b/src/app/api/auth/login/route.test.ts @@ -0,0 +1,108 @@ +import { beforeEach, expect, test, vi } from 'vitest'; + +const mocks = vi.hoisted(() => ({ + parseRequest: vi.fn(), + getUserByUsername: vi.fn(), + checkPassword: vi.fn(), + findTwoFactorAuth: vi.fn(), + createSecureToken: vi.fn(), + secret: vi.fn(), + hash: vi.fn(), + getAllUserTeams: vi.fn(), + saveAuth: vi.fn(), + isTwoFactorConfigured: vi.fn(), +})); + +vi.mock('@/lib/request', () => ({ + parseRequest: mocks.parseRequest, +})); + +vi.mock('@/queries/prisma', () => ({ + getAllUserTeams: mocks.getAllUserTeams, + getUserByUsername: mocks.getUserByUsername, +})); + +vi.mock('@/lib/password', () => ({ + checkPassword: mocks.checkPassword, +})); + +vi.mock('@/lib/prisma', () => ({ + default: { + client: { + twoFactorAuth: { + findUnique: mocks.findTwoFactorAuth, + }, + }, + }, +})); + +vi.mock('@/lib/jwt', () => ({ + createSecureToken: mocks.createSecureToken, +})); + +vi.mock('@/lib/crypto', () => ({ + hash: mocks.hash, + secret: mocks.secret, +})); + +vi.mock('@/lib/auth', () => ({ + saveAuth: mocks.saveAuth, +})); + +vi.mock('@/lib/redis', () => ({ + default: { + enabled: false, + }, +})); + +vi.mock('@/lib/two-factor/crypto', () => ({ + getTwoFactorConfigurationError: () => ({ + code: 'two-factor-error-not-configured', + message: 'TWO_FACTOR_ENCRYPTION_KEY is missing or invalid', + }), + isTwoFactorConfigured: mocks.isTwoFactorConfigured, +})); + +import { POST } from './route'; + +beforeEach(() => { + mocks.parseRequest.mockReset(); + mocks.getUserByUsername.mockReset(); + mocks.checkPassword.mockReset(); + mocks.findTwoFactorAuth.mockReset(); + mocks.createSecureToken.mockReset(); + mocks.secret.mockReset(); + mocks.hash.mockReset(); + mocks.getAllUserTeams.mockReset(); + mocks.saveAuth.mockReset(); + mocks.isTwoFactorConfigured.mockReset(); + + mocks.parseRequest.mockResolvedValue({ + body: { username: 'alice', password: 'secret' }, + error: undefined, + }); + mocks.getUserByUsername.mockResolvedValue({ + id: 'user-1', + username: 'alice', + password: 'hashed-password', + role: 'admin', + createdAt: new Date('2026-08-01T00:00:00.000Z'), + }); + mocks.checkPassword.mockReturnValue(true); + mocks.findTwoFactorAuth.mockResolvedValue({ userId: 'user-1', isEnabled: true }); + mocks.isTwoFactorConfigured.mockReturnValue(true); +}); + +test('POST returns a configuration error instead of partial auth when 2FA is enabled but unavailable', async () => { + mocks.isTwoFactorConfigured.mockReturnValue(false); + + const response = await POST(new Request('http://localhost/api/auth/login', { method: 'POST' })); + + expect(mocks.createSecureToken).not.toHaveBeenCalled(); + await expect(response.json()).resolves.toMatchObject({ + error: { + code: 'two-factor-error-not-configured', + }, + }); + expect(response.status).toBe(503); +}); diff --git a/src/app/api/auth/login/route.ts b/src/app/api/auth/login/route.ts index 208717f17..bb63e0fba 100644 --- a/src/app/api/auth/login/route.ts +++ b/src/app/api/auth/login/route.ts @@ -7,7 +7,8 @@ import { checkPassword } from '@/lib/password'; import prisma from '@/lib/prisma'; import redis from '@/lib/redis'; import { parseRequest } from '@/lib/request'; -import { json, unauthorized } from '@/lib/response'; +import { json, serviceUnavailable, unauthorized } from '@/lib/response'; +import { getTwoFactorConfigurationError, isTwoFactorConfigured } from '@/lib/two-factor/crypto'; import { getAllUserTeams, getUserByUsername } from '@/queries/prisma'; export async function POST(request: Request) { @@ -39,6 +40,10 @@ export async function POST(request: Request) { : null; if (twoFactor?.isEnabled) { + if (!isTwoFactorConfigured()) { + return serviceUnavailable(getTwoFactorConfigurationError()); + } + const partialToken = createSecureToken({ userId: id, type: 'partial-auth' }, secret(), { expiresIn: '5m', }); diff --git a/src/components/hooks/queries/useTwoFactorStatusQuery.ts b/src/components/hooks/queries/useTwoFactorStatusQuery.ts index 17720fece..2f09feed0 100644 --- a/src/components/hooks/queries/useTwoFactorStatusQuery.ts +++ b/src/components/hooks/queries/useTwoFactorStatusQuery.ts @@ -1,9 +1,17 @@ import { useApi } from '../useApi'; +export interface TwoFactorStatusData { + isEnabled: boolean; + isRequired: boolean; + isConfigured: boolean; + globalRequired: boolean; + requiredReason: 'global' | 'user' | 'team' | null; +} + export function useTwoFactorStatusQuery(enabled: boolean) { const { get, useQuery } = useApi(); - return useQuery({ + return useQuery({ queryKey: ['2fa-status'], queryFn: () => get('/2fa/status'), enabled, diff --git a/src/lib/two-factor/crypto.ts b/src/lib/two-factor/crypto.ts index 097c8b26a..52fba6db8 100644 --- a/src/lib/two-factor/crypto.ts +++ b/src/lib/two-factor/crypto.ts @@ -2,6 +2,10 @@ import { createCipheriv, createDecipheriv, randomBytes } from 'crypto'; const ALGORITHM = 'aes-256-gcm'; const KEY_REGEX = /^[0-9a-fA-F]{64}$/; +const TWO_FACTOR_CONFIGURATION_ERROR = { + code: 'two-factor-error-not-configured', + message: 'TWO_FACTOR_ENCRYPTION_KEY is missing or invalid', +} as const; /** Checks that TWO_FACTOR_ENCRYPTION_KEY holds a valid 256-bit key, without throwing. */ export function isTwoFactorConfigured(): boolean { @@ -9,10 +13,14 @@ export function isTwoFactorConfigured(): boolean { return !!hex && KEY_REGEX.test(hex); } +export function getTwoFactorConfigurationError() { + return TWO_FACTOR_CONFIGURATION_ERROR; +} + /** Loads and validates the 256-bit encryption key from TWO_FACTOR_ENCRYPTION_KEY. */ function getKey(): Buffer { if (!isTwoFactorConfigured()) { - throw new Error('TWO_FACTOR_ENCRYPTION_KEY is missing or invalid'); + throw new Error(TWO_FACTOR_CONFIGURATION_ERROR.message); } return Buffer.from(process.env.TWO_FACTOR_ENCRYPTION_KEY, 'hex'); }