Skip to content

Commit f871e7e

Browse files
committed
Review 2FA QR generation.
1 parent 60f2ace commit f871e7e

4 files changed

Lines changed: 22 additions & 6 deletions

File tree

TODO.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -88,7 +88,7 @@ Defence-in-depth work to land shortly after launch.
8888
- [x] **CSP violation reporting.** PHP exposed `api/csp_report.php` and logged breaches.
8989
- [x] Added `report-uri /api/csp-report` and `report-to csp-endpoint` to both CSP lines in `public/_headers`, plus a `Reporting-Endpoints` header naming `csp-endpoint` for the modern Reporting API.
9090
- [x] Added `functions/api/csp-report.ts` — a DB-free, unauthenticated collector that parses both the legacy `{ "csp-report": … }` body and the modern `application/reports+json` array, logs each violation (`console.warn`, capped per request), and returns `204`.
91-
- [ ] **Review 2FA QR generation.** Consider a more secure method for generating QR codes — `functions/api/db/auth/2fa/setup/start.ts:132`.
91+
- [x] **Review 2FA QR generation.** The QR code was generated by sending the `otpauth://` URI — which embeds the TOTP secret — to the third-party `api.qrserver.com`, leaking every user's 2FA seed. Now rendered in the Worker as an inline `<svg>` via `uqr` (zero-dependency, Workers-native); the secret never leaves the origin. Inline SVG is page markup, so it is also unaffected by the page's `default-src https:` CSP (a `data:` URI would not be).
9292
- [ ] **2FA bypass flow.** Replace the password-reset fallback with a dedicated email-based flow to bypass 2FA — `public/2fa.html:56`.
9393

9494
## Phase 3 — Account & password parity

functions/api/db/auth/2fa/setup/start.ts

Lines changed: 11 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
import { generateSecret, generateURI } from "otplib"
2+
import { renderSVG } from "uqr"
23
import { read2fa, create2fa } from "../../../../../../src/2fa.js"
34
import { readUser } from "../../../../../../src/users.js"
45

@@ -121,21 +122,28 @@ export const onRequestPost: Handler = async (context) => {
121122
)
122123
}
123124

124-
// Step 6: Generate QR Code Data (TOTP Auth URI)
125+
// Step 6: Generate the TOTP auth URI and render it to a QR code.
126+
// The URI embeds the TOTP secret, so the QR code is rendered here, in the
127+
// Worker, as an inline SVG — the secret is never sent to a third-party
128+
// image service. Inline SVG is page markup, not a fetched resource, so it
129+
// is also unaffected by the page's `default-src https:` CSP.
125130
const otpauthUri = generateURI({
126131
issuer: APP_NAME,
127132
label: userName,
128133
secret: display_secret,
129134
})
135+
const qrCodeSvg = renderSVG(otpauthUri, { border: 2 }).replace(
136+
"<svg",
137+
'<svg class="tfa-qr-code" width="200" height="200" role="img" aria-label="Two-factor authentication QR code"'
138+
)
130139

131140
// Step 7: Response - HTML for HTMX
132-
// TODO: [Security] Consider using a more secure method for generating QR codes
133141
const htmlResponse = `
134142
<div>
135143
<h3>Setup Two-Factor Authentication</h3>
136144
<p>Scan the QR code with your authenticator app or enter the setup code manually.</p>
137145
<div class="tfa-qr-layout">
138-
<img src="https://api.qrserver.com/v1/create-qr-code/?size=200x200&data=${encodeURIComponent(otpauthUri)}" alt="QR Code" class="tfa-qr-code"/>
146+
${qrCodeSvg}
139147
<div>
140148
<p><strong>Manual Setup Code:</strong></p>
141149
<p class="tfa-secret-display">${display_secret}</p>

package-lock.json

Lines changed: 8 additions & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

package.json

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,8 @@
1212
},
1313
"dependencies": {
1414
"otplib": "^13.4.0",
15-
"pg": "^8.20.0"
15+
"pg": "^8.20.0",
16+
"uqr": "^0.1.3"
1617
},
1718
"devDependencies": {
1819
"@types/node": "^25.8.0",

0 commit comments

Comments
 (0)