diff --git a/app/src/main/java/com/x8bit/bitwarden/data/auth/repository/AuthRepositoryImpl.kt b/app/src/main/java/com/x8bit/bitwarden/data/auth/repository/AuthRepositoryImpl.kt index 3cddcb820..cf3b119ae 100644 --- a/app/src/main/java/com/x8bit/bitwarden/data/auth/repository/AuthRepositoryImpl.kt +++ b/app/src/main/java/com/x8bit/bitwarden/data/auth/repository/AuthRepositoryImpl.kt @@ -1,6 +1,7 @@ package com.x8bit.bitwarden.data.auth.repository import android.os.SystemClock +import com.bitwarden.core.AuthRequestMethod import com.bitwarden.core.AuthRequestResponse import com.bitwarden.core.InitUserCryptoMethod import com.bitwarden.crypto.HashPurpose @@ -502,7 +503,7 @@ class AuthRepositoryImpl( } // Attempt to unlock the vault with auth request if possible. - deviceData?.let { + deviceData?.let { model -> vaultRepository.clearUnlockedData() vaultRepository.unlockVault( userId = userStateJson.activeUserId, @@ -510,17 +511,27 @@ class AuthRepositoryImpl( kdf = userStateJson.activeAccount.profile.toSdkParams(), privateKey = loginResponse.privateKey, initUserCryptoMethod = InitUserCryptoMethod.AuthRequest( - requestPrivateKey = it.privateKey, - protectedUserKey = it.asymmetricalKey, + requestPrivateKey = model.privateKey, + method = model + .masterPasswordHash + ?.let { + AuthRequestMethod.MasterKey( + protectedMasterKey = model.asymmetricalKey, + authRequestKey = loginResponse.key, + ) + } + ?: AuthRequestMethod.UserKey( + protectedUserKey = model.asymmetricalKey, + ), ), // We can separately unlock the vault for organization data after // receiving the sync response if this data is currently absent. organizationKeys = null, ) - authDiskSource.storeMasterPasswordHash( - userId = userStateJson.activeUserId, - passwordHash = it.masterPasswordHash, - ) + // We are purposely not storing the master password hash here since it + // is not formatted in in a manner that we can use. We will store it + // properly the next time the user enters their master password and + // it is validated. } authDiskSource.userState = userStateJson @@ -1064,14 +1075,13 @@ class AuthRepositoryImpl( userId = userId, ) .flatMap { - authRequestsService - .updateAuthRequest( - requestId = requestId, - key = it, - deviceId = authDiskSource.uniqueAppId, - masterPasswordHash = masterPasswordHash, - isApproved = isApproved, - ) + authRequestsService.updateAuthRequest( + requestId = requestId, + key = it, + deviceId = authDiskSource.uniqueAppId, + masterPasswordHash = null, + isApproved = isApproved, + ) } .map { request -> AuthRequestResult.Success( @@ -1138,22 +1148,46 @@ class AuthRepositoryImpl( @Suppress("ReturnCount") override suspend fun validatePassword(password: String): ValidatePasswordResult { val userId = activeUserId ?: return ValidatePasswordResult.Error - val masterPasswordHash = authDiskSource.getMasterPasswordHash(userId = userId) - ?: return ValidatePasswordResult.Error - return vaultSdkSource - .validatePassword( - userId = userId, - password = password, - passwordHash = masterPasswordHash, - ) - .fold( - onSuccess = { - ValidatePasswordResult.Success(isValid = it) - }, - onFailure = { - ValidatePasswordResult.Error - }, - ) + return authDiskSource + .getMasterPasswordHash(userId = userId) + ?.let { masterPasswordHash -> + vaultSdkSource + .validatePassword( + userId = userId, + password = password, + passwordHash = masterPasswordHash, + ) + .fold( + onSuccess = { ValidatePasswordResult.Success(isValid = it) }, + onFailure = { ValidatePasswordResult.Error }, + ) + } + ?: run { + val encryptedKey = authDiskSource + .getUserKey(userId) + ?: return ValidatePasswordResult.Error + vaultSdkSource + .validatePasswordUserKey( + userId = userId, + password = password, + encryptedUserKey = encryptedKey, + ) + .onSuccess { masterPasswordHash -> + authDiskSource.storeMasterPasswordHash( + userId = userId, + passwordHash = masterPasswordHash, + ) + } + .fold( + onSuccess = { ValidatePasswordResult.Success(isValid = true) }, + onFailure = { + // We currently assume that all errors are caused by the user entering + // an invalid password, this is not necessarily the case but we have no + // way to differentiate between the different errors. + ValidatePasswordResult.Success(isValid = false) + }, + ) + } } @Suppress("CyclomaticComplexMethod", "ReturnCount") diff --git a/app/src/main/java/com/x8bit/bitwarden/data/vault/datasource/sdk/VaultSdkSource.kt b/app/src/main/java/com/x8bit/bitwarden/data/vault/datasource/sdk/VaultSdkSource.kt index ff079aacc..68d5c4cef 100644 --- a/app/src/main/java/com/x8bit/bitwarden/data/vault/datasource/sdk/VaultSdkSource.kt +++ b/app/src/main/java/com/x8bit/bitwarden/data/vault/datasource/sdk/VaultSdkSource.kt @@ -327,6 +327,16 @@ interface VaultSdkSource { passwordHash: String, ): Result<Boolean> + /** + * Validates that the given password with the encrypted user key and returns the master + * password hash on validation or an error on failure. + */ + suspend fun validatePasswordUserKey( + userId: String, + password: String, + encryptedUserKey: String, + ): Result<String> + /** * Get the keys needed to update the user's password. */ diff --git a/app/src/main/java/com/x8bit/bitwarden/data/vault/datasource/sdk/VaultSdkSourceImpl.kt b/app/src/main/java/com/x8bit/bitwarden/data/vault/datasource/sdk/VaultSdkSourceImpl.kt index 65cc57c69..1d6f7b7de 100644 --- a/app/src/main/java/com/x8bit/bitwarden/data/vault/datasource/sdk/VaultSdkSourceImpl.kt +++ b/app/src/main/java/com/x8bit/bitwarden/data/vault/datasource/sdk/VaultSdkSourceImpl.kt @@ -350,6 +350,19 @@ class VaultSdkSourceImpl( ) } + override suspend fun validatePasswordUserKey( + userId: String, + password: String, + encryptedUserKey: String, + ): Result<String> = runCatching { + getClient(userId = userId) + .auth() + .validatePasswordUserKey( + password = password, + encryptedUserKey = encryptedUserKey, + ) + } + override suspend fun updatePassword( userId: String, newPassword: String, diff --git a/app/src/test/java/com/x8bit/bitwarden/data/auth/repository/AuthRepositoryTest.kt b/app/src/test/java/com/x8bit/bitwarden/data/auth/repository/AuthRepositoryTest.kt index 19985a073..b385ab425 100644 --- a/app/src/test/java/com/x8bit/bitwarden/data/auth/repository/AuthRepositoryTest.kt +++ b/app/src/test/java/com/x8bit/bitwarden/data/auth/repository/AuthRepositoryTest.kt @@ -1,6 +1,7 @@ package com.x8bit.bitwarden.data.auth.repository import app.cash.turbine.test +import com.bitwarden.core.AuthRequestMethod import com.bitwarden.core.AuthRequestResponse import com.bitwarden.core.InitUserCryptoMethod import com.bitwarden.core.RegisterKeyResponse @@ -1326,7 +1327,10 @@ class AuthRepositoryTest { organizationKeys = null, initUserCryptoMethod = InitUserCryptoMethod.AuthRequest( requestPrivateKey = DEVICE_REQUEST_PRIVATE_KEY, - protectedUserKey = DEVICE_ASYMMETRICAL_KEY, + method = AuthRequestMethod.MasterKey( + authRequestKey = successResponse.key, + protectedMasterKey = DEVICE_ASYMMETRICAL_KEY, + ), ), ) } returns VaultUnlockResult.Success @@ -1369,7 +1373,10 @@ class AuthRepositoryTest { organizationKeys = null, initUserCryptoMethod = InitUserCryptoMethod.AuthRequest( requestPrivateKey = DEVICE_REQUEST_PRIVATE_KEY, - protectedUserKey = DEVICE_ASYMMETRICAL_KEY, + method = AuthRequestMethod.MasterKey( + authRequestKey = successResponse.key, + protectedMasterKey = DEVICE_ASYMMETRICAL_KEY, + ), ), ) } @@ -3680,7 +3687,7 @@ class AuthRepositoryTest { coEvery { authRequestsService.updateAuthRequest( requestId = requestId, - masterPasswordHash = passwordHash, + masterPasswordHash = null, key = encodedKey, deviceId = UNIQUE_APP_ID, isApproved = false, @@ -3689,8 +3696,8 @@ class AuthRepositoryTest { fakeAuthDiskSource.userState = SINGLE_USER_STATE_1 val result = repository.updateAuthRequest( - requestId = "requestId", - masterPasswordHash = "masterPasswordHash", + requestId = requestId, + masterPasswordHash = passwordHash, publicKey = PUBLIC_KEY, isApproved = false, ) @@ -3746,7 +3753,7 @@ class AuthRepositoryTest { coEvery { authRequestsService.updateAuthRequest( requestId = requestId, - masterPasswordHash = passwordHash, + masterPasswordHash = null, key = encodedKey, deviceId = UNIQUE_APP_ID, isApproved = false, @@ -3768,7 +3775,7 @@ class AuthRepositoryTest { ) authRequestsService.updateAuthRequest( requestId = requestId, - masterPasswordHash = passwordHash, + masterPasswordHash = null, key = encodedKey, deviceId = UNIQUE_APP_ID, isApproved = false, @@ -3912,7 +3919,7 @@ class AuthRepositoryTest { @Suppress("MaxLineLength") @Test - fun `validatePassword with no stored password hash returns ValidatePasswordResult Error`() = + fun `validatePassword with no stored password hash and no stored user key returns ValidatePasswordResult Error`() = runTest { val userId = USER_ID_1 val password = "password" @@ -3989,6 +3996,55 @@ class AuthRepositoryTest { ) } + @Suppress("MaxLineLength") + @Test + fun `validatePassword with no stored password hash and a stored user key with sdk failure returns ValidatePasswordResult Success invalid`() = + runTest { + val userId = USER_ID_1 + val password = "password" + val userKey = "userKey" + fakeAuthDiskSource.userState = SINGLE_USER_STATE_1 + fakeAuthDiskSource.storeUserKey(userId = userId, userKey = userKey) + coEvery { + vaultSdkSource.validatePasswordUserKey( + userId = userId, + password = password, + encryptedUserKey = userKey, + ) + } returns Throwable("Fail").asFailure() + + val result = repository.validatePassword(password = password) + + assertEquals(ValidatePasswordResult.Success(isValid = false), result) + } + + @Suppress("MaxLineLength") + @Test + fun `validatePassword with no stored password hash and a stored user key with sdk success returns ValidatePasswordResult Success valid`() = + runTest { + val userId = USER_ID_1 + val password = "password" + val userKey = "userKey" + val passwordHash = "passwordHash" + fakeAuthDiskSource.userState = SINGLE_USER_STATE_1 + fakeAuthDiskSource.storeUserKey(userId = userId, userKey = userKey) + coEvery { + vaultSdkSource.validatePasswordUserKey( + userId = userId, + password = password, + encryptedUserKey = userKey, + ) + } returns passwordHash.asSuccess() + + val result = repository.validatePassword(password = password) + + assertEquals(ValidatePasswordResult.Success(isValid = true), result) + fakeAuthDiskSource.assertMasterPasswordHash( + userId = userId, + passwordHash = passwordHash, + ) + } + @Suppress("MaxLineLength") @Test fun `logOutFlow emission for action account should call logout on the UserLogoutManager and clear the user's in memory vault data`() { diff --git a/app/src/test/java/com/x8bit/bitwarden/data/vault/datasource/sdk/VaultSdkSourceTest.kt b/app/src/test/java/com/x8bit/bitwarden/data/vault/datasource/sdk/VaultSdkSourceTest.kt index faba1469c..16f56572b 100644 --- a/app/src/test/java/com/x8bit/bitwarden/data/vault/datasource/sdk/VaultSdkSourceTest.kt +++ b/app/src/test/java/com/x8bit/bitwarden/data/vault/datasource/sdk/VaultSdkSourceTest.kt @@ -801,6 +801,27 @@ class VaultSdkSourceTest { ) } + @Test + fun `validatePasswordUserKey should call SDK and a Result with correct data`() = runTest { + val userId = "userId" + val password = "password" + val encryptedUserKey = "encryptedUserKey" + val masterPasswordHash = "masterPasswordHash" + coEvery { + clientAuth.validatePasswordUserKey( + password = password, + encryptedUserKey = encryptedUserKey, + ) + } returns masterPasswordHash + + val result = vaultSdkSource.validatePasswordUserKey( + userId = userId, + password = password, + encryptedUserKey = encryptedUserKey, + ) + assertEquals(masterPasswordHash.asSuccess(), result) + } + @Test fun `updatePassword should call SDK and a Result with correct data`() = runTest { val userId = "userId" diff --git a/gradle/libs.versions.toml b/gradle/libs.versions.toml index 6dd927c86..95e03a5a3 100644 --- a/gradle/libs.versions.toml +++ b/gradle/libs.versions.toml @@ -27,7 +27,7 @@ androidxSplash = "1.1.0-alpha02" androidXAppCompat = "1.6.1" androdixAutofill = "1.1.0" androidxWork = "2.9.0" -bitwardenSdk = "0.4.0-20240131.132449-88" +bitwardenSdk = "0.4.0-20240205.155354-106" crashlytics = "2.9.9" detekt = "1.23.1" firebaseBom = "32.7.0"