Skip to content
Merged
Show file tree
Hide file tree
Changes from 6 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.

When the code is rejected because of too many wrong attempts, the message now carries a **Resend code** link beside it. That case wipes the stored code, so retyping cannot work and a fresh code is the only way forward; the standalone **Resend code** button below the form was easy to overlook. A simple mistyped code is unchanged — the boxes clear and refocus so you can just type it again.
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
78 changes: 72 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,13 +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()
// 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('falls back to the plain showError on a merely-wrong code', () => {
const html = renderDefault()
// A typo must NOT carry a resend CTA: the boxes are cleared and
// refocused, so retyping is the intended recovery. The final else
// — reached when the error is neither expired nor code-
// invalidating — stays on the plain path.
expect(html).toMatch(/\} else \{\s*showError\(result\.error\);\s*\}/)
})

it('offers an inline resend when the stored code was invalidated', () => {
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).
// "Too many attempts" deletes the stored code server-side, so
// there is nothing left to retype and resending is the only route
// forward — unlike a typo, which the plain path above covers.
expect(html).toMatch(
/if \(isExpired\) \{[\s\S]*?\} else \{[\s\S]*?showError\(result\.error\);\s*\}/,
/needsFreshCode\s*=\s*\/too many attempts\|invalidated\/i/,
)
expect(html).toMatch(
/else if \(needsFreshCode && !parLikelyDead\(\)\)[\s\S]*?showErrorWithAction\(\s*result\.error,\s*'Resend code'/,
)
})
})
Expand Down Expand Up @@ -723,6 +767,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
95 changes: 89 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 @@ -1269,6 +1308,13 @@ export function renderLoginPage(opts: {
// expired code") and the auth-service wording ("OTP
// expired") plus generic "expir"/"too long" variants.
var isExpired = /expir|too long/i.test(result.error);
// "Too many attempts" invalidates the stored code, so the
// user has nothing left to retype and a resend is the only
// route forward. Matched separately from isExpired because
// the wording shares no substring with it.
var needsFreshCode = /too many attempts|invalidated/i.test(
result.error,
);
if (isExpired) {
// Only offer "Send a new code" when the PAR is still
// alive. If it isn't, a fresh OTP would issue but
Expand All @@ -1285,6 +1331,32 @@ export function renderLoginPage(opts: {
document.getElementById('btn-resend').click();
});
}
} else if (needsFreshCode && !parLikelyDead()) {
// A rejected or invalidated code is the other case a
// fresh one actually fixes, so it gets the same inline
// shortcut as the expired path, for the same reason:
// the standalone Resend button sits below the form and
// is easy to miss.
//
// 3d31876 originally kept every non-expired error on the
// plain path so a typo wouldn't carry a "Send a new
// code" CTA. That reasoning holds for a typo — the user
// should retype, and the boxes are already cleared and
// focused for exactly that — but not for "too many
// attempts", which deletes the stored code server-side
// (better-auth 1.4.18 email-otp/routes.mjs, per the note
// in better-auth.ts). After a lockout there is no code
// left to retype, so resending is the only way forward.
//
// 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 +1396,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