diff --git a/.changeset/account-login-honest-lockout-message.md b/.changeset/account-login-honest-lockout-message.md new file mode 100644 index 00000000..7346670e --- /dev/null +++ b/.changeset/account-login-honest-lockout-message.md @@ -0,0 +1,9 @@ +--- +'ePDS': patch +--- + +Account-settings sign-in shows useful guidance when the code can no longer be used. + +**Affects:** End users + +**End users:** when signing in to manage your account, typing the wrong code enough times locked out the original code — but the page kept saying "Invalid or expired code. Please try again.", which implied more typing might help. The page now distinguishes that case and says "That code can no longer be used. Click 'Resend code' below to get a fresh one." pointing you at the Resend button right below the error. diff --git a/.changeset/account-settings-flash-messages.md b/.changeset/account-settings-flash-messages.md new file mode 100644 index 00000000..ed76ab31 --- /dev/null +++ b/.changeset/account-settings-flash-messages.md @@ -0,0 +1,9 @@ +--- +'ePDS': patch +--- + +Account Settings now confirms every action with a visible banner. + +**Affects:** End users + +**End users:** when you added a backup email, removed one, changed your handle, revoked a session, or hit a validation error on any of those, the page silently bounced back to the same form with no indication that anything had changed (or what went wrong). The page now shows a green "Backup email added" / "Handle updated" / etc. banner on success, and a red "That handle is not available" / "We couldn't send the verification email" / etc. banner on error — so you know whether your action took effect. diff --git a/.changeset/charset-filter-on-server-rendered-otp-forms.md b/.changeset/charset-filter-on-server-rendered-otp-forms.md new file mode 100644 index 00000000..1ac88970 --- /dev/null +++ b/.changeset/charset-filter-on-server-rendered-otp-forms.md @@ -0,0 +1,9 @@ +--- +'ePDS': patch +--- + +Account-settings and account-recovery code inputs also strip out characters that aren't part of the code. + +**Affects:** End users + +**End users:** the same paste/keystroke filter that the main sign-in form already had now applies to the standalone Account Settings sign-in and to the account recovery flow. If you copy your code from somewhere that wraps it in punctuation, or accidentally type a letter into a digits-only code, the input drops the stray characters silently instead of letting them through and rejecting the code as invalid. diff --git a/.changeset/clear-otp-boxes-on-resend.md b/.changeset/clear-otp-boxes-on-resend.md new file mode 100644 index 00000000..c77f8681 --- /dev/null +++ b/.changeset/clear-otp-boxes-on-resend.md @@ -0,0 +1,9 @@ +--- +'ePDS': patch +--- + +Clicking "Resend code" now empties the digit boxes for the new code. + +**Affects:** End users + +**End users:** if you started typing your sign-in code, realised you'd made a mistake, and clicked **Resend code**, the boxes still held the digits you had typed. You had to delete them yourself before you could enter the fresh code. The boxes now reset on Resend so you can just type the new code straight away. diff --git a/.changeset/dont-flash-invalid-otp-on-empty-submit.md b/.changeset/dont-flash-invalid-otp-on-empty-submit.md new file mode 100644 index 00000000..42344e05 --- /dev/null +++ b/.changeset/dont-flash-invalid-otp-on-empty-submit.md @@ -0,0 +1,9 @@ +--- +'ePDS': patch +--- + +Empty Verify clicks no longer flash "Invalid OTP". + +**Affects:** End users + +**End users:** clicking **Verify** before typing the code (or pressing Enter on an empty form) used to flash a red "Invalid OTP" error, which was both misleading — you didn't type an invalid code, you typed nothing — and counted against the per-account rate limit. The form now just moves the cursor into the first empty digit box and waits for you to type, without bothering anyone with a fake error. diff --git a/.changeset/fallback-app-name-on-malformed-client-id.md b/.changeset/fallback-app-name-on-malformed-client-id.md new file mode 100644 index 00000000..376284f8 --- /dev/null +++ b/.changeset/fallback-app-name-on-malformed-client-id.md @@ -0,0 +1,9 @@ +--- +'ePDS': patch +--- + +Sign-in pages no longer leak a malformed client_id as the displayed app name. + +**Affects:** End users + +**End users:** if you arrived at a sign-in page with a broken `client_id` parameter on the URL (e.g. from a misconfigured app or a tampered link), the page would display that raw broken value as the app's name, e.g. "Sign in to not-a-url" or "Sign in to my-local-app". The page now falls back to "an application" in that case, so a malformed value in the URL doesn't leak into the visible page text or browser tab title. diff --git a/.changeset/filter-paste-to-otp-charset.md b/.changeset/filter-paste-to-otp-charset.md new file mode 100644 index 00000000..5bfbe1ec --- /dev/null +++ b/.changeset/filter-paste-to-otp-charset.md @@ -0,0 +1,9 @@ +--- +'ePDS': patch +--- + +Pasted sign-in codes are no longer auto-submitted with stray punctuation or letters. + +**Affects:** End users + +**End users:** if you copied your sign-in code from somewhere that wrapped it in punctuation or extra characters (e.g. an email reading "Your code is 1234-5678"), the page used to drop the punctuation straight into the digit boxes and submit the result, which the server would then reject as an invalid code. The same was true if you typed a stray letter into a digits-only code by accident. The boxes now filter both pasted and typed input to the format the code is in (digits only, or letters and digits) — so a paste cleans itself up and a stray keystroke is silently ignored. diff --git a/.changeset/friendlier-demo-error-banners.md b/.changeset/friendlier-demo-error-banners.md new file mode 100644 index 00000000..14128b02 --- /dev/null +++ b/.changeset/friendlier-demo-error-banners.md @@ -0,0 +1,9 @@ +--- +'ePDS': patch +--- + +Demo client's error banners now use plain user-friendly language instead of developer wording. + +**Affects:** End users + +**End users:** several of the demo's sign-in error banners used technical wording aimed at developers ("PAR rejected the request — check server logs", "token exchange failed") that didn't tell you anything actionable. The wording is now plain English and points at the right next step ("Please try again in a moment", "Please sign in again"). diff --git a/.changeset/friendlier-stale-authorize-link-error.md b/.changeset/friendlier-stale-authorize-link-error.md new file mode 100644 index 00000000..940d9f17 --- /dev/null +++ b/.changeset/friendlier-stale-authorize-link-error.md @@ -0,0 +1,9 @@ +--- +'ePDS': patch +--- + +Visiting a stale sign-in link no longer leaks an internal OAuth field name. + +**Affects:** End users + +**End users:** if you arrived at the sign-in page from a stale link or a direct paste — i.e. without an active sign-in flow — the page used to say "Missing request_uri parameter", which tells you nothing useful and leaks the name of an internal OAuth field. The page now says "Sign-in has to be started from the app you are signing into. Please return to that app and try again." so you know what to do next. diff --git a/.changeset/friendlier-stale-recovery-link-error.md b/.changeset/friendlier-stale-recovery-link-error.md new file mode 100644 index 00000000..e5b6eaad --- /dev/null +++ b/.changeset/friendlier-stale-recovery-link-error.md @@ -0,0 +1,9 @@ +--- +'ePDS': patch +--- + +Visiting a stale account-recovery link no longer leaks an internal OAuth field name. + +**Affects:** End users + +**End users:** if you arrived at the account-recovery page from a stale link or a direct paste — i.e. without an active sign-in flow — the page used to say "Missing request_uri parameter", which tells you nothing useful and leaks the name of an internal OAuth field. The page now says "Account recovery has to be started from the sign-in page. Please sign in again from the app you came from." so you know what to do next. diff --git a/.changeset/handle-not-available-message.md b/.changeset/handle-not-available-message.md new file mode 100644 index 00000000..6eb0bf43 --- /dev/null +++ b/.changeset/handle-not-available-message.md @@ -0,0 +1,9 @@ +--- +'ePDS': patch +--- + +Handle-picker error wording is now accurate when the handle is reserved. + +**Affects:** End users + +**End users:** if you tried to claim a handle that was on the reserved list (admin, www, …) the picker said "That handle was just taken — please choose another." That's misleading — it wasn't just taken, it's permanently unavailable. The wording is now "That handle is not available — please choose another." which doesn't imply you can wait it out. diff --git a/.changeset/handle-picker-flags-reserved-handles.md b/.changeset/handle-picker-flags-reserved-handles.md new file mode 100644 index 00000000..b415bf1f --- /dev/null +++ b/.changeset/handle-picker-flags-reserved-handles.md @@ -0,0 +1,9 @@ +--- +'ePDS': patch +--- + +Handle picker now flags reserved handles as unavailable upfront. + +**Affects:** End users + +**End users:** the handle picker has a live availability check that says ✓ Available or ✗ Already taken as you type. For reserved handles (admin, support, www, …) it used to show ✓ Available, then refuse the handle on submit with a "not available" error — wasting the time you spent on the form. The live check now flags reserved handles as unavailable up front so you can keep going without losing what you typed. diff --git a/.changeset/hide-resend-when-sign-in-cannot-recover.md b/.changeset/hide-resend-when-sign-in-cannot-recover.md new file mode 100644 index 00000000..3bc48815 --- /dev/null +++ b/.changeset/hide-resend-when-sign-in-cannot-recover.md @@ -0,0 +1,13 @@ +--- +'ePDS': patch +--- + +Sign-in no longer offers "Resend code" when the new code wouldn't have worked anyway. + +**Affects:** End users + +**End users:** Previously, if you sat on the email-code step long enough that the underlying sign-in had silently timed out (most often: leaving the tab in the background while reading email on your phone, or coming back after an interruption), the page would still show **Resend code**. Clicking it sent you a fresh email, but the moment you typed the new code you'd see "Sign in failed" — the code was issued for a sign-in that could no longer complete, so it never had a chance. + +The page now hides the Resend button as soon as it knows the sign-in can't be recovered, and shows **Start over** in its place. Clicking Start over takes you back to the app you came from to begin again, instead of letting you waste time on a code that couldn't work. + +If you're actively using the page (the tab in the foreground), nothing changes: Resend stays available and works the same way it always has. diff --git a/.changeset/inline-resend-on-too-many-attempts.md b/.changeset/inline-resend-on-too-many-attempts.md new file mode 100644 index 00000000..99857222 --- /dev/null +++ b/.changeset/inline-resend-on-too-many-attempts.md @@ -0,0 +1,9 @@ +--- +'ePDS': patch +--- + +After too many wrong codes, the sign-in page now offers a one-click "Send a new code" instead of looking like more typing might help. + +**Affects:** End users + +**End users:** if you typed the wrong code enough times to get a "Too many attempts" lockout, the page used to leave you fighting an error that more typing couldn't fix — the underlying code had been thrown away by then, so any further attempts came back as "Invalid OTP" against nothing. The page now surfaces a **Send a new code** action right next to the error so you can get a fresh code in one click instead of hunting for the resend button or wondering if you have a typo. diff --git a/.changeset/keyboard-focus-visible-on-buttons.md b/.changeset/keyboard-focus-visible-on-buttons.md new file mode 100644 index 00000000..bf99df16 --- /dev/null +++ b/.changeset/keyboard-focus-visible-on-buttons.md @@ -0,0 +1,9 @@ +--- +'ePDS': patch +--- + +Sign-in buttons now show a visible keyboard focus ring. + +**Affects:** End users + +**End users:** keyboard users tabbing through the sign-in pages couldn't tell where focus was — the buttons styled out the browser default focus ring without replacing it. Verify, Resend, Use different email, Recover, Send recovery code, Continue with email, Update handle, etc. all now show a clear outline when focused via the keyboard, while still hiding the ring on a mouse click. diff --git a/.changeset/recovery-honest-lockout-message.md b/.changeset/recovery-honest-lockout-message.md new file mode 100644 index 00000000..4d123992 --- /dev/null +++ b/.changeset/recovery-honest-lockout-message.md @@ -0,0 +1,9 @@ +--- +'ePDS': patch +--- + +Account-recovery sign-in shows useful guidance when the code can no longer be used. + +**Affects:** End users + +**End users:** when recovering your account via a backup email, typing the wrong code enough times locked out the original code — but the page kept saying "Invalid or expired code. Please try again.", which implied more typing might help. The page now distinguishes that case and says "That code can no longer be used. Click 'Resend code' below to get a fresh one." pointing you at the Resend button right below the error, matching the standalone Account Settings sign-in flow. diff --git a/.changeset/recovery-link-uses-real-request-uri.md b/.changeset/recovery-link-uses-real-request-uri.md new file mode 100644 index 00000000..6a0b9856 --- /dev/null +++ b/.changeset/recovery-link-uses-real-request-uri.md @@ -0,0 +1,9 @@ +--- +'ePDS': patch +--- + +The "Recover with backup email" link no longer breaks the "Back to sign in" path. + +**Affects:** End users + +**End users:** clicking **Recover with backup email** on the sign-in page used to leave you on a recovery flow that couldn't return you to the original sign-in: hitting **Back to sign in** at the end landed on a "data you submitted is invalid" error page from the underlying OAuth machinery, because the link forwarded a placeholder URL instead of the real sign-in context. The link now carries the active sign-in's actual context, so the back path round-trips cleanly even if you decide not to use recovery after all. diff --git a/.changeset/sms-autofill-distributes-across-otp-boxes.md b/.changeset/sms-autofill-distributes-across-otp-boxes.md new file mode 100644 index 00000000..f5d4b94f --- /dev/null +++ b/.changeset/sms-autofill-distributes-across-otp-boxes.md @@ -0,0 +1,9 @@ +--- +'ePDS': patch +--- + +iOS and Android SMS / email autofill now fills the digit boxes correctly. + +**Affects:** End users + +**End users:** if your phone offered to autofill the sign-in code from a recent text message or email, the autofill used to put the whole code into the first box and discard everything but the last digit — leaving you to retype the rest. The boxes now distribute the autofilled code across them, the same way they do when you paste it manually. diff --git a/.changeset/spam-hint-on-otp-form.md b/.changeset/spam-hint-on-otp-form.md new file mode 100644 index 00000000..91965375 --- /dev/null +++ b/.changeset/spam-hint-on-otp-form.md @@ -0,0 +1,9 @@ +--- +'ePDS': patch +--- + +Sign-in code page hints to check spam folder when the email doesn't arrive. + +**Affects:** End users + +**End users:** the page now shows a small "If you don't see the email, check your spam folder." note below the digit boxes. If you typo'd your email or the message landed in spam, you used to wait and wait and then get frustrated; the hint surfaces the most common reason early so you can either check spam or click "Use different email" sooner. diff --git a/.changeset/use-different-email-clears-form.md b/.changeset/use-different-email-clears-form.md new file mode 100644 index 00000000..3fa6e176 --- /dev/null +++ b/.changeset/use-different-email-clears-form.md @@ -0,0 +1,9 @@ +--- +'ePDS': patch +--- + +"Use different email" on the sign-in code page now clears the email field. + +**Affects:** End users + +**End users:** clicking **Use different email** on the code-entry page used to take you back to the email form with your previous address still filled in — exactly the opposite of what the button suggests, and forcing you to clear the field manually before you could type a new address. The form now starts empty, with the cursor in the field, so you can just type the new address and continue. diff --git a/e2e/step-definitions/account-recovery.steps.ts b/e2e/step-definitions/account-recovery.steps.ts index 870d4224..81514d88 100644 --- a/e2e/step-definitions/account-recovery.steps.ts +++ b/e2e/step-definitions/account-recovery.steps.ts @@ -329,3 +329,72 @@ Then( await assertNoEmailFor(this.backupEmail) }, ) + +// --------------------------------------------------------------------------- +// Recovery-link round-trip +// --------------------------------------------------------------------------- + +Then( + "the recovery link points at the active OAuth flow's request_uri", + async function (this: EpdsWorld) { + const page = getPage(this) + // Pull the request_uri the auth-service was driving for this + // page from the URL we landed on. + const flowUrl = new URL(page.url()) + const expectedRequestUri = flowUrl.searchParams.get('request_uri') + if (!expectedRequestUri) { + throw new Error( + `Expected the current page URL to carry a request_uri so the test can compare it to the recovery link, but URL was: ${page.url()}`, + ) + } + const recoveryHref = await page + .locator('#recovery-link') + .getAttribute('href') + if (!recoveryHref) { + throw new Error('Recovery link not found on page') + } + const recoveryUrl = new URL(recoveryHref, flowUrl.origin) + const linkRequestUri = recoveryUrl.searchParams.get('request_uri') + if (linkRequestUri !== expectedRequestUri) { + throw new Error( + `Recovery link request_uri mismatch:\n expected: ${expectedRequestUri}\n got: ${linkRequestUri}`, + ) + } + }, +) + +// --------------------------------------------------------------------------- +// Stale-recovery-link UX +// --------------------------------------------------------------------------- + +When( + 'the user navigates directly to the recovery page without an active sign-in', + async function (this: EpdsWorld) { + const page = getPage(this) + await page.goto(`${testEnv.authUrl}/auth/recover`) + }, +) + +Then( + 'the page explains that recovery has to start from the sign-in page', + async function (this: EpdsWorld) { + const page = getPage(this) + await expect(page.locator('body')).toContainText( + /recovery has to be started from the sign-in page/i, + { timeout: 10_000 }, + ) + }, +) + +Then( + 'the page does not mention the technical field name {string}', + async function (this: EpdsWorld, fieldName: string) { + const page = getPage(this) + const body = (await page.locator('body').textContent()) ?? '' + if (body.toLowerCase().includes(fieldName.toLowerCase())) { + throw new Error( + `Expected the page to not surface the technical field name "${fieldName}", but its body text contained it. Page body: ${body}`, + ) + } + }, +) diff --git a/e2e/step-definitions/auth.steps.ts b/e2e/step-definitions/auth.steps.ts index 89343dba..c2e77f33 100644 --- a/e2e/step-definitions/auth.steps.ts +++ b/e2e/step-definitions/auth.steps.ts @@ -802,9 +802,18 @@ Then( async function (this: EpdsWorld) { const origin = new URL(testEnv.demoUrl).origin const page = getPage(this) - await page.waitForURL(`${origin}/?error=auth_failed*`, { - timeout: 30_000, - }) + // Both `session_expired` (timeout-shaped error_description from + // the PDS clean-exit path) and `auth_failed` (everything else) + // satisfy "an auth error" — they're both demo error codes that + // surface a banner on the landing page. Older scenarios were + // written before the demo distinguished the two; rather than + // re-tagging every one of them, accept either. + await page.waitForURL( + (url) => + url.origin === origin && + /^[?&](error=(auth_failed|session_expired))/.test(url.search), + { timeout: 30_000 }, + ) }, ) @@ -839,3 +848,239 @@ Then( } }, ) + +// --------------------------------------------------------------------------- +// Resend-button visibility (fix for the "fresh OTP wasted on dead PAR" UX) +// --------------------------------------------------------------------------- +// +// The page never offers an action that cannot complete the flow. When the +// PAR is dead, the standalone Resend button is removed from view and a +// Start over button takes its place — clicking it bails to /auth/abort +// rather than issuing an OTP that would only fail downstream. The steps +// below trigger the page's reactive ping (via the visibilitychange +// handler that fires on tab-foreground) so it can observe the dead PAR +// and reconcile the UI without a 5-minute wall-clock wait. + +When('the OTP form re-checks PAR liveness', async function (this: EpdsWorld) { + const page = getPage(this) + // Drive the page's reactive ping via a string-source script. + // Using page.evaluate(() => ...) inlines esbuild's __name helper, + // which then fails in Playwright's evaluation context with + // "ReferenceError: __name is not defined". Passing a string + // bypasses the bundler. + await page.evaluate(`(function () { + Object.defineProperty(document, 'visibilityState', { + configurable: true, + get: function () { return 'visible' }, + }) + document.dispatchEvent(new Event('visibilitychange')) + })()`) +}) + +Then( + 'the Resend code button is no longer offered', + async function (this: EpdsWorld) { + const page = getPage(this) + await expect(page.locator('#btn-resend')).toBeHidden({ timeout: 5_000 }) + }, +) + +Then( + 'a Start over button is offered instead', + async function (this: EpdsWorld) { + const page = getPage(this) + await expect(page.locator('#btn-start-over')).toBeVisible({ + timeout: 5_000, + }) + }, +) + +// --------------------------------------------------------------------------- +// Demo client cookie expiry simulation +// --------------------------------------------------------------------------- +// +// The demo client stores OAuth state (state value, codeVerifier, token +// endpoint, issuer) in a signed cookie called `oauth_state` with +// `maxAge: 600` (see packages/demo/src/app/api/oauth/login/route.ts). +// If the user spends longer than 10 minutes between starting the OAuth +// flow and the callback firing — most realistic cause: dawdling on the +// OTP form, then clicking Resend after the 10-minute mark — the cookie +// expires before the callback runs, so the callback handler can't find +// the OAuth state and silently bounces to /?error=auth_failed. +// +// This step deletes the cookie programmatically so we can exercise the +// post-cookie-expiry callback path without a 10-minute wall-clock wait. + +When( + 'the user navigates to the demo callback with code and state but no cookie', + async function (this: EpdsWorld) { + const page = getPage(this) + // Visit the demo origin first so document.cookie has the right + // origin context (needed for clearCookies to work reliably). + await page.goto(testEnv.demoUrl) + // Make sure no oauth_state cookie is present. + await page.context().clearCookies({ name: 'oauth_state' }) + // Land on /api/oauth/callback with plausible-looking query params. + // The callback's first thing it checks (after `error`) is + // !code || !state — both present here. Then it tries to read the + // signed cookie, which is gone, and routes to session_expired. + const url = new URL('/api/oauth/callback', testEnv.demoUrl) + url.searchParams.set('code', 'test-code') + url.searchParams.set('state', 'test-state') + await page.goto(url.toString()) + }, +) + +When( + "the demo client's OAuth state cookie has expired", + async function (this: EpdsWorld) { + const page = getPage(this) + const ctx = page.context() + const before = await ctx.cookies() + const target = before.find((c) => c.name === 'oauth_state') + if (!target) { + throw new Error( + `Expected to find an oauth_state cookie set by the demo client but only saw: ${before.map((c) => c.name).join(', ')}`, + ) + } + await ctx.clearCookies({ name: 'oauth_state' }) + }, +) + +Then( + 'the demo client surfaces a session-expired error', + async function (this: EpdsWorld) { + const origin = new URL(testEnv.demoUrl).origin + const page = getPage(this) + await page.waitForURL(`${origin}/?error=session_expired*`, { + timeout: 30_000, + }) + }, +) + +// --------------------------------------------------------------------------- +// "Use different email" UX +// --------------------------------------------------------------------------- + +Then('the email input is empty and focused', async function (this: EpdsWorld) { + const page = getPage(this) + const input = page.locator('#email') + await expect(input).toHaveValue('', { timeout: 5_000 }) + await expect(input).toBeFocused({ timeout: 5_000 }) +}) + +// --------------------------------------------------------------------------- +// Empty-OTP submit guard +// --------------------------------------------------------------------------- + +When( + 'the user clicks the Verify button without entering a code', + async function (this: EpdsWorld) { + const page = getPage(this) + await page.click('#form-verify-otp button[type=submit]') + }, +) + +Then('no "Invalid OTP" error is shown', async function (this: EpdsWorld) { + const page = getPage(this) + // Give any async submit a moment to land before asserting absence. + await page.waitForTimeout(1_000) + const errorText = await page.locator('#error-msg').textContent() + if (errorText && /invalid otp/i.test(errorText)) { + throw new Error( + `Expected no "Invalid OTP" error after empty submit but saw: "${errorText}"`, + ) + } +}) + +// --------------------------------------------------------------------------- +// "Too many attempts" lockout UX +// --------------------------------------------------------------------------- + +When( + 'the user submits enough wrong OTPs to trigger the lockout', + async function (this: EpdsWorld) { + const page = getPage(this) + const boxCount = await page.locator('.otp-box').count() + if (!this.testEmail) { + throw new Error( + 'No testEmail in world; the email-submit step must run first', + ) + } + // better-auth's allowedAttempts is 5 (see auth-service better-auth.ts). + // After 5 wrong submits the next call sees attempts >= 5 and throws + // TOO_MANY_ATTEMPTS while deleting the row. Burn 5 attempts via direct + // fetch (the form path is racy from the driver's side because of + // auto-submit + clearOtpBoxes interleaving), then trigger the 6th + // through the form so the submit handler's error rendering — the UX + // we want to assert on — actually fires. + const burnDigits = ['0', '1', '2', '3', '4'] + for (const digit of burnDigits) { + await page.evaluate( + `fetch('/api/auth/sign-in/email-otp', { + method: 'POST', + headers: { 'Content-Type': 'application/json' }, + body: JSON.stringify({ email: ${JSON.stringify(this.testEmail)}, otp: ${JSON.stringify(digit.repeat(boxCount))} }), + }).catch(function () {})`, + ) + } + // Final attempt through the form. + await page.evaluate(`(function () { + var boxes = document.querySelectorAll('.otp-box'); + for (var i = 0; i < boxes.length; i++) boxes[i].value = ''; + })()`) + await page.locator('.otp-box').first().focus() + await page.keyboard.type('5'.repeat(boxCount)) + await expect(page.locator('#error-msg')).toBeVisible({ timeout: 10_000 }) + }, +) + +Then( + 'a "Send a new code" inline action is offered', + async function (this: EpdsWorld) { + const page = getPage(this) + await expect( + page.locator('#error-msg button.flash-action', { + hasText: /send a new code/i, + }), + ).toBeVisible({ timeout: 5_000 }) + }, +) + +// --------------------------------------------------------------------------- +// Stale-authorize-link UX +// --------------------------------------------------------------------------- + +When( + 'the user navigates directly to the authorize page without an active sign-in', + async function (this: EpdsWorld) { + const page = getPage(this) + await page.goto(`${testEnv.authUrl}/oauth/authorize`) + }, +) + +Then( + 'the page explains that sign-in has to start from the app', + async function (this: EpdsWorld) { + const page = getPage(this) + await expect(page.locator('body')).toContainText( + /sign-in has to be started from the app/i, + { timeout: 10_000 }, + ) + }, +) + +When( + 'the user submits one more wrong OTP after the lockout', + async function (this: EpdsWorld) { + const page = getPage(this) + const boxCount = await page.locator('.otp-box').count() + await page.evaluate(`(function () { + var boxes = document.querySelectorAll('.otp-box'); + for (var i = 0; i < boxes.length; i++) boxes[i].value = ''; + })()`) + await page.locator('.otp-box').first().focus() + await page.keyboard.type('9'.repeat(boxCount)) + await expect(page.locator('#error-msg')).toBeVisible({ timeout: 10_000 }) + }, +) diff --git a/features/account-recovery.feature b/features/account-recovery.feature index 4ded73d1..0f58364d 100644 --- a/features/account-recovery.feature +++ b/features/account-recovery.feature @@ -39,6 +39,33 @@ Feature: Account recovery via backup emails Then the recovery OTP form is displayed And no email arrives for that non-existent address + # The "Recover with backup email" link on the OTP form used to + # carry a hard-coded "/placeholder" URL instead of the active + # OAuth flow's actual request_uri. The user could complete + # recovery, but clicking "Back to sign in" landed them on + # upstream's "data you submitted is invalid" error page (because + # /placeholder isn't a real PAR). This scenario asserts the link + # carries the actual request_uri so the back path round-trips + # cleanly. + @recovery-link-roundtrip + Scenario: Recovery link carries the active OAuth flow's request_uri + Given the demo client initiates OAuth with the test email as login_hint + Then an OTP email arrives in the mail trap + And the login page shows an OTP verification form + Then the recovery link points at the active OAuth flow's request_uri + + # /auth/recover requires an active sign-in flow (it carries a + # request_uri that points at the upstream PAR). Hitting the URL + # directly — typically by following a stale link or pasting from + # somewhere — used to surface "Missing request_uri parameter", + # which leaks the internal OAuth field name and tells the user + # nothing actionable. + @stale-recovery-link + Scenario: Direct visit to recovery URL surfaces a friendly explanation, not a technical field name + When the user navigates directly to the recovery page without an active sign-in + Then the page explains that recovery has to start from the sign-in page + And the page does not mention the technical field name "request_uri" + # --- Backup email management --- Scenario: User removes a backup email diff --git a/features/passwordless-authentication.feature b/features/passwordless-authentication.feature index b0692184..e17a0a0e 100644 --- a/features/passwordless-authentication.feature +++ b/features/passwordless-authentication.feature @@ -343,6 +343,176 @@ Feature: Passwordless authentication via email OTP And the user requests a new OTP via the resend button Then the browser lands back at the demo client with an auth error + # The page never offers actions that cannot complete the flow. When + # the upstream PAR has died (silent timeout, suspended tab, + # heartbeat throttling), the standalone Resend button is removed + # from view and replaced with a Start over button — so the user + # never wastes time typing a fresh OTP that could not have worked. + # This is the proactive complement to @resend-after-par-dead's + # reactive abort gate: rather than letting the click happen and + # bouncing it server-side, we surface only forward paths that can + # actually succeed. + @email @otp-and-par-expiry @resend-hidden-when-par-dead + Scenario: Resend button is hidden when the PAR has died — Start over is offered instead + When the demo client initiates an OAuth login + Then the browser is redirected to the auth service login page + And the login page displays an email input form + When the user enters a unique test email and submits + Then an OTP email arrives in the mail trap for the test email + And the login page shows an OTP verification form + When the PAR request_uri has expired before the bridge fires + And the OTP form re-checks PAR liveness + Then the Resend code button is no longer offered + And a Start over button is offered instead + + # The PDS clean-exit redirect uses error=access_denied with an + # error_description shaped like "Your sign-in took too long...". + # The demo's landing page used to map every PDS-side error to the + # same generic "auth_failed" banner, which discarded that context + # — telling the user the sign-in failed without saying *why*. + # Translating timeout-shaped descriptions to a `session_expired` + # banner gives the user the actual reason and a sensible next + # step ("Please sign in again") instead of leaving them unsure + # whether they typed the wrong code, are blocked, or hit a bug. + @email @otp-and-par-expiry @passes-through-timeout-context + Scenario: PDS timeout error_description surfaces as a session-expired banner + When the demo client initiates an OAuth login + Then the browser is redirected to the auth service login page + And the login page displays an email input form + When the user enters a unique test email and submits + Then an OTP email arrives in the mail trap for the test email + And the login page shows an OTP verification form + When the PAR request_uri has expired before the bridge fires + And the user enters the OTP code + Then the demo client surfaces a session-expired error + + # The demo OAuth client stores its OAuth state (state value, code + # verifier, token endpoint, issuer) in a signed cookie that has its + # own lifetime. If that cookie expires before the auth-service + # bridges the user back via /oauth/epds-callback (e.g. user + # dawdled on the OTP form long enough that the cookie aged out + # mid-flow), the demo's callback handler can't find the OAuth state + # and silently bounces to the demo home page with + # `?error=auth_failed`. The user has just typed a fresh OTP + # successfully, so this is genuinely time-wasting and misleading: + # the auth-service did everything right but the demo dropped the + # ball. + # + # The contract the demo MUST satisfy: as long as the OAuth flow is + # still recoverable from the auth-service side (auth_flow row + # alive, PAR alive or refreshable), the demo's session cookie must + # also be alive. This scenario reproduces the failure mode by + # programmatically clearing the demo's `oauth_state` cookie just + # before the OTP submission, which is equivalent to the cookie + # having lapsed by wall-clock. + # better-auth's email-otp plugin allows up to 3 wrong attempts on a + # single code. The 4th wrong submit returns "Too many attempts" AND + # deletes the verification row — so any further attempts on that + # same code path are doomed to fail (better-auth then returns + # "Invalid OTP" because the row no longer exists, which would mislead + # the user into thinking they have a typo). + # + # Once the user is in the too-many-attempts state, the only forward + # path is to request a fresh code. The form must surface that + # forward path inline alongside the error, the same way it does for + # OTP expiry — anything else lets the user fight an error that + # cannot be resolved by typing more carefully. + @email @too-many-attempts + Scenario: "Too many attempts" surfaces a Send-a-new-code action + When the demo client initiates an OAuth login + Then the browser is redirected to the auth service login page + And the login page displays an email input form + When the user enters a unique test email and submits + Then the login page shows an OTP verification form + When the user submits enough wrong OTPs to trigger the lockout + Then the verification form shows an "Too many attempts" error + And a "Send a new code" inline action is offered + + # After the first lockout response, better-auth has deleted the + # verification row, so any further /sign-in/email-otp call falls + # through the row-not-found path and returns "Invalid OTP" — even + # though the row is gone, not invalid. The page latches the + # lockout state so post-lockout submits inherit the same inline + # action treatment as the original "Too many attempts" response, + # rather than letting the user fight an "Invalid OTP" error that + # more typing can't possibly fix. + @email @too-many-attempts @post-lockout + Scenario: After the lockout, further submits still surface a Send-a-new-code action + When the demo client initiates an OAuth login + Then the browser is redirected to the auth service login page + And the login page displays an email input form + When the user enters a unique test email and submits + Then the login page shows an OTP verification form + When the user submits enough wrong OTPs to trigger the lockout + And the user submits one more wrong OTP after the lockout + Then a "Send a new code" inline action is offered + + # /oauth/authorize requires an active sign-in flow (it carries a + # request_uri that points at the upstream PAR). Direct visits to + # the URL — typically a stale link or a paste — used to surface + # "Missing request_uri parameter", which leaks an internal OAuth + # field name and tells the user nothing actionable. + @stale-authorize-link + Scenario: Direct visit to /oauth/authorize surfaces a friendly explanation + When the user navigates directly to the authorize page without an active sign-in + Then the page explains that sign-in has to start from the app + And the page does not mention the technical field name "request_uri" + + # Clicking Verify with no code typed used to flash "Invalid OTP", + # which is dishonest: the user didn't type an invalid code, they + # typed nothing. It also burned a real call to better-auth's + # /sign-in/email-otp endpoint, which counts against the + # rate-limiter — so a confused user who tab-clicked Verify could + # lock themselves out faster than necessary. The Verify button + # must not allow a submit until the user has typed the full code, + # and an empty submit (e.g. via Enter on a still-empty form) must + # surface a useful prompt rather than a misleading error. + @email @verify-empty-otp + Scenario: Submitting Verify with an empty code does not produce a misleading "Invalid OTP" + When the demo client initiates an OAuth login + Then the browser is redirected to the auth service login page + And the login page displays an email input form + When the user enters a unique test email and submits + Then the login page shows an OTP verification form + When the user clicks the Verify button without entering a code + Then no "Invalid OTP" error is shown + + # The "Use different email" button on the OTP step takes the user + # back to the email-entry form so they can sign in with a different + # address. The form must be EMPTY when they get there — leaving the + # prior email pre-filled is exactly the misleading "looks like the + # form remembered me" UX that the button was meant to escape from, + # and forces the user to manually clear the field before they can + # type their actual email. + @email @use-different-email + Scenario: "Use different email" returns the user to a clean email form + When the demo client initiates an OAuth login + Then the browser is redirected to the auth service login page + And the login page displays an email input form + When the user enters a unique test email and submits + Then the login page shows an OTP verification form + When the user clicks "Use different email" + Then the email input is empty and focused + + @demo-cookie-expiry @bug-report + Scenario: Demo client's OAuth callback with no oauth_state cookie surfaces a session-expired error, not generic auth_failed + # When the user has stalled long enough for the demo's OAuth state + # cookie to expire by the time they finish the OTP cycle, the + # demo callback can't find the cookie. Without the bug-report fix + # this used to surface as "Authentication failed" — misleading + # because the sign-in itself succeeded; the demo just dropped the + # ball at the end. This scenario hits the callback with valid- + # looking parameters but no cookie, asserts the session-expired + # branch fires. + # + # The scenario synthesises code+state on the URL because it's + # only testing the demo's callback branching — not the full + # sign-in flow. That avoids dependency on Mailpit (which can + # be slow on Railway preview envs and was flaking the older + # form of this scenario). + When the user navigates to the demo callback with code and state but no cookie + Then the demo client surfaces a session-expired error + @email @otp-and-par-expiry @prompt-login Scenario: prompt=login + expired PAR — clean exit back to the OAuth client Given a returning user has a PDS account diff --git a/packages/auth-service/src/__tests__/account-settings-flash.test.ts b/packages/auth-service/src/__tests__/account-settings-flash.test.ts new file mode 100644 index 00000000..5de5785d --- /dev/null +++ b/packages/auth-service/src/__tests__/account-settings-flash.test.ts @@ -0,0 +1,109 @@ +import { describe, it, expect } from 'vitest' +import { + FLASH_SUCCESS_MESSAGES, + FLASH_ERROR_MESSAGES, + resolveAccountFlashFromQuery, +} from '../routes/account-settings.js' + +describe('account-settings flash messages', () => { + // Every code referenced in a redirect from a POST handler needs an + // entry in the matching lookup, otherwise the user lands on the + // settings page with no acknowledgement of the action they took. + // These tests guard against drift — adding a new code without a + // corresponding entry would mean the redirect silently dropped. + + it('has a success message for each code redirected to from a POST handler', () => { + const expectedSuccessCodes = [ + 'backup_added', + 'backup_verified', + 'backup_removed', + 'handle_updated', + 'session_revoked', + ] + for (const code of expectedSuccessCodes) { + expect( + FLASH_SUCCESS_MESSAGES[code], + `missing FLASH_SUCCESS_MESSAGES["${code}"]`, + ).toBeTruthy() + } + }) + + it('has an error message for each code redirected to from a POST handler', () => { + const expectedErrorCodes = [ + 'invalid_email', + 'already_primary', + 'account_not_found', + 'send_failed', + 'verify_failed', + 'invalid_handle', + 'handle_failed', + 'handle_taken', + 'delete_failed', + 'confirm_delete', + ] + for (const code of expectedErrorCodes) { + expect( + FLASH_ERROR_MESSAGES[code], + `missing FLASH_ERROR_MESSAGES["${code}"]`, + ).toBeTruthy() + } + }) + + it('returns undefined for unknown codes — the GET handler treats this as "no banner"', () => { + expect(FLASH_SUCCESS_MESSAGES['attacker_injected_text']).toBeUndefined() + expect(FLASH_ERROR_MESSAGES['']).toBeUndefined() + }) + + it('all messages are plain text, no HTML', () => { + // The renderer escapeHtml's the lookup result anyway (defence in + // depth) but the values themselves shouldn't carry markup. + for (const msg of Object.values(FLASH_SUCCESS_MESSAGES)) { + expect(msg).not.toMatch(/<[a-z]/i) + } + for (const msg of Object.values(FLASH_ERROR_MESSAGES)) { + expect(msg).not.toMatch(/<[a-z]/i) + } + }) +}) + +describe('resolveAccountFlashFromQuery', () => { + it('returns the success message when the success code is known', () => { + const result = resolveAccountFlashFromQuery({ success: 'backup_added' }) + expect(result.successMessage).toBe(FLASH_SUCCESS_MESSAGES.backup_added) + expect(result.errorMessage).toBeNull() + }) + + it('returns the error message when the error code is known', () => { + const result = resolveAccountFlashFromQuery({ + success: '', + error: 'invalid_handle', + }) + expect(result.successMessage).toBeNull() + expect(result.errorMessage).toBe(FLASH_ERROR_MESSAGES.invalid_handle) + }) + + it('returns null on both sides when the query is empty', () => { + expect(resolveAccountFlashFromQuery({})).toEqual({ + successMessage: null, + errorMessage: null, + }) + }) + + it('returns null on both sides for unknown codes (safety against URL-injection of attacker text)', () => { + expect( + resolveAccountFlashFromQuery({ + success: 'attacker_chosen_text', + error: '', + }), + ).toEqual({ successMessage: null, errorMessage: null }) + }) + + it('ignores non-string query values gracefully (e.g. ?success=foo&success=bar arrays)', () => { + expect( + resolveAccountFlashFromQuery({ + success: ['backup_added', 'backup_removed'], + error: 12345, + }), + ).toEqual({ successMessage: null, errorMessage: null }) + }) +}) diff --git a/packages/auth-service/src/__tests__/heartbeat-toggle.test.ts b/packages/auth-service/src/__tests__/heartbeat-toggle.test.ts index cb05ed42..98dcc517 100644 --- a/packages/auth-service/src/__tests__/heartbeat-toggle.test.ts +++ b/packages/auth-service/src/__tests__/heartbeat-toggle.test.ts @@ -92,6 +92,7 @@ function renderLoginPageWithHeartbeat(heartbeatEnabled: boolean): string { loginHint: '', initialStep: 'email', otpAlreadySent: false, + requestUri: 'urn:ietf:params:oauth:request_uri:req-abc', csrfToken: 'csrf', authBasePath: '/api/auth', pdsPublicUrl: 'https://pds.example.com', diff --git a/packages/auth-service/src/__tests__/login-page.test.ts b/packages/auth-service/src/__tests__/login-page.test.ts index 5376f713..f5aa7be3 100644 --- a/packages/auth-service/src/__tests__/login-page.test.ts +++ b/packages/auth-service/src/__tests__/login-page.test.ts @@ -425,6 +425,7 @@ describe('renderLoginPage handle login button', () => { loginHint: '', initialStep: 'email', otpAlreadySent: false, + requestUri: 'urn:ietf:params:oauth:request_uri:test-req', csrfToken: 'csrf', authBasePath: '/api/auth', pdsPublicUrl: 'https://pds.example.com', @@ -512,6 +513,7 @@ function renderDefault(): string { loginHint: '', initialStep: 'email', otpAlreadySent: false, + requestUri: 'urn:ietf:params:oauth:request_uri:test-req', csrfToken: 'csrf', authBasePath: '/api/auth', pdsPublicUrl: 'https://pds.example.com', @@ -602,13 +604,24 @@ describe('renderLoginPage inline Resend action on expired OTP', () => { expect(fnBody).not.toContain('innerHTML') }) - it('detects OTP-expired errors via a substring-stable regex', () => { + it('detects unrecoverable verify errors via a substring-stable regex', () => { const html = renderDefault() - // The detection must catch: - // - better-auth's "Invalid or expired code" - // - auth-service's "OTP expired" - // - any future wording with "expir" or "too long" in it - expect(html).toMatch(/var isExpired = \/expir\|too long\/i\.test/) + // The detection must catch every error string that means the + // current code path is dead and the user must Resend (or Start + // over): + // - better-auth's "Invalid or expired code" / "OTP expired" + // ("expir") + // - auth-service's age-out wording ("too long") + // - better-auth's "Too many attempts" lockout + // ("too many" / "attempt") — see ERROR_CODES in + // better-auth/dist/plugins/email-otp/routes.mjs + // The page also OR-combines the regex result with a + // verifyLockedOut latch so post-lockout INVALID_OTP errors + // (better-auth has deleted the row by then, so further calls + // fall through to the row-not-found path) inherit the same + // inline-action treatment. + expect(html).toMatch(/\/expir\|too long\|too many\|attempt\/i\.test/) + expect(html).toContain('verifyLockedOut') }) it('renders the inline action with the "Send a new code" label and triggers the Resend button', () => { @@ -618,15 +631,112 @@ describe('renderLoginPage inline Resend action on expired OTP', () => { expect(html).toContain("document.getElementById('btn-resend').click()") }) - it('falls back to the plain showError on non-expired errors', () => { + it('falls back to the plain showError on recoverable (typo) errors', () => { const html = renderDefault() - // The non-expired branch must NOT route through - // showErrorWithAction (otherwise an "Invalid code" message - // would carry an inappropriate "Send a new code" link). + // The recoverable-error branch must NOT route through + // showErrorWithAction (otherwise a typo "Invalid OTP" message + // would carry an inappropriate "Send a new code" link). The + // structure to verify: if (isUnrecoverable) { ... } else { + // showError(result.error); }. expect(html).toMatch( - /if \(isExpired\) \{[\s\S]*?\} else \{[\s\S]*?showError\(result\.error\);\s*\}/, + /if \(isUnrecoverable\) \{[\s\S]*?\} else \{[\s\S]*?showError\(result\.error\);\s*\}/, ) }) + + it('clears the email input on "Use different email" so the form does not appear to remember the previous user', () => { + const html = renderDefault() + // showEmailStep() must reset emailInput.value AND focus the + // input so the next keystroke goes to a clean form. Leaving + // the previous email pre-filled was the regression we want to + // catch — assert both effects appear in the inlined script. + expect(html).toMatch(/emailInput\.value\s*=\s*''/) + expect(html).toMatch(/emailInput\.focus\(\)/) + }) + + it('short-circuits Verify when the OTP boxes are not all filled', () => { + const html = renderDefault() + // The submit handler bails early when otp.length is shorter + // than the number of OTP boxes — that's what stops empty + // Verify clicks from flashing a misleading "Invalid OTP" and + // burning a rate-limit slot. Assert the guard is present. + expect(html).toMatch(/if \(otp\.length < otpBoxes\.length\)/) + }) + + it('latches verifyLockedOut on a "too many"-shaped error and resets it on Resend', () => { + const html = renderDefault() + // The post-lockout fix needs both halves: a latch that gets + // SET on the lockout error, and a reset on a successful + // Resend so a fresh code reopens the verification cycle. + expect(html).toMatch(/verifyLockedOut\s*=\s*true/) + expect(html).toMatch(/verifyLockedOut\s*=\s*false/) + }) + + it('clears the OTP boxes after a successful Resend so old typing does not linger', () => { + const html = renderDefault() + // When the user clicks Resend the verification row is rotated, + // so any digits they had typed for the old code are stale. + // The Resend success path must reset the boxes so the user + // can paste / type the fresh code without first deleting + // what's there. + const resendIdx = html.indexOf("'Code resent!'") + expect(resendIdx).toBeGreaterThan(-1) + // Look for clearOtpBoxes() near the success message — the + // success branch is what the test is asserting on, not the + // expired-otp branch which is much later in the file. + const window = html.slice(Math.max(0, resendIdx - 500), resendIdx + 200) + expect(window).toContain('clearOtpBoxes()') + }) + + it('filters pasted OTP content to the configured charset before auto-submitting', () => { + const html = renderDefault() + // Numeric OTPs must drop letters / hyphens from a paste rather + // than letting them through and auto-submitting a garbage + // value that better-auth will reject — wastes a rate-limit + // slot AND flashes a misleading "Invalid OTP". The handler + // builds a charsetRegex from the JSON-injected otpCharset + // var; assert both branches are present. + expect(html).toMatch( + /charsetRegex\s*=\s*otpCharset\s*===\s*'alphanumeric'\s*\?\s*\/\[\^A-Za-z0-9\]\/g\s*:\s*\/\[\^0-9\]\/g/, + ) + }) + + it('filters typed OTP keystrokes to the configured charset', () => { + const html = renderDefault() + // The same charset filter must apply on the per-box input + // handler — typing "A" into a numeric form should be dropped + // silently rather than auto-submitting a bad code at the end. + expect(html).toMatch( + /inputCharsetRegex\s*=\s*otpCharset\s*===\s*'alphanumeric'\s*\?\s*\/\[\^A-Za-z0-9\]\/g\s*:\s*\/\[\^0-9\]\/g/, + ) + }) + + it('distributes a multi-char input event across the OTP boxes (iOS SMS autofill)', () => { + const html = renderDefault() + // iOS / Android SMS autofill drops the whole code into the + // first box (the one tagged autocomplete=one-time-code) as a + // single input event with the full string. Without + // distribution the user would lose every digit but the last. + // The handler must spread v[0..N] across otpBoxes starting at + // the current idx, then focus the last filled box. + expect(html).toMatch(/if \(v\.length > 1\)/) + expect(html).toMatch(/otpBoxes\[idx \+ i\]\.value = v\[i\]/) + }) + + it('renders the recovery link with the actual request_uri, not a placeholder', () => { + // The "Recover with backup email" link must round-trip the + // active OAuth flow's request_uri, so the recovery page's + // "Back to sign in" can return the user to the original + // /oauth/authorize. Earlier code hard-coded a "/placeholder" + // suffix on pdsPublicUrl, which silently broke the back path. + const html = renderDefault() + expect(html).toContain( + 'href="/auth/recover?request_uri=urn%3Aietf%3Aparams%3Aoauth%3Arequest_uri%3Atest-req"', + ) + // Make sure the placeholder fallback did not survive the + // refactor (catches a regression where the literal "placeholder" + // ends up back in the rendered HTML). + expect(html).not.toContain('/placeholder') + }) }) describe('renderLoginPage flow-aborted notice + reactive abort gates', () => { diff --git a/packages/auth-service/src/__tests__/otp-verify-error.test.ts b/packages/auth-service/src/__tests__/otp-verify-error.test.ts new file mode 100644 index 00000000..d94c2eac --- /dev/null +++ b/packages/auth-service/src/__tests__/otp-verify-error.test.ts @@ -0,0 +1,47 @@ +import { describe, it, expect } from 'vitest' +import { pickOtpVerifyErrorMessage } from '../lib/otp-verify-error.js' + +// Expected user-facing messages for each error class. Lifted to +// constants so the tests don't repeat the exact copy at every call +// site (Sonar flags >3% line duplication on new code). +const RESEND_MSG = + 'That code can no longer be used. Click "Resend code" below to get a fresh one.' +const TYPO_MSG = 'Invalid code. Please try again.' +const FALLBACK_MSG = 'Verification failed. Please try again.' + +describe('pickOtpVerifyErrorMessage', () => { + // The OTP verify flows (account-login, recovery) surface three + // distinct error states through their server-rendered OTP forms. + // The user-facing copy must distinguish them — typing more in + // an unrecoverable state just rolls up failed attempts that + // could never succeed. + + it.each([ + ['Too many attempts', RESEND_MSG], + ['OTP expired', RESEND_MSG], + ['Too many attempts on this code', RESEND_MSG], + ['TOO MANY ATTEMPTS', RESEND_MSG], // case-insensitive + ])( + 'points the user at Resend on lockout/aged-out: %s', + (errMessage, expected) => { + expect(pickOtpVerifyErrorMessage(new Error(errMessage))).toBe(expected) + }, + ) + + it('asks the user to re-type on a typo', () => { + expect(pickOtpVerifyErrorMessage(new Error('Invalid OTP'))).toBe(TYPO_MSG) + }) + + it('falls back to generic verification-failed on unknown errors', () => { + expect( + pickOtpVerifyErrorMessage(new Error('Internal database problem')), + ).toBe(FALLBACK_MSG) + }) + + it.each([['string-thrown'], [null], [undefined]])( + 'falls back to generic verification-failed on non-Error rejections (%p)', + (err) => { + expect(pickOtpVerifyErrorMessage(err)).toBe(FALLBACK_MSG) + }, + ) +}) diff --git a/packages/auth-service/src/lib/otp-verify-error.ts b/packages/auth-service/src/lib/otp-verify-error.ts new file mode 100644 index 00000000..f86dd197 --- /dev/null +++ b/packages/auth-service/src/lib/otp-verify-error.ts @@ -0,0 +1,37 @@ +/** + * Shared OTP-verify error-message picker for the server-rendered + * sign-in flows (`/account/verify-otp` and `/auth/recover/verify`). + * + * Both routes call better-auth's `signInEmailOTP` and need to + * translate the caught error into user-facing copy. They have the + * same three meaningful cases: + * + * 1. Lockout / aged-out — "Too many attempts" or "OTP expired" + * from better-auth, OR the post-lockout INVALID_OTP that fires + * against a deleted verification row. The current code path is + * dead; more typing cannot succeed. Point the user at Resend. + * + * 2. Recoverable typo — "Invalid OTP" against a live row. Just + * ask the user to re-type. + * + * 3. Internal failure — anything else (network, DB, unexpected). + * Show a generic try-again message. + * + * Returning the message string rather than a structured kind keeps + * the call sites simple — they just feed it straight into their + * `renderOtpForm({ error: ... })` helper. + * + * The branching is exported as a pure function so unit tests can + * cover all three branches without standing up a router. + */ + +export function pickOtpVerifyErrorMessage(err: unknown): string { + const errText = err instanceof Error ? err.message.toLowerCase() : '' + if (/too many|attempt|expir/.test(errText)) { + return 'That code can no longer be used. Click "Resend code" below to get a fresh one.' + } + if (errText.includes('invalid')) { + return 'Invalid code. Please try again.' + } + return 'Verification failed. Please try again.' +} diff --git a/packages/auth-service/src/routes/account-login.ts b/packages/auth-service/src/routes/account-login.ts index 7b044ab2..e822403e 100644 --- a/packages/auth-service/src/routes/account-login.ts +++ b/packages/auth-service/src/routes/account-login.ts @@ -20,6 +20,7 @@ import type { AuthServiceContext } from '../context.js' import { buildOtpInputProps } from '../otp-input.js' import type { BetterAuthInstance } from '../better-auth.js' import { POWERED_BY_CSS, POWERED_BY_HTML } from '../lib/page-helpers.js' +import { pickOtpVerifyErrorMessage } from '../lib/otp-verify-error.js' const logger = createLogger('auth:account-login') @@ -116,17 +117,13 @@ export function createAccountLoginRouter( return } catch (err: unknown) { logger.warn({ err, email }, 'OTP verification failed') - const errMsg = - err instanceof Error && err.message.includes('invalid') - ? 'Invalid or expired code. Please try again.' - : 'Verification failed. Please try again.' res.type('html').send( renderOtpForm({ email, csrfToken: res.locals.csrfToken, otpLength: ctx.config.otpLength, otpCharset: ctx.config.otpCharset, - error: errMsg, + error: pickOtpVerifyErrorMessage(err), }), ) } @@ -151,12 +148,13 @@ function renderLoginForm(opts: { csrfToken: string; error?: string }): string {

