Skip to content

Commit c932f4c

Browse files
authored
Merge pull request #189 from hypercerts-org/fix/log-otp-failures-email
feat(auth-service): log OTP verification failures with the user's email
2 parents 9765fd8 + 4a0861d commit c932f4c

4 files changed

Lines changed: 366 additions & 1 deletion

File tree

.agents/skills/writing-changesets/SKILL.md

Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -224,6 +224,49 @@ to adapt their environment or code **without reading the PR or
224224
the source**. Changesets are not release-note marketing copy —
225225
they are migration instructions.
226226

227+
### Keep it as short as the change deserves
228+
229+
"Concrete" is not "exhaustive". Include exactly what the reader
230+
needs to adapt, and stop. Most changesets are **two to four
231+
sentences per audience**; a small change is often a single
232+
sentence. Length must track the size of what the reader has to do,
233+
not the effort that went into the change. A one-line behaviour
234+
tweak gets a one-line changeset even if the PR was large.
235+
236+
Brevity and bullets are not in tension: cutting is about _how many_
237+
points survive, bullets are about _how you lay them out_ so the
238+
survivors stay scannable. First cut to the points the reader must
239+
act on; then, if 3+ remain, lay them out as bullets rather than
240+
packing them into one paragraph — a short multi-point section is
241+
still a wall of text when run together. Keep one- or two-point
242+
sections as inline prose. If you find yourself writing a fourth or
243+
fifth bullet, that is a signal to cut, not to keep listing.
244+
245+
Before you finish, delete anything the reader does not act on:
246+
247+
- **A verbatim example** (a full log line, a whole JSON body) when
248+
naming the fields already tells them what to grep or send. Show
249+
a shape only when the exact serialization is the thing they must
250+
match.
251+
- **Rationale and "why" prose** — "since this can signal X", "so
252+
that Y". State the observable behaviour; the reasoning belongs
253+
in the commit or PR, not the release note.
254+
- **Exhaustive enumerations** — every endpoint, every reason
255+
string, every excluded case — when a representative name plus
256+
"and related …" conveys the same adaptation. List the full set
257+
only when the reader must match against each member.
258+
- **Internal filtering the reader never sees** — "3xx are filtered
259+
before this runs, 5xx are logged elsewhere". If it doesn't change
260+
what they do, it doesn't belong.
261+
- **Caveats about edge cases** the reader cannot act on. One
262+
clause at most; usually cut.
263+
264+
Rule of thumb: if removing a sentence would not change what the
265+
reader _does_, remove it. A tight four-line changeset that names
266+
the fields and the observable effect beats a fifteen-line one that
267+
also explains the mechanism, quotes a sample, and enumerates every
268+
case.
269+
227270
### What "concrete detail" means
228271

