Background
PR #34 added a server-side ToS acceptance hook (enforceTosAcceptance in packages/auth-service/src/better-auth.ts) on better-auth's hooks.before. The hook gates /sign-in/email-otp on tosAccepted: true when getDidByEmail(email) returns null (new user).
The recovery verify endpoint (POST /auth/recover/verify in packages/auth-service/src/routes/recovery.ts) also calls auth.api.signInEmailOTP(...) to consume the OTP that was sent via the recovery path. Recovery's OTP recipient is a backup email, not a primary PDS email, so getDidByEmail(backupEmail) returns null and the hook treats recovery as a new-user sign-in — which would 400 every legitimate recovery.
#34 works around this by passing tosAccepted: true from the recovery path, guarded (in be97f04) by ctx.db.getDidByBackupEmail(email) — only emails that are actually verified backup emails for a real account can bypass the hook.
Why this is still a smell
The underlying architectural issue is that login and recovery share a single OTP store, keyed only by email, with no notion of intent. Both paths:
- call
auth.api.sendVerificationOTP({ body: { email, type: 'sign-in' } }) to issue the OTP
- call
auth.api.signInEmailOTP({ body: { email, otp, ... } }) to consume it
An OTP minted on the login path can be redeemed on the recovery path and vice versa. The authorization-layer guard (getDidByBackupEmail) closes the specific hole where an attacker obtained a login OTP and tried to submit it to /auth/recover/verify to bypass ToS, but it's a band-aid over a shared-namespace problem.
Proposal
Give login and recovery distinct OTP namespaces so an OTP minted for one purpose literally cannot satisfy the other. Options to explore:
- Type-namespaced OTPs. Pass a different
type to better-auth's emailOTP plugin for recovery (e.g. 'recovery'). The plugin keys the stored OTP by (email, type), so cross-path redemption becomes structurally impossible. Requires checking better-auth supports non-built-in types or forking the plugin.
- Separate OTP store for recovery. Stop routing recovery through
auth.api.signInEmailOTP entirely. Have recovery own its own recovery_otp table + verification, then mint a session via a different better-auth API (e.g. directly set up the session by DID).
Option 1 is the smaller diff. Option 2 is more isolation but more code.
Benefits
- Removes the need for
tosAccepted: true on the recovery path entirely — the hook simply never fires there, because recovery doesn't use the login API.
- The
getDidByBackupEmail guard in /auth/recover/verify can stay as a rate-limit/abuse signal but is no longer load-bearing for ToS enforcement.
- Defense in depth: a future code change that drops the authorization guard would still be safe against the ToS-bypass attack because there's no OTP to satisfy.
Related
Background
PR #34 added a server-side ToS acceptance hook (
enforceTosAcceptanceinpackages/auth-service/src/better-auth.ts) on better-auth'shooks.before. The hook gates/sign-in/email-otpontosAccepted: truewhengetDidByEmail(email)returns null (new user).The recovery verify endpoint (
POST /auth/recover/verifyinpackages/auth-service/src/routes/recovery.ts) also callsauth.api.signInEmailOTP(...)to consume the OTP that was sent via the recovery path. Recovery's OTP recipient is a backup email, not a primary PDS email, sogetDidByEmail(backupEmail)returns null and the hook treats recovery as a new-user sign-in — which would 400 every legitimate recovery.#34 works around this by passing
tosAccepted: truefrom the recovery path, guarded (in be97f04) byctx.db.getDidByBackupEmail(email)— only emails that are actually verified backup emails for a real account can bypass the hook.Why this is still a smell
The underlying architectural issue is that login and recovery share a single OTP store, keyed only by email, with no notion of intent. Both paths:
auth.api.sendVerificationOTP({ body: { email, type: 'sign-in' } })to issue the OTPauth.api.signInEmailOTP({ body: { email, otp, ... } })to consume itAn OTP minted on the login path can be redeemed on the recovery path and vice versa. The authorization-layer guard (
getDidByBackupEmail) closes the specific hole where an attacker obtained a login OTP and tried to submit it to/auth/recover/verifyto bypass ToS, but it's a band-aid over a shared-namespace problem.Proposal
Give login and recovery distinct OTP namespaces so an OTP minted for one purpose literally cannot satisfy the other. Options to explore:
typeto better-auth's emailOTP plugin for recovery (e.g.'recovery'). The plugin keys the stored OTP by(email, type), so cross-path redemption becomes structurally impossible. Requires checking better-auth supports non-built-in types or forking the plugin.auth.api.signInEmailOTPentirely. Have recovery own its ownrecovery_otptable + verification, then mint a session via a different better-auth API (e.g. directly set up the session by DID).Option 1 is the smaller diff. Option 2 is more isolation but more code.
Benefits
tosAccepted: trueon the recovery path entirely — the hook simply never fires there, because recovery doesn't use the login API.getDidByBackupEmailguard in/auth/recover/verifycan stay as a rate-limit/abuse signal but is no longer load-bearing for ToS enforcement.Related