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

@@ -316,7 +316,7 @@ export async function handleRegister(request: Request, env: Env): Promise<Respon
 
   const now = new Date().toISOString();
   const auth = new AuthService(env);
-  const serverHash = await auth.hashPasswordServer(masterPasswordHash, email);
+  const serverHash = await auth.hashPasswordServer(masterPasswordHash);
 
   const user: User = {
     id: generateUUID(),
@@ -764,7 +764,7 @@ export async function handleChangePassword(request: Request, env: Env, userId: s
     return errorResponse('masterPasswordHint must be 120 characters or fewer', 400);
   }
 
-  user.masterPasswordHash = await auth.hashPasswordServer(newMasterPasswordHash, user.email);
+  user.masterPasswordHash = await auth.hashPasswordServer(newMasterPasswordHash);
   user.key = nextKey;
   if (nextPrivateKey) user.privateKey = nextPrivateKey;
   if (nextPublicKey) user.publicKey = nextPublicKey;

src/handlers/identity.ts

@@ -391,35 +391,18 @@ export async function handleToken(request: Request, env: Env): Promise<Response>
     }
 
     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<Response>
         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<Response>
       }
     }
 
+    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) {

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<string> {
-    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<string> {
+    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<boolean> {
-    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<boolean> {
+    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<void> {
+    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<void> {
+    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

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<string> {
+  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<string> {
+  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<void> {
+  // 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<boolean> {
+  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('$');
+}

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<boolean> {
+  // 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<boolean> {
   const email = user.email.toLowerCase();
   const stmt = db.prepare(

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<boolean> {
+    return upgradeStoredPasswordVerifier(this.db, userId, expectedVerifier, securityStamp, newVerifier);
+  }
+
   async createFirstUser(user: User): Promise<boolean> {
     return createFirstStoredUser(this.db, this.safeBind.bind(this), user);
   }