From 1b09306da2238b33a5a764e959bd06c9d718d041 Mon Sep 17 00:00:00 2001 From: Michael Gasperini Date: Wed, 21 Oct 2026 21:57:20 +0200 Subject: [PATCH] fix(security): serialise vault writes and make KDF upgrades crash-safe KS-2: route every whole-collection read-modify-write in StorageService through a per-storage-key FIFO mutex, acquiring multiple keys in a fixed order. Two interleaved mutations (import while a save is in flight, autosave plus manual save, a save during a rekey) can no longer overwrite each other from the same in-memory snapshot. A queued mutation rebuilds from the committed cache and a re-encryption excludes concurrent writes on the collections it rewrites. KS-1: record the new KDF metadata as a pending recovery record before re-encrypting the vault and commit the active metadata with a single SecureStore write. The next master-password login derives both candidates, inspects which key decrypts the stored collections, and either promotes the pending metadata or discards the record and keeps the previous password working. A process kill between the vault write and the metadata commit no longer leaves ciphertext whose salt was never stored. Biometric unlock defers to the master password while a recovery record exists, and the login messages state whether the new or the previous password is required. Crypto parameters and error checks are unchanged. Tests: interleaved saves with a delayed AsyncStorage mock; simulated crash between the vault write and the metadata commit for both the legacy KDF upgrade and the master-password change, covering the new-password, old-then-new password and pre-vault-write rollback states. Docs and CHANGELOG updated. --- CHANGELOG.md | 17 +- docs/architecture.md | 15 +- docs/security.md | 15 +- src/__tests__/services/AuthService.test.ts | 113 ++++ ...AuthServiceKdfUpgradeCrashRecovery.test.ts | 213 +++++++ ...ServicePasswordChangeCrashRecovery.test.ts | 231 ++++++++ src/__tests__/services/StorageService.test.ts | 81 +++ src/locales/en.ts | 6 + src/locales/it.ts | 6 + src/screens/AuthScreen.tsx | 12 + src/services/auth/authService.ts | 413 +++++++++++--- src/services/storage/storageService.ts | 520 ++++++++++++------ 12 files changed, 1410 insertions(+), 232 deletions(-) create mode 100644 src/__tests__/services/AuthServiceKdfUpgradeCrashRecovery.test.ts create mode 100644 src/__tests__/services/AuthServicePasswordChangeCrashRecovery.test.ts diff --git a/CHANGELOG.md b/CHANGELOG.md index b1b1a8f..92fdb23 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,7 +4,22 @@ All notable project changes are documented here. ## [Unreleased] -No unreleased changes. +### Security + +- Made transparent KDF upgrades and master-password changes crash-safe. The vault is + re-encrypted only after a pending recovery record describing the new KDF metadata is + persisted alongside the still-active previous metadata, and the new metadata commit is a + single SecureStore write. A process kill between the vault write and the metadata commit + no longer leaves ciphertext encrypted under a salt that was never stored: the next + password login inspects the vault records and either promotes the pending metadata or + discards it, keeping the previous password usable when the vault was not re-encrypted + yet. Biometric unlock defers to the master password while a recovery record exists, and + the login screen reports whether the new or the previous password is required. +- Serialised whole-vault writes with a per-storage-key async mutex. Two mutations that + start from the same in-memory snapshot (for example an import while a save is in flight, + or autosave plus a manual save) can no longer overwrite each other; a queued mutation + always rebuilds from the committed cache and re-encryptions run before any concurrent + write on the same collection. ## [3.3.1] - 2026-08-09 diff --git a/docs/architecture.md b/docs/architecture.md index ef1c6ba..ef56a37 100644 --- a/docs/architecture.md +++ b/docs/architecture.md @@ -109,11 +109,16 @@ A top-level error boundary (`src/components/ErrorBoundary.tsx`) wraps the naviga 3. Biometric login asks SecureStore for the stored biometric vault key with device authentication, verifies it against the same master-key verifier, and then loads it into memory. 4. `StorageService` initializes the encrypted vault cache with the active key. 5. `AuthContext` completes the shared post-login pipeline: authentication state, login notification, post-auth migration, notification settings, clipboard timeout, and auto-lock timeout. -6. PIN setup, PIN rotation, and transparent KDF upgrades treat ciphertext and verifier - metadata as one authenticated transaction. If rollback also fails, the session and - in-memory key are cleared instead of continuing against uncertain storage. -7. Password and note writes are serialized, encrypted with KS1, persisted to AsyncStorage, - and only then committed to the decrypted in-memory cache. +6. PIN setup, PIN rotation, and transparent KDF upgrades write a pending recovery record + before touching the vault, re-encrypt the vault in one batched write, and commit the new + verifier metadata with a single SecureStore write. A process kill between the vault write + and the metadata commit is reconciled on the next master-password login; biometric unlock + waits for that login. If rollback also fails, the session and in-memory key are cleared + instead of continuing against uncertain storage, and the recovery record is kept so the + next login can complete the promotion. +7. Whole-collection writes are queued per storage key, encrypted with KS1, persisted to + AsyncStorage, and only then committed to the decrypted in-memory cache, so two concurrent + mutations cannot overwrite each other from the same snapshot. 8. Logout and auto-lock clear authentication state and the in-memory key. ## Storage Boundaries diff --git a/docs/security.md b/docs/security.md index dd3e480..924c2f1 100644 --- a/docs/security.md +++ b/docs/security.md @@ -42,7 +42,18 @@ Derived keys are normalized to 64-character lowercase hex strings. Invalid deriv ### Transparent KDF upgrade on login -Vaults created with legacy parameters — either PBKDF2 (`memory = 0`) or the old heavy Argon2id (`64 MiB / t = 3`) — are transparently upgraded to the current Argon2id parameters on the next successful password login, while the vault is decrypted in memory. The upgrade re-derives a new key (new salt and verifier), precomputes both encrypted collections, persists them in one batched storage call, and uses explicit rollback if the following verifier update fails. Recoverable failures restore the previous working key and retry on the next login. If both verifier persistence and rollback fail, Keysoft closes the authenticated session and clears the in-memory key rather than allowing further writes against uncertain storage. Biometric logins do not trigger the upgrade (no password is available); the next password login performs it. +Vaults created with legacy parameters — either PBKDF2 (`memory = 0`) or the old heavy Argon2id (`64 MiB / t = 3`) — are transparently upgraded to the current Argon2id parameters on the next successful password login, while the vault is decrypted in memory. The same crash-safe sequence is used when the master password changes: + +1. A pending recovery record containing the new KDF metadata (salt, iterations, memory, verifier) is written to SecureStore while the active metadata still describes the previous key. +2. Both encrypted collections are re-encrypted and persisted in one batched AsyncStorage write. +3. The new metadata is committed with a single SecureStore write, which is the atomic switch point. +4. The recovery record is removed. + +A process kill between the vault write and the metadata commit would previously leave ciphertext encrypted with a key whose salt was never stored — permanent vault loss. With the recovery record, the next master-password login derives both candidates, inspects which key decrypts the stored collections, and either promotes the pending metadata (vault already re-encrypted) or discards the record and keeps the previous password working (vault not re-encrypted yet). If the password only matches metadata whose key no longer decrypts the vault, login fails with a diagnostic naming the password that is required and keeps the recovery record untouched. A crash after the metadata commit leaves only a stale record, which the next login removes. Biometric unlock cannot derive the pending key: it refuses to run while a record exists and does not disable biometrics, and the following password login refreshes the biometric key best-effort. + +Recoverable failures during the regular flow restore the previous working key and retry on a later login. If both the metadata commit and the rollback fail, Keysoft closes the authenticated session and clears the in-memory key rather than allowing further writes against uncertain storage; the recovery record is kept so the next password login can complete the promotion instead of losing the vault. The recovery record holds no key material, only the same class of KDF metadata already stored in SecureStore. + +This safety relies on two platform guarantees: a single SecureStore key write is atomic, and `AsyncStorage.multiSet` persists both collections in one transaction on Android (the supported release platform). The residual risk is the same as any password change: once the vault has been re-encrypted, only the new password can open it, so a user who forgets the new password after a crash cannot fall back to the previous one. When creating or changing the PIN, the verifier metadata and the vault key are produced from the same KDF result. This avoids a duplicate KDF pass for the same new PIN while preserving the configured KDF cost and the 64-character derived-key requirement. @@ -71,6 +82,8 @@ Vault data uses the KS1 format: Decryption verifies the MAC before trusting plaintext output. +Each collection is stored as one whole encrypted record. Mutations rebuild that record from the decrypted in-memory cache, so writes are queued per storage key: two concurrent mutations cannot both start from the same snapshot and silently drop each other's record, and a re-encryption of the whole vault excludes concurrent saves on the collections it rewrites. + ### Key separation KS1 derives a single 256-bit key and uses that same value both as the AES-256-CBC diff --git a/src/__tests__/services/AuthService.test.ts b/src/__tests__/services/AuthService.test.ts index 9421c43..7ad08f5 100644 --- a/src/__tests__/services/AuthService.test.ts +++ b/src/__tests__/services/AuthService.test.ts @@ -60,6 +60,10 @@ describe('AuthService', () => { (StorageService.saveMasterKeyInfo as jest.Mock).mockResolvedValue(undefined); (StorageService.deleteMasterKeyInfo as jest.Mock).mockResolvedValue(undefined); + (StorageService.savePendingMasterKeyMigration as jest.Mock).mockResolvedValue(undefined); + (StorageService.getPendingMasterKeyMigration as jest.Mock).mockResolvedValue(null); + (StorageService.deletePendingMasterKeyMigration as jest.Mock).mockResolvedValue(undefined); + (StorageService.canDecryptVaultWithKey as jest.Mock).mockResolvedValue(true); (StorageService.setEncryptionKey as jest.Mock).mockImplementation(() => {}); (StorageService.initDatabase as jest.Mock).mockResolvedValue(undefined); (StorageService.getMasterKeyInfo as jest.Mock).mockResolvedValue(mockMasterKeyInfo); @@ -589,6 +593,115 @@ describe('AuthService', () => { }); }); + describe('Crash-safe KDF transaction', () => { + const newInfo: UserMasterKey = { + salt: 'new-salt', + iterations: 2, + memory: 19456, + verifier: 'new-verifier', + }; + const newKey = 'aabbccddeeff00112233445566778899aabbccddeeff00112233445566778899'; + + beforeEach(() => { + (CryptoService.createMasterKeyInfoWithDerivedKey as jest.Mock).mockResolvedValue({ + masterKeyInfo: newInfo, + derivedKey: newKey, + }); + (StorageService.getMasterKeyInfo as jest.Mock).mockResolvedValue(mockMasterKeyInfo); + (StorageService.getEncryptionKey as jest.Mock).mockReturnValue(mockEncryptionKey); + }); + + it('writes the recovery record before the vault and commits metadata last', async () => { + (CryptoService.isNativeKdfAvailable as jest.Mock).mockReturnValue(true); + (CryptoService.isLegacyKdf as jest.Mock).mockReturnValue(true); + + const callOrder: string[] = []; + (StorageService.savePendingMasterKeyMigration as jest.Mock).mockImplementation(async () => { + callOrder.push('write-recovery-record'); + }); + (StorageService.reEncryptAllData as jest.Mock).mockImplementation(async () => { + callOrder.push('vault-write'); + }); + (StorageService.saveMasterKeyInfo as jest.Mock).mockImplementation(async () => { + callOrder.push('metadata-commit'); + }); + (StorageService.deletePendingMasterKeyMigration as jest.Mock).mockImplementation(async () => { + callOrder.push('clear-recovery-record'); + }); + + await expect(AuthService.loginWithMasterPassword(mockMasterPassword)).resolves.toBe(true); + + expect(callOrder).toEqual([ + 'write-recovery-record', + 'vault-write', + 'metadata-commit', + 'clear-recovery-record', + ]); + expect(StorageService.savePendingMasterKeyMigration).toHaveBeenCalledWith( + expect.objectContaining({ masterKeyInfo: newInfo }), + ); + }); + + it('keeps the recovery record when the metadata commit and the rollback both fail', async () => { + (CryptoService.isNativeKdfAvailable as jest.Mock).mockReturnValue(true); + (CryptoService.isLegacyKdf as jest.Mock).mockReturnValue(true); + (StorageService.reEncryptAllData as jest.Mock) + .mockResolvedValueOnce(undefined) + .mockRejectedValueOnce(new Error('rollback failed')); + (StorageService.saveMasterKeyInfo as jest.Mock).mockRejectedValueOnce( + new Error('SecureStore unavailable'), + ); + + await expect(AuthService.loginWithMasterPassword(mockMasterPassword)).resolves.toBe(false); + + expect(StorageService.savePendingMasterKeyMigration).toHaveBeenCalled(); + expect(StorageService.deletePendingMasterKeyMigration).not.toHaveBeenCalled(); + expect(AuthService.getLastAuthFailure()).toEqual( + expect.objectContaining({ reason: 'kdf_upgrade_rollback_failed' }), + ); + }); + + it('writes the recovery record when rotating the master password', async () => { + await AuthService.loginWithMasterPassword(mockMasterPassword); + (CryptoService.isLegacyKdf as jest.Mock).mockReturnValue(false); + + await expect(AuthService.updateMasterPassword('new-password')).resolves.toBe(true); + + expect(StorageService.savePendingMasterKeyMigration).toHaveBeenCalledWith( + expect.objectContaining({ masterKeyInfo: newInfo }), + ); + const pendingWriteOrder = (StorageService.savePendingMasterKeyMigration as jest.Mock).mock + .invocationCallOrder[0]; + const vaultWriteOrder = (StorageService.reEncryptAllData as jest.Mock).mock + .invocationCallOrder[0]; + const metadataOrder = (StorageService.saveMasterKeyInfo as jest.Mock).mock + .invocationCallOrder[0]; + const pendingClearOrders = (StorageService.deletePendingMasterKeyMigration as jest.Mock).mock + .invocationCallOrder; + expect(pendingWriteOrder).toBeLessThan(vaultWriteOrder); + expect(vaultWriteOrder).toBeLessThan(metadataOrder); + expect(Math.max(...pendingClearOrders)).toBeGreaterThan(metadataOrder); + }); + + it('refuses biometric login while a KDF recovery record is pending', async () => { + (StorageService.getPendingMasterKeyMigration as jest.Mock).mockResolvedValue({ + masterKeyInfo: newInfo, + startedAt: 1, + }); + (StorageService.getUserPreferences as jest.Mock).mockResolvedValue({ + biometricsEnabled: true, + }); + + await expect(AuthService.loginWithBiometrics()).resolves.toBe(false); + + expect(StorageService.getBiometricKey).not.toHaveBeenCalled(); + expect(StorageService.deleteBiometricKey).not.toHaveBeenCalled(); + expect(AuthService.getLastAuthFailure()).toEqual( + expect.objectContaining({ reason: 'kdf_recovery_pending' }), + ); + }); + }); + describe('verifyMasterPassword', () => { it('should return true for correct password without modifying auth state', async () => { // beforeEach -> logout calls setEncryptionKey(''). Reset mocks so diff --git a/src/__tests__/services/AuthServiceKdfUpgradeCrashRecovery.test.ts b/src/__tests__/services/AuthServiceKdfUpgradeCrashRecovery.test.ts new file mode 100644 index 0000000..c2a1487 --- /dev/null +++ b/src/__tests__/services/AuthServiceKdfUpgradeCrashRecovery.test.ts @@ -0,0 +1,213 @@ +import CryptoJS from 'crypto-js'; +import AsyncStorage from '@react-native-async-storage/async-storage'; +import * as SecureStore from 'expo-secure-store'; +import * as AuthService from '../../services/auth/authService'; +import * as StorageService from '../../services/storage/storageService'; +import * as CryptoService from '../../services/crypto/cryptoService'; +import { Password } from '../../models/Password'; +import { UserMasterKey } from '../../models/User'; + +// The native KDF is mocked globally in jest.setup.js. This suite replaces its +// implementation with a deterministic 256-bit derivation so the real CryptoService +// (Argon2 metadata path, KS1 encryption) can run end-to-end in Jest. +const argon2Mock = jest.requireMock('react-native-argon2') as { argon2: jest.Mock }; + +// --- Simulated device storage --------------------------------------------------------- +// Records live in Maps so a test can simulate a process kill: module memory is lost, +// the device storage survives and a fresh login has to recover from it. + +const MASTER_KEY_INFO_STORAGE_KEY = 'keysoft_master_key_info'; +const PENDING_MIGRATION_STORAGE_KEY = 'keysoft_pending_master_key_info'; +const PASSWORDS_STORAGE_KEY = 'keysoft_passwords'; +const NOTES_STORAGE_KEY = 'keysoft_notes'; + +const mockAsyncStorageRecords = new Map(); +const mockSecureStoreRecords = new Map(); +let mockCrashOnSecureStoreKey: string | null = null; +let mockCrashPointReached = false; + +function installDeterministicArgon2(): void { + argon2Mock.argon2.mockImplementation( + async (password: string, salt: string, options?: { iterations?: number }) => { + const hash = CryptoJS.PBKDF2(`${password}:${salt}`, 'keysoft-test-kdf', { + keySize: 256 / 32, + iterations: Math.max(1, options?.iterations ?? 1), + }); + return { rawHash: hash.toString(CryptoJS.enc.Hex) }; + }, + ); +} + +function installMockStorage(): void { + const asyncStorage = AsyncStorage as unknown as { + getItem: jest.Mock; + setItem: jest.Mock; + removeItem: jest.Mock; + multiGet: jest.Mock; + multiSet: jest.Mock; + multiRemove: jest.Mock; + }; + + asyncStorage.getItem.mockImplementation( + async (key: string) => mockAsyncStorageRecords.get(key) ?? null, + ); + asyncStorage.setItem.mockImplementation(async (key: string, value: string) => { + mockAsyncStorageRecords.set(key, value); + }); + asyncStorage.removeItem.mockImplementation(async (key: string) => { + mockAsyncStorageRecords.delete(key); + }); + asyncStorage.multiGet.mockImplementation(async (keys: string[]) => + keys.map((key) => [key, mockAsyncStorageRecords.get(key) ?? null] as [string, string | null]), + ); + asyncStorage.multiSet.mockImplementation(async (pairs: [string, string][]) => { + for (const [key, value] of pairs) { + mockAsyncStorageRecords.set(key, value); + } + }); + asyncStorage.multiRemove.mockImplementation(async (keys: string[]) => { + for (const key of keys) { + mockAsyncStorageRecords.delete(key); + } + }); + + const secureStore = SecureStore as unknown as { + getItemAsync: jest.Mock; + setItemAsync: jest.Mock; + deleteItemAsync: jest.Mock; + }; + + secureStore.getItemAsync.mockImplementation( + async (key: string) => mockSecureStoreRecords.get(key) ?? null, + ); + secureStore.setItemAsync.mockImplementation(async (key: string, value: string) => { + if (mockCrashOnSecureStoreKey === key) { + mockCrashOnSecureStoreKey = null; + mockCrashPointReached = true; + // Never settles: the process is killed before this write reaches the disk. + return new Promise(() => {}); + } + mockSecureStoreRecords.set(key, value); + }); + secureStore.deleteItemAsync.mockImplementation(async (key: string) => { + mockSecureStoreRecords.delete(key); + }); +} + +const seededPassword: Password = { + id: 'seeded-password', + title: 'Seeded account', + username: 'user', + password: 'secret-value', + createdAt: 1, + updatedAt: 1, +}; + +async function deriveKeyFor(masterKeyInfo: UserMasterKey, password: string): Promise { + return CryptoService.deriveKey( + password, + masterKeyInfo.salt, + masterKeyInfo.iterations, + masterKeyInfo.memory, + ); +} + +async function seedLegacyVault( + masterPassword: string, +): Promise<{ masterKeyInfo: UserMasterKey; vaultKey: string }> { + const salt = CryptoService.generateSalt(); + // Legacy metadata: memory = 0 makes the metadata a PBKDF2 vault, which login treats as + // a KDF upgrade candidate. The low iteration count only keeps the test fast. + const iterations = 1000; + const memory = 0; + const vaultKey = await CryptoService.deriveKey(masterPassword, salt, iterations, memory); + const masterKeyInfo: UserMasterKey = { + salt, + iterations, + memory, + verifier: CryptoJS.SHA256(vaultKey).toString(), + }; + + mockSecureStoreRecords.set(MASTER_KEY_INFO_STORAGE_KEY, JSON.stringify(masterKeyInfo)); + mockAsyncStorageRecords.set( + PASSWORDS_STORAGE_KEY, + await CryptoService.encrypt(JSON.stringify([seededPassword]), vaultKey), + ); + mockAsyncStorageRecords.set( + NOTES_STORAGE_KEY, + await CryptoService.encrypt(JSON.stringify([]), vaultKey), + ); + + return { masterKeyInfo, vaultKey }; +} + +async function waitForSimulatedCrash(): Promise { + for (let attempt = 0; attempt < 500; attempt += 1) { + if (mockCrashPointReached) { + return; + } + await new Promise((resolve) => setTimeout(resolve, 1)); + } + throw new Error('Simulated crash point was not reached'); +} + +describe('AuthService transparent KDF upgrade crash recovery', () => { + const masterPassword = 'correct horse battery staple'; + + beforeEach(async () => { + jest.clearAllMocks(); + mockAsyncStorageRecords.clear(); + mockSecureStoreRecords.clear(); + mockCrashOnSecureStoreKey = null; + mockCrashPointReached = false; + installDeterministicArgon2(); + installMockStorage(); + await AuthService.logout(); + await StorageService.clearAllData(); + }); + + it('opens the vault after a crash between the vault write and the metadata commit', async () => { + const seeded = await seedLegacyVault(masterPassword); + + // Kill the process at the metadata commit, after the re-encrypted vault was written. + mockCrashOnSecureStoreKey = MASTER_KEY_INFO_STORAGE_KEY; + const crashedLogin = AuthService.loginWithMasterPassword(masterPassword); + void crashedLogin.catch(() => undefined); + await waitForSimulatedCrash(); + + const pendingJson = mockSecureStoreRecords.get(PENDING_MIGRATION_STORAGE_KEY); + expect(pendingJson).toBeDefined(); + const pendingMigration = JSON.parse(pendingJson as string) as { + masterKeyInfo: UserMasterKey; + startedAt: number; + }; + + // This is the state that used to lock the user out: the vault uses the new key while + // the active metadata still describes the legacy one. + const pendingKey = await deriveKeyFor(pendingMigration.masterKeyInfo, masterPassword); + await expect(StorageService.canDecryptVaultWithKey(pendingKey)).resolves.toBe(true); + await expect(StorageService.canDecryptVaultWithKey(seeded.vaultKey)).resolves.toBe(false); + expect(JSON.parse(mockSecureStoreRecords.get(MASTER_KEY_INFO_STORAGE_KEY) as string)).toEqual( + seeded.masterKeyInfo, + ); + + // Simulated process restart: only device storage survives. + await AuthService.logout(); + + await expect(AuthService.loginWithMasterPassword(masterPassword)).resolves.toBe(true); + + expect(mockSecureStoreRecords.has(PENDING_MIGRATION_STORAGE_KEY)).toBe(false); + expect(JSON.parse(mockSecureStoreRecords.get(MASTER_KEY_INFO_STORAGE_KEY) as string)).toEqual( + pendingMigration.masterKeyInfo, + ); + expect((await StorageService.getAllPasswords()).map((record) => record.id)).toEqual([ + seededPassword.id, + ]); + await expect(StorageService.canDecryptVaultWithKey(pendingKey)).resolves.toBe(true); + + // The recovered metadata still requires a password: the vault is not left unlocked. + await AuthService.logout(); + expect(AuthService.getIsAuthenticated()).toBe(false); + await expect(AuthService.loginWithMasterPassword('wrong password')).resolves.toBe(false); + }); +}); diff --git a/src/__tests__/services/AuthServicePasswordChangeCrashRecovery.test.ts b/src/__tests__/services/AuthServicePasswordChangeCrashRecovery.test.ts new file mode 100644 index 0000000..594af32 --- /dev/null +++ b/src/__tests__/services/AuthServicePasswordChangeCrashRecovery.test.ts @@ -0,0 +1,231 @@ +import CryptoJS from 'crypto-js'; +import AsyncStorage from '@react-native-async-storage/async-storage'; +import * as SecureStore from 'expo-secure-store'; +import * as AuthService from '../../services/auth/authService'; +import * as StorageService from '../../services/storage/storageService'; +import * as CryptoService from '../../services/crypto/cryptoService'; +import { Password } from '../../models/Password'; +import { UserMasterKey } from '../../models/User'; + +// The native KDF is mocked globally in jest.setup.js. This suite replaces its +// implementation with a deterministic 256-bit derivation so the real CryptoService +// (Argon2 metadata path, KS1 encryption) can run end-to-end in Jest. +const argon2Mock = jest.requireMock('react-native-argon2') as { argon2: jest.Mock }; + +// --- Simulated device storage --------------------------------------------------------- +// Records live in Maps so a test can simulate a process kill: module memory is lost, +// the device storage survives and a fresh login has to recover from it. + +const MASTER_KEY_INFO_STORAGE_KEY = 'keysoft_master_key_info'; +const PENDING_MIGRATION_STORAGE_KEY = 'keysoft_pending_master_key_info'; +const PASSWORDS_STORAGE_KEY = 'keysoft_passwords'; +const NOTES_STORAGE_KEY = 'keysoft_notes'; + +const mockAsyncStorageRecords = new Map(); +const mockSecureStoreRecords = new Map(); +let mockCrashOnSecureStoreKey: string | null = null; +let mockCrashPointReached = false; + +function installDeterministicArgon2(): void { + argon2Mock.argon2.mockImplementation( + async (password: string, salt: string, options?: { iterations?: number }) => { + const hash = CryptoJS.PBKDF2(`${password}:${salt}`, 'keysoft-test-kdf', { + keySize: 256 / 32, + iterations: Math.max(1, options?.iterations ?? 1), + }); + return { rawHash: hash.toString(CryptoJS.enc.Hex) }; + }, + ); +} + +function installMockStorage(): void { + const asyncStorage = AsyncStorage as unknown as { + getItem: jest.Mock; + setItem: jest.Mock; + removeItem: jest.Mock; + multiGet: jest.Mock; + multiSet: jest.Mock; + multiRemove: jest.Mock; + }; + + asyncStorage.getItem.mockImplementation( + async (key: string) => mockAsyncStorageRecords.get(key) ?? null, + ); + asyncStorage.setItem.mockImplementation(async (key: string, value: string) => { + mockAsyncStorageRecords.set(key, value); + }); + asyncStorage.removeItem.mockImplementation(async (key: string) => { + mockAsyncStorageRecords.delete(key); + }); + asyncStorage.multiGet.mockImplementation(async (keys: string[]) => + keys.map((key) => [key, mockAsyncStorageRecords.get(key) ?? null] as [string, string | null]), + ); + asyncStorage.multiSet.mockImplementation(async (pairs: [string, string][]) => { + for (const [key, value] of pairs) { + mockAsyncStorageRecords.set(key, value); + } + }); + asyncStorage.multiRemove.mockImplementation(async (keys: string[]) => { + for (const key of keys) { + mockAsyncStorageRecords.delete(key); + } + }); + + const secureStore = SecureStore as unknown as { + getItemAsync: jest.Mock; + setItemAsync: jest.Mock; + deleteItemAsync: jest.Mock; + }; + + secureStore.getItemAsync.mockImplementation( + async (key: string) => mockSecureStoreRecords.get(key) ?? null, + ); + secureStore.setItemAsync.mockImplementation(async (key: string, value: string) => { + if (mockCrashOnSecureStoreKey === key) { + mockCrashOnSecureStoreKey = null; + mockCrashPointReached = true; + // Never settles: the process is killed before this write reaches the disk. + return new Promise(() => {}); + } + mockSecureStoreRecords.set(key, value); + }); + secureStore.deleteItemAsync.mockImplementation(async (key: string) => { + mockSecureStoreRecords.delete(key); + }); +} + +const seededPassword: Password = { + id: 'seeded-password', + title: 'Seeded account', + username: 'user', + password: 'secret-value', + createdAt: 1, + updatedAt: 1, +}; + +async function deriveKeyFor(masterKeyInfo: UserMasterKey, password: string): Promise { + return CryptoService.deriveKey( + password, + masterKeyInfo.salt, + masterKeyInfo.iterations, + masterKeyInfo.memory, + ); +} + +async function seedCurrentVault( + masterPassword: string, +): Promise<{ masterKeyInfo: UserMasterKey; vaultKey: string }> { + const { masterKeyInfo, derivedKey: vaultKey } = + await CryptoService.createMasterKeyInfoWithDerivedKey(masterPassword); + + mockSecureStoreRecords.set(MASTER_KEY_INFO_STORAGE_KEY, JSON.stringify(masterKeyInfo)); + mockAsyncStorageRecords.set( + PASSWORDS_STORAGE_KEY, + await CryptoService.encrypt(JSON.stringify([seededPassword]), vaultKey), + ); + mockAsyncStorageRecords.set( + NOTES_STORAGE_KEY, + await CryptoService.encrypt(JSON.stringify([]), vaultKey), + ); + + return { masterKeyInfo, vaultKey }; +} + +function readPendingMigration(): { masterKeyInfo: UserMasterKey; startedAt: number } | null { + const json = mockSecureStoreRecords.get(PENDING_MIGRATION_STORAGE_KEY); + return json ? (JSON.parse(json) as { masterKeyInfo: UserMasterKey; startedAt: number }) : null; +} + +async function waitForSimulatedCrash(): Promise { + for (let attempt = 0; attempt < 500; attempt += 1) { + if (mockCrashPointReached) { + return; + } + await new Promise((resolve) => setTimeout(resolve, 1)); + } + throw new Error('Simulated crash point was not reached'); +} + +describe('AuthService master password change crash recovery', () => { + const oldPassword = 'old master password'; + const newPassword = 'new master password'; + + beforeEach(async () => { + jest.clearAllMocks(); + mockAsyncStorageRecords.clear(); + mockSecureStoreRecords.clear(); + mockCrashOnSecureStoreKey = null; + mockCrashPointReached = false; + installDeterministicArgon2(); + installMockStorage(); + await AuthService.logout(); + await StorageService.clearAllData(); + }); + + it('accepts the new password after a crash between the vault write and the metadata commit', async () => { + const seeded = await seedCurrentVault(oldPassword); + await expect(AuthService.loginWithMasterPassword(oldPassword)).resolves.toBe(true); + + // Kill the process at the metadata commit, after the vault was re-encrypted. + mockCrashOnSecureStoreKey = MASTER_KEY_INFO_STORAGE_KEY; + const crashedUpdate = AuthService.updateMasterPassword(newPassword); + void crashedUpdate.catch(() => undefined); + await waitForSimulatedCrash(); + + const pendingMigration = readPendingMigration(); + expect(pendingMigration).not.toBeNull(); + const pendingKey = await deriveKeyFor( + (pendingMigration as { masterKeyInfo: UserMasterKey }).masterKeyInfo, + newPassword, + ); + await expect(StorageService.canDecryptVaultWithKey(pendingKey)).resolves.toBe(true); + await expect(StorageService.canDecryptVaultWithKey(seeded.vaultKey)).resolves.toBe(false); + + // Simulated process restart: only device storage survives. + await AuthService.logout(); + + // Trying the previous password first must not destroy the recovery record; recovery + // reports which password is needed instead of silently failing with "invalid PIN". + await expect(AuthService.loginWithMasterPassword(oldPassword)).resolves.toBe(false); + expect(AuthService.getLastAuthFailure()?.reason).toBe('kdf_recovery_requires_new_password'); + expect(readPendingMigration()).not.toBeNull(); + + await expect(AuthService.loginWithMasterPassword(newPassword)).resolves.toBe(true); + expect(readPendingMigration()).toBeNull(); + expect(JSON.parse(mockSecureStoreRecords.get(MASTER_KEY_INFO_STORAGE_KEY) as string)).toEqual( + (pendingMigration as { masterKeyInfo: UserMasterKey }).masterKeyInfo, + ); + expect((await StorageService.getAllPasswords()).map((record) => record.id)).toEqual([ + seededPassword.id, + ]); + + // The change is committed: the previous password is no longer accepted. + await AuthService.logout(); + await expect(AuthService.loginWithMasterPassword(oldPassword)).resolves.toBe(false); + }); + + it('rolls back a recovery record whose vault write never happened', async () => { + const seeded = await seedCurrentVault(oldPassword); + const { masterKeyInfo: pendingMasterKeyInfo } = + await CryptoService.createMasterKeyInfoWithDerivedKey(newPassword); + + // Durable state of a crash after the recovery record was written and before the + // vault write: the previous metadata and ciphertext are still the active ones. + mockSecureStoreRecords.set( + PENDING_MIGRATION_STORAGE_KEY, + JSON.stringify({ masterKeyInfo: pendingMasterKeyInfo, startedAt: Date.now() }), + ); + + await expect(AuthService.loginWithMasterPassword(oldPassword)).resolves.toBe(true); + + expect(readPendingMigration()).toBeNull(); + expect(JSON.parse(mockSecureStoreRecords.get(MASTER_KEY_INFO_STORAGE_KEY) as string)).toEqual( + seeded.masterKeyInfo, + ); + await expect(StorageService.canDecryptVaultWithKey(seeded.vaultKey)).resolves.toBe(true); + + // The never-committed change is discarded, so the new password is not active. + await AuthService.logout(); + await expect(AuthService.loginWithMasterPassword(newPassword)).resolves.toBe(false); + }); +}); diff --git a/src/__tests__/services/StorageService.test.ts b/src/__tests__/services/StorageService.test.ts index 1585399..df1b3ba 100644 --- a/src/__tests__/services/StorageService.test.ts +++ b/src/__tests__/services/StorageService.test.ts @@ -487,4 +487,85 @@ describe('StorageService', () => { expect(AsyncStorage.clear).not.toHaveBeenCalled(); }); }); + + describe('Serialised vault writes', () => { + const buildPassword = (id: string): Password => ({ + id, + title: `Title ${id}`, + username: 'user', + password: `secret-${id}`, + createdAt: 0, + updatedAt: 0, + }); + + it('does not lose a password when two saves start from the same snapshot', async () => { + const persisted = new Map(); + (AsyncStorage.setItem as jest.Mock).mockImplementation(async (key: string, value: string) => { + const records = JSON.parse(value.replace('encrypted-', '')) as Password[]; + // The first save is the slow write. Without serialisation the second save + // commits first from the same empty snapshot and is then overwritten by the + // first save's stale snapshot: the last writer silently drops the other record. + await new Promise((resolve) => + setTimeout(resolve, records.some((record) => record.id === 'first') ? 20 : 1), + ); + persisted.set(key, value); + }); + + await Promise.all([ + StorageService.savePassword(buildPassword('first')), + StorageService.savePassword(buildPassword('second')), + ]); + + const persistedPasswords = JSON.parse( + (persisted.get('keysoft_passwords') as string).replace('encrypted-', ''), + ) as Password[]; + expect(persistedPasswords.map((record) => record.id)).toEqual(['first', 'second']); + await expect(StorageService.getAllPasswords()).resolves.toHaveLength(2); + }); + + it('runs a queued save after a concurrent re-encryption and uses the new key', async () => { + await StorageService.savePassword(buildPassword('existing')); + jest.clearAllMocks(); + (CryptoService.encrypt as jest.Mock).mockImplementation((data: string, _key: string) => + Promise.resolve(`encrypted-${data}`), + ); + let stagedSnapshotTaken = false; + (AsyncStorage.multiSet as jest.Mock).mockImplementation(async () => { + stagedSnapshotTaken = true; + await new Promise((resolve) => setTimeout(resolve, 20)); + }); + (AsyncStorage.setItem as jest.Mock).mockResolvedValue(undefined); + + const newKey = '1112131415161718191a1b1c1d1e1f201112131415161718191a1b1c1d1e1f20'; + const rekey = StorageService.reEncryptAllData(newKey); + // Let the re-encryption take its snapshot and acquire the collection locks before + // the save starts; the save must then wait and encrypt with the new key. + while (!stagedSnapshotTaken) { + await new Promise((resolve) => setTimeout(resolve, 0)); + } + const queuedSave = StorageService.savePassword(buildPassword('queued')); + await Promise.all([rekey, queuedSave]); + + const queuedSaveEncrypt = (CryptoService.encrypt as jest.Mock).mock.calls.find( + ([data]) => typeof data === 'string' && data.includes('"id":"queued"'), + ); + expect(queuedSaveEncrypt?.[1]).toBe(newKey); + + const storedIds = (await StorageService.getAllPasswords()).map((record) => record.id); + expect(storedIds).toEqual(['existing', 'queued']); + }); + + it('keeps the write queue usable after a failed persistence', async () => { + (AsyncStorage.setItem as jest.Mock).mockRejectedValueOnce(new Error('disk full')); + + await expect(StorageService.savePassword(buildPassword('failed'))).rejects.toThrow( + 'disk full', + ); + + await StorageService.savePassword(buildPassword('after')); + + const storedIds = (await StorageService.getAllPasswords()).map((record) => record.id); + expect(storedIds).toEqual(['after']); + }); + }); }); diff --git a/src/locales/en.ts b/src/locales/en.ts index f22b7fd..36f403d 100644 --- a/src/locales/en.ts +++ b/src/locales/en.ts @@ -59,6 +59,12 @@ export const en = { 'The native security module is unavailable. Update the app or contact support.', kdf_timeout_message: 'Key derivation is taking too long. Try again shortly.', vault_init_error: 'The vault could not be opened. Restart the app and try again.', + kdf_recovery_new_password_message: + 'The vault was already updated with the new password. Enter the new password to unlock it.', + kdf_recovery_previous_password_message: + 'The vault was not updated yet. Enter the previous password and try changing it again.', + kdf_recovery_master_password_message: + 'A security upgrade is pending. Sign in with the master password to complete it.', // HomeScreen search_passwords: 'Search passwords...', diff --git a/src/locales/it.ts b/src/locales/it.ts index 70780bf..18cc888 100644 --- a/src/locales/it.ts +++ b/src/locales/it.ts @@ -61,6 +61,12 @@ export const it = { kdf_timeout_message: 'La derivazione della chiave sta richiedendo troppo tempo. Riprova tra poco.', vault_init_error: "Non è stato possibile aprire il vault. Riprova dopo aver riavviato l'app.", + kdf_recovery_new_password_message: + 'Il vault è già stato aggiornato con la nuova password. Inserisci la nuova password per sbloccarlo.', + kdf_recovery_previous_password_message: + 'Il vault non è ancora stato aggiornato. Inserisci la password precedente e riprova il cambio password.', + kdf_recovery_master_password_message: + 'Un aggiornamento di sicurezza è in sospeso. Accedi con la master password per completarlo.', // HomeScreen search_passwords: 'Cerca password...', diff --git a/src/screens/AuthScreen.tsx b/src/screens/AuthScreen.tsx index 6214455..fe1dedb 100644 --- a/src/screens/AuthScreen.tsx +++ b/src/screens/AuthScreen.tsx @@ -384,6 +384,14 @@ function getPinLoginFailureMessageKey(): string { return 'vault_init_error'; } + if (failure?.reason === 'kdf_recovery_requires_new_password') { + return 'kdf_recovery_new_password_message'; + } + + if (failure?.reason === 'kdf_recovery_requires_previous_password') { + return 'kdf_recovery_previous_password_message'; + } + return 'invalid_pin'; } @@ -394,6 +402,10 @@ function getBiometricFailureMessageKey(): string { return 'biometric_key_unavailable_message'; } + if (failure?.reason === 'kdf_recovery_pending') { + return 'kdf_recovery_master_password_message'; + } + if (failure?.reason === 'init_database_failed') { return 'vault_init_error'; } diff --git a/src/services/auth/authService.ts b/src/services/auth/authService.ts index 64caac7..c762dc5 100644 --- a/src/services/auth/authService.ts +++ b/src/services/auth/authService.ts @@ -9,6 +9,9 @@ import Logger from '../../utils/logger'; let isAuthenticated = false; let masterKeyInfo: UserMasterKey | null = null; let lastAuthFailure: AuthFailure | null = null; +// Serialises rekey operations in-process: two concurrent rekeys would each write their +// own recovery record and could commit the wrong metadata for the other's ciphertext. +let isVaultRekeyInProgress = false; const isExpoGo = Constants.executionEnvironment === 'storeClient'; @@ -21,6 +24,9 @@ export type AuthFailureReason = | 'verifier_mismatch' | 'init_database_failed' | 'kdf_upgrade_rollback_failed' + | 'kdf_recovery_pending' + | 'kdf_recovery_requires_new_password' + | 'kdf_recovery_requires_previous_password' | 'biometrics_unavailable' | 'biometrics_disabled' | 'biometric_key_unavailable' @@ -107,6 +113,247 @@ async function disableBiometrics( }); } +/** + * Derives a key without recording an authentication failure. Crash recovery has to try + * the password against both the active and the pending metadata, and a mismatch is a + * normal outcome there, not a login error. + */ +async function tryDeriveKey(masterPassword: string, mkInfo: UserMasterKey): Promise { + try { + return await CryptoService.deriveKey( + masterPassword, + mkInfo.salt, + mkInfo.iterations, + mkInfo.memory, + ); + } catch (error) { + Logger.warn( + 'AuthService: key derivation failed while reconciling a pending KDF migration', + error, + ); + return null; + } +} + +class VaultRekeyRollbackError extends Error { + constructor(public readonly cause?: unknown) { + super('Vault re-encryption rollback failed'); + this.name = 'VaultRekeyRollbackError'; + } +} + +/** + * Rekeys the whole vault and commits the new master-key metadata as one crash-safe + * sequence: + * 1. persist a recovery record describing the new metadata while the previous metadata + * is still active and still derivable from the previous password; + * 2. re-encrypt the vault (batched write, transactional on Android); + * 3. commit the new metadata with a single SecureStore write. + * A process kill before (3) leaves the previous metadata usable, and the recovery record + * lets the next password login either roll the vault back or finish the promotion. A + * kill after (3) only leaves a stale recovery record that the next login removes. + */ +async function rekeyVaultWithRecoveryRecord( + newMasterKeyInfo: UserMasterKey, + newKey: string, + previousKey: string, +): Promise { + await StorageService.savePendingMasterKeyMigration({ + masterKeyInfo: newMasterKeyInfo, + startedAt: Date.now(), + }); + + try { + await StorageService.reEncryptAllData(newKey); + } catch (error) { + // The vault still uses the previous key. Drop the recovery record; if deleting it + // fails, the next login removes it once it sees the vault decrypt with the active key. + await StorageService.deletePendingMasterKeyMigration(); + StorageService.setEncryptionKey(previousKey); + throw error; + } + + try { + await StorageService.saveMasterKeyInfo(newMasterKeyInfo); + } catch (error) { + // The vault is on the new key while the active metadata still describes the old one. + // Roll the ciphertext back so the previous password keeps working. + try { + await StorageService.reEncryptAllData(previousKey); + StorageService.setEncryptionKey(previousKey); + await StorageService.deletePendingMasterKeyMigration(); + } catch (rollbackError) { + // Keep the recovery record: the next password login can still finish the promotion. + Logger.error( + 'AuthService: CRITICAL - vault re-encryption rollback failed after a metadata commit error', + rollbackError, + ); + throw new VaultRekeyRollbackError(rollbackError); + } + throw error; + } + + await StorageService.deletePendingMasterKeyMigration(); + StorageService.setEncryptionKey(newKey); +} + +/** + * Makes the biometric vault key match the active key after a crash recovery. Best-effort: + * the password change is already durable and must stay successful even if this fails. + */ +async function refreshBiometricKeyAfterKdfRecovery(vaultKey: string): Promise { + try { + const prefs = await StorageService.getUserPreferences(); + if (!prefs.biometricsEnabled) { + return; + } + + try { + await StorageService.saveBiometricKey(vaultKey); + } catch (biometricError) { + Logger.warn( + 'AuthService: failed to refresh biometric key after KDF recovery', + biometricError, + ); + try { + await disableBiometrics(prefs); + } catch (cleanupError) { + Logger.warn('AuthService: failed to disable biometrics after KDF recovery', cleanupError); + } + } + } catch (error) { + Logger.warn('AuthService: failed to read preferences after KDF recovery', error); + } +} + +interface PendingKdfRecoveryResult { + masterKeyInfo: UserMasterKey; + vaultKey: string; +} + +/** + * Resolves a recovery record left by an interrupted KDF upgrade or master-password + * update. The vault may have been written with the previous key, with the pending key, + * or not yet touched at all, so both passwords are checked against both metadata records + * and the vault records are inspected with the keys that actually derive. + */ +async function recoverPendingKdfMigration( + masterPassword: string, + activeMasterKeyInfo: UserMasterKey, + activeDerivedKey: string, + isActivePasswordValid: boolean, + pendingMigration: StorageService.PendingMasterKeyMigration, +): Promise { + const pendingMasterKeyInfo = pendingMigration.masterKeyInfo; + const pendingDerivedKey = await tryDeriveKey(masterPassword, pendingMasterKeyInfo); + const isPendingPasswordValid = pendingDerivedKey + ? CryptoService.verifyDerivedKey(pendingDerivedKey, pendingMasterKeyInfo) + : false; + + const activeVaultDecrypts = + isActivePasswordValid && (await StorageService.canDecryptVaultWithKey(activeDerivedKey)); + const pendingVaultDecrypts = + isPendingPasswordValid && pendingDerivedKey !== null + ? await StorageService.canDecryptVaultWithKey(pendingDerivedKey) + : false; + + if (isActivePasswordValid && activeVaultDecrypts) { + // The vault still matches the active metadata: the crash happened before the vault + // write, or the record is stale after a committed rekey. Drop it and log in. + await StorageService.deletePendingMasterKeyMigration(); + Logger.info('AuthService: discarded a stale pending KDF migration; vault unchanged'); + return { + masterKeyInfo: activeMasterKeyInfo, + vaultKey: activeDerivedKey, + }; + } + + if (isPendingPasswordValid && pendingDerivedKey !== null && pendingVaultDecrypts) { + // The vault was re-encrypted before the crash: commit the pending metadata first. + // That single SecureStore write is the point where the new key becomes derivable + // from stored metadata; until it succeeds the recovery record is kept intact. + try { + await StorageService.saveMasterKeyInfo(pendingMasterKeyInfo); + } catch (error) { + Logger.error('AuthService: failed to commit recovered KDF metadata', error); + setAuthFailure('kdf_recovery_pending', 'Could not persist recovered vault metadata'); + return null; + } + await StorageService.deletePendingMasterKeyMigration(); + Logger.info('AuthService: completed a pending KDF migration after crash recovery'); + return { + masterKeyInfo: pendingMasterKeyInfo, + vaultKey: pendingDerivedKey, + }; + } + + if (isActivePasswordValid && !activeVaultDecrypts) { + if (isPendingPasswordValid) { + // The same password derives both metadata records, yet the vault decrypts with + // neither key: the records are unreadable or corrupted. Keep the recovery record. + setAuthFailure('init_database_failed', 'Vault records are unreadable after crash recovery'); + return null; + } + // The previous password still matches the active metadata, but the vault is already + // on the new key. Only the password chosen during the interrupted change can open it. + setAuthFailure( + 'kdf_recovery_requires_new_password', + 'Vault already re-encrypted with the new password', + ); + return null; + } + + if (isPendingPasswordValid) { + // The new password matches the pending metadata, but the vault was not re-encrypted + // before the crash: the previous password still opens it. + setAuthFailure( + 'kdf_recovery_requires_previous_password', + 'Vault still requires the previous password', + ); + return null; + } + + setAuthFailure('verifier_mismatch', 'Master password verifier mismatch'); + return null; +} + +/** + * Loads the decrypted vault with a verified key and completes the post-login steps. + * Shared by the regular password flow and crash recovery. + */ +async function completePasswordLogin( + masterPassword: string, + mkInfo: UserMasterKey, + derivedKey: string, +): Promise { + StorageService.setEncryptionKey(derivedKey); + try { + const storageStartedAt = Date.now(); + await StorageService.initDatabase(); + Logger.debug( + `AuthService: encrypted vault load completed in ${Date.now() - storageStartedAt}ms`, + ); + } catch (error) { + StorageService.setEncryptionKey(''); + setAuthFailure('init_database_failed', 'Database initialization failed after login'); + Logger.error('AuthService: initDatabase failed after master password login', error); + return false; + } + + masterKeyInfo = mkInfo; + isAuthenticated = true; + + // Transparently migrate legacy/heavy KDFs to the lighter OWASP Argon2id parameters now + // that the vault is unlocked. Recoverable failures do not fail login; an unrecoverable + // rollback closes the session to avoid using an uncertain key. + const kdfUpgradeSafe = await upgradeVaultKdfIfLegacy(masterPassword, mkInfo); + if (!kdfUpgradeSafe) { + return false; + } + + return true; +} + /** * Best-effort, non-destructive upgrade of a legacy vault (PBKDF2 or the old heavy * Argon2 parameters) to the current Argon2id parameters. Runs after a successful @@ -121,10 +368,12 @@ async function upgradeVaultKdfIfLegacy( ): Promise { if (!CryptoService.isNativeKdfAvailable()) return true; // Expo Go: cannot derive Argon2 if (!CryptoService.isLegacyKdf(mkInfo)) return true; + if (isVaultRekeyInProgress) return true; const currentKey = StorageService.getEncryptionKey(); if (!currentKey) return true; + isVaultRekeyInProgress = true; try { Logger.info('AuthService: upgrading vault KDF to current Argon2id parameters'); @@ -135,19 +384,16 @@ async function upgradeVaultKdfIfLegacy( const { masterKeyInfo: newMkInfo, derivedKey: newKey } = await CryptoService.createMasterKeyInfoWithDerivedKey(masterPassword); - // Re-encrypt the in-memory vault with the new key, then persist the new metadata. - await StorageService.reEncryptAllData(newKey); try { - await StorageService.saveMasterKeyInfo(newMkInfo); + // Recovery record, then batched vault write, then atomic metadata commit. + await rekeyVaultWithRecoveryRecord(newMkInfo, newKey, currentKey); } catch (error) { - // Roll back to the previous key so storage and metadata stay consistent. - try { - await StorageService.reEncryptAllData(currentKey); - StorageService.setEncryptionKey(currentKey); - } catch (rollbackError) { + if (error instanceof VaultRekeyRollbackError) { + // The vault may be inconsistent; end the session and keep the recovery record so + // the next password login can complete the promotion instead of losing the vault. Logger.error( 'AuthService: CRITICAL - KDF upgrade rollback failed. Forcing logout to protect vault integrity.', - rollbackError, + error, ); isAuthenticated = false; masterKeyInfo = null; @@ -187,6 +433,8 @@ async function upgradeVaultKdfIfLegacy( } catch (error) { Logger.warn('AuthService: vault KDF upgrade skipped (will retry next login)', error); return true; + } finally { + isVaultRekeyInProgress = false; } } @@ -258,6 +506,12 @@ export const setupMasterPassword = async (masterPassword: string): Promise => { try { clearAuthFailure(); + + // A pending KDF migration can only be reconciled with the master password. Do not + // read or disable the biometric key here: the password login may still promote it. + const pendingMigration = await StorageService.getPendingMasterKeyMigration(); + if (pendingMigration) { + setAuthFailure( + 'kdf_recovery_pending', + 'A pending vault security upgrade requires the master password', + ); + return false; + } + const { available } = await isBiometricsAvailable(); if (!available) { setAuthFailure('biometrics_unavailable', 'Biometrics unavailable'); @@ -573,67 +850,63 @@ export const updateMasterPassword = async (password: string): Promise = return false; } - // Snapshot of current state for rollback - const oldMkInfo = masterKeyInfo; - let dataReEncryptedWithNewKey = false; + if (isVaultRekeyInProgress) { + Logger.warn('AuthService: ignoring master password update while a vault rekey is in progress'); + return false; + } + let newEncryptionKey: string | null = null; let prefs: Awaited>; + isVaultRekeyInProgress = true; try { // Resolve optional side-effect state before changing encrypted storage. If // this read fails, the vault remains untouched. prefs = await StorageService.getUserPreferences(); + // A recovery record here can only be stale: password login reconciles it and + // biometric login refuses to run while one exists. Drop it before recording the + // new migration so its companion metadata cannot be overwritten. + await StorageService.deletePendingMasterKeyMigration(); + // 2. Create new Master Key Info const { masterKeyInfo: newMkInfo, derivedKey } = await CryptoService.createMasterKeyInfoWithDerivedKey(password); newEncryptionKey = derivedKey; - // 4. Re-encrypt all data with new key - // This updates AsyncStorage AND the in-memory encryption key. - await StorageService.reEncryptAllData(newEncryptionKey); - dataReEncryptedWithNewKey = true; + // 3. Recovery record, then batched vault write, then atomic metadata commit. + // A process kill between the vault write and the commit is reconciled on the next + // password login instead of leaving ciphertext whose salt was never persisted. + await rekeyVaultWithRecoveryRecord(newMkInfo, derivedKey, currentKey); - // 5. Save new Master Key Info - // CRITICAL: If this fails, the rollback below restores the old encrypted state. - await StorageService.saveMasterKeyInfo(newMkInfo); - - // 6. Ciphertext and verifier metadata are committed. Update in-memory state + // 4. Ciphertext and verifier metadata are committed. Update in-memory state // before optional biometric maintenance so later cleanup failures cannot // roll the vault back while leaving the new verifier in SecureStore. masterKeyInfo = newMkInfo; - StorageService.setEncryptionKey(newEncryptionKey); + StorageService.setEncryptionKey(derivedKey); } catch (error) { Logger.error('AuthService: Error updating master password', error); setAuthFailure('derive_key_failed', 'Master password update failed'); - // Rollback: if the data was already re-encrypted with the new key, restore old encryption. - if (dataReEncryptedWithNewKey) { - try { - Logger.warn('AuthService: Attempting rollback to old encryption key...'); - await StorageService.reEncryptAllData(currentKey); - StorageService.setEncryptionKey(currentKey); - masterKeyInfo = oldMkInfo; - Logger.info('AuthService: Rollback completed - vault restored to old key'); - } catch (rollbackError) { - // Critical: rollback failed, so the vault may be inconsistent. - // Force logout: the user must authenticate again and initDatabase will reload - // the current state from disk. This is safer than continuing with in-memory state - // that may be out of sync with AsyncStorage data. - Logger.error( - 'AuthService: CRITICAL - rollback failed. Forcing logout to prevent data corruption.', - rollbackError, - ); - isAuthenticated = false; - masterKeyInfo = null; - StorageService.setEncryptionKey(''); - } + if (error instanceof VaultRekeyRollbackError) { + // Critical: the vault may be inconsistent. Force logout: the user must + // authenticate again and initDatabase will reload the persisted state. The recovery + // record is kept so the next password login can still complete the promotion. + Logger.error( + 'AuthService: CRITICAL - rollback failed. Forcing logout to prevent data corruption.', + error, + ); + isAuthenticated = false; + masterKeyInfo = null; + StorageService.setEncryptionKey(''); } return false; + } finally { + isVaultRekeyInProgress = false; } - // 7. Biometric persistence is a separate, best-effort transaction. The + // 5. Biometric persistence is a separate, best-effort transaction. The // password change is already durable and must remain successful even if the // biometric key or its cleanup cannot be written. if (prefs.biometricsEnabled && newEncryptionKey) { diff --git a/src/services/storage/storageService.ts b/src/services/storage/storageService.ts index 197ef90..a3a9297 100644 --- a/src/services/storage/storageService.ts +++ b/src/services/storage/storageService.ts @@ -108,6 +108,61 @@ const decryptionErrors = { notes: false, }; +// --- Serialised collection writes --- + +// Every mutation below rebuilds a whole collection from the shared in-memory cache and +// overwrites its AsyncStorage record. Two mutations that start from the same snapshot +// would make the last writer silently drop the other operation's change, leaving the +// cache and the persisted record disagreeing. Each storage key owns a FIFO mutex; a +// mutation runs only after the previous mutation on the same key settled. Operations +// that touch more than one collection acquire their mutexes in a fixed order, so they +// cannot deadlock. +class AsyncMutex { + private tail: Promise = Promise.resolve(); + + async runExclusive(operation: () => Promise): Promise { + const previous = this.tail; + let release!: () => void; + this.tail = new Promise((resolve) => { + release = resolve; + }); + + await previous; + try { + return await operation(); + } finally { + release(); + } + } +} + +const storageMutexes = new Map(); + +function getStorageMutex(storageKey: string): AsyncMutex { + let mutex = storageMutexes.get(storageKey); + if (!mutex) { + mutex = new AsyncMutex(); + storageMutexes.set(storageKey, mutex); + } + return mutex; +} + +async function withStorageLocks( + storageKeys: readonly string[], + operation: () => Promise, +): Promise { + const orderedMutexes = [...new Set(storageKeys)].sort().map(getStorageMutex); + + const acquire = async (index: number): Promise => { + if (index >= orderedMutexes.length) { + return operation(); + } + return orderedMutexes[index].runExclusive(() => acquire(index + 1)); + }; + + return acquire(0); +} + function generateStorageId(): string { return `${Date.now().toString(36)}_${bytesToHex(getRandomBytes(8))}`; } @@ -125,23 +180,29 @@ export const reEncryptAllData = async (newKey: string): Promise => { } try { - const [encryptedPasswords, encryptedNotes] = await Promise.all([ - CryptoService.encrypt(JSON.stringify(cache.passwords), newKey), - CryptoService.encrypt(JSON.stringify(cache.notes), newKey), - ]); - - // Compute both ciphertexts before the storage operation so an encryption - // failure cannot leave only half of the vault on the new key. - await AsyncStorage.multiSet([ - [STORAGE_KEYS.PASSWORDS, encryptedPasswords], - [STORAGE_KEYS.NOTES, encryptedNotes], - ]); + // Lock both collections: a concurrent save on either one must run before or after + // the rekey, never against the previous key while the vault is being re-encrypted. + await withStorageLocks([STORAGE_KEYS.PASSWORDS, STORAGE_KEYS.NOTES], async () => { + // Compute both ciphertexts before the storage operation so an encryption + // failure cannot leave only half of the vault on the new key. + const [encryptedPasswords, encryptedNotes] = await Promise.all([ + CryptoService.encrypt(JSON.stringify(cache.passwords), newKey), + CryptoService.encrypt(JSON.stringify(cache.notes), newKey), + ]); + + // AsyncStorage.multiSet is a single transaction on Android, so a process kill + // cannot persist only one collection. The in-memory key changes only afterwards. + await AsyncStorage.multiSet([ + [STORAGE_KEYS.PASSWORDS, encryptedPasswords], + [STORAGE_KEYS.NOTES, encryptedNotes], + ]); + encryptionKey = newKey; + }); } catch (error) { Logger.error('StorageService: Error re-encrypting vault data', error); throw new Error('Failed to re-encrypt vault data'); } - encryptionKey = newKey; Logger.info('StorageService: Data re-encryption completed successfully'); }; @@ -273,11 +334,15 @@ const loadDataFromStorage = async (): Promise => { } if (shouldPersistPasswords && !decryptionErrors.passwords) { - const encryptedPasswords = await CryptoService.encrypt( - JSON.stringify(cache.passwords), - encryptionKey, - ); - await AsyncStorage.setItem(STORAGE_KEYS.PASSWORDS, encryptedPasswords); + await withStorageLocks([STORAGE_KEYS.PASSWORDS], async () => { + const activeKey = encryptionKey; + if (!activeKey) return; + const encryptedPasswords = await CryptoService.encrypt( + JSON.stringify(cache.passwords), + activeKey, + ); + await AsyncStorage.setItem(STORAGE_KEYS.PASSWORDS, encryptedPasswords); + }); Logger.info('StorageService: Password storage migrated in one atomic write'); } @@ -294,11 +359,15 @@ const loadDataFromStorage = async (): Promise => { } } else if (notesJson.trim().startsWith('[')) { cache.notes = JSON.parse(notesJson); - const encryptedNotes = await CryptoService.encrypt( - JSON.stringify(cache.notes), - encryptionKey, - ); - await AsyncStorage.setItem(STORAGE_KEYS.NOTES, encryptedNotes); + await withStorageLocks([STORAGE_KEYS.NOTES], async () => { + const activeKey = encryptionKey; + if (!activeKey) return; + const encryptedNotes = await CryptoService.encrypt( + JSON.stringify(cache.notes), + activeKey, + ); + await AsyncStorage.setItem(STORAGE_KEYS.NOTES, encryptedNotes); + }); Logger.info('StorageService: Legacy plaintext notes migrated to encrypted storage'); } } @@ -340,6 +409,113 @@ export const deleteMasterKeyInfo = async (): Promise => { cache.masterKeyInfo = null; }; +// --- Crash-safe master-key migration --- + +const PENDING_MASTER_KEY_MIGRATION_SECURE = 'keysoft_pending_master_key_info'; + +export interface PendingMasterKeyMigration { + masterKeyInfo: UserMasterKey; + startedAt: number; +} + +function isMasterKeyInfoShape(value: unknown): value is UserMasterKey { + if (typeof value !== 'object' || value === null) { + return false; + } + + const candidate = value as Partial; + return ( + typeof candidate.salt === 'string' && + candidate.salt.length > 0 && + typeof candidate.verifier === 'string' && + candidate.verifier.length > 0 && + Number.isSafeInteger(candidate.iterations) && + Number.isSafeInteger(candidate.memory) + ); +} + +/** + * Records the metadata the vault is about to be re-encrypted with before the vault + * write starts. The active metadata keeps describing the previous key until the + * migration is committed, so a crash at any point leaves at least one password that + * can still be used to open the vault. + */ +export const savePendingMasterKeyMigration = async ( + migration: PendingMasterKeyMigration, +): Promise => { + try { + await SecureStore.setItemAsync(PENDING_MASTER_KEY_MIGRATION_SECURE, JSON.stringify(migration)); + Logger.info('StorageService: Pending master-key migration recorded'); + } catch (error) { + Logger.error('StorageService: Error saving pending master-key migration', error); + throw error; + } +}; + +export const getPendingMasterKeyMigration = async (): Promise => { + try { + const json = await SecureStore.getItemAsync(PENDING_MASTER_KEY_MIGRATION_SECURE); + if (!json) { + return null; + } + + const parsed = JSON.parse(json) as Partial | null; + if ( + !parsed || + !isMasterKeyInfoShape(parsed.masterKeyInfo) || + typeof parsed.startedAt !== 'number' || + !Number.isSafeInteger(parsed.startedAt) + ) { + Logger.warn('StorageService: Ignoring a malformed pending master-key migration record'); + return null; + } + + return { masterKeyInfo: parsed.masterKeyInfo, startedAt: parsed.startedAt }; + } catch (error) { + // A transient read failure is reported as "no recovery record" instead of blocking + // the app: the stored metadata and vault either open normally, or login fails without + // mutating anything and the recovery record is still there on the next attempt. + Logger.error('StorageService: Error reading pending master-key migration', error); + return null; + } +}; + +export const deletePendingMasterKeyMigration = async (): Promise => { + try { + await SecureStore.deleteItemAsync(PENDING_MASTER_KEY_MIGRATION_SECURE); + Logger.debug('StorageService: Pending master-key migration cleared'); + } catch (error) { + // A stale recovery record is harmless: the next password login reconciles it. + Logger.error('StorageService: Error deleting pending master-key migration', error); + } +}; + +/** + * Reports whether the persisted vault records can be decrypted with `key`, without + * touching the decrypted cache or the active key. Crash recovery uses it to decide + * which of the two known derivations actually encrypted the vault. + */ +export const canDecryptVaultWithKey = async (key: string): Promise => { + const vaultKeys = [STORAGE_KEYS.PASSWORDS, STORAGE_KEYS.NOTES]; + const entries = new Map(await AsyncStorage.multiGet(vaultKeys)); + + for (const storageKey of vaultKeys) { + const record = entries.get(storageKey); + if (!record || record.trim().startsWith('[')) { + // Missing collections and legacy plaintext records do not depend on the key. + continue; + } + + try { + await CryptoService.decrypt(record, key); + } catch { + return false; + } + } + + return true; +}; + export const setEncryptionKey = (key: string): void => { if (!key) { encryptionKey = null; @@ -403,45 +579,52 @@ function mergeRecordsById(current: T[], imported: T[]) * the active session. */ export const importBackupData = async (imported: BackupImportData): Promise => { - const activeKey = getEncryptionKeyOrThrow('import backup'); - if (decryptionErrors.passwords || decryptionErrors.notes) { - throw new Error('Decryption error detected. Cannot import backup safely.'); - } + const collectionsToLock = [ + ...(imported.passwords ? [STORAGE_KEYS.PASSWORDS] : []), + ...(imported.notes ? [STORAGE_KEYS.NOTES] : []), + ]; + + return withStorageLocks(collectionsToLock, async () => { + const activeKey = getEncryptionKeyOrThrow('import backup'); + if (decryptionErrors.passwords || decryptionErrors.notes) { + throw new Error('Decryption error detected. Cannot import backup safely.'); + } - const importedPasswords = imported.passwords ?? []; - const importedNotes = imported.notes ?? []; - const nextPasswords = mergeRecordsById(cache.passwords, importedPasswords); - const nextNotes = mergeRecordsById(cache.notes, importedNotes); + const importedPasswords = imported.passwords ?? []; + const importedNotes = imported.notes ?? []; + const nextPasswords = mergeRecordsById(cache.passwords, importedPasswords); + const nextNotes = mergeRecordsById(cache.notes, importedNotes); - if (nextPasswords.length > MAX_PASSWORDS_LIMIT) { - throw new Error(`Password limit of ${MAX_PASSWORDS_LIMIT} would be exceeded.`); - } + if (nextPasswords.length > MAX_PASSWORDS_LIMIT) { + throw new Error(`Password limit of ${MAX_PASSWORDS_LIMIT} would be exceeded.`); + } - const writes: [string, string][] = []; - if (imported.passwords) { - writes.push([ - STORAGE_KEYS.PASSWORDS, - await CryptoService.encrypt(JSON.stringify(nextPasswords), activeKey), - ]); - } - if (imported.notes) { - writes.push([ - STORAGE_KEYS.NOTES, - await CryptoService.encrypt(JSON.stringify(nextNotes), activeKey), - ]); - } + const writes: [string, string][] = []; + if (imported.passwords) { + writes.push([ + STORAGE_KEYS.PASSWORDS, + await CryptoService.encrypt(JSON.stringify(nextPasswords), activeKey), + ]); + } + if (imported.notes) { + writes.push([ + STORAGE_KEYS.NOTES, + await CryptoService.encrypt(JSON.stringify(nextNotes), activeKey), + ]); + } - if (writes.length > 0) { - await AsyncStorage.multiSet(writes); - } + if (writes.length > 0) { + await AsyncStorage.multiSet(writes); + } - if (imported.passwords) cache.passwords = nextPasswords; - if (imported.notes) cache.notes = nextNotes; + if (imported.passwords) cache.passwords = nextPasswords; + if (imported.notes) cache.notes = nextNotes; - return { - passwords: importedPasswords.length, - notes: importedNotes.length, - }; + return { + passwords: importedPasswords.length, + notes: importedNotes.length, + }; + }); }; function getEncryptionKeyOrThrow(action: string, scope?: 'passwords' | 'notes'): string { @@ -460,37 +643,41 @@ const checkPasswordLimit = () => { export const savePassword = async (password: Password): Promise => { try { - const activeKey = getEncryptionKeyOrThrow('save password', 'passwords'); - const existingIndex = password.id ? cache.passwords.findIndex((p) => p.id === password.id) : -1; - const now = Date.now(); - let savedPassword: Password; - let nextPasswords: Password[]; - - if (existingIndex !== -1) { - savedPassword = { - ...password, - updatedAt: now, - }; - nextPasswords = cache.passwords.map((current, index) => - index === existingIndex ? savedPassword : current, - ); - } else { - checkPasswordLimit(); - savedPassword = { - ...password, - id: password.id || generateStorageId(), - createdAt: now, - updatedAt: now, - }; - nextPasswords = [...cache.passwords, savedPassword]; - } + return await withStorageLocks([STORAGE_KEYS.PASSWORDS], async () => { + const activeKey = getEncryptionKeyOrThrow('save password', 'passwords'); + const existingIndex = password.id + ? cache.passwords.findIndex((p) => p.id === password.id) + : -1; + const now = Date.now(); + let savedPassword: Password; + let nextPasswords: Password[]; + + if (existingIndex !== -1) { + savedPassword = { + ...password, + updatedAt: now, + }; + nextPasswords = cache.passwords.map((current, index) => + index === existingIndex ? savedPassword : current, + ); + } else { + checkPasswordLimit(); + savedPassword = { + ...password, + id: password.id || generateStorageId(), + createdAt: now, + updatedAt: now, + }; + nextPasswords = [...cache.passwords, savedPassword]; + } - const data = JSON.stringify(nextPasswords); - const encrypted = await CryptoService.encrypt(data, activeKey); - await AsyncStorage.setItem(STORAGE_KEYS.PASSWORDS, encrypted); - cache.passwords = nextPasswords; + const data = JSON.stringify(nextPasswords); + const encrypted = await CryptoService.encrypt(data, activeKey); + await AsyncStorage.setItem(STORAGE_KEYS.PASSWORDS, encrypted); + cache.passwords = nextPasswords; - return savedPassword.id; + return savedPassword.id; + }); } catch (error) { Logger.error('StorageService: Error saving password', error); throw error; @@ -536,13 +723,15 @@ export const getPasswordsPaginated = async ( export const deletePassword = async (id: string): Promise => { try { - const activeKey = getEncryptionKeyOrThrow('delete password', 'passwords'); - const nextPasswords = cache.passwords.filter((password) => password.id !== id); - - const data = JSON.stringify(nextPasswords); - const encrypted = await CryptoService.encrypt(data, activeKey); - await AsyncStorage.setItem(STORAGE_KEYS.PASSWORDS, encrypted); - cache.passwords = nextPasswords; + await withStorageLocks([STORAGE_KEYS.PASSWORDS], async () => { + const activeKey = getEncryptionKeyOrThrow('delete password', 'passwords'); + const nextPasswords = cache.passwords.filter((password) => password.id !== id); + + const data = JSON.stringify(nextPasswords); + const encrypted = await CryptoService.encrypt(data, activeKey); + await AsyncStorage.setItem(STORAGE_KEYS.PASSWORDS, encrypted); + cache.passwords = nextPasswords; + }); } catch (error) { Logger.error('StorageService: Error deleting password', error); throw error; @@ -551,10 +740,12 @@ export const deletePassword = async (id: string): Promise => { export const clearAllPasswords = async (): Promise => { try { - getEncryptionKeyOrThrow('clear passwords', 'passwords'); - await AsyncStorage.removeItem(STORAGE_KEYS.PASSWORDS); - cache.passwords = []; - Logger.info('StorageService: All passwords cleared'); + await withStorageLocks([STORAGE_KEYS.PASSWORDS], async () => { + getEncryptionKeyOrThrow('clear passwords', 'passwords'); + await AsyncStorage.removeItem(STORAGE_KEYS.PASSWORDS); + cache.passwords = []; + Logger.info('StorageService: All passwords cleared'); + }); } catch (error) { Logger.error('StorageService: Error clearing passwords', error); throw error; @@ -563,19 +754,22 @@ export const clearAllPasswords = async (): Promise => { export const clearAllData = async (): Promise => { try { - // Remove only Keysoft-owned records. AsyncStorage can also be used by - // third-party libraries, so a global clear risks deleting unrelated state. - await AsyncStorage.multiRemove(ASYNC_STORAGE_KEYS); - await SecureStore.deleteItemAsync(STORAGE_KEYS.MASTER_KEY_INFO_SECURE); - await SecureStore.deleteItemAsync(BIOMETRIC_KEY_STORAGE); - - // Reset cache - cache.passwords = []; - cache.categories = []; - cache.notes = []; - cache.userPreferences = cloneUserPreferences(defaultUserPreferences); - cache.masterKeyInfo = null; - encryptionKey = null; + await withStorageLocks(ASYNC_STORAGE_KEYS, async () => { + // Remove only Keysoft-owned records. AsyncStorage can also be used by + // third-party libraries, so a global clear risks deleting unrelated state. + await AsyncStorage.multiRemove(ASYNC_STORAGE_KEYS); + await SecureStore.deleteItemAsync(STORAGE_KEYS.MASTER_KEY_INFO_SECURE); + await SecureStore.deleteItemAsync(PENDING_MASTER_KEY_MIGRATION_SECURE); + await SecureStore.deleteItemAsync(BIOMETRIC_KEY_STORAGE); + + // Reset cache + cache.passwords = []; + cache.categories = []; + cache.notes = []; + cache.userPreferences = cloneUserPreferences(defaultUserPreferences); + cache.masterKeyInfo = null; + encryptionKey = null; + }); Logger.info('StorageService: All data cleared'); } catch (error) { @@ -592,14 +786,16 @@ export const getCategories = async (): Promise => { export const createCategory = async (category: Omit): Promise => { try { - const newCategory: PasswordCategory = { - ...category, - id: generateStorageId(), - }; - const nextCategories = [...cache.categories, newCategory]; - await AsyncStorage.setItem(STORAGE_KEYS.CATEGORIES, JSON.stringify(nextCategories)); - cache.categories = nextCategories; - return newCategory.id; + return await withStorageLocks([STORAGE_KEYS.CATEGORIES], async () => { + const newCategory: PasswordCategory = { + ...category, + id: generateStorageId(), + }; + const nextCategories = [...cache.categories, newCategory]; + await AsyncStorage.setItem(STORAGE_KEYS.CATEGORIES, JSON.stringify(nextCategories)); + cache.categories = nextCategories; + return newCategory.id; + }); } catch (error) { Logger.error('StorageService: Error creating category', error); throw error; @@ -608,14 +804,16 @@ export const createCategory = async (category: Omit): Pr export const updateCategory = async (category: PasswordCategory): Promise => { try { - const index = cache.categories.findIndex((c) => c.id === category.id); - if (index !== -1) { - const nextCategories = cache.categories.map((current, currentIndex) => - currentIndex === index ? category : current, - ); - await AsyncStorage.setItem(STORAGE_KEYS.CATEGORIES, JSON.stringify(nextCategories)); - cache.categories = nextCategories; - } + await withStorageLocks([STORAGE_KEYS.CATEGORIES], async () => { + const index = cache.categories.findIndex((c) => c.id === category.id); + if (index !== -1) { + const nextCategories = cache.categories.map((current, currentIndex) => + currentIndex === index ? category : current, + ); + await AsyncStorage.setItem(STORAGE_KEYS.CATEGORIES, JSON.stringify(nextCategories)); + cache.categories = nextCategories; + } + }); } catch (error) { Logger.error('StorageService: Error updating category', error); throw error; @@ -624,9 +822,11 @@ export const updateCategory = async (category: PasswordCategory): Promise export const deleteCategory = async (id: string): Promise => { try { - const nextCategories = cache.categories.filter((category) => category.id !== id); - await AsyncStorage.setItem(STORAGE_KEYS.CATEGORIES, JSON.stringify(nextCategories)); - cache.categories = nextCategories; + await withStorageLocks([STORAGE_KEYS.CATEGORIES], async () => { + const nextCategories = cache.categories.filter((category) => category.id !== id); + await AsyncStorage.setItem(STORAGE_KEYS.CATEGORIES, JSON.stringify(nextCategories)); + cache.categories = nextCategories; + }); } catch (error) { Logger.error('StorageService: Error deleting category', error); throw error; @@ -637,17 +837,19 @@ export const deleteCategory = async (id: string): Promise => { export const saveNote = async (note: Note): Promise => { try { - const activeKey = getEncryptionKeyOrThrow('save note', 'notes'); - const existingIndex = cache.notes.findIndex((n) => n.id === note.id); - const nextNotes = - existingIndex >= 0 - ? cache.notes.map((current, index) => (index === existingIndex ? note : current)) - : [...cache.notes, note]; - - const data = JSON.stringify(nextNotes); - const encrypted = await CryptoService.encrypt(data, activeKey); - await AsyncStorage.setItem(STORAGE_KEYS.NOTES, encrypted); - cache.notes = nextNotes; + await withStorageLocks([STORAGE_KEYS.NOTES], async () => { + const activeKey = getEncryptionKeyOrThrow('save note', 'notes'); + const existingIndex = cache.notes.findIndex((n) => n.id === note.id); + const nextNotes = + existingIndex >= 0 + ? cache.notes.map((current, index) => (index === existingIndex ? note : current)) + : [...cache.notes, note]; + + const data = JSON.stringify(nextNotes); + const encrypted = await CryptoService.encrypt(data, activeKey); + await AsyncStorage.setItem(STORAGE_KEYS.NOTES, encrypted); + cache.notes = nextNotes; + }); } catch (error) { Logger.error('StorageService: Error saving note', error); throw error; @@ -669,13 +871,15 @@ export const getNoteById = async (noteId: string): Promise => { export const deleteNote = async (noteId: string): Promise => { try { - const activeKey = getEncryptionKeyOrThrow('delete note', 'notes'); - const nextNotes = cache.notes.filter((note) => note.id !== noteId); - - const data = JSON.stringify(nextNotes); - const encrypted = await CryptoService.encrypt(data, activeKey); - await AsyncStorage.setItem(STORAGE_KEYS.NOTES, encrypted); - cache.notes = nextNotes; + await withStorageLocks([STORAGE_KEYS.NOTES], async () => { + const activeKey = getEncryptionKeyOrThrow('delete note', 'notes'); + const nextNotes = cache.notes.filter((note) => note.id !== noteId); + + const data = JSON.stringify(nextNotes); + const encrypted = await CryptoService.encrypt(data, activeKey); + await AsyncStorage.setItem(STORAGE_KEYS.NOTES, encrypted); + cache.notes = nextNotes; + }); } catch (error) { Logger.error('StorageService: Error deleting note', error); throw error; @@ -684,10 +888,12 @@ export const deleteNote = async (noteId: string): Promise => { export const clearAllNotes = async (): Promise => { try { - getEncryptionKeyOrThrow('clear notes', 'notes'); - await AsyncStorage.removeItem(STORAGE_KEYS.NOTES); - cache.notes = []; - Logger.info('StorageService: All notes cleared'); + await withStorageLocks([STORAGE_KEYS.NOTES], async () => { + getEncryptionKeyOrThrow('clear notes', 'notes'); + await AsyncStorage.removeItem(STORAGE_KEYS.NOTES); + cache.notes = []; + Logger.info('StorageService: All notes cleared'); + }); } catch (error) { Logger.error('StorageService: Error clearing notes', error); throw error; @@ -702,9 +908,11 @@ export const getUserPreferences = async (): Promise => { export const saveUserPreferences = async (preferences: UserPreferences): Promise => { try { - await AsyncStorage.setItem(STORAGE_KEYS.USER_PREFERENCES, JSON.stringify(preferences)); - cache.userPreferences = cloneUserPreferences(preferences); - Logger.debug('StorageService: User preferences saved'); + await withStorageLocks([STORAGE_KEYS.USER_PREFERENCES], async () => { + await AsyncStorage.setItem(STORAGE_KEYS.USER_PREFERENCES, JSON.stringify(preferences)); + cache.userPreferences = cloneUserPreferences(preferences); + Logger.debug('StorageService: User preferences saved'); + }); } catch (error) { Logger.error('StorageService: Error saving user preferences', error); throw error; @@ -713,9 +921,11 @@ export const saveUserPreferences = async (preferences: UserPreferences): Promise export const clearPreferences = async (): Promise => { try { - await AsyncStorage.removeItem(STORAGE_KEYS.USER_PREFERENCES); - cache.userPreferences = cloneUserPreferences(defaultUserPreferences); - Logger.info('StorageService: Preferences cleared'); + await withStorageLocks([STORAGE_KEYS.USER_PREFERENCES], async () => { + await AsyncStorage.removeItem(STORAGE_KEYS.USER_PREFERENCES); + cache.userPreferences = cloneUserPreferences(defaultUserPreferences); + Logger.info('StorageService: Preferences cleared'); + }); } catch (error) { Logger.error('StorageService: Error clearing preferences', error); throw error;