From ce683aa0e3883e673a301c27a38d94583750e2cf Mon Sep 17 00:00:00 2001 From: Gildas Garcia <1122076+djhi@users.noreply.github.com> Date: Mon, 21 Sep 2026 15:50:19 +0200 Subject: [PATCH] Recovery Codes: improve code format (#50646) ## Problem 1. Recovery are displayed as returned by the backend 2. Recovery codes are not displayed even if present when more than 1 MFA is set up ## Solution 1. Format them as uppercased groups of 4 characters 3. Fix the condition check to display recovery codes ## How to test - Generate or regenerate your recovery codes: check the format is correct - If you haven't already, add a 2nd MFA: check recovery codes are still displayed ## Summary by CodeRabbit - **Improvements** - Recovery codes are now displayed in uppercase with clear hyphen-separated groups. - Copied recovery codes use the same formatted presentation for easier sharing and entry. - The recovery codes section is available whenever recovery codes are enabled, regardless of the number of authenticator apps configured. - Codes that do not match the expected format remain unchanged. - **Tests** - Added coverage to verify consistent recovery code formatting and clipboard behavior. --- .../GenerateRecoveryCodesModal.test.tsx | 27 ++++++++++++++----- .../TOTPFactors/RecoveryCodesModal.tsx | 14 ++++++---- .../RecoveryCodesModal.utils.test.ts | 12 +++++++++ .../TOTPFactors/RecoveryCodesModal.utils.ts | 11 ++++++++ .../RegenerateRecoveryCodesModal.test.tsx | 27 ++++++++++++++----- .../interfaces/Account/TOTPFactors/index.tsx | 2 +- 6 files changed, 75 insertions(+), 18 deletions(-) create mode 100644 apps/studio/components/interfaces/Account/TOTPFactors/RecoveryCodesModal.utils.test.ts create mode 100644 apps/studio/components/interfaces/Account/TOTPFactors/RecoveryCodesModal.utils.ts diff --git a/apps/studio/components/interfaces/Account/TOTPFactors/GenerateRecoveryCodesModal.test.tsx b/apps/studio/components/interfaces/Account/TOTPFactors/GenerateRecoveryCodesModal.test.tsx index 4ac820f96a1..562ca916e2a 100644 --- a/apps/studio/components/interfaces/Account/TOTPFactors/GenerateRecoveryCodesModal.test.tsx +++ b/apps/studio/components/interfaces/Account/TOTPFactors/GenerateRecoveryCodesModal.test.tsx @@ -3,6 +3,7 @@ import { fireEvent, screen, waitFor } from '@testing-library/react' import { describe, expect, test, vi } from 'vitest' import { GenerateRecoveryCodesModal } from './GenerateRecoveryCodesModal' +import { formatRecoveryCode } from './RecoveryCodesModal.utils' import { auth } from '@/lib/gotrue' import { customRender } from '@/tests/lib/custom-render' @@ -16,7 +17,18 @@ vi.mock('ui', async (importOriginal) => ({ copyToClipboard: mockCopyToClipboard, })) -const codes = Array.from(Array(10).keys()).map((i) => `code_${i}`) +const codes = [ + 'wto24t5xbeulvjmi', + 'ade6in2sufndbbwd', + 'oge3npsrhrr66k25', + 'pxbkfvxlbi6ggvnf', + 'kc3oaioqdjn4htc3', + '363gseoknplambiy', + '3urgae2p2jegem4m', + 'tbxog2guayp6uvud', + 'rpcvcf4owbclxrfp', + 'kyyfsp3eqjydj53t', +] describe('GenerateRecoveryCodesModal', () => { test('generate the recovery codes and allow users to copy them', async () => { @@ -34,8 +46,8 @@ describe('GenerateRecoveryCodesModal', () => { // Codes are generated await screen.findByText('Save your recovery codes') - await screen.findByText('code_0') - await screen.findByText('code_9') + await screen.findByText('WTO2-4T5X-BEUL-VJMI') + await screen.findByText('KYYF-SP3E-QJYD-J53T') // Users have to copy the codes to close the modal, next click should fail if they managed to close it expect(await screen.findAllByRole('button', { name: 'Close' })).toHaveLength(1) @@ -44,7 +56,10 @@ describe('GenerateRecoveryCodesModal', () => { await waitFor(() => expect(screen.getByRole('checkbox', { name: 'I have copied the codes' })).toBeChecked() ) - expect(mockCopyToClipboard).toHaveBeenCalledWith(codes.join('\n'), expect.any(Function)) + expect(mockCopyToClipboard).toHaveBeenCalledWith( + codes.map((code) => formatRecoveryCode(code)).join('\n'), + expect.any(Function) + ) // We should have 2 close buttons (header icon and a standard button) expect(await screen.findAllByRole('button', { name: 'Close' })).toHaveLength(2) @@ -82,7 +97,7 @@ describe('GenerateRecoveryCodesModal', () => { fireEvent.click(await screen.findByRole('button', { name: 'Generate recovery codes' })) // Codes are generated await screen.findByText('Save your recovery codes') - await screen.findByText('code_0') - await screen.findByText('code_9') + await screen.findByText('WTO2-4T5X-BEUL-VJMI') + await screen.findByText('KYYF-SP3E-QJYD-J53T') }) }) diff --git a/apps/studio/components/interfaces/Account/TOTPFactors/RecoveryCodesModal.tsx b/apps/studio/components/interfaces/Account/TOTPFactors/RecoveryCodesModal.tsx index 4efec68cf56..c16cb52bfa3 100644 --- a/apps/studio/components/interfaces/Account/TOTPFactors/RecoveryCodesModal.tsx +++ b/apps/studio/components/interfaces/Account/TOTPFactors/RecoveryCodesModal.tsx @@ -14,6 +14,7 @@ import { DialogTitle, } from 'ui' +import { formatRecoveryCode } from './RecoveryCodesModal.utils' import { recoveryCodeKeys } from '@/data/recovery-codes/keys' interface RecoveryCodesModalProps @@ -82,10 +83,13 @@ export const RecoveryCodesModal = ({ - copyToClipboard(mutation.data?.codes.join('\n') ?? '', () => { - setCopiedToClipboard(true) - setCopied(true) - }) + copyToClipboard( + mutation.data?.codes.map((code) => formatRecoveryCode(code)).join('\n') ?? '', + () => { + setCopiedToClipboard(true) + setCopied(true) + } + ) } > Copy to clipboard @@ -139,7 +143,7 @@ const GenerateRecoveryCodesModalContent = ({ {codes?.map((code) => ( - {code} + {formatRecoveryCode(code)} ))} diff --git a/apps/studio/components/interfaces/Account/TOTPFactors/RecoveryCodesModal.utils.test.ts b/apps/studio/components/interfaces/Account/TOTPFactors/RecoveryCodesModal.utils.test.ts new file mode 100644 index 00000000000..1ffe070ebd2 --- /dev/null +++ b/apps/studio/components/interfaces/Account/TOTPFactors/RecoveryCodesModal.utils.test.ts @@ -0,0 +1,12 @@ +import { describe, expect, test } from 'vitest' + +import { formatRecoveryCode } from './RecoveryCodesModal.utils' + +describe('formatRecoveryCode', () => { + test('transforms a backend code to a human readable format', () => { + expect(formatRecoveryCode('p6pcl32vl6q6l6nt')).toEqual('P6PC-L32V-L6Q6-L6NT') + }) + test('returns the code unchanged if it does not match the expected format', () => { + expect(formatRecoveryCode('bazinga')).toEqual('bazinga') + }) +}) diff --git a/apps/studio/components/interfaces/Account/TOTPFactors/RecoveryCodesModal.utils.ts b/apps/studio/components/interfaces/Account/TOTPFactors/RecoveryCodesModal.utils.ts new file mode 100644 index 00000000000..3f4e499ac69 --- /dev/null +++ b/apps/studio/components/interfaces/Account/TOTPFactors/RecoveryCodesModal.utils.ts @@ -0,0 +1,11 @@ +// Codes are 4 groups of alphanumeric characters +const CodeRegex = new RegExp('([a-zA-Z0-9]{4})([a-zA-Z0-9]{4})([a-zA-Z0-9]{4})([a-zA-Z0-9]{4})') + +/* + * Format recovery code to make it easier to read by humans by separating the 4 groups of alphanumeric characters by a + * dash and transforming them to uppercase + */ +export const formatRecoveryCode = (code: string) => { + if (code.length !== 16) return code + return code.toUpperCase().replace(CodeRegex, '$1-$2-$3-$4') +} diff --git a/apps/studio/components/interfaces/Account/TOTPFactors/RegenerateRecoveryCodesModal.test.tsx b/apps/studio/components/interfaces/Account/TOTPFactors/RegenerateRecoveryCodesModal.test.tsx index bc12a9f8181..85836832e08 100644 --- a/apps/studio/components/interfaces/Account/TOTPFactors/RegenerateRecoveryCodesModal.test.tsx +++ b/apps/studio/components/interfaces/Account/TOTPFactors/RegenerateRecoveryCodesModal.test.tsx @@ -3,6 +3,7 @@ import { fireEvent, screen, waitFor, within } from '@testing-library/react' import userEvent from '@testing-library/user-event' import { describe, expect, test, vi } from 'vitest' +import { formatRecoveryCode } from './RecoveryCodesModal.utils' import { RegenerateRecoveryCodesModal } from './RegenerateRecoveryCodesModal' import { auth } from '@/lib/gotrue' import { customRender } from '@/tests/lib/custom-render' @@ -17,7 +18,18 @@ vi.mock('ui', async (importOriginal) => ({ copyToClipboard: mockCopyToClipboard, })) -const codes = Array.from(Array(10).keys()).map((i) => `code_${i}`) +const codes = [ + 'wto24t5xbeulvjmi', + 'ade6in2sufndbbwd', + 'oge3npsrhrr66k25', + 'pxbkfvxlbi6ggvnf', + 'kc3oaioqdjn4htc3', + '363gseoknplambiy', + '3urgae2p2jegem4m', + 'tbxog2guayp6uvud', + 'rpcvcf4owbclxrfp', + 'kyyfsp3eqjydj53t', +] describe('RegenerateRecoveryCodesModal', () => { test('regenerate the recovery codes after confirmation and allow users to copy them', async () => { @@ -46,8 +58,8 @@ describe('RegenerateRecoveryCodesModal', () => { // Codes are generated await screen.findByText('Save your recovery codes') - await screen.findByText('code_0') - await screen.findByText('code_9') + await screen.findByText('WTO2-4T5X-BEUL-VJMI') + await screen.findByText('KYYF-SP3E-QJYD-J53T') // Users have to copy the codes to close the modal, next click should fail if they managed to close it expect(await screen.findAllByRole('button', { name: 'Close' })).toHaveLength(1) @@ -56,7 +68,10 @@ describe('RegenerateRecoveryCodesModal', () => { await waitFor(() => expect(screen.getByRole('checkbox', { name: 'I have copied the codes' })).toBeChecked() ) - expect(mockCopyToClipboard).toHaveBeenCalledWith(codes.join('\n'), expect.any(Function)) + expect(mockCopyToClipboard).toHaveBeenCalledWith( + codes.map((code) => formatRecoveryCode(code)).join('\n'), + expect.any(Function) + ) // We should have 2 close buttons (header icon and a standard button) expect(await screen.findAllByRole('button', { name: 'Close' })).toHaveLength(2) @@ -115,7 +130,7 @@ describe('RegenerateRecoveryCodesModal', () => { ) // Codes are generated await screen.findByText('Save your recovery codes') - await screen.findByText('code_0') - await screen.findByText('code_9') + await screen.findByText('WTO2-4T5X-BEUL-VJMI') + await screen.findByText('KYYF-SP3E-QJYD-J53T') }) }) diff --git a/apps/studio/components/interfaces/Account/TOTPFactors/index.tsx b/apps/studio/components/interfaces/Account/TOTPFactors/index.tsx index e328a2824e5..5bf6bba9cb1 100644 --- a/apps/studio/components/interfaces/Account/TOTPFactors/index.tsx +++ b/apps/studio/components/interfaces/Account/TOTPFactors/index.tsx @@ -34,7 +34,7 @@ export const TOTPFactors = () => { const totpFactors = data?.totp ?? [] const canAddApp = isSuccess && totpFactors.length < 2 const shouldShowLockoutWarning = isSuccess && totpFactors.length === 1 - const shouldVerifyRecoveryCodes = enableAuthRecoveryCodes && totpFactors.length === 1 + const shouldVerifyRecoveryCodes = enableAuthRecoveryCodes const { data: recoveryCodesStatus } = useRecoveryCodesStatusQuery({ enabled: shouldVerifyRecoveryCodes,
{codes?.map((code) => ( - {code} + {formatRecoveryCode(code)} ))}