Account Settings

Sign in to manage your account

- ${opts.error ? '

' + escapeHtml(opts.error) + '

' : ''} + ${opts.error ? '' : ''}
@@ -193,7 +191,7 @@ function renderOtpForm(opts: {

Enter your code

We sent a ${opts.otpLength}-${opts.otpCharset === 'alphanumeric' ? 'character' : 'digit'} code to ${escapeHtml(maskedEmail)}

- ${opts.error ? '

' + escapeHtml(opts.error) + '

' : ''} + ${opts.error ? '' : ''} @@ -208,6 +206,7 @@ function renderOtpForm(opts: { autocapitalize="${inputProps.autocapitalize}" placeholder="${inputProps.placeholder}" class="otp-input" + oninput="this.value=this.value.replace(${opts.otpCharset === 'alphanumeric' ? '/[^A-Za-z0-9]/g' : '/[^0-9]/g'},'')${opts.otpCharset === 'alphanumeric' ? '.toUpperCase()' : ''}" style="letter-spacing: ${Math.max(2, Math.round(32 / opts.otpLength))}px">
@@ -239,6 +238,8 @@ const CSS = ` .otp-input { font-size: 28px !important; text-align: center; font-family: 'SF Mono', Menlo, Consolas, monospace !important; padding: 14px !important; } .btn-primary { width: 100%; padding: 12px; background: #0f1828; color: white; border: none; border-radius: 8px; font-size: 16px; font-weight: 500; cursor: pointer; } .btn-primary:hover { background: #1a2a40; } - .btn-secondary { display: inline-block; color: #0f1828; background: none; border: none; font-size: 14px; cursor: pointer; text-decoration: underline; } + .btn-primary:focus-visible { outline: 2px solid #0f1828; outline-offset: 2px; } + .btn-secondary { display: inline-block; color: #0f1828; background: none; border: none; font-size: 14px; cursor: pointer; text-decoration: underline; border-radius: 4px; } + .btn-secondary:focus-visible { outline: 2px solid #0f1828; outline-offset: 2px; } .error { color: #dc3545; background: #fdf0f0; padding: 12px; border-radius: 8px; margin: 12px 0; } ` diff --git a/packages/auth-service/src/routes/account-settings.ts b/packages/auth-service/src/routes/account-settings.ts index 8923039a..90f34de7 100644 --- a/packages/auth-service/src/routes/account-settings.ts +++ b/packages/auth-service/src/routes/account-settings.ts @@ -16,6 +16,67 @@ import { POWERED_BY_CSS, POWERED_BY_HTML } from '../lib/page-helpers.js' const logger = createLogger('auth:account-settings') +/** + * User-visible flash-message lookup tables for the GET /account + * page. The POST handlers redirect back with `?success=` or + * `?error=` and the GET handler renders the matching message. + * + * Whitelisted lookup (not `?success=Some+raw+text`) so attackers + * can't inject arbitrary text into the settings page via a crafted + * URL. + * + * Exported for unit testing. + */ +export const FLASH_SUCCESS_MESSAGES: Record = { + backup_added: + 'Backup email added. Click the verification link we sent to confirm.', + backup_verified: 'Backup email verified.', + backup_removed: 'Backup email removed.', + handle_updated: 'Handle updated.', + session_revoked: 'Session revoked.', +} + +export const FLASH_ERROR_MESSAGES: Record = { + invalid_email: 'That email address is not valid.', + already_primary: + 'That email is already your primary — you can sign in with it directly.', + account_not_found: "We couldn't find an account for your sign-in.", + send_failed: + "We couldn't send the verification email. Please try again in a moment.", + verify_failed: + 'That verification link is no longer valid. Add the backup email again to receive a fresh link.', + invalid_handle: + "That handle isn't valid. Use 5–20 characters: letters, numbers, or hyphens.", + handle_failed: + "We couldn't change your handle. It may be reserved or already taken.", + delete_failed: + "We couldn't delete your account right now. Please try again in a moment.", + handle_taken: 'That handle is not available — please choose another.', + confirm_delete: + 'Type DELETE in the confirmation box to permanently delete your account.', +} + +/** + * Resolve the success / error message pair from a /account request's + * query params. Whitelist the keys via the FLASH_*_MESSAGES tables so + * an attacker can't craft a URL like + * `/account?error=Some+raw+text` to inject arbitrary copy. + * + * Exported so the route handler can stay declarative AND be unit- + * tested without needing a fake express request. + */ +export function resolveAccountFlashFromQuery(query: { + success?: unknown + error?: unknown +}): { successMessage: string | null; errorMessage: string | null } { + const successCode = typeof query.success === 'string' ? query.success : '' + const errorCode = typeof query.error === 'string' ? query.error : '' + return { + successMessage: FLASH_SUCCESS_MESSAGES[successCode] ?? null, + errorMessage: FLASH_ERROR_MESSAGES[errorCode] ?? null, + } +} + /** * Middleware that validates a better-auth session and injects it into res.locals. * If not authenticated, redirects to /account/login. @@ -80,6 +141,14 @@ export function createAccountSettingsRouter( logger.warn({ err }, 'Failed to list sessions') } + // The POST handlers below redirect back to /account with a + // ?success=… or ?error=… query param to confirm the action. + // resolveAccountFlashFromQuery whitelists the recognised codes + // so an attacker can't craft a URL that injects arbitrary text. + const { successMessage, errorMessage } = resolveAccountFlashFromQuery( + req.query as { success?: unknown; error?: unknown }, + ) + res.type('html').send( renderSettingsPage({ did: did ?? '(unknown)', @@ -90,6 +159,8 @@ export function createAccountSettingsRouter( sessions, currentSessionToken: session.session.token, csrfToken: res.locals.csrfToken, + successMessage, + errorMessage, }), ) }) @@ -217,7 +288,7 @@ export function createAccountSettingsRouter( if (did && email) { ctx.db.removeBackupEmail(did, email) } - res.redirect(303, '/account') + res.redirect(303, '/account?success=backup_removed') }, ) @@ -237,7 +308,7 @@ export function createAccountSettingsRouter( logger.warn({ err }, 'Failed to revoke session') } } - res.redirect(303, '/account') + res.redirect(303, '/account?success=session_revoked') }, ) @@ -427,7 +498,21 @@ function renderSettingsPage(opts: { }> currentSessionToken: string csrfToken: string + successMessage?: string | null + errorMessage?: string | null }): string { + // Prepend any flash banners INSIDE the existing page-wrap so they + // sit above the settings content. Both go through escapeHtml even + // though the source dictionary is whitelisted — defence-in-depth + // against a future maintainer accidentally widening the lookup. + const flashHtml = [ + opts.successMessage + ? `
${escapeHtml(opts.successMessage)}
` + : '', + opts.errorMessage + ? `` + : '', + ].join('') const backupRows = opts.backupEmails .map( (be) => ` @@ -489,6 +574,8 @@ function renderSettingsPage(opts: {
+ ${flashHtml} +

Account

DID: ${escapeHtml(opts.did)}
@@ -589,6 +676,9 @@ const SETTINGS_CSS = ` .header { display: flex; justify-content: space-between; align-items: center; margin-bottom: 24px; border-bottom: 1px solid #eee; padding-bottom: 16px; } h1 { font-size: 24px; color: #111; } h2 { font-size: 18px; color: #333; margin-bottom: 12px; } + .flash { padding: 12px 16px; border-radius: 8px; margin-bottom: 20px; font-size: 14px; } + .flash-success { background: #f0fff4; color: #28a745; border: 1px solid #c3e6cb; } + .flash-error { background: #fdf0f0; color: #dc3545; border: 1px solid #f5c6cb; } .section { margin-bottom: 28px; padding-bottom: 20px; border-bottom: 1px solid #f0f0f0; } .section:last-child { border-bottom: none; margin-bottom: 0; } .setting-row { display: flex; justify-content: space-between; align-items: center; padding: 8px 0; font-size: 14px; color: #333; } diff --git a/packages/auth-service/src/routes/choose-handle.ts b/packages/auth-service/src/routes/choose-handle.ts index 555a912f..866493fd 100644 --- a/packages/auth-service/src/routes/choose-handle.ts +++ b/packages/auth-service/src/routes/choose-handle.ts @@ -209,7 +209,7 @@ export function createChooseHandleRouter( } const KNOWN_ERROR_MESSAGES: Record = { - handle_taken: 'That handle was just taken — please choose another.', + handle_taken: 'That handle is not available — please choose another.', } const rawError = req.query.error as string | undefined const error = rawError @@ -536,9 +536,11 @@ export function renderChooseHandlePage( .error { color: #dc3545; background: #fdf0f0; padding: 12px; border-radius: 8px; margin-bottom: 16px; font-size: 14px; } .btn-primary { width: 100%; padding: 12px; background: #0f1828; color: white; border: none; border-radius: 8px; font-size: 16px; font-weight: 500; cursor: pointer; margin-top: 8px; } .btn-primary:hover:not(:disabled) { background: #1a2a40; } + .btn-primary:focus-visible { outline: 2px solid #0f1828; outline-offset: 2px; } .btn-primary:disabled { opacity: 0.5; cursor: not-allowed; } .btn-secondary { width: 100%; padding: 10px; background: white; color: #0f1828; border: 1px solid #0f1828; border-radius: 8px; font-size: 15px; font-weight: 500; cursor: pointer; margin-top: 8px; } .btn-secondary:hover:not(:disabled) { background: #f0f2f5; } + .btn-secondary:focus-visible { outline: 2px solid #0f1828; outline-offset: 2px; } .btn-secondary:disabled { opacity: 0.5; cursor: not-allowed; } ${renderOptionalStyleTag(customCss)} diff --git a/packages/auth-service/src/routes/login-page.ts b/packages/auth-service/src/routes/login-page.ts index 302d8902..eba4837b 100644 --- a/packages/auth-service/src/routes/login-page.ts +++ b/packages/auth-service/src/routes/login-page.ts @@ -163,10 +163,19 @@ export function createLoginPageRouter(ctx: AuthServiceContext): Router { const clientId = req.query.client_id as string | undefined const loginHint = req.query.login_hint as string | undefined if (!requestUri) { + // The user landed here without an active sign-in flow — + // typically a stale link or direct visit. The technical + // "Missing request_uri parameter" tells them nothing + // actionable; surface the honest, useful message instead. res .status(400) .type('html') - .send(renderError('Missing request_uri parameter')) + .send( + renderError( + 'Sign-in has to be started from the app you are signing into. Please return to that app and try again.', + { title: 'No active sign-in' }, + ), + ) return } @@ -449,6 +458,7 @@ export function createLoginPageRouter(ctx: AuthServiceContext): Router { loginHint: emailHint, initialStep, otpAlreadySent, + requestUri, csrfToken: res.locals.csrfToken, authBasePath: '/api/auth', pdsPublicUrl: ctx.config.pdsPublicUrl, @@ -476,6 +486,14 @@ export function renderLoginPage(opts: { loginHint: string initialStep: 'email' | 'otp' otpAlreadySent: boolean + /** + * The actual PAR request_uri that started this flow. Forwarded to + * the recovery link's query string so the recovery page can + * round-trip the user back to their original /oauth/authorize on + * "Back to sign in". Previously the link used a placeholder URL, + * which silently broke the back-to-sign-in path after recovery. + */ + requestUri: string csrfToken: string authBasePath: string pdsPublicUrl: string @@ -614,11 +632,14 @@ export function renderLoginPage(opts: { .otp-box::placeholder { color: #d4d4d4; } .otp-box:focus { border-color: var(--focus-border); } .otp-actions { display: flex; gap: 32px; justify-content: center; margin-top: 12px; } + .otp-spam-hint { color: var(--muted-foreground); font-size: 13px; text-align: center; margin: 8px 0 16px; } .btn-primary { width: 100%; padding: 15px; background: ${brandColor}; color: white; border: none; border-radius: 9999px; font-size: 15px; font-weight: 500; cursor: pointer; transition: opacity 0.15s; } .btn-primary:hover { opacity: 0.9; } + .btn-primary:focus-visible { outline: 2px solid var(--focus-border, #2563eb); outline-offset: 2px; } .btn-primary:disabled { opacity: 0.7; cursor: not-allowed; } - .btn-secondary { display: inline-block; color: #6b6b6b; background: none; border: none; font-size: 14px; font-weight: 500; cursor: pointer; padding: 4px 0; } + .btn-secondary { display: inline-block; color: #6b6b6b; background: none; border: none; font-size: 14px; font-weight: 500; cursor: pointer; padding: 4px 0; border-radius: 4px; } .btn-secondary:hover { color: #1A130F; } + .btn-secondary:focus-visible { outline: 2px solid var(--focus-border, #2563eb); outline-offset: 2px; } .btn-social { display: flex; align-items: center; justify-content: center; gap: 8px; width: 100%; padding: 13px 20px; border: 1px solid var(--btn-secondary-border); border-radius: 9999px; font-size: 15px; font-weight: 500; cursor: pointer; text-decoration: none; background: white; color: #333; margin-bottom: 8px; transition: background 0.15s; } .btn-social:hover { background: #fafafa; } .btn-atproto { margin-top: 12px; margin-bottom: 0; color: #1A130F !important; background: var(--input-bg) !important; border-color: var(--input-border) !important; } @@ -654,7 +675,7 @@ export function renderLoginPage(opts: { ${logoHtml}

${opts.initialStep === 'otp' ? 'Enter your code' : 'Sign in'}

- + ${socialButtonsHtml} @@ -664,6 +685,7 @@ export function renderLoginPage(opts: {
@@ -696,11 +718,14 @@ export function renderLoginPage(opts: { +

+ If you don't see the email, check your spam folder. +

- Recover with backup email @@ -743,17 +768,49 @@ export function renderLoginPage(opts: { var heartbeatEnabled = ${JSON.stringify(opts.heartbeatEnabled)}; var heartbeatHandle = null; var heartbeatIntervalMs = 3 * 60 * 1000; + // Upstream's AUTHORIZATION_INACTIVITY_TIMEOUT — once this much + // wall-clock time has elapsed since our last successful PAR + // refresh, the upstream row is guaranteed to be dead. Used by + // parLikelyDead() to hide Resend before the user can click it. + var parInactivityTimeoutMs = 5 * 60 * 1000; + // Page load is the implicit first PAR refresh — atproto's + // PAR_EXPIRES_IN gives a fresh row 5 min on creation, and the + // user just hit /oauth/authorize seconds ago. Treat now as + // last-known-alive until the first ping confirms otherwise. + var lastSuccessfulHeartbeatAt = Date.now(); // Set to true the moment we know the flow can no longer // complete (PAR or auth_flow gone). Resend / Verify gates // check this so a click that races the proactive notice // still bails to /auth/abort instead of issuing a fresh OTP // that would only fail. var flowAborted = false; + // Set to true the moment better-auth surfaces a lockout + // ("Too many attempts"). The verification row is deleted at + // that point, so every subsequent /sign-in/email-otp call + // returns INVALID_OTP ("Invalid OTP") against nothing — and + // a user who keeps typing would just rack up dishonest + // "Invalid OTP" errors when in reality the only forward + // path is a fresh Resend. Latching this lets the + // post-lockout INVALID_OTP path inherit the same inline + // "Send a new code" treatment as the original "Too many + // attempts" response. + var verifyLockedOut = false; + // True iff we have proof the PAR is still alive (last ping + // was ok:true and was recent enough to fall inside the + // upstream inactivity window). Used to gate every "offer the + // user a Resend" decision so they only ever see actions that + // can actually complete the flow. + function parLikelyDead() { + if (flowAborted) return true; + return Date.now() - lastSuccessfulHeartbeatAt >= parInactivityTimeoutMs; + } function pingHeartbeat() { return fetch('/auth/ping', { credentials: 'include', cache: 'no-store' }) .then(function(r) { return r.json(); }) .then(function(body) { - if (body && body.ok === false && body.reason !== 'transient') { + if (body && body.ok === true) { + lastSuccessfulHeartbeatAt = Date.now(); + } else if (body && body.ok === false && body.reason !== 'transient') { // Auth flow / PAR genuinely dead — no point pinging again, // and no point letting the user keep typing. 'transient' // (5xx / network blip) does NOT stop the interval; the @@ -763,7 +820,13 @@ export function renderLoginPage(opts: { } return body; }) - .catch(function() { return null; /* network blip — caller may retry */ }); + .catch(function() { return null; /* network blip — caller may retry */ }) + .finally(function() { + // Always reconcile visibility — a 'transient' tick that + // pushes us past the inactivity window must hide Resend + // even though we never got a definitive 'par_expired'. + refreshResendVisibility(); + }); } function startHeartbeat() { if (!heartbeatEnabled) return; @@ -777,6 +840,16 @@ export function renderLoginPage(opts: { } } window.addEventListener('beforeunload', stopHeartbeat); + // When the tab returns to the foreground after being hidden, + // setInterval may have been throttled enough that PAR has + // silently lapsed. Re-ping immediately so the UI reflects + // reality before the user clicks anything. + document.addEventListener('visibilitychange', function() { + if (document.visibilityState === 'visible' && heartbeatEnabled) { + pingHeartbeat(); + refreshResendVisibility(); + } + }); // Show the proactive "this won't work — start over" notice when // the flow is unrecoverable. Disables the OTP boxes, the verify @@ -819,6 +892,41 @@ export function renderLoginPage(opts: { errorEl.appendChild(startOverBtn); } + /** + * Toggle the standalone Resend button between visible and + * hidden based on whether the PAR is still alive. The button + * is removed from view (display:none) rather than just + * disabled — a button the user cannot productively click + * shouldn't be on the page at all. When hidden, a "Start over" + * link is shown in its place so the user always has a forward + * path. Idempotent — safe to call from heartbeat ticks, + * visibility change handlers, and inline render paths. + */ + function refreshResendVisibility() { + var resendBtn = document.getElementById('btn-resend'); + var startOverLink = document.getElementById('btn-start-over'); + if (!resendBtn) return; + if (parLikelyDead()) { + resendBtn.style.display = 'none'; + if (!startOverLink) { + startOverLink = document.createElement('button'); + startOverLink.type = 'button'; + startOverLink.id = 'btn-start-over'; + startOverLink.className = 'btn-secondary'; + startOverLink.textContent = 'Start over'; + startOverLink.addEventListener('click', function() { + window.location.href = '/auth/abort'; + }); + resendBtn.parentNode.insertBefore(startOverLink, resendBtn); + } + } else { + resendBtn.style.display = ''; + if (startOverLink && startOverLink.parentNode) { + startOverLink.parentNode.removeChild(startOverLink); + } + } + } + /** * Reactive gate used by the Resend and Verify click handlers. * Pings /auth/ping synchronously; if the result indicates the @@ -854,11 +962,35 @@ export function renderLoginPage(opts: { hiddenCode.value = ''; } + // Mirror the paste filter so a single typed keystroke gets + // dropped silently when it isn't part of the OTP charset. + // Without this a user typing "1A2B3C" into a numeric form + // ends up with "1A2B3C" in the boxes and triggers an + // auto-submit that returns "Invalid OTP", same misleading + // path the paste filter closes off. + var inputCharsetRegex = otpCharset === 'alphanumeric' ? /[^A-Za-z0-9]/g : /[^0-9]/g; otpBoxes.forEach(function(box, idx) { box.addEventListener('input', function() { - // keep only the last typed char (handles paste into a single box) - var v = box.value.replace(/\\s/g, ''); - if (v.length > 1) v = v.slice(-1); + var v = box.value.replace(/\\s/g, '').replace(inputCharsetRegex, ''); + if (v && otpCharset === 'alphanumeric') v = v.toUpperCase(); + if (v.length > 1) { + // iOS / Android SMS autofill drops the entire code into + // the first box (the one tagged autocomplete=one-time-code) + // as a single input event with the full string. Without + // distribution the user would see only the last digit + // and lose the rest. Distribute across boxes starting + // here, mirroring the paste handler. + for (var i = 0; i < v.length && idx + i < otpBoxes.length; i++) { + otpBoxes[idx + i].value = v[i]; + } + updateHiddenCode(); + var nextIdx = Math.min(idx + v.length, otpBoxes.length - 1); + otpBoxes[nextIdx].focus(); + if (hiddenCode.value.length === otpBoxes.length) { + document.getElementById('form-verify-otp').requestSubmit(); + } + return; + } box.value = v; updateHiddenCode(); if (box.value && idx < otpBoxes.length - 1) otpBoxes[idx + 1].focus(); @@ -881,8 +1013,21 @@ export function renderLoginPage(opts: { box.addEventListener('paste', function(e) { e.preventDefault(); var data = (e.clipboardData || window.clipboardData).getData('text') || ''; - var cleaned = data.replace(/\\s/g, '').slice(0, otpBoxes.length - idx); - for (var i = 0; i < cleaned.length; i++) otpBoxes[idx + i].value = cleaned[i]; + // Strip everything that isn't part of the OTP charset. + // Pasting "1234 5678" should fill the boxes; pasting + // "12-AB-34" with a numeric OTP should drop the dashes + // and letters silently rather than auto-submitting a + // garbage code that better-auth will then reject as + // "Invalid OTP" — the user typed nothing wrong, the + // paste source did, so flashing them an error would be + // both misleading and burn a rate-limit slot. + var charsetRegex = otpCharset === 'alphanumeric' ? /[^A-Za-z0-9]/g : /[^0-9]/g; + var cleaned = data.replace(charsetRegex, '').slice(0, otpBoxes.length - idx); + for (var i = 0; i < cleaned.length; i++) { + otpBoxes[idx + i].value = otpCharset === 'alphanumeric' + ? cleaned[i].toUpperCase() + : cleaned[i]; + } updateHiddenCode(); var nextIdx = Math.min(idx + cleaned.length, otpBoxes.length - 1); otpBoxes[nextIdx].focus(); @@ -983,6 +1128,7 @@ export function renderLoginPage(opts: { if (otpBoxes.length) otpBoxes[0].focus(); clearError(); startHeartbeat(); + refreshResendVisibility(); } function showEmailStep() { @@ -992,6 +1138,15 @@ export function renderLoginPage(opts: { if (termsEl) termsEl.style.display = 'block'; clearError(); stopHeartbeat(); + // Reset the email field — the user clicked "Use different + // email" precisely to escape the previous value, so leaving + // it pre-filled both wastes a clearing keystroke and looks + // like the form remembered them when they wanted a fresh + // start. Focus the input so they can start typing + // immediately. + emailInput.value = ''; + currentEmail = ''; + emailInput.focus(); } // Send OTP via better-auth @@ -1075,12 +1230,22 @@ export function renderLoginPage(opts: { // first call consumes the code; a second one races the redirect and // flashes "Invalid OTP" before the page unloads. if (verifying) return; + var otp = document.getElementById('code').value.trim(); + // Don't bother better-auth with an empty / partial submit — + // it would flash a misleading "Invalid OTP" (the user typed + // nothing, not an invalid code) and burn a rate-limit slot. + // Just focus the first empty box and bail. + if (otp.length < otpBoxes.length) { + for (var i = 0; i < otpBoxes.length; i++) { + if (!otpBoxes[i].value) { otpBoxes[i].focus(); break; } + } + return; + } verifying = true; // Stop pinging the moment a verify is in flight — the redirect // is imminent and any further heartbeat is wasted. stopHeartbeat(); clearError(); - var otp = document.getElementById('code').value.trim(); var btn = this.querySelector('button[type=submit]'); btn.disabled = true; btn.textContent = 'Verifying...'; @@ -1096,23 +1261,43 @@ export function renderLoginPage(opts: { try { var result = await verifyOtp(currentEmail, otp); if (result && result.error) { - // Inline a "Send a new code" action when the error - // indicates the OTP has aged out — too easy to miss the - // separate Resend button below the form. The substring - // match catches the better-auth wording ("Invalid or - // expired code") and the auth-service wording ("OTP - // expired") plus generic "expir"/"too long" variants. - var isExpired = /expir|too long/i.test(result.error); - if (isExpired) { - // The inline action triggers the same Resend handler, - // which itself runs abortIfFlowDead() before issuing - // a new code. So even if the PAR is dead the user - // gets the spec-compliant bounce rather than a fresh - // OTP that wouldn't work — no need to gate the - // action's visibility separately here. - showErrorWithAction(result.error, 'Send a new code', function() { - document.getElementById('btn-resend').click(); - }); + // Inline a "Send a new code" action when the current + // code path is dead — too easy to miss the separate + // Resend button below the form, and fighting an + // unrecoverable error is exactly the misleading, + // time-wasting UX we want to avoid. Matches: + // - "Invalid or expired code" / "OTP expired" + // (the original aged-out case) + // - "Too many attempts" (better-auth deletes the + // verification row after N wrong tries — the next + // code typed cannot succeed; the only forward + // path is a fresh Resend) + // Plus: once we have seen a lockout once, every + // subsequent /sign-in/email-otp call hits a deleted + // verification row and returns the generic + // "Invalid OTP" — which without this latch would let + // the user fight a typo error that more typing can't + // fix. + if (/too many|attempt/i.test(result.error)) verifyLockedOut = true; + var isUnrecoverable = + /expir|too long|too many|attempt/i.test(result.error) || + verifyLockedOut; + if (isUnrecoverable) { + // Only offer "Send a new code" when the PAR is still + // alive. If it isn't, a fresh OTP would issue but + // never complete — wasting the user's time on a code + // that can't work. Show "Start over" instead so the + // only forward path we surface is one that will + // actually succeed. + if (parLikelyDead()) { + showErrorWithAction(result.error, 'Start over', function() { + window.location.href = '/auth/abort'; + }); + } else { + showErrorWithAction(result.error, 'Send a new code', function() { + document.getElementById('btn-resend').click(); + }); + } } else { showError(result.error); } @@ -1152,6 +1337,16 @@ export function renderLoginPage(opts: { if (result.error) { showError(result.error); } else { + // A fresh code resets the verify-lockout latch — the + // verification row is brand new, so future submits can + // succeed and shouldn't inherit the old lockout state. + verifyLockedOut = false; + // Clear any digits the user typed for the old code so + // they can paste / type the fresh one without first + // having to delete what's there. Focus the first box so + // the next keystroke goes to the right place. + clearOtpBoxes(); + if (otpBoxes.length) otpBoxes[0].focus(); showSuccess('Code resent!'); } }); @@ -1185,8 +1380,10 @@ export function renderLoginPage(opts: { }); } // OTP form is already visible server-side; showOtpStep() never - // ran, so kick off the heartbeat ourselves. + // ran, so kick off the heartbeat ourselves and reflect the + // current PAR-liveness state in the Resend button visibility. startHeartbeat(); + refreshResendVisibility(); } })(); diff --git a/packages/auth-service/src/routes/preview.ts b/packages/auth-service/src/routes/preview.ts index f9938998..513a2f27 100644 --- a/packages/auth-service/src/routes/preview.ts +++ b/packages/auth-service/src/routes/preview.ts @@ -216,6 +216,11 @@ export function createPreviewRouter(ctx: AuthServiceContext): Router { loginHint: '', initialStep: 'email', otpAlreadySent: false, + // Preview pages don't sit behind a real OAuth flow — pass a + // recognisable fake request_uri so the recovery link renders + // something shaped right without pretending to point at a + // live PAR. + requestUri: 'urn:ietf:params:oauth:request_uri:preview', csrfToken: fakeCsrfToken(), authBasePath: '/api/auth', pdsPublicUrl: ctx.config.pdsPublicUrl, @@ -246,6 +251,8 @@ export function createPreviewRouter(ctx: AuthServiceContext): Router { loginHint: FAKE_EMAIL, initialStep: 'otp', otpAlreadySent: true, + // See preview/login note above — fake request_uri. + requestUri: 'urn:ietf:params:oauth:request_uri:preview', csrfToken: fakeCsrfToken(), authBasePath: '/api/auth', pdsPublicUrl: ctx.config.pdsPublicUrl, diff --git a/packages/auth-service/src/routes/recovery.ts b/packages/auth-service/src/routes/recovery.ts index ac6ae5df..110e88ab 100644 --- a/packages/auth-service/src/routes/recovery.ts +++ b/packages/auth-service/src/routes/recovery.ts @@ -28,6 +28,7 @@ import { import { renderError } from '../lib/render-error.js' import { AUTH_FLOW_COOKIE, AUTH_FLOW_TTL_MS } from '../lib/auth-flow.js' import { heartbeatEnabledFor } from './login-page.js' +import { pickOtpVerifyErrorMessage } from '../lib/otp-verify-error.js' const logger = createLogger('auth:recovery') @@ -76,10 +77,19 @@ export function createRecoveryRouter( const requestUri = req.query.request_uri as string | undefined if (!requestUri) { + // The user landed here without an active sign-in flow — + // typically a stale link or direct visit. The technical + // "Missing request_uri parameter" tells them nothing + // actionable; surface the honest, useful message instead. res .status(400) .type('html') - .send(renderError('Missing request_uri parameter')) + .send( + renderError( + 'Account recovery has to be started from the sign-in page. Please sign in again from the app you came from.', + { title: 'No active sign-in' }, + ), + ) return } @@ -268,11 +278,11 @@ export function createRecoveryRouter( res.redirect(303, '/auth/complete') } catch (err: unknown) { logger.warn({ err, email }, 'Recovery OTP verification failed') - const errMsg = - err instanceof Error && - (err.message.includes('invalid') || err.message.includes('expired')) - ? 'Invalid or expired code. Please try again.' - : 'Verification failed. Please try again.' + // Same shape as the account-login flow: distinguish typo + // from lockout/aged-out so the user gets pointed at Resend + // rather than fighting an "Invalid or expired" error that + // more typing can't fix. + const errMsg = pickOtpVerifyErrorMessage(err) const { customCss, customFaviconUrl, customFaviconUrlDark, backUri } = await getFlowBranding(req) res.send( @@ -340,7 +350,7 @@ export function renderRecoveryForm(opts: {

Account Recovery

Enter the backup email address associated with your account.

- ${opts.error ? '

' + escapeHtml(opts.error) + '

' : ''} + ${opts.error ? '' : ''}
@@ -348,6 +358,7 @@ export function renderRecoveryForm(opts: {
@@ -434,7 +445,7 @@ export function renderRecoveryOtpForm(opts: {

Enter recovery code

If a backup email matches, we sent a ${opts.otpLength}-${opts.otpCharset === 'alphanumeric' ? 'character' : 'digit'} code to ${escapeHtml(maskedEmail)}

- ${opts.error ? '

' + escapeHtml(opts.error) + '

' : ''} + ${opts.error ? '' : ''} @@ -451,7 +462,7 @@ export function renderRecoveryOtpForm(opts: { autocapitalize="${inputProps.autocapitalize}" placeholder="${inputProps.placeholder}" class="otp-input" - oninput="this.value=this.value.replace(/[\\s-]/g,'')" + oninput="this.value=this.value.replace(${opts.otpCharset === 'alphanumeric' ? '/[^A-Za-z0-9]/g' : '/[^0-9]/g'},'')${opts.otpCharset === 'alphanumeric' ? '.toUpperCase()' : ''}" style="letter-spacing: ${Math.max(2, Math.round(32 / opts.otpLength))}px">
@@ -511,6 +522,8 @@ const CSS = ` .otp-input { font-size: 28px !important; text-align: center; font-family: 'SF Mono', Menlo, Consolas, monospace !important; padding: 14px !important; } .btn-primary { width: 100%; padding: 12px; background: #0f1828; color: white; border: none; border-radius: 8px; font-size: 16px; font-weight: 500; cursor: pointer; } .btn-primary:hover { background: #1a2a40; } - .btn-secondary { display: inline-block; margin-top: 12px; color: #0f1828; background: none; border: none; font-size: 14px; cursor: pointer; text-decoration: underline; } + .btn-primary:focus-visible { outline: 2px solid #0f1828; outline-offset: 2px; } + .btn-secondary { display: inline-block; margin-top: 12px; color: #0f1828; background: none; border: none; font-size: 14px; cursor: pointer; text-decoration: underline; border-radius: 4px; } + .btn-secondary:focus-visible { outline: 2px solid #0f1828; outline-offset: 2px; } .error { color: #dc3545; background: #fdf0f0; padding: 12px; border-radius: 8px; margin: 12px 0; } ` diff --git a/packages/demo/src/app/api/oauth/callback/route.ts b/packages/demo/src/app/api/oauth/callback/route.ts index abaa5811..15feb7b0 100644 --- a/packages/demo/src/app/api/oauth/callback/route.ts +++ b/packages/demo/src/app/api/oauth/callback/route.ts @@ -40,19 +40,44 @@ export async function GET(request: NextRequest) { const error = request.nextUrl.searchParams.get('error') if (error) { - console.error('[oauth/callback] Auth error from PDS') - return NextResponse.redirect(new URL('/?error=auth_failed', baseUrl)) + // The PDS clean-exit paths (e.g. dead PAR after a long wait) + // send error=access_denied with an error_description like + // "Your sign-in took too long...". A generic auth_failed + // banner discards that context and tells the user nothing + // about *why* it failed. Surface session-expiry timeouts + // distinctly so the landing page can guide them ("sign in + // again") instead of looking like the credentials were + // wrong. + const errorDescription = + request.nextUrl.searchParams.get('error_description') ?? '' + const isTimeout = + error === 'access_denied' && + /timed out|too long|expired|session/i.test(errorDescription) + const code = isTimeout ? 'session_expired' : 'auth_failed' + console.error( + `[oauth/callback] Auth error from PDS: ${error} (${errorDescription})`, + ) + return NextResponse.redirect(new URL(`/?error=${code}`, baseUrl)) } if (!code || !state) { return NextResponse.redirect(new URL('/?error=auth_failed', baseUrl)) } - // Retrieve OAuth session from signed cookie + // Retrieve OAuth session from signed cookie. The cookie carries + // the state value, code verifier, token endpoint and issuer that + // we recorded when starting the OAuth flow. If it has gone away + // (cookie expired by browser, user cleared cookies, very long + // wait on the OTP form), there is nothing we can do to complete + // the token exchange — but we owe the user a useful error + // (`session_expired`) rather than a generic `auth_failed`, so + // the landing page can guide them to start over instead of + // looking like the sign-in itself just failed. const cookieStore = await cookies() const stateData = getOAuthSessionFromCookie(cookieStore) if (!stateData) { - return NextResponse.redirect(new URL('/?error=auth_failed', baseUrl)) + console.error('[oauth/callback] Missing oauth_state cookie') + return NextResponse.redirect(new URL('/?error=session_expired', baseUrl)) } if (stateData.state !== state) { @@ -161,7 +186,17 @@ export async function GET(request: NextRequest) { console.error( `[oauth/callback] FAILED status=${tokenRes.status} url=${tokenUrl} body=${errBody}`, ) - return NextResponse.redirect(new URL('/?error=auth_failed', baseUrl)) + // invalid_grant from the token endpoint typically means the + // authorization code expired (atproto's auth_request rows + // age out a few minutes after issuance). Surface that as + // session_expired rather than the generic auth_failed banner + // so the user sees an honest "sign-in took too long" rather + // than wondering whether their credentials were wrong. + const code = + tokenRes.status === 400 && /invalid_grant/i.test(errBody) + ? 'session_expired' + : 'auth_failed' + return NextResponse.redirect(new URL(`/?error=${code}`, baseUrl)) } const tokenData = (await tokenRes.json()) as { diff --git a/packages/demo/src/app/api/oauth/login/route.ts b/packages/demo/src/app/api/oauth/login/route.ts index 30a855e0..c544235c 100644 --- a/packages/demo/src/app/api/oauth/login/route.ts +++ b/packages/demo/src/app/api/oauth/login/route.ts @@ -263,7 +263,7 @@ export async function GET(request: Request) { httpOnly: true, secure: true, sameSite: 'lax', - maxAge: 600, + maxAge: 60 * 60, path: '/', }) return resp2 @@ -281,7 +281,7 @@ export async function GET(request: Request) { httpOnly: true, secure: true, sameSite: 'lax', - maxAge: 600, + maxAge: 60 * 60, path: '/', }) return response diff --git a/packages/demo/src/app/components/LoginForm.tsx b/packages/demo/src/app/components/LoginForm.tsx index 06989549..1a86d809 100644 --- a/packages/demo/src/app/components/LoginForm.tsx +++ b/packages/demo/src/app/components/LoginForm.tsx @@ -5,15 +5,17 @@ import { useState } from 'react' import { ForceLoginCheckbox } from './ForceLoginCheckbox' const ERROR_MESSAGES: Record = { - auth_failed: 'Authentication failed. Please try again.', - par_failed: - 'Could not start login — the PDS rejected the request. Check server logs.', + auth_failed: 'Sign-in failed. Please try again.', + session_expired: + 'Your sign-in took too long to finish. Please sign in again.', + par_failed: "Sign-in couldn't start right now. Please try again in a moment.", invalid_email: 'Please enter a valid email address.', invalid_handle: 'Please enter a valid handle (e.g. you.bsky.social).', - invalid_login_hint: 'Invalid login hint format.', - token_failed: 'Login could not be completed — token exchange failed.', + invalid_login_hint: + "We didn't recognise that account. Please sign in with your email instead.", + token_failed: "Sign-in couldn't be completed. Please sign in again.", state_mismatch: - 'Login session expired or was tampered with. Please try again.', + 'Your sign-in session expired or was interrupted. Please sign in again.', } /** @@ -38,6 +40,7 @@ export function LoginForm() { <> {errorMessage && (
{ + it.each([ + // Common reserved subdomains from upstream's @atproto/pds list. + // The list is curated; we don't reproduce it here, just sample + // a few that should always be reserved. + ['admin'], + ['www'], + ['support'], + ['help'], + ['api'], + ['bsky'], + ])('flags reserved subdomain "%s" as reserved', (local) => { + expect(isReservedSubdomain(local)).toBe(true) + }) + + it('treats reserved subdomains case-insensitively', () => { + expect(isReservedSubdomain('ADMIN')).toBe(true) + expect(isReservedSubdomain('Www')).toBe(true) + }) + + it('returns false for normal handles', () => { + expect(isReservedSubdomain('alice')).toBe(false) + expect(isReservedSubdomain('bob42')).toBe(false) + expect(isReservedSubdomain('my-handle')).toBe(false) + }) + + it('returns false for the empty string (handle picker passes "" while still typing)', () => { + expect(isReservedSubdomain('')).toBe(false) + }) + + it('returns false for whitespace-only or punctuation-only inputs', () => { + // The picker normalises before calling, but defence-in-depth on the + // helper. Reserved list doesn't contain spaces, so these all miss. + expect(isReservedSubdomain(' ')).toBe(false) + expect(isReservedSubdomain('---')).toBe(false) + }) +}) + +describe('handleIsUnavailable', () => { + // The handle picker's live availability check treats "exists" as + // "unavailable" — and "unavailable" has two sources: someone owns + // the handle in the DB, OR it's a reserved subdomain. The helper + // OR-combines them so the picker doesn't paint a misleading + // "✓ Available" on a reserved handle. + + it('returns true when the account already exists in the DB', () => { + expect( + handleIsUnavailable({ + fullHandle: 'someone.epds-poc1.test.certified.app', + accountExists: true, + }), + ).toBe(true) + }) + + it('returns true when the local part is a reserved subdomain', () => { + expect( + handleIsUnavailable({ + fullHandle: 'admin.epds-poc1.test.certified.app', + accountExists: false, + }), + ).toBe(true) + }) + + it('returns true when both the account exists AND the local part is reserved', () => { + // Pathological case: someone managed to get a row created with a + // reserved local part (e.g. via a config change after an account + // existed). Either condition alone makes the handle unavailable. + expect( + handleIsUnavailable({ + fullHandle: 'admin.epds-poc1.test.certified.app', + accountExists: true, + }), + ).toBe(true) + }) + + it('returns false for a fresh, non-reserved handle', () => { + expect( + handleIsUnavailable({ + fullHandle: 'alice.epds-poc1.test.certified.app', + accountExists: false, + }), + ).toBe(false) + }) + + it('matches reserved local parts case-insensitively', () => { + expect( + handleIsUnavailable({ + fullHandle: 'ADMIN.epds-poc1.test.certified.app', + accountExists: false, + }), + ).toBe(true) + }) + + it('handles an empty fullHandle gracefully (route handler rejects empty before calling)', () => { + expect(handleIsUnavailable({ fullHandle: '', accountExists: false })).toBe( + false, + ) + }) +}) diff --git a/packages/pds-core/src/index.ts b/packages/pds-core/src/index.ts index bdbe8c71..f3cec47f 100644 --- a/packages/pds-core/src/index.ts +++ b/packages/pds-core/src/index.ts @@ -46,6 +46,7 @@ import { validateClientMetadataForPreview, } from '@certified-app/shared' import { shouldRewriteSecFetchSite } from './lib/sec-fetch-site-rewrite.js' +import { handleIsUnavailable } from './lib/reserved-handle.js' import { findInsertionIndex, installCssInjectionMiddleware, @@ -1022,7 +1023,15 @@ async function main() { } try { const account = await pds.ctx.accountManager.getAccount(handle) - res.json({ exists: !!account }) + // The handle picker treats "exists" as "unavailable" — see + // `handleIsUnavailable` for why both DB-collision and + // reserved-subdomain match feed the same boolean. + res.json({ + exists: handleIsUnavailable({ + fullHandle: handle, + accountExists: !!account, + }), + }) } catch (err) { logger.error({ err, handle }, 'Failed to check handle availability') res.status(503).json({ error: 'handle_check_failed' }) diff --git a/packages/pds-core/src/lib/reserved-handle.ts b/packages/pds-core/src/lib/reserved-handle.ts new file mode 100644 index 00000000..3afa8aed --- /dev/null +++ b/packages/pds-core/src/lib/reserved-handle.ts @@ -0,0 +1,48 @@ +/** + * Reserved-subdomain check that mirrors @atproto/pds's + * `ensureHandleServiceConstraints`. + * + * Upstream's reserved list lives at + * `@atproto/pds/dist/handle/reserved.js` and is not part of the + * public package exports — only `ensureHandleServiceConstraints` is. + * That helper throws an `HandleNotAvailable` error when the local + * part is reserved, but we want a cheap boolean for the live + * availability check on the handle picker (no need to throw + catch + * just to render a status string). + * + * Import the deep path explicitly. Pinned to the installed + * `@atproto/pds` version, so a major upgrade can break the import — + * caught immediately at typecheck / build time. + */ + +import { reservedSubdomains } from '@atproto/pds/dist/handle/reserved.js' + +export function isReservedSubdomain(local: string): boolean { + return local.toLowerCase() in reservedSubdomains +} + +/** + * Compute the `exists` field returned by /_internal/check-handle. + * + * The handle picker's live availability check treats "exists" as + * "this handle is unavailable to claim" — which has two distinct + * sources: + * + * 1. Someone already owns it (DB row in accountManager). + * 2. It's in the upstream reserved-subdomains list (admin, www, + * support, …). createAccount would later throw + * HandleUnavailableError; better to flag it here so the picker + * disables Submit and shows the same "not available" status as + * the already-owned case. + * + * Pure function so the route handler can stay thin and unit tests + * cover the OR-shape directly without standing up the full PDS. + */ +export function handleIsUnavailable(opts: { + fullHandle: string + accountExists: boolean +}): boolean { + if (opts.accountExists) return true + const local = opts.fullHandle.split('.')[0] ?? '' + return isReservedSubdomain(local) +} diff --git a/packages/shared/src/__tests__/client-metadata.test.ts b/packages/shared/src/__tests__/client-metadata.test.ts index 5a971c30..4c7330e5 100644 --- a/packages/shared/src/__tests__/client-metadata.test.ts +++ b/packages/shared/src/__tests__/client-metadata.test.ts @@ -23,9 +23,14 @@ beforeEach(() => { }) describe('resolveClientMetadata', () => { - it('returns client_name for non-URL client_id', async () => { + it('returns empty metadata for a non-URL client_id', async () => { + // Non-URL client_ids used to surface the raw value as + // metadata.client_name, which then ended up in page titles + // ("Sign in to not-a-url") on the auth-service login page. + // Returning empty metadata lets the caller's fallback chain + // (client_name || extractDomain → 'an application') win. const metadata = await resolveClientMetadata('my-local-app') - expect(metadata.client_name).toBe('my-local-app') + expect(metadata).toEqual({}) }) it('returns seeded metadata from cache', async () => { @@ -135,9 +140,14 @@ describe('resolveClientName', () => { expect(name).toBe('no-name.app') }) - it('returns client_id as-is for non-URL', async () => { + it('falls back to "an application" for a non-URL client_id', async () => { + // Non-URL client_ids used to leak through as the displayed + // app name (e.g. "Sign in to unnamed-client"). The empty- + // metadata change in resolveClientMetadata + extractDomain + // returning null on parse failure means resolveClientName's + // final fallback wins, yielding a generic "an application". const name = await resolveClientName('unnamed-client') - expect(name).toBe('unnamed-client') + expect(name).toBe('an application') }) }) diff --git a/packages/shared/src/client-metadata.ts b/packages/shared/src/client-metadata.ts index 1c3d0ded..fec97dc2 100644 --- a/packages/shared/src/client-metadata.ts +++ b/packages/shared/src/client-metadata.ts @@ -160,10 +160,18 @@ export async function resolveClientMetadata( try { parsedUrl = new URL(clientId) } catch { - return { client_name: clientId } + // Malformed clientId — return empty metadata so the caller's + // fallback chain (client_name || extractDomain(clientId) || + // 'an application') doesn't end up surfacing the literal raw + // string ("Sign in to not-a-url") in page titles and copy. + // The caller's extractDomain will also fail; the final + // fallback to "an application" wins. + return {} } if (parsedUrl.protocol !== 'https:' && parsedUrl.protocol !== 'http:') { - return { client_name: clientId } + // Same shape as the unparseable case: don't surface the raw + // schemeless or non-http value to end users. + return {} } if (!options.noCache) { diff --git a/packages/shared/src/render-error.ts b/packages/shared/src/render-error.ts index 22cf2593..56f7942e 100644 --- a/packages/shared/src/render-error.ts +++ b/packages/shared/src/render-error.ts @@ -102,7 +102,7 @@ export function renderError(

${escapeHtml(title)}

-

${escapeHtml(message)}

+ ${startOverHtml}
${bodyExtra}