229272
Good adaptation detail includes:
Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
---
2+
'ePDS': patch
3+
---
4+
5+
Failed one-time code attempts now appear in the server logs, split by reason and tagged with the account's email.
6+
7+
**Affects:** Operators
8+
9+
**Operators:** failed OTP verifications are now logged under the `auth:better-auth` logger.
10+
11+
- Each line carries an `email`, `statusCode`, and `path` field, and names the reason: `code expired`, `invalid or unrecognized code`, or `too many attempts, code invalidated`.
12+
- Routine failures (expired / invalid) log at `info`; too-many-attempts logs at `warn`.
13+
- All are visible at the default `info` level — no `LOG_LEVEL` change needed.
Lines changed: 172 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,172 @@
1+
/**
2+
* Tests for logOtpVerificationFailure — the bridge that surfaces better-auth's
3+
* 4xx OTP verification errors ("OTP expired" vs "Invalid OTP" vs "Too many
4+
* attempts") in our pino logs, with the user's email attached.
5+
*
6+
* These are exactly the errors better-auth throws from its email-otp verify
7+
* endpoint (better-auth 1.4.18 dist/plugins/email-otp/routes.mjs), which the
8+
* browser posts to directly and which were previously logged nowhere.
9+
*/
10+
import { describe, it, expect, vi } from 'vitest'
11+
import { APIError } from 'better-auth/api'
12+
import { isOtpVerifyPath, logOtpVerificationFailure } from '../better-auth.js'
13+
14+
const PATH = '/sign-in/email-otp'
15+
16+
/** Minimal stand-in for the pino logger — only `info` and `warn` are used. */
17+
function makeLogger() {
18+
return { info: vi.fn(), warn: vi.fn() } as unknown as Parameters<
19+
typeof logOtpVerificationFailure
20+
>[3]
21+
}
22+
23+
describe('logOtpVerificationFailure', () => {
24+
it('maps an expired-OTP error to a self-contained message with the email', () => {
25+
const log = makeLogger()
26+
const error = new APIError('BAD_REQUEST', { message: 'OTP expired' })
27+
logOtpVerificationFailure(error, 'alice@example.com', PATH, log)
28+
29+
expect(log.info).toHaveBeenCalledOnce()
30+
expect(log.info).toHaveBeenCalledWith(
31+
{ email: 'alice@example.com', statusCode: 400, path: PATH },
32+
'OTP verification failed: code expired',
33+
)
34+
expect(log.warn).not.toHaveBeenCalled()
35+
})
36+
37+
it('maps an invalid-OTP error, distinct from expiry', () => {
38+
const log = makeLogger()
39+
logOtpVerificationFailure(
40+
new APIError('BAD_REQUEST', { message: 'Invalid OTP' }),
41+
'bob@example.com',
42+
PATH,
43+
log,
44+
)
45+
46+
expect(log.info).toHaveBeenCalledWith(
47+
{ email: 'bob@example.com', statusCode: 400, path: PATH },
48+
'OTP verification failed: invalid or unrecognized code',
49+
)
50+
})
51+
52+
it('logs a 403 too-many-attempts error at warn (possible abuse signal)', () => {
53+
const log = makeLogger()
54+
logOtpVerificationFailure(
55+
new APIError('FORBIDDEN', { message: 'Too many attempts' }),
56+
'carol@example.com',
57+
PATH,
58+
log,
59+
)
60+
61+
expect(log.warn).toHaveBeenCalledWith(
62+
{ email: 'carol@example.com', statusCode: 403, path: PATH },
63+
'OTP verification failed: too many attempts, code invalidated',
64+
)
65+
expect(log.info).not.toHaveBeenCalled()
66+
})
67+
68+
it('logs the broad-scope path verbatim in the path field', () => {
69+
const log = makeLogger()
70+
logOtpVerificationFailure(
71+
new APIError('BAD_REQUEST', { message: 'Invalid OTP' }),
72+
'dave@example.com',
73+
'/email-otp/reset-password',
74+
log,
75+
)
76+
77+
expect(log.info).toHaveBeenCalledWith(
78+
expect.objectContaining({ path: '/email-otp/reset-password' }),
79+
'OTP verification failed: invalid or unrecognized code',
80+
)
81+
})
82+
83+
it('falls back to the prefixed reason for an unmapped 4xx reason', () => {
84+
const log = makeLogger()
85+
logOtpVerificationFailure(
86+
new APIError('BAD_REQUEST', { message: 'Some other reason' }),
87+
'eve@example.com',
88+
PATH,
89+
log,
90+
)
91+
92+
expect(log.info).toHaveBeenCalledWith(
93+
expect.objectContaining({ email: 'eve@example.com', statusCode: 400 }),
94+
'OTP verification failed: Some other reason',
95+
)
96+
})
97+
98+
it('logs email as undefined when the body had no email', () => {
99+
const log = makeLogger()
100+
logOtpVerificationFailure(
101+
new APIError('BAD_REQUEST', { message: 'Invalid OTP' }),
102+
undefined,
103+
PATH,
104+
log,
105+
)
106+
107+
expect(log.info).toHaveBeenCalledWith(
108+
{ email: undefined, statusCode: 400, path: PATH },
109+
'OTP verification failed: invalid or unrecognized code',
110+
)
111+
})
112+
113+
it('ignores 5xx errors (already logged by better-auth itself)', () => {
114+
const log = makeLogger()
115+
logOtpVerificationFailure(
116+
new APIError('INTERNAL_SERVER_ERROR', { message: 'boom' }),
117+
'alice@example.com',
118+
PATH,
119+
log,
120+
)
121+
122+
expect(log.warn).not.toHaveBeenCalled()
123+
expect(log.info).not.toHaveBeenCalled()
124+
})
125+
126+
it('ignores 3xx redirects', () => {
127+
const log = makeLogger()
128+
logOtpVerificationFailure(
129+
new APIError('FOUND'),
130+
'alice@example.com',
131+
PATH,
132+
log,
133+
)
134+
135+
expect(log.warn).not.toHaveBeenCalled()
136+
expect(log.info).not.toHaveBeenCalled()
137+
})
138+
139+
it('ignores non-APIError values (e.g. a successful response)', () => {
140+
const log = makeLogger()
141+
logOtpVerificationFailure(
142+
new Error('some other failure'),
143+
'a@x.com',
144+
PATH,
145+
log,
146+
)
147+
logOtpVerificationFailure({ token: 'ok' }, 'a@x.com', PATH, log)
148+
logOtpVerificationFailure(undefined, 'a@x.com', PATH, log)
149+
150+
expect(log.warn).not.toHaveBeenCalled()
151+
expect(log.info).not.toHaveBeenCalled()
152+
})
153+
})
154+
155+
describe('isOtpVerifyPath', () => {
156+
it('matches the OTP verification endpoints', () => {
157+
expect(isOtpVerifyPath('/sign-in/email-otp')).toBe(true)
158+
expect(isOtpVerifyPath('/email-otp/check-verification-otp')).toBe(true)
159+
expect(isOtpVerifyPath('/email-otp/verify-email')).toBe(true)
160+
expect(isOtpVerifyPath('/email-otp/reset-password')).toBe(true)
161+
})
162+
163+
it('excludes the OTP send endpoints (a send failure is not a verification failure)', () => {
164+
expect(isOtpVerifyPath('/email-otp/send-verification-otp')).toBe(false)
165+
expect(isOtpVerifyPath('/email-otp/request-password-reset')).toBe(false)
166+
})
167+
168+
it('excludes unrelated endpoints', () => {
169+
expect(isOtpVerifyPath('/sign-in/social')).toBe(false)
170+
expect(isOtpVerifyPath('/callback/google')).toBe(false)
171+
})
172+
})

0 commit comments

Comments
 (0)