diff --git a/packages/admin-ui/src/pages/EmailTemplates.tsx b/packages/admin-ui/src/pages/EmailTemplates.tsx index 5c88292..64132c9 100644 --- a/packages/admin-ui/src/pages/EmailTemplates.tsx +++ b/packages/admin-ui/src/pages/EmailTemplates.tsx @@ -24,6 +24,12 @@ const TEMPLATE_DEFINITIONS: TemplateDefinition[] = [ description: "Sent when a user must verify their email during signup.", variables: ["name", "verification_link"], }, + { + key: "signup_existing_account_notice", + label: "Existing account signup notice", + description: "Sent when signup is attempted with an email that already has an account.", + variables: ["name", "recovery_link"], + }, { key: "verification_resend_confirmation", label: "Resend confirmation", diff --git a/packages/admin-ui/src/services/api.ts b/packages/admin-ui/src/services/api.ts index 11b6d54..b98f752 100644 --- a/packages/admin-ui/src/services/api.ts +++ b/packages/admin-ui/src/services/api.ts @@ -309,6 +309,7 @@ export interface AdminSetting { export type EmailTemplateKey = | "signup_verification" + | "signup_existing_account_notice" | "verification_resend_confirmation" | "email_change_verification" | "password_recovery" diff --git a/packages/api/drizzle/0019_prevent_email_enumeration_on_registration.sql b/packages/api/drizzle/0019_prevent_email_enumeration_on_registration.sql new file mode 100644 index 0000000..3f1e750 --- /dev/null +++ b/packages/api/drizzle/0019_prevent_email_enumeration_on_registration.sql @@ -0,0 +1,12 @@ +INSERT INTO "settings" ("key", "name", "type", "category", "description", "tags", "default_value", "value", "secure", "updated_at") VALUES +('users.prevent_email_enumeration_on_registration', 'Prevent Email Enumeration on Registration', 'boolean', 'Users', 'Pretend registration works for existing users when registering using an existing email. Only applies when email verification and SMTP are enabled.', ARRAY['users', 'email', 'security']::text[], 'false'::jsonb, 'false'::jsonb, false, now()), +('email.templates.signup_existing_account_notice', 'signup_existing_account_notice Template', 'object', 'Email / Templates', 'Template for signup_existing_account_notice', ARRAY['email', 'templates']::text[], '{"subject":"Someone tried to create an account with this email","text":"Hello {{name}},\n\nSomeone attempted to create a new account using this email address, but an account already exists.\n\nIf this was you and you forgot your password, recover access here:\n{{recovery_link}}","html":"

Hello {{name}},

Someone attempted to create a new account using this email address, but an account already exists.

If this was you and you forgot your password, recover access here:

Recover account

"}'::jsonb, '{"subject":"Someone tried to create an account with this email","text":"Hello {{name}},\n\nSomeone attempted to create a new account using this email address, but an account already exists.\n\nIf this was you and you forgot your password, recover access here:\n{{recovery_link}}","html":"

Hello {{name}},

Someone attempted to create a new account using this email address, but an account already exists.

If this was you and you forgot your password, recover access here:

Recover account

"}'::jsonb, false, now()) +ON CONFLICT ("key") DO UPDATE SET +"name" = excluded."name", +"type" = excluded."type", +"category" = excluded."category", +"description" = excluded."description", +"tags" = excluded."tags", +"default_value" = excluded."default_value", +"secure" = excluded."secure", +"updated_at" = now(); diff --git a/packages/api/drizzle/meta/_journal.json b/packages/api/drizzle/meta/_journal.json index 226d61b..92664b8 100644 --- a/packages/api/drizzle/meta/_journal.json +++ b/packages/api/drizzle/meta/_journal.json @@ -120,6 +120,13 @@ "when": 1772449200000, "tag": "0018_default_clients_and_org_rbac", "breakpoints": true + }, + { + "idx": 17, + "version": "7", + "when": 1772538000000, + "tag": "0019_prevent_email_enumeration_on_registration", + "breakpoints": true } ] } diff --git a/packages/api/src/controllers/admin/emailTemplates.ts b/packages/api/src/controllers/admin/emailTemplates.ts index d84aa6d..f78f6b5 100644 --- a/packages/api/src/controllers/admin/emailTemplates.ts +++ b/packages/api/src/controllers/admin/emailTemplates.ts @@ -20,6 +20,7 @@ const TemplateSchema = z.object({ const KeySchema = z.enum([ "signup_verification", + "signup_existing_account_notice", "verification_resend_confirmation", "email_change_verification", "password_recovery", diff --git a/packages/api/src/controllers/user/opaqueLoginStart.ts b/packages/api/src/controllers/user/opaqueLoginStart.ts index 9ecc178..b6af438 100644 --- a/packages/api/src/controllers/user/opaqueLoginStart.ts +++ b/packages/api/src/controllers/user/opaqueLoginStart.ts @@ -1,6 +1,6 @@ import type { IncomingMessage, ServerResponse } from "node:http"; import { z } from "zod/v4"; -import { ValidationError } from "../../errors.ts"; +import { NotFoundError, ValidationError } from "../../errors.ts"; import { genericErrors } from "../../http/openapi-helpers.ts"; import { getCachedBody, withRateLimit } from "../../middleware/rateLimit.ts"; import { getUserOpaqueRecordByEmail } from "../../models/users.ts"; @@ -48,11 +48,18 @@ export const postOpaqueLoginStart = withRateLimit("opaque", (body) => } context.logger.debug({ reqLen: requestBuffer.length }, "decoded request"); - const userLookup = await getUserOpaqueRecordByEmail(context, parsed.email); - context.logger.debug({ found: !!userLookup.user }, "user lookup"); + let userLookup: Awaited> | null = null; + try { + userLookup = await getUserOpaqueRecordByEmail(context, parsed.email); + } catch (error) { + if (!(error instanceof NotFoundError)) { + throw error; + } + } + context.logger.debug({ found: !!userLookup?.user }, "user lookup"); let loginResponse: OpaqueLoginResponse; - if (!userLookup.user) { + if (!userLookup?.user) { loginResponse = await opaque.startLoginWithDummy(requestBuffer, parsed.email); } else { const envelopeBuffer = userLookup.envelope as unknown as Buffer | string | null; diff --git a/packages/api/src/models/registration.test.ts b/packages/api/src/models/registration.test.ts new file mode 100644 index 0000000..985e0e4 --- /dev/null +++ b/packages/api/src/models/registration.test.ts @@ -0,0 +1,168 @@ +import assert from "node:assert/strict"; +import fs from "node:fs"; +import os from "node:os"; +import path from "node:path"; +import { test } from "node:test"; +import { createPglite } from "../db/pglite.ts"; +import { users } from "../db/schema.ts"; +import { ConflictError } from "../errors.ts"; +import { setSetting } from "../services/settings.ts"; +import type { Context } from "../types.ts"; +import { userOpaqueRegisterFinish } from "./registration.ts"; + +function createLogger() { + return { + error() {}, + warn() {}, + info() {}, + debug() {}, + trace() {}, + fatal() {}, + }; +} + +async function createTestContext() { + const directory = fs.mkdtempSync(path.join(os.tmpdir(), "darkauth-registration-test-")); + const { db, close } = await createPglite(directory); + const context = { + db, + config: { + publicOrigin: "http://localhost:9080", + }, + logger: createLogger(), + services: { + opaque: { + finishRegistration: async () => ({ + envelope: new Uint8Array([1, 2, 3]), + serverPublicKey: new Uint8Array([4, 5, 6]), + }), + }, + }, + } as unknown as Context; + + const cleanup = async () => { + await close(); + fs.rmSync(directory, { recursive: true, force: true }); + }; + + return { context, cleanup }; +} + +test("duplicate registration returns conflict by default", async () => { + const { context, cleanup } = await createTestContext(); + try { + await context.db.insert(users).values({ + sub: "user-existing-default", + email: "existing-default@example.com", + name: "Existing Default", + createdAt: new Date(), + }); + + await assert.rejects( + () => + userOpaqueRegisterFinish(context, { + record: new Uint8Array([9, 9, 9]), + email: "existing-default@example.com", + name: "New Attempt", + }), + (error: unknown) => + error instanceof ConflictError && + error.message === "A user with this email address already exists" + ); + } finally { + await cleanup(); + } +}); + +test("duplicate registration returns conflict when anti-enumeration is enabled but verification is disabled", async () => { + const { context, cleanup } = await createTestContext(); + try { + await context.db.insert(users).values({ + sub: "user-existing-no-verify", + email: "existing-no-verify@example.com", + name: "Existing No Verify", + createdAt: new Date(), + }); + await setSetting(context, "users.prevent_email_enumeration_on_registration", true); + await setSetting(context, "users.require_email_verification", false); + + await assert.rejects( + () => + userOpaqueRegisterFinish(context, { + record: new Uint8Array([9, 9, 9]), + email: "existing-no-verify@example.com", + name: "New Attempt", + }), + (error: unknown) => + error instanceof ConflictError && + error.message === "A user with this email address already exists" + ); + } finally { + await cleanup(); + } +}); + +test("duplicate registration returns conflict when anti-enumeration is enabled and verification is enabled but smtp is unavailable", async () => { + const { context, cleanup } = await createTestContext(); + try { + await context.db.insert(users).values({ + sub: "user-existing-no-smtp", + email: "existing-no-smtp@example.com", + name: "Existing No SMTP", + createdAt: new Date(), + }); + await setSetting(context, "users.prevent_email_enumeration_on_registration", true); + await setSetting(context, "users.require_email_verification", true); + await setSetting(context, "email.smtp.enabled", false); + + await assert.rejects( + () => + userOpaqueRegisterFinish(context, { + record: new Uint8Array([9, 9, 9]), + email: "existing-no-smtp@example.com", + name: "New Attempt", + }), + (error: unknown) => + error instanceof ConflictError && + error.message === "A user with this email address already exists" + ); + } finally { + await cleanup(); + } +}); + +test("duplicate registration returns conflict when anti-enumeration path cannot send the notice email", async () => { + const { context, cleanup } = await createTestContext(); + try { + await context.db.insert(users).values({ + sub: "user-existing-send-fail", + email: "existing-send-fail@example.com", + name: "Existing Send Fail", + createdAt: new Date(), + }); + + await setSetting(context, "users.prevent_email_enumeration_on_registration", true); + await setSetting(context, "users.require_email_verification", true); + await setSetting(context, "email.smtp.enabled", true); + await setSetting(context, "email.transport", "smtp"); + await setSetting(context, "email.from", "noreply@example.com"); + await setSetting(context, "email.smtp.host", "127.0.0.1"); + await setSetting(context, "email.smtp.port", 1); + await setSetting(context, "email.smtp.user", "user"); + await setSetting(context, "email.smtp.password", "pass"); + + await assert.rejects( + () => + userOpaqueRegisterFinish(context, { + record: new Uint8Array([9, 9, 9]), + email: "existing-send-fail@example.com", + name: "New Attempt", + }), + (error: unknown) => + error instanceof ConflictError && + error.message === "A user with this email address already exists" + ); + } finally { + await cleanup(); + } +}); diff --git a/packages/api/src/models/registration.ts b/packages/api/src/models/registration.ts index 7b5d58b..0d7effe 100644 --- a/packages/api/src/models/registration.ts +++ b/packages/api/src/models/registration.ts @@ -7,8 +7,12 @@ import { roles, users, } from "../db/schema.ts"; -import { ValidationError } from "../errors.ts"; -import { sendSignupVerification } from "../services/emailVerification.ts"; +import { ConflictError, ValidationError } from "../errors.ts"; +import { isEmailSendingAvailable } from "../services/email.ts"; +import { + sendSignupExistingAccountNotice, + sendSignupVerification, +} from "../services/emailVerification.ts"; import { createSession } from "../services/sessions.ts"; import { getSetting } from "../services/settings.ts"; import type { Context } from "../types.ts"; @@ -23,19 +27,28 @@ export async function userOpaqueRegisterFinish( const opaqueRecord = await context.services.opaque.finishRegistration(data.record, data.email); const requireEmailVerification = (await getSetting(context, "users.require_email_verification")) === true; + const preventEmailEnumeration = + (await getSetting(context, "users.prevent_email_enumeration_on_registration")) === true; const { generateRandomString } = await import("../utils/crypto.ts"); const sub = generateRandomString(16); - // Check if user already exists before transaction - // Return a generic success response to prevent user enumeration attacks const existingUser = await context.db.query.users.findFirst({ where: eq(users.email, data.email), }); if (existingUser) { - const { generateRandomString: genFakeId } = await import("../utils/crypto.ts"); - return { - sub: genFakeId(16), - requiresEmailVerification: true, - }; + const emailSendingAvailable = await isEmailSendingAvailable(context); + if (preventEmailEnumeration && requireEmailVerification && emailSendingAvailable) { + try { + await sendSignupExistingAccountNotice(context, { + email: data.email, + name: existingUser.name || data.name, + }); + } catch (error) { + context.logger.warn(error, "Failed to send existing-account signup notice"); + throw new ConflictError("A user with this email address already exists"); + } + return { sub, requiresEmailVerification: true }; + } + throw new ConflictError("A user with this email address already exists"); } await context.db.transaction(async (tx) => { diff --git a/packages/api/src/services/emailTemplates.ts b/packages/api/src/services/emailTemplates.ts index 97e13b9..4adce26 100644 --- a/packages/api/src/services/emailTemplates.ts +++ b/packages/api/src/services/emailTemplates.ts @@ -3,6 +3,7 @@ import { getSetting, setSetting } from "./settings.ts"; export type EmailTemplateKey = | "signup_verification" + | "signup_existing_account_notice" | "verification_resend_confirmation" | "email_change_verification" | "password_recovery" @@ -20,6 +21,11 @@ const DEFAULT_TEMPLATES: Record = { text: "Hello {{name}},\n\nPlease verify your email by opening this link:\n{{verification_link}}\n\nIf you did not create this account, ignore this email.", html: '

Hello {{name}},

Please verify your email by opening this link:

Verify email

If you did not create this account, ignore this email.

', }, + signup_existing_account_notice: { + subject: "Someone tried to create an account with this email", + text: "Hello {{name}},\n\nSomeone attempted to create a new account using this email address, but an account already exists.\n\nIf this was you and you forgot your password, recover access here:\n{{recovery_link}}", + html: '

Hello {{name}},

Someone attempted to create a new account using this email address, but an account already exists.

If this was you and you forgot your password, recover access here:

Recover account

', + }, verification_resend_confirmation: { subject: "A new verification link has been sent", text: "Hello {{name}},\n\nA new verification link has been requested for this account.\n\nIf this was you, use the newest email in your inbox.", diff --git a/packages/api/src/services/emailVerification.ts b/packages/api/src/services/emailVerification.ts index 99adcf5..58f3d64 100644 --- a/packages/api/src/services/emailVerification.ts +++ b/packages/api/src/services/emailVerification.ts @@ -87,6 +87,20 @@ export async function sendSignupVerification( }); } +export async function sendSignupExistingAccountNotice( + context: Context, + params: { email: string; name: string } +): Promise { + await sendTemplatedEmail(context, { + to: params.email, + template: "signup_existing_account_notice", + variables: { + name: params.name || params.email, + recovery_link: `${context.config.publicOrigin}/login`, + }, + }); +} + export async function resendSignupVerificationByEmail( context: Context, email: string diff --git a/packages/test-suite/setup/helpers/admin.ts b/packages/test-suite/setup/helpers/admin.ts index 1de8f07..47790b3 100644 --- a/packages/test-suite/setup/helpers/admin.ts +++ b/packages/test-suite/setup/helpers/admin.ts @@ -33,26 +33,28 @@ export async function ensureAdminDashboard( }); } -export async function ensureSelfRegistrationEnabled(page: Page): Promise { - await page.click('a[href="/settings"], button:has-text("Settings")'); - const usersSectionTrigger = page.getByRole('button', { name: 'Users', exact: true }).first(); - const triggerState = await usersSectionTrigger.getAttribute('data-state'); - if (triggerState !== 'open') { - await usersSectionTrigger.click(); - } - const usersRegion = page.getByRole('region', { name: 'Users', exact: true }); - await expect(usersRegion).toBeVisible({ timeout: 10000 }); - const checkbox = usersRegion.getByRole('checkbox').nth(1); - await expect(checkbox).toBeVisible({ timeout: 5000 }); - const state = await checkbox.getAttribute('data-state'); - if (state === 'checked') return; - const updateResponse = page.waitForResponse((response) => { - return response.url().endsWith('/admin/settings') && response.request().method() === 'PUT'; +export async function ensureSelfRegistrationEnabled( + servers: TestServers, + admin: AdminCredentials +): Promise { + const adminSession = await getAdminSession(servers, admin); + const updateResponse = await fetch(`${servers.adminUrl}/admin/settings`, { + method: 'PUT', + headers: { + Cookie: adminSession.cookieHeader, + Origin: servers.adminUrl, + 'Content-Type': 'application/json', + 'x-csrf-token': adminSession.csrfToken, + }, + body: JSON.stringify({ + key: 'users.self_registration_enabled', + value: true, + }), }); - await checkbox.click(); - const response = await updateResponse; - expect(response.ok()).toBeTruthy(); - await expect(checkbox).toHaveAttribute('data-state', 'checked', { timeout: 5000 }); + if (!updateResponse.ok) { + const errorText = await updateResponse.text().catch(() => ''); + throw new Error(`failed to enable self-registration: ${updateResponse.status} ${errorText}`); + } } export async function configureDemoClient( diff --git a/packages/test-suite/tests/demo/demo-app-note-flow.spec.ts b/packages/test-suite/tests/demo/demo-app-note-flow.spec.ts index 5cdccb3..8fe3900 100644 --- a/packages/test-suite/tests/demo/demo-app-note-flow.spec.ts +++ b/packages/test-suite/tests/demo/demo-app-note-flow.spec.ts @@ -59,7 +59,7 @@ test.describe('Demo App Note Flow', () => { } const bundle: DemoServerBundle = { servers, demoApi, demoUi }; await ensureAdminDashboard(page, servers, secondaryAdmin); - await ensureSelfRegistrationEnabled(page); + await ensureSelfRegistrationEnabled(servers, secondaryAdmin); await configureDemoClient(servers, secondaryAdmin, demoUi.url); const { user, page: userPage, snapshot } = await registerDemoUser(context, servers); const demoPage = await openDemoDashboard(context, bundle, user, snapshot); @@ -74,7 +74,7 @@ test.describe('Demo App Note Flow', () => { const bundle: DemoServerBundle = { servers, demoApi, demoUi }; const adminPage = await context.newPage(); await ensureAdminDashboard(adminPage, servers, secondaryAdmin); - await ensureSelfRegistrationEnabled(adminPage); + await ensureSelfRegistrationEnabled(servers, secondaryAdmin); await configureDemoClient(servers, secondaryAdmin, demoUi.url); await adminPage.close(); const userData = await registerDemoUser(context, servers); diff --git a/packages/user-ui/src/components/Login.tsx b/packages/user-ui/src/components/Login.tsx index c9dd352..f3fa38b 100644 --- a/packages/user-ui/src/components/Login.tsx +++ b/packages/user-ui/src/components/Login.tsx @@ -268,6 +268,7 @@ export default function Login({ let errorMessage = "Login failed. Please try again."; if (error instanceof Error) { + const normalizedMessage = error.message.toLowerCase(); const authError = error as Error & { unverified?: boolean; resendAllowed?: boolean; @@ -279,11 +280,13 @@ export default function Login({ setErrors({ general: errorMessage }); return; } - if (error.message.includes("not found")) { - errorMessage = "No account found with this email address."; - } else if (error.message.includes("authentication")) { + if ( + normalizedMessage.includes("not found") || + normalizedMessage.includes("authentication") || + normalizedMessage.includes("invalid") + ) { errorMessage = "Invalid email or password."; - } else if (error.message.includes("network") || error.message.includes("fetch")) { + } else if (normalizedMessage.includes("network") || normalizedMessage.includes("fetch")) { errorMessage = "Network error. Please check your connection and try again."; } else { errorMessage = error.message;