diff --git a/.changeset/auth-reset-passwordless-signin.md b/.changeset/auth-reset-passwordless-signin.md new file mode 100644 index 0000000..9292abc --- /dev/null +++ b/.changeset/auth-reset-passwordless-signin.md @@ -0,0 +1,7 @@ +--- +'@factiii/auth': patch +--- + +A password reset requested for an account with no password now sends an email sign-in (link and code) when the request names an app with email sign-in, instead of refusing. The sign-in goes through the same path as `auth.emailLogin.request`, so it shares its rate limits and single open attempt. + +`sendPasswordResetEmail` now gives one answer for every address — `{ message: 'If an account exists with that email, we sent a link.' }` — padded to the email sign-in response floor. It no longer returns different messages or errors for an address with no account, an account with no password, or an account with no email, and a reset email the provider rejects is logged instead of thrown. An unknown `app` key is still refused with `Unknown app.` diff --git a/packages/auth/src/procedures/base.ts b/packages/auth/src/procedures/base.ts index 7b4ff9d..1764964 100644 --- a/packages/auth/src/procedures/base.ts +++ b/packages/auth/src/procedures/base.ts @@ -6,6 +6,7 @@ import { type AuthProcedure, type BaseProcedure } from '../types/trpc'; import { assertCanMintSession } from '../utilities/accountStatus'; import { detectBrowser } from '../utilities/browser'; import { sameIdentifier } from '../utilities/emailMatch'; +import { issueEmailLoginAttempt, logEmailSendFailure, padToResponseFloor } from './emailLogin'; import { runDeviceStep } from './twoFa/deviceStep'; import type { ResolvedAuthConfig } from '../utilities/config'; import { clearAuthCookies, setAuthCookies } from '../utilities/cookies'; @@ -16,7 +17,7 @@ import { } from '../utilities/issueCookies'; import { createAuthToken } from '../utilities/jwt'; import { comparePassword, hashPassword } from '../utilities/password'; -import type { UsernameMode } from '../types/config'; +import type { EmailLoginAppConfig, UsernameMode } from '../types/config'; import type { ExtendedSignupHookInput, SchemaExtensions } from '../types/hooks'; import { changePasswordSchema, @@ -30,6 +31,13 @@ import { type LoginSchemaInput, } from '../validators'; +/** + * The one answer a password reset request gets, whatever happened. A different + * answer for "no account", "no password" or "no email" would tell anyone who asks + * whether an address has an account. + */ +const RESET_REQUEST_ANSWER = 'If an account exists with that email, we sent a link.'; + /** * Factory for core authentication procedures: register, login, logout, * token refresh, session management, and password reset flows. @@ -568,7 +576,7 @@ export class BaseProcedureFactory< } private sendPasswordResetEmail() { - return this.procedure.input(requestPasswordResetSchema).mutation(async ({ input }) => { + return this.procedure.input(requestPasswordResetSchema).mutation(async ({ ctx, input }) => { const { email, app } = input; // Checked before the account lookup, so an unknown key answers the same @@ -581,48 +589,69 @@ export class BaseProcedureFactory< throw new TRPCError({ code: 'BAD_REQUEST', message: 'Unknown app.' }); } - const found = await this.config.database.user.findByEmailInsensitive(email); - // A reset link for a look-alike address must never go to another account. - const user = found && sameIdentifier(found.email, email) ? found : null; - - if (!user || user.status !== 'ACTIVE') { - return { message: 'If an account exists with that email, a reset link has been sent.' }; + // Every outcome — no account, an inactive one, one with no email, one with + // no password, one with a password, a refused rate limit — gets the same + // answer padded to the same floor as an email sign-in request. + const startedAt = Date.now(); + try { + await this.issueReset(email, app, appSettings, ctx.ip); + } finally { + await padToResponseFloor(startedAt, this.config.emailLogin?.responseFloorMs ?? 0); } - if (!user.password) { - throw new TRPCError({ - code: 'BAD_REQUEST', - message: 'This account uses social login. Please use that method.', - }); - } + return { message: RESET_REQUEST_ANSWER }; + }); + } - // Username-first consumers allow accounts with no email, which have no - // way to receive the link. - if (!user.email) { - throw new TRPCError({ - code: 'BAD_REQUEST', - message: 'This account has no email address to send a reset link to.', + /** Sends whatever a reset request should send, or nothing; never says which. */ + private async issueReset( + email: string, + app: string | undefined, + appSettings: EmailLoginAppConfig | undefined, + ip: string | undefined + ): Promise { + const found = await this.config.database.user.findByEmailInsensitive(email); + // A reset link for a look-alike address must never go to another account. + const user = found && sameIdentifier(found.email, email) ? found : null; + + // Username-first consumers allow accounts with no email, which have no + // way to receive the link. + if (!user || user.status !== 'ACTIVE' || !user.email) return; + + if (!user.password) { + // Nothing to reset. An app with email sign-in sends that instead, so the + // person still gets in; without one there is nothing to send. + const { emailLogin } = this.config; + const attempts = this.config.database.emailLoginAttempt; + if (this.config.features.emailLogin && emailLogin && attempts && app && appSettings) { + await issueEmailLoginAttempt(this.config, emailLogin, attempts, { + email: user.email, + appKey: app, + ip, }); } + return; + } - await this.config.database.passwordReset.deleteAllByUserId(user.id); - - const passwordReset = await this.config.database.passwordReset.create(user.id); - - if (this.config.emailService) { - const token = String(passwordReset.id); - if (app && appSettings) { - await this.config.emailService.sendPasswordResetEmail(user.email, token, { - app, - resetUrl: `${appSettings.siteUrl}${appSettings.resetPath}/${encodeURIComponent(token)}`, - }); - } else { - await this.config.emailService.sendPasswordResetEmail(user.email, token); - } - } - - return { message: 'Password reset email sent.' }; - }); + await this.config.database.passwordReset.deleteAllByUserId(user.id); + + const passwordReset = await this.config.database.passwordReset.create(user.id); + + if (this.config.emailService) { + const token = String(passwordReset.id); + const send = + app && appSettings + ? this.config.emailService.sendPasswordResetEmail(user.email, token, { + app, + resetUrl: `${appSettings.siteUrl}${appSettings.resetPath}/${encodeURIComponent(token)}`, + }) + : this.config.emailService.sendPasswordResetEmail(user.email, token); + // Not awaited, for the reason the sign-in email is not (emailLogin.ts): the + // provider's latency and its per-address errors must not reach the answer. + void send.catch((err: unknown) => + logEmailSendFailure(this.config, err, ip, 'passwordReset', 'the password reset email') + ); + } } private checkPasswordReset() { diff --git a/packages/auth/src/procedures/emailLogin.ts b/packages/auth/src/procedures/emailLogin.ts index 744ab29..e2238d0 100644 --- a/packages/auth/src/procedures/emailLogin.ts +++ b/packages/auth/src/procedures/emailLogin.ts @@ -56,7 +56,8 @@ const LINK_EXPIRED = 'That link has expired. Ask for a new one.'; const EMAIL_NOT_CONFIRMED = 'Sign in another way, then confirm this email in your account.'; const TOO_MANY_TRIES = 'Too many tries. Wait a few minutes and try again.'; -type AttemptStore = NonNullable; +export type EmailLoginAttemptStore = NonNullable; +type AttemptStore = EmailLoginAttemptStore; /** Trim and lowercase, so one inbox is one rate-limit key and one attempt chain. */ export function normalizeLoginEmail(email: string): string { @@ -141,6 +142,132 @@ const PEEKS_PER_IP = 30; type PeekLinkResult = { valid: true; maskedEmail: string } | { valid: false }; +/** Pad an outcome to `floorMs` after `startedAt`, so the timing does not show which outcome it was. */ +export async function padToResponseFloor(startedAt: number, floorMs: number): Promise { + const elapsed = Date.now() - startedAt; + if (elapsed < floorMs) { + await sleep(floorMs - elapsed); + } +} + +/** + * Log an email that could not be sent. Never throws: its callers fire their + * sends without awaiting them, and a send failure must not reach any response. + */ +export function logEmailSendFailure( + config: ResolvedAuthConfig, + err: unknown, + ip: string | undefined, + source: string, + what: string +): void { + const error = err instanceof Error ? err : new Error(String(err)); + if (config.hooks?.logError) { + void config.hooks + .logError({ + type: 'OTHER', + description: `${source}: ${what} could not be sent: ${error.message}`, + stack: error.stack ?? '', + ip, + }) + .catch(() => undefined); + return; + } + console.error(`@factiii/auth: ${what} could not be sent`, error); +} + +export interface IssueEmailLoginAttemptInput { + /** The address to sign in; normalized here. */ + email: string; + /** An `emailLogin.apps` key. Checked again here (own keys only). */ + appKey: string; + ip: string | undefined; +} + +/** + * Issue one email sign-in attempt and send its email. This is the one path behind + * `auth.emailLogin.request` and behind a password reset asked for by an account + * that has no password, so both get the same rate limits, the same single open + * attempt per address, and nothing stored in plaintext. + * + * Returns false, having sent nothing, when a rate limit refused the request or + * `appKey` is not a configured app. The send is never awaited and never throws. + */ +export async function issueEmailLoginAttempt( + config: ResolvedAuthConfig, + emailLogin: ResolvedEmailLoginConfig, + attempts: EmailLoginAttemptStore, + input: IssueEmailLoginAttemptInput +): Promise { + // Own keys only, so `constructor` or `__proto__` is not an app. + const app = Object.prototype.hasOwnProperty.call(emailLogin.apps, input.appKey) + ? emailLogin.apps[input.appKey] + : undefined; + if (!app) return false; + + const email = normalizeLoginEmail(input.email); + const ip = input.ip ?? 'unknown'; + // Each limit is asked only when the one before it passed, so a refused + // request does not also spend the wider budgets. + const allowed = + (await emailLogin.rateLimit( + `emailLogin:request:emailip:${email}:${ip}`, + REQUESTS_PER_EMAIL_AND_IP, + LIMIT_WINDOW_SEC + )) && + (await emailLogin.rateLimit(`emailLogin:request:ip:${ip}`, REQUESTS_PER_IP, LIMIT_WINDOW_SEC)) && + (await emailLogin.rateLimit( + `emailLogin:request:email:${email}`, + REQUESTS_PER_EMAIL, + LIMIT_WINDOW_SEC + )); + + // Rate-limited: send nothing, and say nothing different about it. + if (!allowed) return false; + + const found = await config.database.user.findByEmailInsensitive(email); + // A lookup is only as exact as its adapter. The address the inbox proves is + // the one that has to be on the account. + const user = found && sameIdentifier(found.email, email) ? found : null; + + // A newer request replaces every open one, so an email the user did + // not act on stops working the moment they ask again. + await attempts.consumeOpenByEmail(email); + + const id = randomUUID(); + const token = randomBytes(TOKEN_BYTES).toString('base64url'); + const code = String(randomInt(0, CODE_SPACE)).padStart(CODE_DIGITS, '0'); + const expiresAt = new Date(Date.now() + emailLogin.ttlMs); + + await attempts.create({ + id, + email, + userId: user?.id ?? null, + app: input.appKey, + tokenHash: hashLoginToken(token), + codeHash: hashLoginCode(emailLogin.pepper, id, code), + expiresAt, + }); + + // Not awaited. The mail provider's latency would make an allowed request + // slower than a rate-limited one, and its errors differ by address (a + // suppressed or bouncing inbox fails where a good one does not) — neither + // may reach the response. + void emailLogin + .sendLoginEmail({ + to: email, + app: input.appKey, + brand: app.brand, + link: `${app.siteUrl}${app.verifyPath}?token=${encodeURIComponent(token)}`, + code, + expiresAt, + }) + .catch((err: unknown) => + logEmailSendFailure(config, err, input.ip, 'emailLogin', 'the sign-in email') + ); + return true; +} + /** Factory for `auth.emailLogin.*`. */ export class EmailLoginProcedureFactory { constructor( @@ -224,94 +351,23 @@ export class EmailLoginProcedureFactory { const startedAt = Date.now(); try { - const email = normalizeLoginEmail(input.email); - const ip = ctx.ip ?? 'unknown'; - // Each limit is asked only when the one before it passed, so a refused - // request does not also spend the wider budgets. - const allowed = - (await emailLogin.rateLimit( - `emailLogin:request:emailip:${email}:${ip}`, - REQUESTS_PER_EMAIL_AND_IP, - LIMIT_WINDOW_SEC - )) && - (await emailLogin.rateLimit( - `emailLogin:request:ip:${ip}`, - REQUESTS_PER_IP, - LIMIT_WINDOW_SEC - )) && - (await emailLogin.rateLimit( - `emailLogin:request:email:${email}`, - REQUESTS_PER_EMAIL, - LIMIT_WINDOW_SEC - )); - - // Rate-limited: send nothing, and say nothing different about it. - if (allowed) { - const user = await this.accountFor(email); - - // A newer request replaces every open one, so an email the user did - // not act on stops working the moment they ask again. - await attempts.consumeOpenByEmail(email); - - const id = randomUUID(); - const token = randomBytes(TOKEN_BYTES).toString('base64url'); - const code = String(randomInt(0, CODE_SPACE)).padStart(CODE_DIGITS, '0'); - const expiresAt = new Date(Date.now() + emailLogin.ttlMs); - - await attempts.create({ - id, - email, - userId: user?.id ?? null, - app: input.app, - tokenHash: hashLoginToken(token), - codeHash: hashLoginCode(emailLogin.pepper, id, code), - expiresAt, - }); - - // Not awaited. The mail provider's latency would make an allowed request - // slower than a rate-limited one, and its errors differ by address (a - // suppressed or bouncing inbox fails where a good one does not) — neither - // may reach the response. - void emailLogin - .sendLoginEmail({ - to: email, - app: input.app, - brand: app.brand, - link: `${app.siteUrl}${app.verifyPath}?token=${encodeURIComponent(token)}`, - code, - expiresAt, - }) - .catch((err: unknown) => this.logSendFailure(err, ctx)); - } + // The rate limits, the attempt and the send live in the shared path, which + // a password reset for an account with no password also takes. + await issueEmailLoginAttempt(this.config, emailLogin, attempts, { + email: input.email, + appKey: input.app, + ip: ctx.ip, + }); } finally { // Pad every outcome to one floor, so an address with an account does not // answer faster or slower than one without. - const elapsed = Date.now() - startedAt; - if (elapsed < emailLogin.responseFloorMs) { - await sleep(emailLogin.responseFloorMs - elapsed); - } + await padToResponseFloor(startedAt, emailLogin.responseFloorMs); } return { sent: true as const }; }); } - private logSendFailure(err: unknown, ctx: TrpcContext) { - const error = err instanceof Error ? err : new Error(String(err)); - if (this.config.hooks?.logError) { - void this.config.hooks - .logError({ - type: 'OTHER', - description: `emailLogin: the sign-in email could not be sent: ${error.message}`, - stack: error.stack ?? '', - ip: ctx.ip, - }) - .catch(() => undefined); - return; - } - console.error('@factiii/auth: the sign-in email could not be sent', error); - } - private verifyLink() { return this.procedure.input(emailLoginVerifyLinkSchema).mutation(async ({ ctx, input }) => { const { emailLogin, attempts } = this.settings(); diff --git a/packages/auth/tests/emailLogin.test.ts b/packages/auth/tests/emailLogin.test.ts index 52bde8a..91641b3 100644 --- a/packages/auth/tests/emailLogin.test.ts +++ b/packages/auth/tests/emailLogin.test.ts @@ -774,6 +774,90 @@ describe('password reset per app', () => { }); }); +describe('password reset for an account without a password', () => { + const RESET_ANSWER = { message: 'If an account exists with that email, we sent a link.' }; + + it('sends an email sign-in instead, and its code signs in', async () => { + const h = harness({ users: [account()] }); + + const result = await h.caller.sendPasswordResetEmail({ email: 'Ada@Example.com', app: 'oakbox' }); + + expect(result).toEqual(RESET_ANSWER); + expect(h.database.passwordReset.create).not.toHaveBeenCalled(); + expect(h.emailService.sendPasswordResetEmail).not.toHaveBeenCalled(); + expect(h.attempts).toHaveLength(1); + const email = h.lastEmail(); + expect(email.to).toBe('ada@example.com'); + expect(email.link.startsWith('https://oakbox.me/auth/email?token=')).toBe(true); + expect(email.code).toMatch(/^\d{6}$/); + + const signIn = await h.caller.emailLogin.verifyCode({ email: 'ada@example.com', code: email.code }); + expect(signIn).toMatchObject({ success: true, created: false }); + }); + + it('still sends a reset link to an account with a password, with the same answer', async () => { + const h = harness({ users: [account({ password: 'hashed' })] }); + + const result = await h.caller.sendPasswordResetEmail({ email: 'ada@example.com', app: 'oakbox' }); + + expect(result).toEqual(RESET_ANSWER); + expect(h.emailService.sendPasswordResetEmail).toHaveBeenCalledOnce(); + expect(h.sent).toHaveLength(0); + }); + + it('answers the same and sends nothing when there is nothing to send', async () => { + const h = harness({ + users: [ + account(), + account({ id: 8, email: 'inactive@example.com', username: 'inactive', status: 'DEACTIVATED' }), + ], + }); + + const answers = [ + // No account (an account with no email is found by no address either). + await h.caller.sendPasswordResetEmail({ email: 'nobody@example.com', app: 'oakbox' }), + await h.caller.sendPasswordResetEmail({ email: 'inactive@example.com', app: 'oakbox' }), + // No password, and no app to send an email sign-in for. + await h.caller.sendPasswordResetEmail({ email: 'ada@example.com' }), + ]; + + expect(answers).toEqual([RESET_ANSWER, RESET_ANSWER, RESET_ANSWER]); + expect(h.sent).toHaveLength(0); + expect(h.attempts).toHaveLength(0); + expect(h.emailService.sendPasswordResetEmail).not.toHaveBeenCalled(); + }); + + it('shares its rate limit with emailLogin.request', async () => { + const h = harness({ users: [account()] }); + for (let i = 0; i < 3; i += 1) { + await h.caller.emailLogin.request({ email: 'ada@example.com', app: 'oakbox' }); + } + expect(h.sent).toHaveLength(3); + + const result = await h.caller.sendPasswordResetEmail({ email: 'ada@example.com', app: 'oakbox' }); + + expect(result).toEqual(RESET_ANSWER); + expect(h.sent).toHaveLength(3); + }); + + it('a reset email the provider rejects still gets the same answer', async () => { + const quiet = vi.spyOn(console, 'error').mockImplementation(() => {}); + try { + const h = harness({ users: [account({ password: 'hashed' })] }); + h.emailService.sendPasswordResetEmail.mockRejectedValueOnce(new Error('suppressed')); + + await expect( + h.caller.sendPasswordResetEmail({ email: 'ada@example.com', app: 'oakbox' }) + ).resolves.toEqual(RESET_ANSWER); + // Let the fired send settle and log before the spy is restored. + await new Promise((resolve) => setTimeout(resolve, 0)); + expect(quiet).toHaveBeenCalled(); + } finally { + quiet.mockRestore(); + } + }); +}); + describe('createAuthConfig with features.emailLogin', () => { const valid = () => ({ database: { emailLoginAttempt: {} }, diff --git a/packages/auth/tests/insensitiveLookup.test.ts b/packages/auth/tests/insensitiveLookup.test.ts index 48ce2c9..d3ae411 100644 --- a/packages/auth/tests/insensitiveLookup.test.ts +++ b/packages/auth/tests/insensitiveLookup.test.ts @@ -219,7 +219,7 @@ describe('callers re-check the lookup result', () => { .sendPasswordResetEmail({ email: 'j_hn@outlook.com' }); expect(result).toEqual({ - message: 'If an account exists with that email, a reset link has been sent.', + message: 'If an account exists with that email, we sent a link.', }); expect(passwordReset.create).not.toHaveBeenCalled(); expect(sendPasswordResetEmail).not.toHaveBeenCalled();