diff --git a/.changeset/clear-otp-boxes-on-resend.md b/.changeset/clear-otp-boxes-on-resend.md new file mode 100644 index 00000000..65ab35fd --- /dev/null +++ b/.changeset/clear-otp-boxes-on-resend.md @@ -0,0 +1,9 @@ +--- +'ePDS': patch +--- + +Asking for a new sign-in code now clears the boxes and tells you the old code has stopped working. + +**Affects:** End users + +**End users:** the boxes reset on **Resend code**, so a half-typed old code no longer has to be deleted by hand before you can type the new one. The confirmation message that replaces "Code resent!" also warns that only the newest code will be accepted, and points at the spam folder — the two things most likely to be going wrong for anyone who got as far as resending. diff --git a/.changeset/sign-in-errors-at-point-of-failure.md b/.changeset/sign-in-errors-at-point-of-failure.md new file mode 100644 index 00000000..7b15520a --- /dev/null +++ b/.changeset/sign-in-errors-at-point-of-failure.md @@ -0,0 +1,11 @@ +--- +'ePDS': patch +--- + +Sign-in error messages now appear next to the field that caused them, instead of at the top of the page. + +**Affects:** End users + +**End users:** a rejected sign-in code used to report the problem above the page heading, several elements away from the boxes you had just typed into — easy to miss, and it left the **Verify** button looking like the thing to press again. The message now sits directly between the code boxes and **Verify**, and a failed email submission likewise reads under the email field rather than above it. + +A rejected code now also carries a **Resend code** link beside the message. The standalone button below the form was easy to overlook, and "that code didn't work" often means there is no usable code left at all rather than that you mistyped it — after too many wrong attempts, after signing in from another tab, or once an old code has aged out. Retyping cannot recover any of those. The boxes still clear and refocus, so retyping remains one keystroke away when that is what you need. diff --git a/docs/screenshots/pr-232-before-low-contrast-muted-text.png b/docs/screenshots/pr-232-before-low-contrast-muted-text.png deleted file mode 100644 index 31a90426..00000000 Binary files a/docs/screenshots/pr-232-before-low-contrast-muted-text.png and /dev/null differ diff --git a/docs/screenshots/pr-232-muted-text-contrast.png b/docs/screenshots/pr-232-muted-text-contrast.png deleted file mode 100644 index e5bdfbbb..00000000 Binary files a/docs/screenshots/pr-232-muted-text-contrast.png and /dev/null differ diff --git a/e2e/step-definitions/auth.steps.ts b/e2e/step-definitions/auth.steps.ts index 2e18271c..58f0e36d 100644 --- a/e2e/step-definitions/auth.steps.ts +++ b/e2e/step-definitions/auth.steps.ts @@ -625,6 +625,20 @@ Then( }, ) +When( + 'the user enters two digits from the old OTP', + async function (this: EpdsWorld) { + const otpBoxes = getPage(this).locator('.otp-box') + await otpBoxes.nth(0).fill('1') + await otpBoxes.nth(1).fill('2') + // Prove the digits actually landed, so the later empty-box assertion + // demonstrates that resend cleared them rather than that they were + // never entered. + await expect(otpBoxes.nth(0)).toHaveValue('1') + await expect(otpBoxes.nth(1)).toHaveValue('2') + }, +) + When( 'the user requests a new OTP via the resend button', async function (this: EpdsWorld) { @@ -665,6 +679,21 @@ Then( }, ) +Then( + 'the OTP entry boxes are empty with the first box focused', + async function (this: EpdsWorld) { + if (!this.otpCode) { + throw new Error('No fresh OTP was captured from the mail trap') + } + const otpBoxes = getPage(this).locator('.otp-box') + await expect(otpBoxes).toHaveCount(this.otpCode.length) + for (let index = 0; index < this.otpCode.length; index += 1) { + await expect(otpBoxes.nth(index)).toHaveValue('') + } + await expect(otpBoxes.first()).toBeFocused() + }, +) + // --------------------------------------------------------------------------- // Refresh / idempotency scenario // --------------------------------------------------------------------------- diff --git a/features/passwordless-authentication.feature b/features/passwordless-authentication.feature index 1492e1a9..05c74e3d 100644 --- a/features/passwordless-authentication.feature +++ b/features/passwordless-authentication.feature @@ -265,8 +265,10 @@ Feature: Passwordless authentication via email OTP And the user enters the OTP code Then the verification form shows an "OTP expired" error And the OTP entry boxes are visible and enabled - When the user requests a new OTP via the resend button + When the user enters two digits from the old OTP + And the user requests a new OTP via the resend button Then a fresh OTP email arrives in the mail trap for the test email + And the OTP entry boxes are empty with the first box focused When the user enters the OTP code And the user picks a handle Then the browser is redirected back to the demo client diff --git a/packages/auth-service/src/__tests__/login-page.test.ts b/packages/auth-service/src/__tests__/login-page.test.ts index 32cb57c2..d9eb070a 100644 --- a/packages/auth-service/src/__tests__/login-page.test.ts +++ b/packages/auth-service/src/__tests__/login-page.test.ts @@ -500,7 +500,9 @@ describe('renderLoginPage handle login button', () => { // tests pin its structure so accidental refactors (removing the guard, // moving it after the fetch, resetting the flag unconditionally on // success) fail loudly. -function renderDefault(): string { +type LoginPageOpts = Parameters[0] + +function renderDefault(overrides: Partial = {}): string { return renderLoginPage({ flowId: 'flow-1', clientId: 'https://example.com/client-metadata.json', @@ -518,6 +520,7 @@ function renderDefault(): string { otpLength: 6, otpCharset: 'numeric', heartbeatEnabled: false, + ...overrides, }) } @@ -618,15 +621,54 @@ 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('renders exactly one flash region, inside the active step', () => { + // One element, so there is only ever one aria-live region for a + // screen reader to track; it is reparented between the two slots + // on step transitions rather than duplicated. + const emailStep = renderDefault() + expect(emailStep.match(/id="error-msg"/g)).toHaveLength(1) + expect(emailStep).toMatch(/flash-slot-email">
{ 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). + // Position is the point of the slot: a rejected code must read at + // the input the user just filled, not above the subtitle. + const boxes = html.indexOf('id="otp-boxes"') + const slot = html.indexOf('id="flash-slot-otp"') + const verify = html.indexOf('>Verify<') + expect(boxes).toBeGreaterThan(-1) + expect(slot).toBeGreaterThan(boxes) + expect(verify).toBeGreaterThan(slot) + }) + + it('offers an inline resend on any rejected code', () => { + const html = renderDefault() + // "Invalid OTP" is not reliably a typo — better-auth also throws it + // when no stored code exists at all, which is the terminal state + // after a lockout, after the code was consumed in another tab, and + // after expiry cleanup. Retyping cannot recover any of those, so + // the action must not be withheld from the plain-invalid branch. expect(html).toMatch( - /if \(isExpired\) \{[\s\S]*?\} else \{[\s\S]*?showError\(result\.error\);\s*\}/, + /else if \(!parLikelyDead\(\)\)[\s\S]*?showErrorWithAction\(\s*result\.error,\s*'Resend code'/, ) }) + + it('withholds the inline resend once the flow is dead', () => { + const html = renderDefault() + // refreshResendVisibility() hides the standalone Resend button when + // the PAR is dead; an inline one would re-offer a withdrawn action. + // The bare else is that path — plain message, no CTA. + expect(html).toMatch(/\} else \{\s*showError\(result\.error\);\s*\}/) + }) }) describe('renderLoginPage flow-aborted notice + reactive abort gates', () => { @@ -723,6 +765,28 @@ describe('renderLoginPage flow-aborted notice + reactive abort gates', () => { expect(branchSlice).toContain('showFlowAbortedNotice();') }) + it('tells the user on resend that earlier codes are dead', () => { + const html = renderDefault() + // Resending invalidates every earlier OTP, and a user who got as + // far as resending may have mail sitting in spam. Both facts are + // noise for the majority who sign in on the first code, so they + // live in the resend confirmation rather than in permanently + // visible page copy — which is what makes this worth pinning: a + // refactor that "tidies" the message back to a bare + // acknowledgement silently loses both. + const handlerStart = html.indexOf("'btn-resend').addEventListener") + expect(handlerStart).toBeGreaterThan(0) + const handlerEnd = html.indexOf( + "'btn-back').addEventListener", + handlerStart, + ) + const handlerBody = html.slice(handlerStart, handlerEnd) + expect(handlerBody).toContain('earlier ones no longer work') + expect(handlerBody).toContain('spam folder') + // The success branch must not regress to a bare acknowledgement. + expect(handlerBody).not.toContain("showSuccess('Code resent!')") + }) + it('gates the Resend click on abortIfFlowDead', () => { const html = renderDefault() // The Resend click handler must call abortIfFlowDead and diff --git a/packages/auth-service/src/routes/login-page.ts b/packages/auth-service/src/routes/login-page.ts index 14416e38..f4459fb3 100644 --- a/packages/auth-service/src/routes/login-page.ts +++ b/packages/auth-service/src/routes/login-page.ts @@ -559,6 +559,14 @@ export function renderLoginPage(opts: { const hasGithub = 'github' in socialProviders const hasSocialProviders = hasGoogle || hasGithub + // The flash region is a single element shared by both steps, so there + // is only ever one live region for assistive tech to track. It is + // server-rendered into whichever step is initially visible and moved + // between the two slots on step transitions; both transitions call + // clearError() first, so it is always empty when it moves. + const flashRegionHtml = + '' + // Social login buttons — redirect to better-auth provider endpoints const socialButtonsHtml = hasSocialProviders ? ` @@ -633,6 +641,7 @@ export function renderLoginPage(opts: { .divider { display: flex; align-items: center; gap: 12px; margin: 20px 0; color: var(--muted-foreground); font-size: 13px; } .divider::before, .divider::after { content: ''; flex: 1; height: 1px; background: #ececec; } .flash-msg { padding: 12px; border-radius: 10px; margin: 12px 0; font-size: 14px; text-align: center; } + .flash-msg.hidden { display: none; } .flash-msg.error { color: #dc3545; background: #fdf0f0; } .flash-msg.success { color: #28a745; background: #f0fff4; } /* Inline action button rendered next to an OTP-expired error so @@ -662,8 +671,6 @@ export function renderLoginPage(opts: { ${logoHtml}

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

- - ${socialButtonsHtml} @@ -677,6 +684,11 @@ export function renderLoginPage(opts: { value="${escapeHtml(opts.loginHint)}">
${renderEmailTypoGuardMarkup()} + +
${opts.initialStep === 'otp' ? '' : flashRegionHtml}
${handleLoginButtonHtml} @@ -704,6 +716,12 @@ export function renderLoginPage(opts: { ) .join('\n ')}
+ +
${opts.initialStep === 'otp' ? flashRegionHtml : ''}
@@ -1023,7 +1041,7 @@ export function renderLoginPage(opts: { function setFlash(kind, buildContent) { errorEl.classList.remove('error', 'success'); errorEl.classList.add(kind); - errorEl.style.display = 'block'; + errorEl.classList.remove('hidden'); var frag = document.createDocumentFragment(); buildContent(frag); @@ -1078,13 +1096,32 @@ export function renderLoginPage(opts: { }); } + /** + * Reparent the single flash region into the active step's slot, + * so a message always renders at the point of failure — under + * the email field on the email step, between the code boxes and + * Verify on the OTP step. + * + * Callers must clearError() first: moving a *populated* live + * region across parents can re-announce or drop the message + * depending on the screen reader. Both step transitions already + * clear before switching, so the region is empty whenever it + * moves here. + */ + function moveFlashTo(slotId) { + var slot = document.getElementById(slotId); + if (slot && errorEl && errorEl.parentNode !== slot) { + slot.appendChild(errorEl); + } + } + function clearError() { // Empty the region before hiding it. Clearing after the - // display:none would mutate a region that is already out of + // region is hidden would mutate one that is already out of // the accessibility tree, which some assistive tech reports // as a stale announcement. errorEl.replaceChildren(); - errorEl.style.display = 'none'; + errorEl.classList.add('hidden'); errorEl.classList.remove('error', 'success'); } @@ -1138,6 +1175,7 @@ export function renderLoginPage(opts: { clearOtpBoxes(); if (otpBoxes.length) otpBoxes[0].focus(); clearError(); + moveFlashTo('flash-slot-otp'); startHeartbeat(); refreshResendVisibility(); } @@ -1148,6 +1186,7 @@ export function renderLoginPage(opts: { headingEl.textContent = 'Sign in'; if (termsEl) termsEl.style.display = 'block'; clearError(); + moveFlashTo('flash-slot-email'); stopHeartbeat(); // Reset the email field — the user clicked "Use different // email" precisely to escape the previous value, so leaving @@ -1285,6 +1324,43 @@ export function renderLoginPage(opts: { document.getElementById('btn-resend').click(); }); } + } else if (!parLikelyDead()) { + // Every other verify failure gets the same inline + // shortcut, for the same reason as the expired path: the + // standalone Resend button sits below the form and is + // easy to miss. + // + // 3d31876 originally kept non-expired errors on the plain + // path, reasoning that a typo should be retyped rather + // than resent. But "Invalid OTP" does not reliably mean + // a typo: better-auth throws it from two places + // (1.4.18 email-otp/routes.mjs) — the wrong-code + // comparison, and a missing stored code. The second + // covers several states where retyping cannot possibly + // work and a fresh code is the only recovery: + // + // - after a lockout. TOO_MANY_ATTEMPTS deletes the + // stored value, so it is reported exactly once and + // every later submit falls through to "Invalid OTP". + // - after the code was consumed elsewhere, e.g. the + // user completed sign-in in another tab. + // - after expiry cleanup deleted the value, so a later + // submit reads "Invalid OTP" rather than "expired". + // + // Since the branch cannot distinguish those from a typo, + // withholding the action strands the users who need it + // most. Offering it costs a typo-ing user nothing: the + // boxes are cleared and focused for retyping either way. + // + // Gated on parLikelyDead() because + // refreshResendVisibility() hides the standalone Resend + // button in that state; surfacing an inline one anyway + // would re-offer an action the page has deliberately + // withdrawn. The aborted-flow notice carries its own + // restart action, so nothing is lost by staying quiet. + showErrorWithAction(result.error, 'Resend code', function() { + document.getElementById('btn-resend').click(); + }); } else { showError(result.error); } @@ -1324,7 +1400,18 @@ export function renderLoginPage(opts: { if (result.error) { showError(result.error); } else { - showSuccess('Code resent!'); + // Clear any characters typed for the old code so the new code + // starts from a clean, focused input grid. + clearOtpBoxes(); + if (otpBoxes.length) otpBoxes[0].focus(); + // Both facts only matter once a resend has happened, so they + // live here rather than in permanently-visible page copy: + // sending a new OTP invalidates every earlier one, and a user + // who needed to resend is the user whose mail may be in spam. + showSuccess( + 'Resent! Make sure to use the new code; earlier ones no longer work. ' + + 'It may be in your spam folder.', + ); } });