From 89c1751e79966676bb97c7f2a784ba7b7a4bc1cf Mon Sep 17 00:00:00 2001 From: openhands Date: Thu, 27 Aug 2026 15:17:54 +0200 Subject: [PATCH] refactor: extract shared login credential verification into auth/login-core The username normalization, dummy-hash constant, password check and email-verification gate were duplicated between precheckLogin and the NextAuth credentials authorize handler. Move them into a single login-core module so both paths share one source of truth and stay consistent. --- src/actions/auth-precheck.test.ts | 70 +++++++++-------- src/actions/auth-precheck.ts | 56 +++----------- src/lib/auth.ts | 113 +++++---------------------- src/lib/auth/login-core.ts | 122 ++++++++++++++++++++++++++++++ 4 files changed, 187 insertions(+), 174 deletions(-) create mode 100644 src/lib/auth/login-core.ts diff --git a/src/actions/auth-precheck.test.ts b/src/actions/auth-precheck.test.ts index 6c6df35c..da37a925 100644 --- a/src/actions/auth-precheck.test.ts +++ b/src/actions/auth-precheck.test.ts @@ -1,19 +1,25 @@ // @ts-nocheck import { beforeEach, describe, expect, it, vi } from "vitest"; -import { checkLogin } from "@/lib/auth/password"; import { clientIp, rateLimit } from "@/lib/rate-limit"; import { captchaConfig, verifyCaptcha } from "@/lib/services/captcha"; import { siteSettings } from "@/lib/services/site-settings"; import { precheckLogin } from "./auth-precheck"; -const { queryPreparedOne } = vi.hoisted(() => { - const queryPreparedOne = vi.fn().mockResolvedValue(null); - return { queryPreparedOne }; -}); +const core = vi.hoisted(() => ({ + getLoginUser: vi.fn(), + verifyLoginPassword: vi.fn(), + isEmailUnverified: vi.fn(), + runDummyHashCheck: vi.fn(), + normalizeLoginInput: (username: unknown, password: unknown) => ({ + username: String(username ?? "") + .normalize("NFC") + .trim(), + password: String(password ?? "").normalize("NFC"), + }), +})); vi.mock("@/env", () => ({ env: { CONVERT_PASSWORDS: false } })); -vi.mock("@/lib/auth/password", () => ({ checkLogin: vi.fn() })); -vi.mock("@/lib/db", () => ({ queryPreparedOne })); +vi.mock("@/lib/auth/login-core", () => core); vi.mock("@/lib/rate-limit", () => ({ clientIp: vi.fn(), rateLimit: vi.fn() })); vi.mock("@/lib/services/captcha", () => ({ captchaConfig: vi.fn(), @@ -23,33 +29,35 @@ vi.mock("@/lib/services/site-settings", () => ({ siteSettings: { getBool: vi.fn() }, })); +const user = (overrides = {}) => ({ + password: "hash", + twoFactorConfirmedAt: null, + mail: null, + mailVerified: "0", + ...overrides, +}); + beforeEach(() => { vi.clearAllMocks(); vi.mocked(clientIp).mockResolvedValue("1.2.3.4"); vi.mocked(rateLimit).mockResolvedValue({ ok: true }); - vi.mocked(checkLogin).mockResolvedValue({ valid: true } as never); vi.mocked(captchaConfig).mockResolvedValue({ provider: "none" } as never); - queryPreparedOne.mockResolvedValue(null); + core.getLoginUser.mockResolvedValue(null); + core.verifyLoginPassword.mockResolvedValue({ valid: true }); + core.isEmailUnverified.mockResolvedValue(false); + core.runDummyHashCheck.mockResolvedValue(undefined); }); describe("precheckLogin", () => { it("returns ok for valid login without 2FA", async () => { - queryPreparedOne.mockResolvedValue({ - password: "hash", - twoFactorConfirmedAt: null, - mail: null, - mailVerified: "0", - }); + core.getLoginUser.mockResolvedValue(user()); expect(await precheckLogin("user", "pass")).toBe("ok"); }); it("returns twofactor when 2FA is set up", async () => { - queryPreparedOne.mockResolvedValue({ - password: "hash", - twoFactorConfirmedAt: new Date(), - mail: null, - mailVerified: "0", - }); + core.getLoginUser.mockResolvedValue( + user({ twoFactorConfirmedAt: new Date() }), + ); expect(await precheckLogin("user", "pass")).toBe("twofactor"); }); @@ -62,30 +70,20 @@ describe("precheckLogin", () => { provider: "hcaptcha", } as never); vi.mocked(verifyCaptcha).mockResolvedValue(false); - queryPreparedOne.mockResolvedValue({ - password: "hash", - twoFactorConfirmedAt: null, - mail: null, - mailVerified: "0", - }); + core.getLoginUser.mockResolvedValue(user()); expect(await precheckLogin("user", "pass", "bad-token")).toBe("captcha"); }); it("returns invalid when user not found (dummy hash check)", async () => { - queryPreparedOne.mockResolvedValue(null); + core.getLoginUser.mockResolvedValue(null); const result = await precheckLogin("nonexistent", "pass"); expect(result).toBe("invalid"); - expect(checkLogin).toHaveBeenCalled(); + expect(core.runDummyHashCheck).toHaveBeenCalled(); }); it("returns unverified when email verification required", async () => { - queryPreparedOne.mockResolvedValue({ - password: "hash", - twoFactorConfirmedAt: null, - mail: "user@example.com", - mailVerified: "0", - }); - vi.mocked(siteSettings.getBool).mockResolvedValue(true); + core.getLoginUser.mockResolvedValue(user({ mail: "user@example.com" })); + core.isEmailUnverified.mockResolvedValue(true); expect(await precheckLogin("user", "pass")).toBe("unverified"); }); }); diff --git a/src/actions/auth-precheck.ts b/src/actions/auth-precheck.ts index 3c353503..64acb0f9 100644 --- a/src/actions/auth-precheck.ts +++ b/src/actions/auth-precheck.ts @@ -1,11 +1,14 @@ "use server"; -import { env } from "@/env"; -import { checkLogin } from "@/lib/auth/password"; -import { queryPreparedOne } from "@/lib/db"; +import { + getLoginUser, + isEmailUnverified, + normalizeLoginInput, + runDummyHashCheck, + verifyLoginPassword, +} from "@/lib/auth/login-core"; import { clientIp, rateLimit } from "@/lib/rate-limit"; import { captchaConfig, verifyCaptcha } from "@/lib/services/captcha"; -import { siteSettings } from "@/lib/services/site-settings"; export type PrecheckResult = | "ok" @@ -24,10 +27,7 @@ export async function precheckLogin( password: string, captchaToken?: string | null, ): Promise { - const u = String(username ?? "") - .normalize("NFC") - .trim(); - const p = String(password ?? "").normalize("NFC"); + const { username: u, password: p } = normalizeLoginInput(username, password); if (!u || !p) return "invalid"; const ip = await clientIp(); @@ -38,49 +38,17 @@ export async function precheckLogin( if (!(await verifyCaptcha(captchaToken ?? null, ip))) return "captcha"; } - let user: { - password: string; - twoFactorConfirmedAt: Date | null; - mail: string | null; - mailVerified: string; - } | null; - try { - user = await queryPreparedOne<{ - password: string; - twoFactorConfirmedAt: Date | null; - mail: string | null; - mailVerified: string; - }>( - `SELECT password, two_factor_confirmed_at AS twoFactorConfirmedAt, - mail, mail_verified AS mailVerified - FROM users WHERE username = ? LIMIT 1`, - [u], - ); - } catch { - return "invalid"; - } + const user = await getLoginUser(u); if (!user) { // Prevent timing-based enumeration: always run a dummy hash check. - await checkLogin( - p, - "$2y$12$abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZabcd", - { - convertPasswords: false, - }, - ); + await runDummyHashCheck(p); return "invalid"; } - const res = await checkLogin(p, user.password, { - convertPasswords: env.CONVERT_PASSWORDS, - }); + const res = await verifyLoginPassword(user, p); if (!res.valid) return "invalid"; - if ( - (await siteSettings.getBool("require_email_verification", false)) && - user.mail && - user.mailVerified !== "1" - ) { + if (await isEmailUnverified(user)) { return "unverified"; } diff --git a/src/lib/auth.ts b/src/lib/auth.ts index 594b06ac..7e5b42a7 100644 --- a/src/lib/auth.ts +++ b/src/lib/auth.ts @@ -1,85 +1,24 @@ -import { eq, sql } from "drizzle-orm"; +import { eq } from "drizzle-orm"; import NextAuth from "next-auth"; import Credentials from "next-auth/providers/credentials"; import { env } from "@/env"; import { getCachedJwtVersion } from "@/lib/auth/jwt-version-cache"; +import { + getLoginUser, + invalidateLoginCache, + isEmailUnverified, + normalizeLoginInput, + runDummyHashCheck, + verifyLoginPassword, +} from "@/lib/auth/login-core"; + +export { invalidateLoginCache }; + import { LaravelEncrypter } from "@/lib/auth/laravel-encrypter"; -import { checkLogin } from "@/lib/auth/password"; import { verifyTotp } from "@/lib/auth/totp"; -import { cachedQuery, invalidateKey } from "@/lib/cached-db"; import { db, User, WebsiteLoginLogs } from "@/lib/db"; import { logger } from "@/lib/logger"; import { clientIp, rateLimit } from "@/lib/rate-limit"; -import { siteSettings } from "@/lib/services/site-settings"; - -interface LoginUser { - id: number; - username: string; - password: string | null; - rank: number; - mail: string | null; - mailVerified: string | null; - twoFactorConfirmedAt: string | null; - twoFactorSecret: string | null; -} - -/** - * Cached login user lookup — short TTL to survive brute-force attempts - * while still reflecting recent password/account changes reasonably fast. - */ -async function getLoginUser(username: string): Promise { - return cachedQuery( - `login:user:${username}`, - async () => { - const [result] = await db.execute<{ - id: number; - username: string; - password: string | null; - rank: number; - mail: string | null; - mail_verified: string | null; - two_factor_confirmed_at: string | null; - two_factor_secret: string | null; - }>(sql` - SELECT id, username, password, rank, mail, - mail_verified, - two_factor_confirmed_at, - two_factor_secret - FROM users - WHERE username = ${username} - LIMIT 1 - `); - const rows = result as unknown as Array<{ - id: number; - username: string; - password: string | null; - rank: number; - mail: string | null; - mail_verified: string | null; - two_factor_confirmed_at: string | null; - two_factor_secret: string | null; - }>; - return rows.length > 0 - ? { - id: rows[0].id, - username: rows[0].username, - password: rows[0].password, - rank: rows[0].rank, - mail: rows[0].mail, - mailVerified: rows[0].mail_verified, - twoFactorConfirmedAt: rows[0].two_factor_confirmed_at, - twoFactorSecret: rows[0].two_factor_secret, - } - : null; - }, - 15, // 15s TTL — brute-force protection without blocking legit changes - ); -} - -/** Call after password reset / rank change to invalidate the cached login row. */ -export async function invalidateLoginCache(username: string): Promise { - await invalidateKey(`login:user:${username}`); -} async function verify2faCode(userId: number, code: string): Promise { const [user] = await db @@ -149,10 +88,10 @@ export const { handlers, signOut, auth } = NextAuth({ code: { label: "2FA code", type: "text" }, }, authorize: async (credentials) => { - const username = String(credentials?.username ?? "") - .normalize("NFC") - .trim(); - const password = String(credentials?.password ?? "").normalize("NFC"); + const { username, password } = normalizeLoginInput( + credentials?.username, + credentials?.password, + ); if (!username || !password) return null; const ip = await clientIp(); @@ -163,28 +102,14 @@ export const { handlers, signOut, auth } = NextAuth({ const user = await getLoginUser(username); if (!user) { // Prevent timing-based enumeration: always run a dummy hash check. - await checkLogin( - password, - "$2y$12$abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZabcd", - { - convertPasswords: false, - }, - ); + await runDummyHashCheck(password); return null; } - // Byte-compatible AtomCMS check (argon2id + legacy md5/bcrypt upgrade). - if (!user.password) return null; - const res = await checkLogin(password, user.password, { - convertPasswords: env.CONVERT_PASSWORDS, - }); + const res = await verifyLoginPassword(user, password); if (!res.valid) return null; - if ( - (await siteSettings.getBool("require_email_verification", false)) && - user.mail && - user.mailVerified !== "1" - ) { + if (await isEmailUnverified(user)) { return null; } diff --git a/src/lib/auth/login-core.ts b/src/lib/auth/login-core.ts new file mode 100644 index 00000000..41e4c354 --- /dev/null +++ b/src/lib/auth/login-core.ts @@ -0,0 +1,122 @@ +import { sql } from "drizzle-orm"; +import { env } from "@/env"; +import { checkLogin } from "@/lib/auth/password"; +import { cachedQuery, invalidateKey } from "@/lib/cached-db"; +import { db } from "@/lib/db"; +import { siteSettings } from "@/lib/services/site-settings"; + +export interface LoginUser { + id: number; + username: string; + password: string | null; + rank: number; + mail: string | null; + mailVerified: string | null; + twoFactorConfirmedAt: string | null; + twoFactorSecret: string | null; +} + +/** + * Fixed dummy bcrypt hash used to keep timing roughly constant when a username + * does not exist, so attackers can't enumerate accounts by response time. + */ +const DUMMY_BCRYPT_HASH = + "$2y$12$abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZabcd"; + +/** + * Normalize credentials exactly like the registration flow hashes them, so + * accounts with accented/non-ASCII usernames or passwords verify correctly. + */ +export function normalizeLoginInput(username: unknown, password: unknown) { + return { + username: String(username ?? "") + .normalize("NFC") + .trim(), + password: String(password ?? "").normalize("NFC"), + }; +} + +/** + * Cached login user lookup — short TTL to survive brute-force attempts + * while still reflecting recent password/account changes reasonably fast. + */ +export async function getLoginUser( + username: string, +): Promise { + return cachedQuery( + `login:user:${username}`, + async () => { + const [result] = await db.execute<{ + id: number; + username: string; + password: string | null; + rank: number; + mail: string | null; + mail_verified: string | null; + two_factor_confirmed_at: string | null; + two_factor_secret: string | null; + }>(sql` + SELECT id, username, password, rank, mail, + mail_verified, + two_factor_confirmed_at, + two_factor_secret + FROM users + WHERE username = ${username} + LIMIT 1 + `); + const rows = result as unknown as Array<{ + id: number; + username: string; + password: string | null; + rank: number; + mail: string | null; + mail_verified: string | null; + two_factor_confirmed_at: string | null; + two_factor_secret: string | null; + }>; + return rows.length > 0 + ? { + id: rows[0].id, + username: rows[0].username, + password: rows[0].password, + rank: rows[0].rank, + mail: rows[0].mail, + mailVerified: rows[0].mail_verified, + twoFactorConfirmedAt: rows[0].two_factor_confirmed_at, + twoFactorSecret: rows[0].two_factor_secret, + } + : null; + }, + 15, // 15s TTL — brute-force protection without blocking legit changes + ); +} + +/** Call after password reset / rank change to invalidate the cached login row. */ +export async function invalidateLoginCache(username: string): Promise { + await invalidateKey(`login:user:${username}`); +} + +/** Runs a dummy hash check so missing-user responses stay timing-constant. */ +export async function runDummyHashCheck(password: string): Promise { + await checkLogin(password, DUMMY_BCRYPT_HASH, { convertPasswords: false }); +} + +/** Verifies the password against the stored hash and reports a possible upgrade. */ +export async function verifyLoginPassword( + user: LoginUser, + password: string, +): Promise<{ valid: boolean; upgradedHash?: string }> { + if (!user.password) return { valid: false }; + return checkLogin(password, user.password, { + convertPasswords: env.CONVERT_PASSWORDS, + }); +} + +/** True when email verification is required but this account hasn't verified yet. */ +export async function isEmailUnverified(user: LoginUser): Promise { + return ( + (await siteSettings.getBool("require_email_verification", false)) && + !!user.mail && + user.mailVerified !== "1" + ); +}