diff --git a/src/server/auth/auth-hardening.test.ts b/src/server/auth/auth-hardening.test.ts new file mode 100644 index 0000000..51d4a21 --- /dev/null +++ b/src/server/auth/auth-hardening.test.ts @@ -0,0 +1,98 @@ +import { beforeAll, describe, expect, it } from "vitest"; +import { eq } from "drizzle-orm"; +import * as authSchema from "@/db/auth-schema"; +import type { Db } from "@/db"; +import { createTestDb, type TestDb } from "@/test/db"; +import { createAuth } from "./auth"; +import { getAuthEnv } from "./env"; +import { consumeOtpQuota, OTP_EMAIL_LIMITS } from "./otp-limit"; + +/** Security audit 2026-10 (COMP-001, 002, 005): regressions for the authentication hardening. */ +describe("auth hardening", () => { + let db: TestDb; + let auth: ReturnType; + const mails: { to: string; subject: string; text: string }[] = []; + + beforeAll(async () => { + db = await createTestDb(); + auth = createAuth({ + db: db as unknown as Db, + nextJsCookies: false, + env: getAuthEnv({ NODE_ENV: "test", BETTER_AUTH_SECRET: "test-secret-test-secret-test-secret-123", BETTER_AUTH_URL: "http://localhost:3000" }), + send: async (m) => { + mails.push(m); + }, + }); + }); + + const githubSource = { method: "oauth", oauth: { providerId: "github" } } as const; + const request = (email: string) => auth.api.sendVerificationOTP({ body: { email, type: "sign-in" } }); + + it("caps sign-in codes per e-mail, not only per IP", async () => { + const hourly = OTP_EMAIL_LIMITS[0].max; + for (let i = 0; i < hourly; i++) await request("target@example.com"); + await expect(request("target@example.com")).rejects.toThrow(/Muitos códigos/); + // the cap is per address and case-insensitive + await expect(request(" TARGET@example.com ")).rejects.toThrow(/Muitos códigos/); + await expect(request("someone-else@example.com")).resolves.toBeDefined(); + }); + + it("a refused request leaves the valid code untouched", async () => { + mails.length = 0; + const hourly = OTP_EMAIL_LIMITS[0].max; + for (let i = 0; i < hourly; i++) await request("rotate@example.com"); + await new Promise((r) => setTimeout(r, 20)); + const otp = /(\d{6})/.exec(mails.at(-1)!.text)![1]; + await expect(request("rotate@example.com")).rejects.toThrow(); + const res = await auth.api.signInEmailOTP({ body: { email: "rotate@example.com", otp } }); + expect(res.user.email).toBe("rotate@example.com"); + }); + + it("opens a new window after the old one expires", async () => { + const t0 = 1_800_000_000_000; + const [{ max, windowMs }] = OTP_EMAIL_LIMITS; + for (let i = 0; i < max; i++) expect(await consumeOtpQuota(db as unknown as Db, "window@example.com", t0)).toBe(true); + expect(await consumeOtpQuota(db as unknown as Db, "window@example.com", t0 + 1000)).toBe(false); + expect(await consumeOtpQuota(db as unknown as Db, "window@example.com", t0 + windowMs + 1)).toBe(true); + }); + + it("keeps only a hash of the address in the counters", async () => { + await consumeOtpQuota(db as unknown as Db, "private@example.com"); + const keys = (await db.select().from(authSchema.rateLimit)).map((r) => r.key); + expect(keys.some((k) => k.startsWith("otp-email:"))).toBe(true); + expect(keys.join(" ")).not.toContain("private@example.com"); + }); + + it("stores the one-time code hashed, not in plain text", async () => { + mails.length = 0; + await request("hashed@example.com"); + await new Promise((r) => setTimeout(r, 20)); + const otp = /(\d{6})/.exec(mails[0].text)![1]; + const rows = await db.select().from(authSchema.verification).where(eq(authSchema.verification.identifier, "sign-in-otp-hashed@example.com")); + expect(rows).toHaveLength(1); + expect(rows[0].value).not.toContain(otp); + }); + + it("refuses to create a user whose e-mail the provider did not verify (pre-account takeover)", async () => { + const ctx = await auth.$context; + // what the social callback does for a first-time GitHub/Google sign-in with an unverified address + // (the hook aborts the insert, createUser returns null and the callback then fails with "unable to create user") + expect(await ctx.internalAdapter.createUser({ email: "victim@example.com", name: "Mallory", emailVerified: false }, githubSource)).toBeNull(); + expect(await db.select().from(authSchema.user).where(eq(authSchema.user.email, "victim@example.com"))).toHaveLength(0); + + // the real owner signs in by e-mail code and ends up with a clean account: no social account hanging off it + mails.length = 0; + await request("victim@example.com"); + await new Promise((r) => setTimeout(r, 20)); + const otp = /(\d{6})/.exec(mails[0].text)![1]; + const res = await auth.api.signInEmailOTP({ body: { email: "victim@example.com", otp } }); + const accounts = await db.select().from(authSchema.account).where(eq(authSchema.account.userId, res.user.id)); + expect(accounts.filter((a) => a.providerId !== "credential")).toHaveLength(0); + }); + + it("still creates verified users (provider-verified social sign-in)", async () => { + const ctx = await auth.$context; + const created = await ctx.internalAdapter.createUser({ email: "ok@example.com", name: "Ok", emailVerified: true }, githubSource); + expect(created.emailVerified).toBe(true); + }); +}); diff --git a/src/server/auth/auth.ts b/src/server/auth/auth.ts index fba3714..6f05b2b 100644 --- a/src/server/auth/auth.ts +++ b/src/server/auth/auth.ts @@ -1,4 +1,5 @@ -import { betterAuth } from "better-auth"; +import { APIError, betterAuth } from "better-auth"; +import { createAuthMiddleware } from "better-auth/api"; import { drizzleAdapter } from "better-auth/adapters/drizzle"; import { nextCookies } from "better-auth/next-js"; import { after } from "next/server"; @@ -8,6 +9,7 @@ import { getDb, type Db } from "@/db"; import * as authSchema from "@/db/auth-schema"; import { users, workspaceMembers, workspaces } from "@/db/schema"; import { getAuthEnv } from "./env"; +import { consumeOtpQuota } from "./otp-limit"; import { sendMail, type Mailer } from "./mail"; import { stripProviderTokens } from "./strip-tokens"; @@ -24,13 +26,15 @@ export function createAuth({ env?: ReturnType; }) { const baseURL = env.BETTER_AUTH_URL; + // Social sign-in must never trust an address the provider did not verify (COMP-001). `requireEmailVerification` refuses the + // session, but Better Auth creates the user and the account first; `databaseHooks.user.create.before` below stops that creation. const google = env.GOOGLE_CLIENT_ID && env.GOOGLE_CLIENT_SECRET - ? { google: { clientId: env.GOOGLE_CLIENT_ID, clientSecret: env.GOOGLE_CLIENT_SECRET } } + ? { google: { clientId: env.GOOGLE_CLIENT_ID, clientSecret: env.GOOGLE_CLIENT_SECRET, requireEmailVerification: true } } : {}; const github = env.GITHUB_CLIENT_ID && env.GITHUB_CLIENT_SECRET - ? { github: { clientId: env.GITHUB_CLIENT_ID, clientSecret: env.GITHUB_CLIENT_SECRET } } + ? { github: { clientId: env.GITHUB_CLIENT_ID, clientSecret: env.GITHUB_CLIENT_SECRET, requireEmailVerification: true } } : {}; return betterAuth({ @@ -46,7 +50,24 @@ export function createAuth({ max: 100, customRules: { "/email-otp/send-verification-otp": { window: 60, max: 3 } }, }, + hooks: { + // Runs before the code exists, so a refused request neither rotates nor invalidates a valid code. Same answer for every + // address (no enumeration). Trade-off: someone can exhaust the quota of a target address and delay its code login for + // up to an hour; social sign-in still works and the alternative was a ~48% chance of guessing a code in a day (COMP-002). + before: createAuthMiddleware(async (ctx) => { + if (ctx.path !== "/email-otp/send-verification-otp") return; + const email = (ctx.body as { email?: unknown } | undefined)?.email; + if (typeof email !== "string") return; + if (!(await consumeOtpQuota(db, email))) { + throw new APIError("TOO_MANY_REQUESTS", { message: "Muitos códigos pedidos para este e-mail. Tente de novo mais tarde." }); + } + }), + }, databaseHooks: { + user: { + // E-mail codes create verified users; an unverified one can only come from a social provider (COMP-001). + create: { before: async (user) => (user.emailVerified ? undefined : false) }, + }, session: { create: { // Sign in lands directly in the user's first organization (Clerk did this with the "active org"). @@ -71,6 +92,7 @@ export function createAuth({ otpLength: 6, expiresIn: 300, allowedAttempts: 3, + storeOTP: "hashed", // a leaked table must not hold usable codes (COMP-005) async sendVerificationOTP({ email, otp }) { // Not awaited on purpose (timing attacks), but kept alive with after(): on Vercel the function can be frozen // as soon as the response is sent, which silently drops a fire-and-forget fetch. Errors never include the body. @@ -85,6 +107,7 @@ export function createAuth({ }, }), organization({ + requireEmailVerificationOnInvitation: true, async sendInvitationEmail({ id, email, organization: org, inviter }) { await send({ to: email, diff --git a/src/server/auth/otp-limit.ts b/src/server/auth/otp-limit.ts new file mode 100644 index 0000000..fd56658 --- /dev/null +++ b/src/server/auth/otp-limit.ts @@ -0,0 +1,37 @@ +import { createHash, randomUUID } from "node:crypto"; +import { sql } from "drizzle-orm"; +import type { Db } from "@/db"; +import { rateLimit } from "@/db/auth-schema"; + +/** + * Per-e-mail cap on sign-in codes (COMP-002). Better Auth's own limiter is per IP, so a botnet could keep asking for + * fresh 6-digit codes for one address. Counters live in `auth_rate_limit` (shared across serverless instances) and are + * bumped with one atomic upsert, which works on neon-http (no transactions). Keys hold a hash, never the address. + */ +export const OTP_EMAIL_LIMITS = [ + { name: "hour", windowMs: 60 * 60 * 1000, max: 5 }, + { name: "day", windowMs: 24 * 60 * 60 * 1000, max: 20 }, +] as const; + +const keyFor = (name: string, email: string) => `otp-email:${name}:${createHash("sha256").update(email.trim().toLowerCase()).digest("hex")}`; + +/** Registers one code request for `email`. Returns false when any window is over its cap (the request must be refused). */ +export async function consumeOtpQuota(db: Db, email: string, now = Date.now()): Promise { + let allowed = true; + for (const { name, windowMs, max } of OTP_EMAIL_LIMITS) { + const expired = sql`${now}::bigint - ${rateLimit.lastRequest} >= ${windowMs}`; + const [row] = await db + .insert(rateLimit) + .values({ id: randomUUID(), key: keyFor(name, email), count: 1, lastRequest: now }) + .onConflictDoUpdate({ + target: rateLimit.key, + set: { + count: sql`case when ${expired} then 1 else ${rateLimit.count} + 1 end`, + lastRequest: sql`case when ${expired} then ${now}::bigint else ${rateLimit.lastRequest} end`, + }, + }) + .returning({ count: rateLimit.count }); + if (row.count > max) allowed = false; + } + return allowed; +} diff --git a/src/server/auth/workspace-context.test.ts b/src/server/auth/workspace-context.test.ts index 0a46d35..9ca8d6b 100644 --- a/src/server/auth/workspace-context.test.ts +++ b/src/server/auth/workspace-context.test.ts @@ -128,3 +128,36 @@ describe("resolveWorkspaceContext", () => { expect((await resolveWorkspaceContext(asDb(), sessionOf(guest.user, org.id))).kind).toBe("no-membership"); }); }); + +describe("ensureUser e-mail linking (COMP-010)", () => { + it("never re-points a domain user that already has an auth_id", async () => { + const db = await createTestDb(); + const mails: { text: string }[] = []; + const auth = createAuth({ + db: db as unknown as Db, + nextJsCookies: false, + env: getAuthEnv({ NODE_ENV: "test", BETTER_AUTH_SECRET: "test-secret-test-secret-test-secret-123" }), + send: async (m) => void mails.push(m), + }); + await db.insert(users).values({ authId: "old-auth-id", email: "taken@example.com", name: "Original" }); + + await auth.api.sendVerificationOTP({ body: { email: "taken@example.com", type: "sign-in" } }); + await new Promise((r) => setTimeout(r, 20)); + const otp = /(\d{6})/.exec(mails[0].text)![1]; + const res = await auth.api.signInEmailOTP({ body: { email: "taken@example.com", otp }, returnHeaders: true }); + const headers = new Headers({ cookie: res.headers.getSetCookie().map((c) => c.split(";")[0]).join("; ") }); + const org = await auth.api.createOrganization({ headers, body: { name: "Taken", slug: "taken" } }); + + // the unique e-mail on `users` makes the insert fail; what matters is that the existing row is left alone + const u = res.response.user; + await resolveWorkspaceContext(db as unknown as Db, { + user: { id: u.id, email: u.email, name: u.name, image: null }, + session: { activeOrganizationId: org.id }, + } as AuthSessionLike).catch(() => undefined); + + const rows = await db.select().from(users).where(eq(users.email, "taken@example.com")); + expect(rows).toHaveLength(1); + expect(rows[0].authId).toBe("old-auth-id"); + expect(rows[0].name).toBe("Original"); + }); +}); diff --git a/src/server/auth/workspace-context.ts b/src/server/auth/workspace-context.ts index 8cd154f..db2571b 100644 --- a/src/server/auth/workspace-context.ts +++ b/src/server/auth/workspace-context.ts @@ -132,7 +132,7 @@ async function ensureUser(db: Db, authUser: AuthSessionLike["user"]) { return updated; } - const [byEmail] = await db.select().from(users).where(sql`lower(${users.email}) = ${email}`); + const [byEmail] = await db.select().from(users).where(and(sql`lower(${users.email}) = ${email}`, isNull(users.authId))); if (byEmail) { if (byEmail.deletedAt) return null; const [linked] = await db