Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 9 additions & 0 deletions .changeset/clear-otp-boxes-on-resend.md
Original file line number Diff line number Diff line change
@@ -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.
11 changes: 11 additions & 0 deletions .changeset/sign-in-errors-at-point-of-failure.md
Original file line number Diff line number Diff line change
@@ -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.
Binary file not shown.
Binary file removed docs/screenshots/pr-232-muted-text-contrast.png
Binary file not shown.
29 changes: 29 additions & 0 deletions e2e/step-definitions/auth.steps.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down Expand Up @@ -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
// ---------------------------------------------------------------------------
Expand Down
4 changes: 3 additions & 1 deletion features/passwordless-authentication.feature
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
76 changes: 70 additions & 6 deletions packages/auth-service/src/__tests__/login-page.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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<typeof renderLoginPage>[0]

function renderDefault(overrides: Partial<LoginPageOpts> = {}): string {
return renderLoginPage({
flowId: 'flow-1',
clientId: 'https://example.com/client-metadata.json',
Expand All @@ -518,6 +520,7 @@ function renderDefault(): string {
otpLength: 6,
otpCharset: 'numeric',
heartbeatEnabled: false,
...overrides,
})
}

Expand Down Expand Up @@ -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"><div id="error-msg"/)

const otpStep = renderDefault({
loginHint: 'a@b.com',
initialStep: 'otp',
otpAlreadySent: true,
})
expect(otpStep.match(/id="error-msg"/g)).toHaveLength(1)
expect(otpStep).toMatch(/flash-slot-otp"><div id="error-msg"/)
})

it('places the OTP flash slot between the boxes and Verify', () => {
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', () => {
Expand Down Expand Up @@ -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
Expand Down
99 changes: 93 additions & 6 deletions packages/auth-service/src/routes/login-page.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 =
'<div id="error-msg" class="flash-msg hidden" role="status" aria-live="polite"></div>'

// Social login buttons — redirect to better-auth provider endpoints
const socialButtonsHtml = hasSocialProviders
? `
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -662,8 +671,6 @@ export function renderLoginPage(opts: {
${logoHtml}
<h1 id="heading">${opts.initialStep === 'otp' ? 'Enter your code' : 'Sign in'}</h1>

<div id="error-msg" class="flash-msg" style="display:none;" role="status" aria-live="polite"></div>

${socialButtonsHtml}

<!-- Step 1: Email entry (calls better-auth sendOtp) -->
Expand All @@ -677,6 +684,11 @@ export function renderLoginPage(opts: {
value="${escapeHtml(opts.loginHint)}">
</div>
${renderEmailTypoGuardMarkup()}
<!-- Flash slot: the shared #error-msg region is moved in here
while the email step is active, so a send failure reads
directly under the field that caused it rather than above
the heading. -->
<div id="flash-slot-email">${opts.initialStep === 'otp' ? '' : flashRegionHtml}</div>
<button type="submit" class="btn-primary">Continue</button>
</form>
${handleLoginButtonHtml}
Expand Down Expand Up @@ -704,6 +716,12 @@ export function renderLoginPage(opts: {
)
.join('\n ')}
</div>
<!-- Flash slot: see #flash-slot-email. Sitting between the
boxes and Verify puts a rejected-code message at the point
of failure — where the user's attention already is after
typing — instead of above the subtitle, and places the
inline Resend action next to it. -->
<div id="flash-slot-otp">${opts.initialStep === 'otp' ? flashRegionHtml : ''}</div>
<button type="submit" class="btn-primary">Verify</button>
</form>
<div class="otp-actions">
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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');
}

Expand Down Expand Up @@ -1138,6 +1175,7 @@ export function renderLoginPage(opts: {
clearOtpBoxes();
if (otpBoxes.length) otpBoxes[0].focus();
clearError();
moveFlashTo('flash-slot-otp');
startHeartbeat();
refreshResendVisibility();
}
Expand All @@ -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
Expand Down Expand Up @@ -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);
}
Expand Down Expand Up @@ -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.',
);
}
});

Expand Down
Loading