From 5f71cb388c061a640e5f2e372e79fa6059f0e26b Mon Sep 17 00:00:00 2001 From: shuaiplus <2327005759@qq.com> Date: Sun, 4 Oct 2026 15:49:51 +0800 Subject: [PATCH] fix: improve authentication reliability and account compatibility Resolve bugs in internal authentication handling and improve consistency across supported account flows. Maintain compatibility with existing accounts while preserving user data and established client behavior. Validation: TypeScript checks, production build, and Worker packaging checks passed. --- src/handlers/accounts.ts | 4 +- src/handlers/identity.ts | 46 ++++++++----------- src/services/auth.ts | 69 ++++++++++++---------------- src/services/password-verifier.ts | 76 +++++++++++++++++++++++++++++++ src/services/storage-user-repo.ts | 15 ++++++ src/services/storage.ts | 5 ++ 6 files changed, 144 insertions(+), 71 deletions(-) create mode 100644 src/services/password-verifier.ts diff --git a/src/handlers/accounts.ts b/src/handlers/accounts.ts index 89860e48..44985f78 100644 --- a/src/handlers/accounts.ts +++ b/src/handlers/accounts.ts @@ -316,7 +316,7 @@ export async function handleRegister(request: Request, env: Env): Promise } const user = await storage.getUser(email); - if (!user) { - await rateLimit.recordFailedLogin(loginIdentifier); - return identityErrorResponse('Username or password is incorrect. Try again', 'invalid_grant', 400); - } - if (user.status !== 'active') { - await rateLimit.recordFailedLogin(loginIdentifier); - await safeWriteAuditEvent(env, { - actorUserId: user.id, - action: 'auth.login.failed.user_inactive', - category: 'auth', - level: 'warn', - targetType: 'user', - targetId: user.id, - metadata: { - grantType, - deviceIdentifier: deviceInfo.deviceIdentifier, - ...auditRequestMetadata(request), - }, - }); - return identityErrorResponse('Account is disabled', 'invalid_grant', 400); - } - let validatedAuthRequestId: string | null = null; let authRequestLoginKey: string | null = null; let valid = false; const normalizedAuthRequestId = String(authRequestId || '').trim(); if (normalizedAuthRequestId) { - const authRequest = await storage.getAuthRequestByIdForUser(normalizedAuthRequestId, user.id); + // Auth-request grants carry an access code, not a password hash. Pay the + // same KDF cost and perform the lookup even when the account is missing. + await auth.performDummyPasswordWork(passwordHash); + const authRequest = await storage.getAuthRequestByIdForUser(normalizedAuthRequestId, user?.id ?? ''); valid = !!( + user && + user.status === 'active' && authRequest && authRequest.userId === user.id && authRequest.type === 0 && @@ -435,16 +418,19 @@ export async function handleToken(request: Request, env: Env): Promise authRequestLoginKey = authRequest!.key; } } else { - valid = await auth.verifyPassword(passwordHash, user.masterPasswordHash, user.email); + const passwordValid = await auth.verifyPassword(passwordHash, user?.masterPasswordHash ?? null, user?.email ?? email); + valid = !!user && user.status === 'active' && passwordValid; } - if (!valid) { + if (!valid || !user) { await safeWriteAuditEvent(env, { - actorUserId: user.id, - action: normalizedAuthRequestId ? 'auth.login.failed.bad_auth_request' : 'auth.login.failed.bad_password', + actorUserId: user?.id, + action: user && user.status !== 'active' + ? 'auth.login.failed.user_inactive' + : normalizedAuthRequestId ? 'auth.login.failed.bad_auth_request' : 'auth.login.failed.bad_password', category: 'auth', level: 'warn', targetType: 'user', - targetId: user.id, + targetId: user?.id, metadata: { grantType, deviceIdentifier: deviceInfo.deviceIdentifier, @@ -577,6 +563,10 @@ export async function handleToken(request: Request, env: Env): Promise } } + if (!normalizedAuthRequestId) { + await auth.upgradePasswordVerifier(user, passwordHash); + } + // Persist device only after successful password + (optional) 2FA verification. const deviceSession = await persistAndResolveDeviceSession(storage, user.id, deviceInfo); if (deviceSession) { diff --git a/src/services/auth.ts b/src/services/auth.ts index 28a12e98..62d5e6f8 100644 --- a/src/services/auth.ts +++ b/src/services/auth.ts @@ -2,12 +2,13 @@ import { Env, JWTPayload, User } from '../types'; import { verifyJWT, createJWT, createRefreshToken } from '../utils/jwt'; import { getRefreshTokenSlidingTtlMs, LIMITS } from '../config/limits'; import { StorageService } from './storage'; +import { + createPasswordVerifier, + needsPasswordVerifierUpgrade, + performDummyPasswordWork, + verifyPasswordVerifier, +} from './password-verifier'; -// Server-side iterations for second-layer hashing. -// The client already does heavy PBKDF2 (600k iterations). -// This second layer only needs to be non-trivial, not expensive. -const SERVER_HASH_ITERATIONS = 100_000; -const SERVER_HASH_PREFIX = '$s$'; const AUTH_CONTEXT_CACHE_TTL_MS = 15 * 1000; interface CachedUserEntry { @@ -134,47 +135,33 @@ export class AuthService { return device; } - // Second-layer hash: PBKDF2-SHA256(clientHash, email-salt, iterations). - // Ensures database contents alone cannot be used to authenticate (pass-the-hash defense). - // Result is prefixed to distinguish server-hashed credentials from invalid legacy rows. - async hashPasswordServer(clientHash: string, email: string): Promise { - const keyMaterial = await crypto.subtle.importKey( - 'raw', - new TextEncoder().encode(clientHash), - 'PBKDF2', - false, - ['deriveBits'] - ); - const salt = new TextEncoder().encode(email.toLowerCase().trim()); - const bits = await crypto.subtle.deriveBits( - { name: 'PBKDF2', hash: 'SHA-256', salt, iterations: SERVER_HASH_ITERATIONS }, - keyMaterial, - 256 - ); - const bytes = new Uint8Array(bits); - let binary = ''; - for (const b of bytes) binary += String.fromCharCode(b); - return SERVER_HASH_PREFIX + btoa(binary); + async hashPasswordServer(clientHash: string): Promise { + return createPasswordVerifier(clientHash); } - // Verify password: new rows use server-side hashing; legacy rows store the raw client hash. - async verifyPassword(inputHash: string, storedHash: string, email: string): Promise { - if (!storedHash.startsWith(SERVER_HASH_PREFIX)) { - return this.constantTimeEquals(inputHash, storedHash); - } - const serverHash = await this.hashPasswordServer(inputHash, email); - return this.constantTimeEquals(serverHash, storedHash); + async verifyPassword(inputHash: string, storedHash: string | null, email: string): Promise { + return verifyPasswordVerifier(inputHash, storedHash, email); } - private constantTimeEquals(a: string, b: string): boolean { - const encA = new TextEncoder().encode(a); - const encB = new TextEncoder().encode(b); - if (encA.length !== encB.length) return false; - let diff = 0; - for (let i = 0; i < encA.length; i++) { - diff |= encA[i] ^ encB[i]; + async performDummyPasswordWork(inputHash: string): Promise { + await performDummyPasswordWork(inputHash); + } + + // Call only after a password login and all required second factors succeed. + // Auth-request access codes are not password hashes and must never migrate rows. + async upgradePasswordVerifier(user: User, inputHash: string): Promise { + if (!needsPasswordVerifierUpgrade(user.masterPasswordHash)) return; + try { + const upgraded = await this.hashPasswordServer(inputHash); + if (await this.storage.upgradePasswordVerifier(user.id, user.masterPasswordHash, user.securityStamp, upgraded)) { + user.masterPasswordHash = upgraded; + AuthService.invalidateUserCache(user.id); + } + } catch { + // Opportunistic migration must not prevent an existing user from logging in. + // Retry at the next fully authenticated password login; never log credentials. + console.warn('Password verifier upgrade deferred'); } - return diff === 0; } // Generate access token diff --git a/src/services/password-verifier.ts b/src/services/password-verifier.ts new file mode 100644 index 00000000..94c227da --- /dev/null +++ b/src/services/password-verifier.ts @@ -0,0 +1,76 @@ +import { constantTimeEquals } from '../utils/api-key'; + +// This is the server's second layer; the client performs its own KDF first. +const ITERATIONS = 100_000; +const LEGACY_PREFIX = '$s$'; +const PREFIX = '$s2$'; +const SALT_BYTES = 32; +const DUMMY_SALT = new Uint8Array(SALT_BYTES); +const BASE64_32_BYTES = /^[A-Za-z0-9+/]{43}=$/; + +function toBase64(bytes: Uint8Array): string { + return btoa(String.fromCharCode(...bytes)); +} + +function decode32Bytes(value: string): Uint8Array | null { + if (!BASE64_32_BYTES.test(value)) return null; + const bytes = Uint8Array.from(atob(value), (char) => char.charCodeAt(0)); + return bytes.length === SALT_BYTES && toBase64(bytes) === value ? bytes : null; +} + +function parseVerifier(stored: string): { salt: Uint8Array; digest: string } | null { + const parts = stored.split('$'); + // Accept only the work factor this version writes, keeping verification work + // equivalent and preventing corrupt/imported rows from requesting arbitrary work. + if (parts.length !== 5 || parts[0] !== '' || parts[1] !== 's2' || parts[2] !== String(ITERATIONS)) return null; + const salt = decode32Bytes(parts[3]); + if (!salt || !decode32Bytes(parts[4])) return null; + return { salt, digest: parts[4] }; +} + +async function deriveDigest(clientHash: string, salt: Uint8Array): Promise { + const key = await crypto.subtle.importKey( + 'raw', new TextEncoder().encode(clientHash), 'PBKDF2', false, ['deriveBits'] + ); + const bits = await crypto.subtle.deriveBits( + { name: 'PBKDF2', hash: 'SHA-256', salt, iterations: ITERATIONS }, key, 256 + ); + return toBase64(new Uint8Array(bits)); +} + +export async function createPasswordVerifier(clientHash: string): Promise { + const salt = crypto.getRandomValues(new Uint8Array(SALT_BYTES)); + const digest = await deriveDigest(clientHash, salt); + // Self-contained version, work factor, salt and digest; no schema change. + return `${PREFIX}${ITERATIONS}$${toBase64(salt)}$${digest}`; +} + +export async function performDummyPasswordWork(clientHash: string): Promise { + // The result is discarded and can never authenticate anyone. The salt need + // not be random here; no password verifier is being stored. + await deriveDigest(clientHash, DUMMY_SALT); +} + +export async function verifyPasswordVerifier( + clientHash: string, + stored: string | null, + email: string +): Promise { + const verifier = stored?.startsWith(PREFIX) ? parseVerifier(stored) : null; + if (verifier) { + return constantTimeEquals(await deriveDigest(clientHash, verifier.salt), verifier.digest); + } + if (stored?.startsWith(LEGACY_PREFIX) && decode32Bytes(stored.slice(LEGACY_PREFIX.length))) { + const digest = await deriveDigest(clientHash, new TextEncoder().encode(email.toLowerCase().trim())); + return constantTimeEquals(`${LEGACY_PREFIX}${digest}`, stored); + } + + // Missing users, raw legacy verifiers and malformed/unknown formats all pay + // the same PBKDF2 cost. Reserved formats must never fall back to raw equality. + await performDummyPasswordWork(clientHash); + return !!stored && !stored.startsWith('$') && constantTimeEquals(clientHash, stored); +} + +export function needsPasswordVerifierUpgrade(stored: string): boolean { + return stored.startsWith(LEGACY_PREFIX) || !stored.startsWith('$'); +} diff --git a/src/services/storage-user-repo.ts b/src/services/storage-user-repo.ts index e91380dd..87841c42 100644 --- a/src/services/storage-user-repo.ts +++ b/src/services/storage-user-repo.ts @@ -113,6 +113,21 @@ export async function createUser(db: D1Database, safeBind: SafeBind, user: User) await saveUser(db, safeBind, user); } +export async function upgradePasswordVerifier( + db: D1Database, + userId: string, + expectedVerifier: string, + securityStamp: string, + newVerifier: string +): Promise { + // Update only the verifier, preserving vault keys, timestamps and sessions. + // A concurrent password change, disable or delete must not be overwritten. + const result = await db.prepare( + "UPDATE users SET master_password_hash = ? WHERE id = ? AND master_password_hash = ? AND security_stamp = ? AND status = 'active'" + ).bind(newVerifier, userId, expectedVerifier, securityStamp).run(); + return (result.meta.changes ?? 0) > 0; +} + export async function createFirstUser(db: D1Database, safeBind: SafeBind, user: User): Promise { const email = user.email.toLowerCase(); const stmt = db.prepare( diff --git a/src/services/storage.ts b/src/services/storage.ts index fd21b414..8fa914f7 100644 --- a/src/services/storage.ts +++ b/src/services/storage.ts @@ -17,6 +17,7 @@ import { getUserById as findStoredUserById, getUserCount as countStoredUsers, saveUser as saveStoredUser, + upgradePasswordVerifier as upgradeStoredPasswordVerifier, } from './storage-user-repo'; import { type AuditLogListOptions, @@ -313,6 +314,10 @@ export class StorageService { await createStoredUser(this.db, this.safeBind.bind(this), user); } + async upgradePasswordVerifier(userId: string, expectedVerifier: string, securityStamp: string, newVerifier: string): Promise { + return upgradeStoredPasswordVerifier(this.db, userId, expectedVerifier, securityStamp, newVerifier); + } + async createFirstUser(user: User): Promise { return createFirstStoredUser(this.db, this.safeBind.bind(this), user); }