Skip to content
Draft
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
6 changes: 6 additions & 0 deletions .agents/skills/writing-changesets/SKILL.md
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,12 @@ Skip changesets for:
routes under `/_internal/`). These are contributor-facing, not
consumer-facing.

Create one changeset per coherent user-facing change, not one per file,
route, or UI surface. Closely related fixes that ship together — such as
the same error-state improvement across sign-in and recovery forms —
share one concise changeset. Split them only when consumers could
meaningfully adopt or release them independently.

If in doubt, add one. An "empty" changeset (intentionally no
release) can be created with `pnpm changeset add --empty` to
document the decision.
Expand Down
9 changes: 9 additions & 0 deletions .changeset/otp-lockout-recovery.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
---
'ePDS': patch
---

After too many wrong codes, sign-in and account recovery now explain that the old code cannot be used and offer a way to get a fresh one.

**Affects:** End users

**End users:** Use **Send a new code** on the sign-in page, or the nearby **Resend code** button on account-management and recovery forms, instead of retrying a code that has been locked out.
69 changes: 68 additions & 1 deletion e2e/step-definitions/auth.steps.ts
Original file line number Diff line number Diff line change
Expand Up @@ -611,7 +611,7 @@ Then(
)

Then(
/^the verification form shows (?:an|the) "([^"]*)" error$/,
/^the verification form shows (?:an|the|a) "([^"]*)" error$/,
async function (this: EpdsWorld, expected: string) {
const page = getPage(this)
await expect(page.locator('#error-msg')).toBeVisible({ timeout: 10_000 })
Expand Down Expand Up @@ -969,3 +969,70 @@ Then('the email input is empty and focused', async function (this: EpdsWorld) {
await expect(input).toHaveValue('', { timeout: 5_000 })
await expect(input).toBeFocused({ timeout: 5_000 })
})

// ---------------------------------------------------------------------------
// "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).
// Burn five attempts directly, then trigger the sixth through the form
// so the submit handler's error rendering is exercised.
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 () {})`,
)
}
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 })
await expect(page.locator('#error-msg')).toHaveText(
/\.\s*Send a new code$/i,
)
},
)

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 })
},
)
33 changes: 33 additions & 0 deletions features/passwordless-authentication.feature
Original file line number Diff line number Diff line change
Expand Up @@ -386,6 +386,7 @@ Feature: Passwordless authentication via email OTP
# programmatically clearing the demo's `oauth_state` cookie just
# before the OTP submission, which is equivalent to the cookie
# having lapsed by wall-clock.

# 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
Expand All @@ -403,6 +404,38 @@ Feature: Passwordless authentication via email OTP
When the user clicks "Use different email"
Then the email input is empty and focused

# better-auth allows five wrong attempts on one code. The sixth
# submit returns "Too many attempts" and deletes the verification
# row, so the only useful next action is requesting a fresh code.
# The banner asserts the rewritten end-user copy from otpErrorText(),
# not better-auth's raw developer string, because that mapped wording
# is what the user actually reads.
@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 a "Too many tries — that code is no longer usable." error
And a "Send a new code" inline action is offered

# Once lockout deletes the verification row, later submissions return
# the generic "Invalid OTP" response. Retyping cannot recover that
# state, so the inline action must still be offered on the follow-up
# submit rather than stranding the user on a bare error.
@email @too-many-attempts
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

@email @demo-cookie-expiry @bug-report
Scenario: Demo client's OAuth cookie has expired by the time of callback — useful error, not generic auth_failed
When the demo client starts a new OAuth flow with random handle mode
Expand Down
47 changes: 47 additions & 0 deletions packages/auth-service/src/__tests__/otp-verify-error.test.ts
Original file line number Diff line number Diff line change
@@ -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)
},
)
})
38 changes: 38 additions & 0 deletions packages/auth-service/src/lib/otp-verify-error.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
/**
* 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. The current code can no longer succeed, so
* point the user at Resend.
*
* 2. "Invalid OTP" — usually a recoverable typo. better-auth uses
* the same text after deleting a locked-out row, so these stateless
* server-rendered routes cannot distinguish that later case and must
* avoid claiming certainty they do not have.
*
* 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 attempts|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.'
}
7 changes: 2 additions & 5 deletions packages/auth-service/src/routes/account-login.ts
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ import {
renderEmailTypoGuardMarkup,
renderEmailTypoGuardScript,
} from '../lib/email-typo-guard.js'
import { pickOtpVerifyErrorMessage } from '../lib/otp-verify-error.js'

const logger = createLogger('auth:account-login')

Expand Down Expand Up @@ -121,17 +122,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),
}),
)
}
Expand Down
11 changes: 6 additions & 5 deletions packages/auth-service/src/routes/recovery.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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')

Expand Down Expand Up @@ -268,11 +269,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(
Expand Down
Loading