Repository navigation
feat(auth): show a QR code during TOTP enrolment - #2269
Merged
Merged
Conversation
Co-authored-by: Cursor <cursoragent@cursor.com>
|
Review the following changes in direct dependencies. Learn more about Socket for GitHub.
|
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #1826
Description
Problem
Setting up an authenticator for a server-store account meant typing the key or opening the
otpauth://link on the same device.beginTotpEnrolment(src/lib/local-accounts.ts:860-872) already returnsotpauthUrlto the browser, so the missing piece was rendering, not data.docs/MFA.mdalso said Studio never mints a QR, which is only true for the environment-variable path.Approach
In step 1 of
src/components/auth/AuthenticatorSettings.tsxI renderQRCodeSVG(qrcode.react4.2.0) from theotpauthUrlreturned bybegin, only when it is present. It sits in abg-white p-3container, black on white, withrole="img"and the label "Scan to add LibreDB to your authenticator app". The key, Copy button and setup link are unchanged. The hint mentions scanning only when a QR is shown. No new endpoint, network request or stored state.docs/MFA.mdnow scopes the "does not mint one" paragraph to the environment-variable path and says server-store accounts get a QR under Sign-in security.sequenceDiagram participant U as User participant C as AuthenticatorSettings participant R as POST /api/auth/totp participant L as beginTotpEnrolment U->>C: password, Set up authenticator C->>R: action begin R->>L: email, body L-->>R: secret, otpauthUrl R-->>C: secret, otpauthUrl C->>C: render key, link, QRCodeSVG(otpauthUrl) U->>C: Confirm code or Cancel C->>C: enrolment state cleared, QR unmountedDesign decisions & trade-offs
otpauthUrlas asked, so the QR and the key cannot disagree.role="img"on the SVG needsoxlint-disable-next-line jsx-a11y/prefer-tag-over-role. This matches 8 existing jsx-a11y suppressions insrc/.nullforgetContext("2d"). The component tests mockqrcode.reactwith a stub that forwards props and adds no role or label, so those attributes must come from the component.e2e/totp-enrolment-qr.spec.tsrepeats the sign-in helpers frome2e/passkey.spec.tsinstead of extracting them, which keeps an unrelated spec out of this diff. I can extract them in a follow-up.Type of Change
Testing
New tests in
tests/components/auth/AuthenticatorSettings.test.tsx, written first and failing before the change:e2e/totp-enrolment-qr.spec.tsruns on thechromium-passkeyproject. It screenshots the QR and decodes the PNG in Node withjsqrandpngjs(e2e/helpers/decode-enrolment-qr-png.ts). It asserts the result equals the page'sotpauthlink, once with<html>confirmed light and once confirmed dark.Results:
bun tests/run-tests.ts tests/components/auth/AuthenticatorSettings.test.tsx: 15 pass.bunx playwright test --project=chromium-passkey e2e/totp-enrolment-qr.spec.ts: 2 passed on93b2444c, rebased ontoe6852bc7. Its webServernext buildsucceeded.bun run test:coveragein a Linux container:AuthenticatorSettings.tsxhas 293 of 293 lines covered.coverage:checkthere flagged 40 lines in files this PR does not touch:sqlite.ts,duckdb/index.ts,instrumentation.tsandunwritable-file.ts.mainCI passes this gate, so I expect CI here to pass too.tar, TLS,gosu). They reproduce with identical counts onupstream/main.format,lint,typecheck,knip,chart:check,channels:showcase:check,readme:checkandsecurity:checkall exit 0.Test Environment
mcr.microsoft.com/playwright:v1.64.1-nobleSTORAGE_PROVIDER=sqlite)Screenshots
Enrolment step in light and dark mode. The key, link and QR are blurred, and I checked that neither QR decodes.
Risk & Rollback
The change is client-only. The QR encodes the same value the page already shows as the setup link, so nothing new is exposed. The new runtime dependency is
qrcode.react, which has no transitive dependencies inbun.lock.jsqr,pngjsand@types/pngjsare dev-only. To roll back, revert the two commits. There is no migration or config change.