PM-12259: Use validatePin SDK to validate the users pin (#4311)

bitwarden · Nov 15, 2024 · 0891365 · 0891365
1 parent 1b0bc13
commit 0891365
Show file tree

Hide file tree

Showing 5 changed files with 74 additions and 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
@@ -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
@@ -94,6 +94,15 @@ interface VaultSdkSource {
         encryptedPin: String,
     ): Result<String>
 
+    /**
+     * Validate the user pin using the [pinProtectedUserKey].
+     */
+    suspend fun validatePin(
+        userId: String,
+        pin: String,
+        pinProtectedUserKey: String,
+    ): Result<Boolean>
+
     /**
      * 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
@@ -109,6 +109,17 @@ class VaultSdkSourceImpl(
                 .derivePinUserKey(encryptedPin = encryptedPin)
         }
 
+    override suspend fun validatePin(
+        userId: String,
+        pin: String,
+        pinProtectedUserKey: String,
+    ): Result<Boolean> =
+        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
@@ -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,84 +5868,79 @@ 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)
 
             assertEquals(
                 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)
 
             assertEquals(
                 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
@@ -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 {