fix(settings): serve the account recovery key as a plain text file - #21003
fix(settings): serve the account recovery key as a plain text file#21003vpomerleau wants to merge 1 commit into
Conversation
There was a problem hiding this comment.
Pull request overview
This PR removes client-side recovery key PDF generation in fxa-settings and switches the download to a plain text file, unblocking the React 19 upgrade while avoiding the upstream @react-pdf/* text corruption + WASM/CSP concerns.
Changes:
- Add a small browser download helper and a new
ButtonDownloadRecoveryKeycomponent that downloads the key as.txt, shows inline failure UI, and tags clicks viadata-glean-id. - Replace PDF download usage at the three call sites and update Jest + Playwright tests accordingly.
- Remove
@react-pdf/*,file-saver,pdf-parse, and the bundled Noto font assets.
Reviewed changes
Copilot reviewed 37 out of 67 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| package.json | Drops @react-pdf/* resolution pins now that PDF rendering is removed. |
| packages/functional-tests/package.json | Removes pdf-parse and its types from functional test deps. |
| packages/functional-tests/tests/resetPassword/oauthResetPasswordRecoveryKey.spec.ts | Updates expected downloaded filename extension to .txt. |
| packages/functional-tests/tests/resetPassword/resetPasswordRecoveryKey.spec.ts | Updates expected downloaded filename extension to .txt. |
| packages/functional-tests/tests/settings/recoveryKey.spec.ts | Validates text-file download contents instead of parsing a PDF. |
| packages/fxa-settings/package.json | Removes @react-pdf/renderer, file-saver, and @types/file-saver. |
| packages/fxa-settings/src/components/ButtonDownloadRecoveryKey/en.ftl | New localized strings/comments for the text download button + error. |
| packages/fxa-settings/src/components/ButtonDownloadRecoveryKey/index.stories.tsx | Renames story to the new text-download button component. |
| packages/fxa-settings/src/components/ButtonDownloadRecoveryKey/index.test.tsx | Adds unit tests for download, error banner, Sentry reporting, and filename logic. |
| packages/fxa-settings/src/components/ButtonDownloadRecoveryKey/index.tsx | New button component that triggers text download + inline error handling + Glean tag. |
| packages/fxa-settings/src/components/ButtonDownloadRecoveryKeyPDF/RecoveryKeyPDF.tsx | Removes PDF document generation component. |
| packages/fxa-settings/src/components/ButtonDownloadRecoveryKeyPDF/VectorImagesForPdf.tsx | Removes inline SVG assets used only for PDF rendering. |
| packages/fxa-settings/src/components/ButtonDownloadRecoveryKeyPDF/snapshots/index.test.tsx.snap | Removes snapshots for deleted PDF button component. |
| packages/fxa-settings/src/components/ButtonDownloadRecoveryKeyPDF/en.ftl | Removes PDF-specific fluent strings. |
| packages/fxa-settings/src/components/ButtonDownloadRecoveryKeyPDF/fonts/NotoSans-Bold.ttf | Removes bundled PDF font asset. |
| packages/fxa-settings/src/components/ButtonDownloadRecoveryKeyPDF/fonts/NotoSans-Regular.ttf | Removes bundled PDF font asset. |
| packages/fxa-settings/src/components/ButtonDownloadRecoveryKeyPDF/fonts/NotoSans-SemiBold.ttf | Removes bundled PDF font asset. |
| packages/fxa-settings/src/components/ButtonDownloadRecoveryKeyPDF/fonts/NotoSansGeorgian-Bold.ttf | Removes bundled PDF font asset. |
| packages/fxa-settings/src/components/ButtonDownloadRecoveryKeyPDF/fonts/NotoSansGeorgian-Regular.ttf | Removes bundled PDF font asset. |
| packages/fxa-settings/src/components/ButtonDownloadRecoveryKeyPDF/fonts/NotoSansGeorgian-SemiBold.ttf | Removes bundled PDF font asset. |
| packages/fxa-settings/src/components/ButtonDownloadRecoveryKeyPDF/fonts/NotoSansHebrew-Bold.ttf | Removes bundled PDF font asset. |
| packages/fxa-settings/src/components/ButtonDownloadRecoveryKeyPDF/fonts/NotoSansHebrew-Regular.ttf | Removes bundled PDF font asset. |
| packages/fxa-settings/src/components/ButtonDownloadRecoveryKeyPDF/fonts/NotoSansHebrew-SemiBold.ttf | Removes bundled PDF font asset. |
| packages/fxa-settings/src/components/ButtonDownloadRecoveryKeyPDF/fonts/NotoSansJP-Bold.ttf | Removes bundled PDF font asset. |
| packages/fxa-settings/src/components/ButtonDownloadRecoveryKeyPDF/fonts/NotoSansJP-Regular.ttf | Removes bundled PDF font asset. |
| packages/fxa-settings/src/components/ButtonDownloadRecoveryKeyPDF/fonts/NotoSansJP-SemiBold.ttf | Removes bundled PDF font asset. |
| packages/fxa-settings/src/components/ButtonDownloadRecoveryKeyPDF/fonts/NotoSansKR-Bold.otf | Removes bundled PDF font asset. |
| packages/fxa-settings/src/components/ButtonDownloadRecoveryKeyPDF/fonts/NotoSansKR-Medium.otf | Removes bundled PDF font asset. |
| packages/fxa-settings/src/components/ButtonDownloadRecoveryKeyPDF/fonts/NotoSansKR-Regular.otf | Removes bundled PDF font asset. |
| packages/fxa-settings/src/components/ButtonDownloadRecoveryKeyPDF/fonts/NotoSansMono-SemiBold.ttf | Removes bundled PDF font asset. |
| packages/fxa-settings/src/components/ButtonDownloadRecoveryKeyPDF/fonts/NotoSansSC-Bold.otf | Removes bundled PDF font asset. |
| packages/fxa-settings/src/components/ButtonDownloadRecoveryKeyPDF/fonts/NotoSansSC-Medium.otf | Removes bundled PDF font asset. |
| packages/fxa-settings/src/components/ButtonDownloadRecoveryKeyPDF/fonts/NotoSansSC-Regular.otf | Removes bundled PDF font asset. |
| packages/fxa-settings/src/components/ButtonDownloadRecoveryKeyPDF/fonts/NotoSansTC-Bold.otf | Removes bundled PDF font asset. |
| packages/fxa-settings/src/components/ButtonDownloadRecoveryKeyPDF/fonts/NotoSansTC-Medium.otf | Removes bundled PDF font asset. |
| packages/fxa-settings/src/components/ButtonDownloadRecoveryKeyPDF/fonts/NotoSansTC-Regular.otf | Removes bundled PDF font asset. |
| packages/fxa-settings/src/components/ButtonDownloadRecoveryKeyPDF/fonts/NotoSansThai-Bold.ttf | Removes bundled PDF font asset. |
| packages/fxa-settings/src/components/ButtonDownloadRecoveryKeyPDF/fonts/NotoSansThai-Regular.ttf | Removes bundled PDF font asset. |
| packages/fxa-settings/src/components/ButtonDownloadRecoveryKeyPDF/fonts/NotoSansThai-SemiBold.ttf | Removes bundled PDF font asset. |
| packages/fxa-settings/src/components/ButtonDownloadRecoveryKeyPDF/fonts/NotoSerifGurmukhi-Bold.ttf | Removes bundled PDF font asset. |
| packages/fxa-settings/src/components/ButtonDownloadRecoveryKeyPDF/fonts/NotoSerifGurmukhi-Regular.ttf | Removes bundled PDF font asset. |
| packages/fxa-settings/src/components/ButtonDownloadRecoveryKeyPDF/fonts/NotoSerifGurmukhi-SemiBold.ttf | Removes bundled PDF font asset. |
| packages/fxa-settings/src/components/ButtonDownloadRecoveryKeyPDF/fonts/README.md | Removes PDF-font documentation (no longer relevant). |
| packages/fxa-settings/src/components/ButtonDownloadRecoveryKeyPDF/fonts/fonts.d.ts | Removes font module declarations used only by PDF code. |
| packages/fxa-settings/src/components/ButtonDownloadRecoveryKeyPDF/index.test.tsx | Removes unit tests for deleted PDF download button component. |
| packages/fxa-settings/src/components/ButtonDownloadRecoveryKeyPDF/index.tsx | Removes the PDF download button implementation. |
| packages/fxa-settings/src/components/ButtonDownloadRecoveryKeyPDF/requiredFont.test.tsx | Removes tests for PDF font selection logic. |
| packages/fxa-settings/src/components/ButtonDownloadRecoveryKeyPDF/requiredFont.tsx | Removes PDF font selection logic. |
| packages/fxa-settings/src/components/DataBlock/index.test.tsx | Stops per-test URL.createObjectURL mocking now that it’s in setup. |
| packages/fxa-settings/src/components/DataBlockInline/index.test.tsx | Stops per-test URL.createObjectURL mocking now that it’s in setup. |
| packages/fxa-settings/src/components/GetDataTrio/index.test.tsx | Stops per-test URL.createObjectURL mocking now that it’s in setup. |
| packages/fxa-settings/src/components/RecoveryKeySetupDownload/snapshots/index.test.tsx.snap | Updates snapshot for the new Glean data attribute on the button. |
| packages/fxa-settings/src/components/RecoveryKeySetupDownload/index.test.tsx | Removes async waiting previously needed for lazy PDF button load. |
| packages/fxa-settings/src/components/RecoveryKeySetupDownload/index.tsx | Switches to the new text-download button component. |
| packages/fxa-settings/src/components/Settings/FlowRecoveryKeyDownload/index.test.tsx | Updates flow test now that legacy metrics call is removed from download click. |
| packages/fxa-settings/src/components/Settings/FlowSetup2faBackupCodeDownload/index.test.tsx | Stops per-test URL.createObjectURL mocking now that it’s in setup. |
| packages/fxa-settings/src/components/Settings/Page2faReplaceBackupCodes/index.test.tsx | Stops per-test URL.createObjectURL mocking now that it’s in setup. |
| packages/fxa-settings/src/components/Settings/PageDeleteAccount/index.test.tsx | Stops per-test URL.createObjectURL mocking now that it’s in setup. |
| packages/fxa-settings/src/components/Settings/PageRecoveryKeyCreate/index.test.tsx | Removes @react-pdf/renderer mocks and per-test URL mocking. |
| packages/fxa-settings/src/lib/download.test.ts | Adds focused unit tests for the new download helper behavior. |
| packages/fxa-settings/src/lib/download.ts | Adds downloadTextFile helper using Blob + object URL + anchor click. |
| packages/fxa-settings/src/pages/ConnectAnotherDevice/index.test.tsx | Stops per-test URL.createObjectURL mocking now that it’s in setup. |
| packages/fxa-settings/src/pages/InlineRecoverySetupFlow/index.test.tsx | Stops per-test URL.createObjectURL mocking now that it’s in setup. |
| packages/fxa-settings/src/pages/InlineTotpSetup/index.test.tsx | Stops per-test URL.createObjectURL mocking now that it’s in setup. |
| packages/fxa-settings/src/pages/ResetPassword/ResetPasswordWithRecoveryKeyVerified/index.tsx | Uses the new text-download button instead of lazy-loading the PDF button. |
| packages/fxa-settings/src/setupTests.tsx | Adds global URL object URL mocks so tests don’t silently take failure paths. |
| yarn.lock | Removes transitive dependencies related to react-pdf, pdf parsing, and fonts. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Because:
* @react-pdf/textkit drops source-character mapping when fontkit
returns a glyph with empty codePoints, corrupting non-Latin text
in the generated PDF. The upstream fix is unreviewed.
* Any WASM-based PDF renderer needs a wasm-unsafe-eval CSP
exception, which we will not add on an auth surface.
This commit:
* Replaces PDF generation with a text/plain download of the key.
Plain text cannot express bidi or shaping, so the file holds the
key alone; the guidance the PDF carried is already on the download
screen, apart from the support link, which is dropped.
* Adds a downloadTextFile util. The object URL is revoked a task
after the click, since revoking within it can abort the download.
* Reports a failed download in an inline banner and stays on the
screen. <AlertBar /> is only mounted by SettingsLayout, so on the
two screens rendering under AppLayout the old alert went nowhere,
and navigating forward hid it on all three. The key, the error and
the retry now stay together, and "Continue without downloading"
is still there for a persistent failure.
* Sends the failure to Sentry. The catch still fires on synchronous
faults, and the key is the only way back into encrypted Sync data.
* Sanitises the email before using it as a filename; FxA permits "/"
in a local part.
* Records the click through Glean via data-glean-id instead of the
legacy Amplitude logViewEvent, which also retires the viewName
prop. A failed download can no longer be detected client-side, so
there is no success/failure event.
* Removes @react-pdf/renderer, file-saver, pdf-parse, both
@react-pdf resolution pins, and 74 MB of bundled Noto fonts.
* Renames ButtonDownloadRecoveryKeyPDF to ButtonDownloadRecoveryKey
and drops the lazy/Suspense wrappers the font bundle required.
* Stubs URL.createObjectURL/revokeObjectURL in setupTests; jsdom
implements neither, so download tests silently ran the error path.
Drops the per-file stubs this replaces, most of which returned
undefined and so shadowed it with a worse one.
Closes: FXA-14334
256c724 to
aabf370
Compare
vbudhram
left a comment
There was a problem hiding this comment.
@vpomerleau Bummer, but this does work on desktop. I haven't tested on mobile but don't think there will be issues.
| }; | ||
| getPDF(filename); | ||
| const contents = await fs.promises.readFile(filename, 'utf8'); | ||
| expect(contents).toBe(newKey); |
|
|
||
| # Button to download the account recovery key as a plain text file and navigate to the next step | ||
| # The next (and final) step is an optional prompt to save a storage hint | ||
| recovery-key-download-button-v3 = Download and continue |
There was a problem hiding this comment.
Does this still need a .title attribute? If so it seems it was removed. If not, then we'll need to update the string ID since adding/removing an attribute requires a new ID.
Because
@react-pdf/renderer@3.2.1, which does not support React 19.@react-pdf/textkit6.x, which corrupts non-Latin text — recovery key PDFs are unreadable in the eight locales that ship bundled fonts, and the upstream fix is unmerged.wasm-unsafe-evalCSP exception we will not add to an auth surface.This pull request
.txtholding the key alone; the guidance the PDF carried is already on the download screen.packages/fxa-settings/src/lib/download.tsand renamesButtonDownloadRecoveryKeyPDFtoButtonDownloadRecoveryKey.useAlertBar, which never rendered on two of the three screens.data-glean-id, replacing the legacy AmplitudelogViewEventcalls.@react-pdf/*,file-saverandpdf-parse(37 packages) plus 74 MB of bundled Noto fonts.Issue that this pull request solves
Closes: FXA-14334
Checklist
Put an
xin the boxes that applyHow to review (Optional)
lib/download.ts,ButtonDownloadRecoveryKey/index.tsxScreenshots (Optional)
Please attach the screenshots of the changes made in case of change in user interface.
Other information (Optional)
Any other information that is important to this pull request.