From 089136552b9d816c93d57408af63f7df0ae3b269 Mon Sep 17 00:00:00 2001 From: David Perez Date: Fri, 15 Nov 2024 13:56:55 -0600 Subject: [PATCH] PM-12259: Use validatePin SDK to validate the users pin (#4311) --- .../auth/repository/AuthRepositoryImpl.kt | 34 +------- .../vault/datasource/sdk/VaultSdkSource.kt | 9 ++ .../datasource/sdk/VaultSdkSourceImpl.kt | 11 +++ .../auth/repository/AuthRepositoryTest.kt | 84 ++++++------------- .../datasource/sdk/VaultSdkSourceTest.kt | 24 ++++++ 5 files changed, 74 insertions(+), 88 deletions(-) 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 5501bb0de..a769f88aa 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 @@ -2,7 +2,6 @@ package com.x8bit.bitwarden.data.auth.repository import com.bitwarden.core.AuthRequestMethod import com.bitwarden.core.InitUserCryptoMethod -import com.bitwarden.core.InitUserCryptoRequest import com.bitwarden.crypto.HashPurpose import com.bitwarden.crypto.Kdf import com.x8bit.bitwarden.data.auth.datasource.disk.AuthDiskSource @@ -114,7 +113,6 @@ import com.x8bit.bitwarden.data.vault.datasource.network.model.OrganizationType import com.x8bit.bitwarden.data.vault.datasource.network.model.PolicyTypeJson import com.x8bit.bitwarden.data.vault.datasource.network.model.SyncResponseJson import com.x8bit.bitwarden.data.vault.datasource.sdk.VaultSdkSource -import com.x8bit.bitwarden.data.vault.datasource.sdk.model.InitializeCryptoResult import com.x8bit.bitwarden.data.vault.repository.VaultRepository import com.x8bit.bitwarden.data.vault.repository.model.VaultUnlockData import com.x8bit.bitwarden.data.vault.repository.model.VaultUnlockError @@ -1249,41 +1247,17 @@ class AuthRepositoryImpl( ?.activeAccount ?.profile ?: return ValidatePinResult.Error - val privateKey = authDiskSource - .getPrivateKey(userId = activeAccount.userId) - ?: return ValidatePinResult.Error val pinProtectedUserKey = authDiskSource .getPinProtectedUserKey(userId = activeAccount.userId) ?: return ValidatePinResult.Error - - // HACK: As the SDK doesn't provide a way to directly validate the pin yet, we instead - // try to initialize the user crypto, and if it succeeds then the PIN is correct, otherwise - // the PIN is incorrect. return vaultSdkSource - .initializeCrypto( + .validatePin( userId = activeAccount.userId, - request = InitUserCryptoRequest( - kdfParams = activeAccount.toSdkParams(), - email = activeAccount.email, - privateKey = privateKey, - method = InitUserCryptoMethod.Pin( - pin = pin, - pinProtectedUserKey = pinProtectedUserKey, - ), - ), + pin = pin, + pinProtectedUserKey = pinProtectedUserKey, ) .fold( - onSuccess = { - when (it) { - InitializeCryptoResult.Success -> { - ValidatePinResult.Success(isValid = true) - } - - is InitializeCryptoResult.AuthenticationError -> { - ValidatePinResult.Success(isValid = false) - } - } - }, + onSuccess = { ValidatePinResult.Success(isValid = it) }, onFailure = { ValidatePinResult.Error }, ) } 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 4e26eb042..120f50176 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 @@ -94,6 +94,15 @@ interface VaultSdkSource { encryptedPin: String, ): Result + /** + * Validate the user pin using the [pinProtectedUserKey]. + */ + suspend fun validatePin( + userId: String, + pin: String, + pinProtectedUserKey: String, + ): Result + /** * Gets the key for an auth request that is required to approve or decline it. */ 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 2c33c7450..95de2ec67 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 @@ -109,6 +109,17 @@ class VaultSdkSourceImpl( .derivePinUserKey(encryptedPin = encryptedPin) } + override suspend fun validatePin( + userId: String, + pin: String, + pinProtectedUserKey: String, + ): Result = + runCatchingWithLogs { + getClient(userId = userId) + .auth() + .validatePin(pin = pin, pinProtectedUserKey = pinProtectedUserKey) + } + override suspend fun getAuthRequestKey( publicKey: String, userId: 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 72b32e01f..5e0882b33 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 @@ -123,7 +123,6 @@ import com.x8bit.bitwarden.data.vault.datasource.network.model.SyncResponseJson import com.x8bit.bitwarden.data.vault.datasource.network.model.createMockOrganization import com.x8bit.bitwarden.data.vault.datasource.network.model.createMockPolicy import com.x8bit.bitwarden.data.vault.datasource.sdk.VaultSdkSource -import com.x8bit.bitwarden.data.vault.datasource.sdk.model.InitializeCryptoResult import com.x8bit.bitwarden.data.vault.repository.VaultRepository import com.x8bit.bitwarden.data.vault.repository.model.VaultUnlockData import com.x8bit.bitwarden.data.vault.repository.model.VaultUnlockResult @@ -5828,33 +5827,11 @@ class AuthRepositoryTest { ) } - @Test - fun `validatePin returns ValidatePinResult Error when no private key found`() = runTest { - val pin = "PIN" - fakeAuthDiskSource.userState = SINGLE_USER_STATE_1 - fakeAuthDiskSource.storePrivateKey( - userId = SINGLE_USER_STATE_1.activeUserId, - privateKey = null, - ) - - val result = repository.validatePin(pin = pin) - - assertEquals( - ValidatePinResult.Error, - result, - ) - } - @Test fun `validatePin returns ValidatePinResult Error when no pin protected user key found`() = runTest { val pin = "PIN" - val privateKey = "privateKey" fakeAuthDiskSource.userState = SINGLE_USER_STATE_1 - fakeAuthDiskSource.storePrivateKey( - userId = SINGLE_USER_STATE_1.activeUserId, - privateKey = privateKey, - ) fakeAuthDiskSource.storePinProtectedUserKey( userId = SINGLE_USER_STATE_1.activeUserId, pinProtectedUserKey = null, @@ -5869,23 +5846,19 @@ class AuthRepositoryTest { } @Test - fun `validatePin returns ValidatePinResult Error when initialize crypto fails`() = runTest { + fun `validatePin returns ValidatePinResult Error when SDK validatePin fails`() = runTest { val pin = "PIN" - val privateKey = "privateKey" val pinProtectedUserKey = "pinProtectedUserKey" fakeAuthDiskSource.userState = SINGLE_USER_STATE_1 - fakeAuthDiskSource.storePrivateKey( - userId = SINGLE_USER_STATE_1.activeUserId, - privateKey = privateKey, - ) fakeAuthDiskSource.storePinProtectedUserKey( userId = SINGLE_USER_STATE_1.activeUserId, pinProtectedUserKey = pinProtectedUserKey, ) coEvery { - vaultSdkSource.initializeCrypto( + vaultSdkSource.validatePin( userId = SINGLE_USER_STATE_1.activeUserId, - request = any(), + pin = pin, + pinProtectedUserKey = pinProtectedUserKey, ) } returns Throwable().asFailure() @@ -5895,36 +5868,33 @@ class AuthRepositoryTest { ValidatePinResult.Error, result, ) - coVerify { - vaultSdkSource.initializeCrypto( + coVerify(exactly = 1) { + vaultSdkSource.validatePin( userId = SINGLE_USER_STATE_1.activeUserId, - request = any(), + pin = pin, + pinProtectedUserKey = pinProtectedUserKey, ) } } @Suppress("MaxLineLength") @Test - fun `validatePin returns ValidatePinResult Success with valid false when initialize cryto returns AuthenticationError`() = + fun `validatePin returns ValidatePinResult Success with valid false when SDK validatePin returns false`() = runTest { val pin = "PIN" - val privateKey = "privateKey" val pinProtectedUserKey = "pinProtectedUserKey" fakeAuthDiskSource.userState = SINGLE_USER_STATE_1 - fakeAuthDiskSource.storePrivateKey( - userId = SINGLE_USER_STATE_1.activeUserId, - privateKey = privateKey, - ) fakeAuthDiskSource.storePinProtectedUserKey( userId = SINGLE_USER_STATE_1.activeUserId, pinProtectedUserKey = pinProtectedUserKey, ) coEvery { - vaultSdkSource.initializeCrypto( + vaultSdkSource.validatePin( userId = SINGLE_USER_STATE_1.activeUserId, - request = any(), + pin = pin, + pinProtectedUserKey = pinProtectedUserKey, ) - } returns InitializeCryptoResult.AuthenticationError().asSuccess() + } returns false.asSuccess() val result = repository.validatePin(pin = pin) @@ -5932,36 +5902,33 @@ class AuthRepositoryTest { ValidatePinResult.Success(isValid = false), result, ) - coVerify { - vaultSdkSource.initializeCrypto( + coVerify(exactly = 1) { + vaultSdkSource.validatePin( userId = SINGLE_USER_STATE_1.activeUserId, - request = any(), + pin = pin, + pinProtectedUserKey = pinProtectedUserKey, ) } } @Suppress("MaxLineLength") @Test - fun `validatePin returns ValidatePinResult Success with valid true when initialize cryto returns Success`() = + fun `validatePin returns ValidatePinResult Success with valid true when SDK validatePin returns true`() = runTest { val pin = "PIN" - val privateKey = "privateKey" val pinProtectedUserKey = "pinProtectedUserKey" fakeAuthDiskSource.userState = SINGLE_USER_STATE_1 - fakeAuthDiskSource.storePrivateKey( - userId = SINGLE_USER_STATE_1.activeUserId, - privateKey = privateKey, - ) fakeAuthDiskSource.storePinProtectedUserKey( userId = SINGLE_USER_STATE_1.activeUserId, pinProtectedUserKey = pinProtectedUserKey, ) coEvery { - vaultSdkSource.initializeCrypto( + vaultSdkSource.validatePin( userId = SINGLE_USER_STATE_1.activeUserId, - request = any(), + pin = pin, + pinProtectedUserKey = pinProtectedUserKey, ) - } returns InitializeCryptoResult.Success.asSuccess() + } returns true.asSuccess() val result = repository.validatePin(pin = pin) @@ -5969,10 +5936,11 @@ class AuthRepositoryTest { ValidatePinResult.Success(isValid = true), result, ) - coVerify { - vaultSdkSource.initializeCrypto( + coVerify(exactly = 1) { + vaultSdkSource.validatePin( userId = SINGLE_USER_STATE_1.activeUserId, - request = any(), + pin = pin, + pinProtectedUserKey = pinProtectedUserKey, ) } } 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 d4626c2c2..c086ee73e 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 @@ -234,6 +234,30 @@ class VaultSdkSourceTest { coVerify { sdkClientManager.getOrCreateClient(userId = userId) } } + @Test + fun `validatePin should call SDK and return a Result with the correct data`() = + runBlocking { + val userId = "userId" + val pin = "pin" + val pinProtectedUserKey = "pinProtectedUserKey" + val expectedResult = true + coEvery { + clientAuth.validatePin(pin = pin, pinProtectedUserKey = pinProtectedUserKey) + } returns expectedResult + + val result = vaultSdkSource.validatePin( + userId = userId, + pin = pin, + pinProtectedUserKey = pinProtectedUserKey, + ) + + assertEquals(expectedResult.asSuccess(), result) + coVerify(exactly = 1) { + clientAuth.validatePin(pin = pin, pinProtectedUserKey = pinProtectedUserKey) + sdkClientManager.getOrCreateClient(userId = userId) + } + } + @Test fun `getAuthRequestKey should call SDK and return a Result with correct data`() = runBlocking {