PM-43459: feat: Update how we hash the master password (#7381)

app/src/main/kotlin/com/x8bit/bitwarden/data/auth/repository/AuthRepositoryImpl.kt

@@ -59,6 +59,7 @@ import com.x8bit.bitwarden.data.auth.datasource.disk.model.ForcePasswordResetRea
 import com.x8bit.bitwarden.data.auth.datasource.disk.model.OnboardingStatus
 import com.x8bit.bitwarden.data.auth.datasource.network.model.DeviceDataModel
 import com.x8bit.bitwarden.data.auth.datasource.sdk.AuthSdkSource
+import com.x8bit.bitwarden.data.auth.datasource.sdk.util.toKdf
 import com.x8bit.bitwarden.data.auth.datasource.sdk.util.toKdfRequestModel
 import com.x8bit.bitwarden.data.auth.manager.AuthRequestManager
 import com.x8bit.bitwarden.data.auth.manager.AuthStateManager
@@ -309,14 +310,22 @@ internal class AuthRepositoryImpl(
     override suspend fun deleteAccountWithMasterPassword(
         masterPassword: String,
     ): DeleteAccountResult {
-        val profile = authDiskSource.userState?.activeAccount?.profile
-            ?: return DeleteAccountResult.Error(message = null, error = NoActiveUserException())
+        val masterPasswordUnlock = authDiskSource
+            .userState
+            ?.activeAccount
+            ?.profile
+            ?.userDecryptionOptions
+            ?.masterPasswordUnlock
+            ?: return DeleteAccountResult.Error(
+                message = null,
+                error = MissingPropertyException("Master Password Unlock"),
+            )
         userStateManager.hasPendingAccountDeletion = true
         return authSdkSource
             .hashPassword(
-                salt = profile.email,
+                salt = masterPasswordUnlock.salt,
                 password = masterPassword,
-                kdf = profile.toSdkParams(),
+                kdf = masterPasswordUnlock.kdf.toKdf(),
                 purpose = HashPurpose.SERVER_AUTHORIZATION,
             )
             .flatMap { hashedPassword ->
@@ -978,12 +987,14 @@ internal class AuthRepositoryImpl(
     ): ResetPasswordResult {
         val profile = authDiskSource.userState?.activeAccount?.profile
             ?: return ResetPasswordResult.Error(error = NoActiveUserException())
+        val masterPasswordUnlock = profile.userDecryptionOptions?.masterPasswordUnlock
+            ?: return ResetPasswordResult.Error(MissingPropertyException("Master Password Unlock"))
         val currentPasswordHash = currentPassword?.let { password ->
             authSdkSource
                 .hashPassword(
-                    salt = profile.email,
+                    salt = masterPasswordUnlock.salt,
                     password = password,
-                    kdf = profile.toSdkParams(),
+                    kdf = masterPasswordUnlock.kdf.toKdf(),
                     purpose = HashPurpose.SERVER_AUTHORIZATION,
                 )
                 .fold(
@@ -1002,8 +1013,8 @@ internal class AuthRepositoryImpl(
                     body = ResetPasswordRequestJson(
                         currentPasswordHash = currentPasswordHash,
                         passwordHint = passwordHint,
-                        kdf = profile.toKdfRequestModel(),
-                        salt = profile.email,
+                        kdf = masterPasswordUnlock.kdf,
+                        salt = masterPasswordUnlock.salt,
                         masterPasswordAuthenticationHash = response.passwordHash,
                         masterKeyWrappedUserKey = response.newKey,
                     ),
@@ -1701,23 +1712,6 @@ internal class AuthRepositoryImpl(
             }
         }
 
-        password?.let {
-            // Save the master password hash.
-            authSdkSource
-                .hashPassword(
-                    salt = email,
-                    password = it,
-                    kdf = profile.toSdkParams(),
-                    purpose = HashPurpose.LOCAL_AUTHORIZATION,
-                )
-                .onSuccess { passwordHash ->
-                    authDiskSource.storeMasterPasswordHash(
-                        userId = userId,
-                        passwordHash = passwordHash,
-                    )
-                }
-        }
-
         settingsRepository.hasUserLoggedInOrCreatedAccount = true
         authDiskSource.userState = userStateJson
         password?.let {

app/src/main/kotlin/com/x8bit/bitwarden/data/vault/manager/VaultLockManagerImpl.kt

@@ -231,8 +231,6 @@ internal class VaultLockManagerImpl(
                                 .also {
                                     processMasterPassword(
                                         initUserCryptoMethod = initUserCryptoMethod,
-                                        email = email,
-                                        kdf = kdf,
                                         userId = userId,
                                     )
                                     if (it is VaultUnlockResult.Success) {
@@ -265,30 +263,28 @@ internal class VaultLockManagerImpl(
      */
     private suspend fun processMasterPassword(
         initUserCryptoMethod: InitUserCryptoMethod,
-        email: String,
-        kdf: Kdf,
         userId: String,
     ) {
-        initUserCryptoMethod.password?.let { password ->
+        (initUserCryptoMethod as? InitUserCryptoMethod.MasterPasswordUnlock)?.let {
             // Save the master password hash.
             authSdkSource
                 .hashPassword(
-                    salt = email,
-                    password = password,
-                    kdf = kdf,
+                    salt = it.masterPasswordUnlock.salt,
+                    password = it.password,
+                    kdf = it.masterPasswordUnlock.kdf,
                     purpose = HashPurpose.LOCAL_AUTHORIZATION,
                 )
-                .onSuccess {
+                .onSuccess { passwordHash ->
                     authDiskSource.storeMasterPasswordHash(
                         userId = userId,
-                        passwordHash = it,
+                        passwordHash = passwordHash,
                     )
                 }
 
             // If there is currently no forcePasswordResetReason, then we want to check to see if
             // the password is strong enough based on known policies.
             if (userState?.accounts[userId]?.profile?.forcePasswordResetReason == null &&
-                !passwordPolicyManager.validatePasswordAgainstPolicies(password, false)
+                !passwordPolicyManager.validatePasswordAgainstPolicies(it.password, false)
             ) {
                 authDiskSource.userState = userState?.updateForcePasswordReset(
                     userId = userId,

app/src/test/kotlin/com/x8bit/bitwarden/data/auth/repository/AuthRepositoryTest.kt

@@ -407,13 +407,36 @@ class AuthRepositoryTest {
     }
 
     @Test
-    fun `delete account fails if not logged in`() = runTest {
+    fun `delete account fails if there is no master password unlock data`() = runTest {
         val masterPassword = "hello world"
+        fakeAuthDiskSource.userState = SINGLE_USER_STATE_1.copy(
+            accounts = mapOf(
+                USER_ID_1 to ACCOUNT_1.copy(
+                    profile = ACCOUNT_1.profile.copy(userDecryptionOptions = null),
+                ),
+            ),
+        )
+
         val result = repository.deleteAccountWithMasterPassword(masterPassword = masterPassword)
+
         assertEquals(
-            DeleteAccountResult.Error(message = null, error = NoActiveUserException()),
+            DeleteAccountResult.Error(
+                message = null,
+                error = MissingPropertyException("Master Password Unlock"),
+            ),
             result,
         )
+        verify(exactly = 0) {
+            userStateManager.hasPendingAccountDeletion = true
+        }
+        coVerify(exactly = 0) {
+            authSdkSource.hashPassword(
+                salt = any(),
+                password = any(),
+                kdf = any(),
+                purpose = any(),
+            )
+        }
     }
 
     @Test
@@ -503,36 +526,73 @@ class AuthRepositoryTest {
         }
     }
 
+    @Suppress("MaxLineLength")
     @Test
-    fun `deleteAccountWithMasterPassword succeeds`() = runTest {
-        val masterPassword = "hello world"
-        val hashedMasterPassword = "hashed password"
-        fakeAuthDiskSource.userState = SINGLE_USER_STATE_1
-        val kdf = SINGLE_USER_STATE_1.activeAccount.profile.toSdkParams()
-        coEvery {
-            authSdkSource.hashPassword(EMAIL, masterPassword, kdf, HashPurpose.SERVER_AUTHORIZATION)
-        } returns hashedMasterPassword.asSuccess()
-        coEvery {
-            accountsService.deleteAccount(
-                masterPasswordHash = hashedMasterPassword,
-                oneTimePassword = null,
+    fun `deleteAccountWithMasterPassword succeeds and uses the master password unlock salt and kdf`() =
+        runTest {
+            val masterPassword = "hello world"
+            val hashedMasterPassword = "hashed password"
+            // Deliberately differs from the profile email and KDF to verify the unlock data is used
+            // instead.
+            val masterPasswordUnlock = MasterPasswordUnlockDataJson(
+                kdf = KdfJson(
+                    kdfType = KdfTypeJson.PBKDF2_SHA256,
+                    iterations = 500_000,
+                    memory = null,
+                    parallelism = null,
+                ),
+                masterKeyWrappedUserKey = ENCRYPTED_USER_KEY,
+                salt = SALT,
             )
-        } returns DeleteAccountResponseJson.Success.asSuccess()
-
-        val result = repository.deleteAccountWithMasterPassword(masterPassword = masterPassword)
-
-        assertEquals(DeleteAccountResult.Success, result)
-        verify(exactly = 1) {
-            userStateManager.hasPendingAccountDeletion = true
-        }
-        coVerify {
-            authSdkSource.hashPassword(EMAIL, masterPassword, kdf, HashPurpose.SERVER_AUTHORIZATION)
-            accountsService.deleteAccount(
-                masterPasswordHash = hashedMasterPassword,
-                oneTimePassword = null,
+            fakeAuthDiskSource.userState = SINGLE_USER_STATE_1.copy(
+                accounts = mapOf(
+                    USER_ID_1 to ACCOUNT_1.copy(
+                        profile = ACCOUNT_1.profile.copy(
+                            userDecryptionOptions = UserDecryptionOptionsJson(
+                                hasMasterPassword = true,
+                                trustedDeviceUserDecryptionOptions = null,
+                                keyConnectorUserDecryptionOptions = null,
+                                masterPasswordUnlock = masterPasswordUnlock,
+                            ),
+                        ),
+                    ),
+                ),
             )
+            val kdf = Kdf.Pbkdf2(iterations = 500000u)
+            coEvery {
+                authSdkSource.hashPassword(
+                    salt = SALT,
+                    password = masterPassword,
+                    kdf = kdf,
+                    purpose = HashPurpose.SERVER_AUTHORIZATION,
+                )
+            } returns hashedMasterPassword.asSuccess()
+            coEvery {
+                accountsService.deleteAccount(
+                    masterPasswordHash = hashedMasterPassword,
+                    oneTimePassword = null,
+                )
+            } returns DeleteAccountResponseJson.Success.asSuccess()
+
+            val result = repository.deleteAccountWithMasterPassword(masterPassword = masterPassword)
+
+            assertEquals(DeleteAccountResult.Success, result)
+            verify(exactly = 1) {
+                userStateManager.hasPendingAccountDeletion = true
+            }
+            coVerify(exactly = 1) {
+                authSdkSource.hashPassword(
+                    salt = SALT,
+                    password = masterPassword,
+                    kdf = kdf,
+                    purpose = HashPurpose.SERVER_AUTHORIZATION,
+                )
+                accountsService.deleteAccount(
+                    masterPasswordHash = hashedMasterPassword,
+                    oneTimePassword = null,
+                )
+            }
         }
-    }
 
     @Test
     fun `deleteAccountWithOneTimePassword succeeds`() = runTest {
@@ -1848,10 +1908,6 @@ class AuthRepositoryTest {
                 userId = USER_ID_1,
                 accountCryptographicState = ACCOUNT_CRYPTOGRAPHIC_STATE_V2,
             )
-            fakeAuthDiskSource.assertMasterPasswordHash(
-                userId = USER_ID_1,
-                passwordHash = PASSWORD_HASH,
-            )
             coVerify {
                 identityService.getToken(
                     email = EMAIL,
@@ -2167,10 +2223,6 @@ class AuthRepositoryTest {
                 userId = USER_ID_1,
                 accountCryptographicState = accountCryptographicState,
             )
-            fakeAuthDiskSource.assertMasterPasswordHash(
-                userId = USER_ID_1,
-                passwordHash = PASSWORD_HASH,
-            )
             coVerify {
                 identityService.getToken(
                     email = EMAIL,
@@ -2342,10 +2394,6 @@ class AuthRepositoryTest {
             )
             assertEquals(LoginResult.Success, result)
             coVerify { identityService.preLogin(email = EMAIL) }
-            fakeAuthDiskSource.assertMasterPasswordHash(
-                userId = USER_ID_1,
-                passwordHash = PASSWORD_HASH,
-            )
             coVerify {
                 identityService.getToken(
                     email = EMAIL,
@@ -7097,10 +7145,6 @@ class AuthRepositoryTest {
                 userId = USER_ID_1,
                 accountCryptographicState = ACCOUNT_CRYPTOGRAPHIC_STATE_V2,
             )
-            fakeAuthDiskSource.assertMasterPasswordHash(
-                userId = USER_ID_1,
-                passwordHash = PASSWORD_HASH,
-            )
             // This should only be set after they complete a registration and not based on login.
             assertNull(fakeAuthDiskSource.getOnboardingStatus(USER_ID_1))
             verify(exactly = 1) {
@@ -7155,10 +7199,6 @@ class AuthRepositoryTest {
                 userId = USER_ID_1,
                 accountCryptographicState = ACCOUNT_CRYPTOGRAPHIC_STATE_V2,
             )
-            fakeAuthDiskSource.assertMasterPasswordHash(
-                userId = USER_ID_1,
-                passwordHash = PASSWORD_HASH,
-            )
             verify(exactly = 1) {
                 userStateManager.hasPendingAccountAddition = false
                 settingsRepository.setDefaultsIfNecessary(userId = USER_ID_1)

app/src/test/kotlin/com/x8bit/bitwarden/data/vault/manager/VaultLockManagerTest.kt

@@ -82,9 +82,9 @@ class VaultLockManagerTest {
     private val authSdkSource: AuthSdkSource = mockk {
         coEvery {
             hashPassword(
-                salt = MOCK_PROFILE.email,
+                salt = MOCK_MASTER_PASSWORD_UNLOCK_DATA.salt,
                 password = "mockValue",
-                kdf = MOCK_PROFILE.toSdkParams(),
+                kdf = MOCK_MASTER_PASSWORD_UNLOCK_DATA.kdf,
                 purpose = HashPurpose.LOCAL_AUTHORIZATION,
             )
         } returns "hashedPassword".asSuccess()
@@ -1848,6 +1848,12 @@ class VaultLockManagerTest {
                 )
                 trustedDeviceManager.trustThisDeviceIfNecessary(userId = USER_ID)
                 kdfManager.updateKdfToMinimumsIfNeeded(password = masterPassword)
+                authSdkSource.hashPassword(
+                    salt = MOCK_MASTER_PASSWORD_UNLOCK_DATA.salt,
+                    password = masterPassword,
+                    kdf = MOCK_MASTER_PASSWORD_UNLOCK_DATA.kdf,
+                    purpose = HashPurpose.LOCAL_AUTHORIZATION,
+                )
             }
         }
 
@@ -2088,12 +2094,21 @@ class VaultLockManagerTest {
 
             assertEquals(VaultUnlockResult.Success, result)
             fakeAuthDiskSource.assertUserState(userState = MOCK_USER_STATE)
+            fakeAuthDiskSource.assertMasterPasswordHash(userId = USER_ID, passwordHash = null)
             verify(exactly = 0) {
                 passwordPolicyManager.validatePasswordAgainstPolicies(
                     password = any(),
                     isCreation = any(),
                 )
             }
+            coVerify(exactly = 0) {
+                authSdkSource.hashPassword(
+                    salt = any(),
+                    password = any(),
+                    kdf = any(),
+                    purpose = any(),
+                )
+            }
         }
 
     @Suppress("MaxLineLength")