From ff72efe0ed2c2ddeb573895de651dfb7f679ebbb Mon Sep 17 00:00:00 2001 From: Patrick Honkonen <1883101+SaintPatrck@users.noreply.github.com> Date: Wed, 16 Apr 2025 11:35:25 -0400 Subject: [PATCH] [PM-20127] Only prompt passkey user verification once (#5026) --- .../java/com/x8bit/bitwarden/MainViewModel.kt | 17 +- .../manager/Fido2CredentialManagerImpl.kt | 175 +++++----- .../model/Fido2CreateCredentialRequest.kt | 3 + .../model/Fido2CredentialAssertionRequest.kt | 11 +- .../autofill/fido2/util/Fido2IntentUtils.kt | 16 + .../com/x8bit/bitwarden/MainViewModelTest.kt | 8 +- .../Fido2CredentialAssertionRequestUtil.kt | 1 + .../fido2/model/Fido2CredentialRequestUtil.kt | 2 + .../fido2/util/Fido2IntentUtilsTest.kt | 83 ++++- .../util/SpecialCircumstanceExtensionsTest.kt | 1 + .../feature/rootnav/RootNavViewModelTest.kt | 1 + .../addedit/VaultAddEditViewModelTest.kt | 3 + .../Fido2CredentialRequestExtensionsTest.kt | 2 + .../VaultItemListingViewModelTest.kt | 310 +++++++++++++++++- .../VaultItemListingDataExtensionsTest.kt | 1 + 15 files changed, 530 insertions(+), 104 deletions(-) diff --git a/app/src/main/java/com/x8bit/bitwarden/MainViewModel.kt b/app/src/main/java/com/x8bit/bitwarden/MainViewModel.kt index d820e0ed92..faf82230e0 100644 --- a/app/src/main/java/com/x8bit/bitwarden/MainViewModel.kt +++ b/app/src/main/java/com/x8bit/bitwarden/MainViewModel.kt @@ -374,10 +374,8 @@ class MainViewModel @Inject constructor( // Set the user's verification status when a new FIDO 2 request is received to force // explicit verification if the user's vault is unlocked when the request is // received. - fido2CreateCredentialRequest.providerRequest - .biometricPromptResult - ?.isSuccessful - ?.let { isVerified -> fido2CredentialManager.isUserVerified = isVerified } + fido2CredentialManager.isUserVerified = + fido2CreateCredentialRequest.isUserPreVerified specialCircumstanceManager.specialCircumstance = SpecialCircumstance.Fido2Save( @@ -393,12 +391,11 @@ class MainViewModel @Inject constructor( } fido2AssertCredentialRequest != null -> { - // If device biometric verification was performed as part of single-tap - // authentication, set the user's verification state to the device result. - // Otherwise, retain the verification state as-is. - fido2AssertCredentialRequest.providerRequest.biometricPromptResult - ?.isSuccessful - ?.let { isVerified -> fido2CredentialManager.isUserVerified = isVerified } + // Set the user's verification status when a new FIDO 2 request is received to force + // explicit verification if the user's vault is unlocked when the request is + // received. + fido2CredentialManager.isUserVerified = + fido2AssertCredentialRequest.isUserPreVerified specialCircumstanceManager.specialCircumstance = SpecialCircumstance.Fido2Assertion( diff --git a/app/src/main/java/com/x8bit/bitwarden/data/autofill/fido2/manager/Fido2CredentialManagerImpl.kt b/app/src/main/java/com/x8bit/bitwarden/data/autofill/fido2/manager/Fido2CredentialManagerImpl.kt index 85a16f9d4c..aa896785fa 100644 --- a/app/src/main/java/com/x8bit/bitwarden/data/autofill/fido2/manager/Fido2CredentialManagerImpl.kt +++ b/app/src/main/java/com/x8bit/bitwarden/data/autofill/fido2/manager/Fido2CredentialManagerImpl.kt @@ -65,92 +65,6 @@ class Fido2CredentialManagerImpl( } } - private suspend fun registerFido2CredentialForUnprivilegedApp( - userId: String, - callingAppInfo: CallingAppInfo, - createPublicKeyCredentialRequest: CreatePublicKeyCredentialRequest, - selectedCipherView: CipherView, - ): Fido2RegisterCredentialResult { - val clientData = ClientData.DefaultWithExtraData(callingAppInfo.packageName) - - val host = getOriginUrlFromAttestationOptionsOrNull( - requestJson = createPublicKeyCredentialRequest.requestJson, - ) - ?: return Fido2RegisterCredentialResult.Error.MissingHostUrl - - val signatureFingerprint = callingAppInfo - .getSignatureFingerprintAsHexString() - ?: return Fido2RegisterCredentialResult.Error.InvalidAppSignature - - val sdkOrigin = Origin.Android( - UnverifiedAssetLink( - packageName = callingAppInfo.packageName, - sha256CertFingerprint = signatureFingerprint, - host = host, - assetLinkUrl = host, - ), - ) - - return registerFido2CredentialInternal( - userId = userId, - sdkOrigin = sdkOrigin, - createPublicKeyCredentialRequest = createPublicKeyCredentialRequest, - selectedCipherView = selectedCipherView, - clientData = clientData, - ) - } - - private suspend fun registerFido2CredentialForPrivilegedApp( - userId: String, - callingAppInfo: CallingAppInfo, - createPublicKeyCredentialRequest: CreatePublicKeyCredentialRequest, - selectedCipherView: CipherView, - ): Fido2RegisterCredentialResult { - val clientData = callingAppInfo - .getAppSigningSignatureFingerprint() - ?.let { ClientData.DefaultWithCustomHash(hash = it) } - ?: return Fido2RegisterCredentialResult.Error.InvalidAppSignature - - val sdkOrigin = createPublicKeyCredentialRequest.origin - ?.let { Origin.Web(it) } - ?: return Fido2RegisterCredentialResult.Error.MissingHostUrl - - return registerFido2CredentialInternal( - userId = userId, - sdkOrigin = sdkOrigin, - createPublicKeyCredentialRequest = createPublicKeyCredentialRequest, - selectedCipherView = selectedCipherView, - clientData = clientData, - ) - } - - private suspend fun registerFido2CredentialInternal( - userId: String, - sdkOrigin: Origin, - createPublicKeyCredentialRequest: CreatePublicKeyCredentialRequest, - selectedCipherView: CipherView, - clientData: ClientData, - ): Fido2RegisterCredentialResult = vaultSdkSource - .registerFido2Credential( - request = RegisterFido2CredentialRequest( - userId = userId, - origin = sdkOrigin, - requestJson = """{"publicKey": ${createPublicKeyCredentialRequest.requestJson}}""", - clientData = clientData, - selectedCipherView = selectedCipherView, - // User verification is handled prior to engaging the SDK. We always respond - // `true` so that the SDK does not fail if the relying party requests UV. - isUserVerificationSupported = true, - ), - fido2CredentialStore = this, - ) - .map { it.toAndroidAttestationResponse() } - .mapCatching { json.encodeToString(it) } - .fold( - onSuccess = { Fido2RegisterCredentialResult.Success(it) }, - onFailure = { Fido2RegisterCredentialResult.Error.InternalError }, - ) - override fun getPasskeyAttestationOptionsOrNull( requestJson: String, ): PasskeyAttestationOptions? = @@ -254,6 +168,95 @@ class Fido2CredentialManagerImpl( ?.userVerification ?: fallbackRequirement + private suspend fun registerFido2CredentialForUnprivilegedApp( + userId: String, + callingAppInfo: CallingAppInfo, + createPublicKeyCredentialRequest: CreatePublicKeyCredentialRequest, + selectedCipherView: CipherView, + ): Fido2RegisterCredentialResult { + val clientData = ClientData.DefaultWithExtraData(callingAppInfo.packageName) + + val host = getOriginUrlFromAttestationOptionsOrNull( + requestJson = createPublicKeyCredentialRequest.requestJson, + ) + ?: return Fido2RegisterCredentialResult.Error.MissingHostUrl + + val signatureFingerprint = callingAppInfo + .getSignatureFingerprintAsHexString() + ?: return Fido2RegisterCredentialResult.Error.InvalidAppSignature + + val sdkOrigin = Origin.Android( + UnverifiedAssetLink( + packageName = callingAppInfo.packageName, + sha256CertFingerprint = signatureFingerprint, + host = host, + assetLinkUrl = host, + ), + ) + + return registerFido2CredentialInternal( + userId = userId, + sdkOrigin = sdkOrigin, + createPublicKeyCredentialRequest = createPublicKeyCredentialRequest, + selectedCipherView = selectedCipherView, + clientData = clientData, + ) + } + + private suspend fun registerFido2CredentialForPrivilegedApp( + userId: String, + callingAppInfo: CallingAppInfo, + createPublicKeyCredentialRequest: CreatePublicKeyCredentialRequest, + selectedCipherView: CipherView, + ): Fido2RegisterCredentialResult { + val clientData = callingAppInfo + .getAppSigningSignatureFingerprint() + ?.let { ClientData.DefaultWithCustomHash(hash = it) } + ?: return Fido2RegisterCredentialResult.Error.InvalidAppSignature + + val sdkOrigin = createPublicKeyCredentialRequest.origin + ?.let { Origin.Web(it) } + ?: return Fido2RegisterCredentialResult.Error.MissingHostUrl + + return registerFido2CredentialInternal( + userId = userId, + sdkOrigin = sdkOrigin, + createPublicKeyCredentialRequest = createPublicKeyCredentialRequest, + selectedCipherView = selectedCipherView, + clientData = clientData, + ) + } + + private suspend fun registerFido2CredentialInternal( + userId: String, + sdkOrigin: Origin, + createPublicKeyCredentialRequest: CreatePublicKeyCredentialRequest, + selectedCipherView: CipherView, + clientData: ClientData, + ): Fido2RegisterCredentialResult = vaultSdkSource + .registerFido2Credential( + request = RegisterFido2CredentialRequest( + userId = userId, + origin = sdkOrigin, + requestJson = """{"publicKey": ${createPublicKeyCredentialRequest.requestJson}}""", + clientData = clientData, + selectedCipherView = selectedCipherView, + // User verification is handled prior to engaging the SDK. We always respond + // `true` so that the SDK does not fail if the relying party requests UV. + isUserVerificationSupported = true, + ), + fido2CredentialStore = this, + ) + .map { it.toAndroidAttestationResponse() } + .mapCatching { json.encodeToString(it) } + .fold( + onSuccess = { Fido2RegisterCredentialResult.Success(it) }, + onFailure = { + Timber.e(it, "Failed to register FIDO2 credential.") + Fido2RegisterCredentialResult.Error.InternalError + }, + ) + private fun getOriginUrlFromAssertionOptionsOrNull(requestJson: String) = getPasskeyAssertionOptionsOrNull(requestJson) ?.relyingPartyId diff --git a/app/src/main/java/com/x8bit/bitwarden/data/autofill/fido2/model/Fido2CreateCredentialRequest.kt b/app/src/main/java/com/x8bit/bitwarden/data/autofill/fido2/model/Fido2CreateCredentialRequest.kt index cbf5dff79b..b94fd165ae 100644 --- a/app/src/main/java/com/x8bit/bitwarden/data/autofill/fido2/model/Fido2CreateCredentialRequest.kt +++ b/app/src/main/java/com/x8bit/bitwarden/data/autofill/fido2/model/Fido2CreateCredentialRequest.kt @@ -14,12 +14,15 @@ import kotlinx.parcelize.Parcelize * credential manager framework. * * @property userId The ID of the user creating the passkey. + * @property isUserPreVerified Whether the user has already been verified by the OS biometric + * prompt. * @property requestData Provider request data in the form of a [Bundle]. */ @Parcelize data class Fido2CreateCredentialRequest( val userId: String, val requestData: Bundle, + val isUserPreVerified: Boolean, ) : Parcelable { /** diff --git a/app/src/main/java/com/x8bit/bitwarden/data/autofill/fido2/model/Fido2CredentialAssertionRequest.kt b/app/src/main/java/com/x8bit/bitwarden/data/autofill/fido2/model/Fido2CredentialAssertionRequest.kt index 5d0acc8e5d..b17139731a 100644 --- a/app/src/main/java/com/x8bit/bitwarden/data/autofill/fido2/model/Fido2CredentialAssertionRequest.kt +++ b/app/src/main/java/com/x8bit/bitwarden/data/autofill/fido2/model/Fido2CredentialAssertionRequest.kt @@ -11,16 +11,19 @@ import kotlinx.parcelize.Parcelize /** * Models a FIDO 2 credential authentication request parsed from the launching intent. * - * @param userId ID of the user requesting credential authentication. - * @param cipherId ID of the cipher to be authenticated against. - * @param credentialId ID of the credential to authenticate. - * @param requestData Provider request data in the form of a [Bundle]. + * @property userId ID of the user requesting credential authentication. + * @property cipherId ID of the cipher to be authenticated against. + * @property credentialId ID of the credential to authenticate. + * @property isUserPreVerified Whether the user has already been verified by the OS biometric + * prompt. + * @property requestData Provider request data in the form of a [Bundle]. */ @Parcelize data class Fido2CredentialAssertionRequest( val userId: String, val cipherId: String, val credentialId: String, + val isUserPreVerified: Boolean, private val requestData: Bundle, ) : Parcelable { diff --git a/app/src/main/java/com/x8bit/bitwarden/data/autofill/fido2/util/Fido2IntentUtils.kt b/app/src/main/java/com/x8bit/bitwarden/data/autofill/fido2/util/Fido2IntentUtils.kt index 573d12e4c4..b0dfb4da61 100644 --- a/app/src/main/java/com/x8bit/bitwarden/data/autofill/fido2/util/Fido2IntentUtils.kt +++ b/app/src/main/java/com/x8bit/bitwarden/data/autofill/fido2/util/Fido2IntentUtils.kt @@ -27,8 +27,16 @@ fun Intent.getFido2CreateCredentialRequestOrNull(): Fido2CreateCredentialRequest val userId = getStringExtra(EXTRA_KEY_USER_ID) ?: return null + // Extract the OS biometric prompt result from the request data because it is not included in + // the bundle returned by `ProviderGetCredentialRequest.asBundle()`. + val isUserPreVerified = systemRequest + .biometricPromptResult + ?.isSuccessful + ?: false + return Fido2CreateCredentialRequest( userId = userId, + isUserPreVerified = isUserPreVerified, requestData = ProviderCreateCredentialRequest.asBundle(systemRequest), ) } @@ -53,10 +61,18 @@ fun Intent.getFido2AssertionRequestOrNull(): Fido2CredentialAssertionRequest? { val userId: String = getStringExtra(EXTRA_KEY_USER_ID) ?: return null + // Extract the OS biometric prompt result from the request data because it is not included in + // the bundle returned by `ProviderGetCredentialRequest.asBundle()`. + val isUserPreVerified = systemRequest + .biometricPromptResult + ?.isSuccessful + ?: false + return Fido2CredentialAssertionRequest( userId = userId, cipherId = cipherId, credentialId = credentialId, + isUserPreVerified = isUserPreVerified, requestData = ProviderGetCredentialRequest.asBundle(systemRequest), ) } diff --git a/app/src/test/java/com/x8bit/bitwarden/MainViewModelTest.kt b/app/src/test/java/com/x8bit/bitwarden/MainViewModelTest.kt index 54edabf82d..9728d0bf5a 100644 --- a/app/src/test/java/com/x8bit/bitwarden/MainViewModelTest.kt +++ b/app/src/test/java/com/x8bit/bitwarden/MainViewModelTest.kt @@ -691,6 +691,7 @@ class MainViewModelTest : BaseViewModelTest() { val viewModel = createViewModel() val fido2CreateCredentialRequest = Fido2CreateCredentialRequest( userId = DEFAULT_USER_STATE.activeUserId, + isUserPreVerified = false, requestData = bundleOf(), ) val fido2Intent = createMockIntent( @@ -715,7 +716,10 @@ class MainViewModelTest : BaseViewModelTest() { @Test fun `on ReceiveFirstIntent with fido2 create request data should set the user verification based on request`() { val viewModel = createViewModel() - val createCredentialRequest = createMockFido2CreateCredentialRequest(number = 1) + val createCredentialRequest = createMockFido2CreateCredentialRequest( + number = 1, + isUserPreVerified = true, + ) val fido2Intent = createMockIntent( mockFido2CreateCredentialRequest = createCredentialRequest, ) @@ -739,6 +743,7 @@ class MainViewModelTest : BaseViewModelTest() { val viewModel = createViewModel() val fido2CreateCredentialRequest = Fido2CreateCredentialRequest( userId = "selectedUserId", + isUserPreVerified = false, requestData = bundleOf(), ) val mockIntent = createMockIntent( @@ -769,6 +774,7 @@ class MainViewModelTest : BaseViewModelTest() { val viewModel = createViewModel() val fido2CreateCredentialRequest = Fido2CreateCredentialRequest( userId = DEFAULT_USER_STATE.activeUserId, + isUserPreVerified = false, requestData = bundleOf(), ) val mockIntent = createMockIntent( diff --git a/app/src/test/java/com/x8bit/bitwarden/data/autofill/fido2/model/Fido2CredentialAssertionRequestUtil.kt b/app/src/test/java/com/x8bit/bitwarden/data/autofill/fido2/model/Fido2CredentialAssertionRequestUtil.kt index 8e8e92b397..b7bd887513 100644 --- a/app/src/test/java/com/x8bit/bitwarden/data/autofill/fido2/model/Fido2CredentialAssertionRequestUtil.kt +++ b/app/src/test/java/com/x8bit/bitwarden/data/autofill/fido2/model/Fido2CredentialAssertionRequestUtil.kt @@ -10,5 +10,6 @@ fun createMockFido2CredentialAssertionRequest( userId = userId, cipherId = "mockCipherId-$number", credentialId = "mockCredentialId-$number", + isUserPreVerified = false, requestData = bundleOf(), ) diff --git a/app/src/test/java/com/x8bit/bitwarden/data/autofill/fido2/model/Fido2CredentialRequestUtil.kt b/app/src/test/java/com/x8bit/bitwarden/data/autofill/fido2/model/Fido2CredentialRequestUtil.kt index 4c7144571f..ef470adff0 100644 --- a/app/src/test/java/com/x8bit/bitwarden/data/autofill/fido2/model/Fido2CredentialRequestUtil.kt +++ b/app/src/test/java/com/x8bit/bitwarden/data/autofill/fido2/model/Fido2CredentialRequestUtil.kt @@ -8,8 +8,10 @@ import androidx.core.os.bundleOf */ fun createMockFido2CreateCredentialRequest( number: Int, + isUserPreVerified: Boolean = false, requestData: Bundle = bundleOf(), ): Fido2CreateCredentialRequest = Fido2CreateCredentialRequest( userId = "mockUserId-$number", + isUserPreVerified = isUserPreVerified, requestData = requestData, ) diff --git a/app/src/test/java/com/x8bit/bitwarden/data/autofill/fido2/util/Fido2IntentUtilsTest.kt b/app/src/test/java/com/x8bit/bitwarden/data/autofill/fido2/util/Fido2IntentUtilsTest.kt index 30ee9d8c32..03790a8668 100644 --- a/app/src/test/java/com/x8bit/bitwarden/data/autofill/fido2/util/Fido2IntentUtilsTest.kt +++ b/app/src/test/java/com/x8bit/bitwarden/data/autofill/fido2/util/Fido2IntentUtilsTest.kt @@ -3,6 +3,7 @@ package com.x8bit.bitwarden.data.autofill.fido2.util import android.content.Intent import androidx.core.os.bundleOf import androidx.credentials.provider.BeginGetCredentialRequest +import androidx.credentials.provider.BiometricPromptResult import androidx.credentials.provider.PendingIntentHandler import androidx.credentials.provider.ProviderCreateCredentialRequest import androidx.credentials.provider.ProviderGetCredentialRequest @@ -18,6 +19,7 @@ import io.mockk.unmockkObject import io.mockk.unmockkStatic import org.junit.jupiter.api.AfterEach import org.junit.jupiter.api.Assertions.assertEquals +import org.junit.jupiter.api.Assertions.assertFalse import org.junit.jupiter.api.Assertions.assertNotNull import org.junit.jupiter.api.Assertions.assertNull import org.junit.jupiter.api.BeforeEach @@ -58,7 +60,11 @@ class Fido2IntentUtilsTest { every { ProviderCreateCredentialRequest.asBundle(any()) } returns bundleOf() every { PendingIntentHandler.retrieveProviderCreateCredentialRequest(intent) - } returns mockk() + } returns mockk(relaxed = true) { + every { biometricPromptResult } returns mockk(relaxed = true) { + every { isSuccessful } returns false + } + } val createRequest = intent.getFido2CreateCredentialRequestOrNull() assertEquals( @@ -67,6 +73,39 @@ class Fido2IntentUtilsTest { ) } + @Suppress("MaxLineLength") + @Test + fun `getFido2CreateCredentialRequestOrNull should set user verification based on biometric prompt result`() { + val intent = mockk { + every { getStringExtra(EXTRA_KEY_USER_ID) } returns "mockUserId" + } + val mockBiometricPromptResult = mockk(relaxed = true) { + every { isSuccessful } returns false + } + val mockProviderCreateCredentialRequest = + mockk(relaxed = true) { + every { biometricPromptResult } returns mockBiometricPromptResult + } + every { ProviderCreateCredentialRequest.asBundle(any()) } returns bundleOf() + every { + PendingIntentHandler.retrieveProviderCreateCredentialRequest(intent) + } returns mockProviderCreateCredentialRequest + + // Verify false is returned when biometric prompt is unsuccessful + var createRequest = intent.getFido2CreateCredentialRequestOrNull() + assertFalse(createRequest!!.isUserPreVerified) + + // Verify true is returned when biometric prompt is successful + every { mockBiometricPromptResult.isSuccessful } returns true + createRequest = intent.getFido2CreateCredentialRequestOrNull() + assert(createRequest!!.isUserPreVerified) + + // Verify false is returned when biometric prompt result is null + every { mockProviderCreateCredentialRequest.biometricPromptResult } returns null + createRequest = intent.getFido2CreateCredentialRequestOrNull() + assertFalse(createRequest!!.isUserPreVerified) + } + @Suppress("MaxLineLength") @Test fun `getFido2CreateCredentialRequestOrNull should return null when build version is below 34`() { @@ -108,7 +147,11 @@ class Fido2IntentUtilsTest { every { ProviderGetCredentialRequest.asBundle(any()) } returns bundleOf() every { PendingIntentHandler.retrieveProviderGetCredentialRequest(intent) - } returns mockk() + } returns mockk(relaxed = true) { + every { biometricPromptResult } returns mockk(relaxed = true) { + every { isSuccessful } returns false + } + } val assertionRequest = intent.getFido2AssertionRequestOrNull() @@ -116,6 +159,42 @@ class Fido2IntentUtilsTest { assertEquals("mockUserId", assertionRequest?.userId) assertEquals("mockCipherId", assertionRequest?.cipherId) assertEquals("mockCredentialId", assertionRequest?.credentialId) + assertEquals(false, assertionRequest?.isUserPreVerified) + } + + @Suppress("MaxLineLength") + @Test + fun `getFido2AssertionRequestOrNull should set user verification based on biometric prompt result`() { + val intent = mockk { + every { getStringExtra(EXTRA_KEY_USER_ID) } returns "mockUserId" + every { getStringExtra(EXTRA_KEY_CIPHER_ID) } returns "mockCipherId" + every { getStringExtra(EXTRA_KEY_CREDENTIAL_ID) } returns "mockCredentialId" + } + val mockBiometricPromptResult = mockk(relaxed = true) { + every { isSuccessful } returns false + } + val mockGetCredentialRequest = + mockk(relaxed = true) { + every { biometricPromptResult } returns mockBiometricPromptResult + } + every { ProviderCreateCredentialRequest.asBundle(any()) } returns bundleOf() + every { + PendingIntentHandler.retrieveProviderGetCredentialRequest(intent) + } returns mockGetCredentialRequest + + // Verify false is returned when biometric prompt is unsuccessful + var assertionRequest = intent.getFido2AssertionRequestOrNull() + assertFalse(assertionRequest!!.isUserPreVerified) + + // Verify true is returned when biometric prompt is successful + every { mockBiometricPromptResult.isSuccessful } returns true + assertionRequest = intent.getFido2AssertionRequestOrNull() + assert(assertionRequest!!.isUserPreVerified) + + // Verify false is returned when biometric prompt result is null + every { mockGetCredentialRequest.biometricPromptResult } returns null + assertionRequest = intent.getFido2AssertionRequestOrNull() + assertFalse(assertionRequest!!.isUserPreVerified) } @Test diff --git a/app/src/test/java/com/x8bit/bitwarden/data/platform/manager/util/SpecialCircumstanceExtensionsTest.kt b/app/src/test/java/com/x8bit/bitwarden/data/platform/manager/util/SpecialCircumstanceExtensionsTest.kt index 1a3e990747..ba648a6246 100644 --- a/app/src/test/java/com/x8bit/bitwarden/data/platform/manager/util/SpecialCircumstanceExtensionsTest.kt +++ b/app/src/test/java/com/x8bit/bitwarden/data/platform/manager/util/SpecialCircumstanceExtensionsTest.kt @@ -148,6 +148,7 @@ class SpecialCircumstanceExtensionsTest { fun `toFido2RequestOrNull should return a non-null value for Fido2Save`() { val fido2CreateCredentialRequest = Fido2CreateCredentialRequest( userId = "mockUserId", + isUserPreVerified = false, requestData = bundleOf(), ) assertEquals( diff --git a/app/src/test/java/com/x8bit/bitwarden/ui/platform/feature/rootnav/RootNavViewModelTest.kt b/app/src/test/java/com/x8bit/bitwarden/ui/platform/feature/rootnav/RootNavViewModelTest.kt index 62064ba5eb..73191aa2ad 100644 --- a/app/src/test/java/com/x8bit/bitwarden/ui/platform/feature/rootnav/RootNavViewModelTest.kt +++ b/app/src/test/java/com/x8bit/bitwarden/ui/platform/feature/rootnav/RootNavViewModelTest.kt @@ -664,6 +664,7 @@ class RootNavViewModelTest : BaseViewModelTest() { fun `when the active user has an unlocked vault but there is a Fido2Save special circumstance the nav state should be VaultUnlockedForFido2Save`() { val fido2CreateCredentialRequest = Fido2CreateCredentialRequest( userId = "activeUserId", + isUserPreVerified = false, requestData = bundleOf(), ) specialCircumstanceManager.specialCircumstance = diff --git a/app/src/test/java/com/x8bit/bitwarden/ui/vault/feature/addedit/VaultAddEditViewModelTest.kt b/app/src/test/java/com/x8bit/bitwarden/ui/vault/feature/addedit/VaultAddEditViewModelTest.kt index bafa3041f0..89c3c1c8be 100644 --- a/app/src/test/java/com/x8bit/bitwarden/ui/vault/feature/addedit/VaultAddEditViewModelTest.kt +++ b/app/src/test/java/com/x8bit/bitwarden/ui/vault/feature/addedit/VaultAddEditViewModelTest.kt @@ -396,6 +396,7 @@ class VaultAddEditViewModelTest : BaseViewModelTest() { } returns mockk(relaxed = true) val fido2CreateCredentialRequest = Fido2CreateCredentialRequest( userId = "mockUserId-1", + isUserPreVerified = false, requestData = bundleOf(), ) specialCircumstanceManager.specialCircumstance = SpecialCircumstance.Fido2Save( @@ -880,6 +881,7 @@ class VaultAddEditViewModelTest : BaseViewModelTest() { runTest { val fido2CreateCredentialRequest = Fido2CreateCredentialRequest( userId = "mockUserId", + isUserPreVerified = false, requestData = bundleOf(), ) specialCircumstanceManager.specialCircumstance = @@ -959,6 +961,7 @@ class VaultAddEditViewModelTest : BaseViewModelTest() { val mockUserId = "mockUserId" val fido2CreateCredentialRequest = Fido2CreateCredentialRequest( userId = mockUserId, + isUserPreVerified = false, requestData = bundleOf(), ) specialCircumstanceManager.specialCircumstance = diff --git a/app/src/test/java/com/x8bit/bitwarden/ui/vault/feature/addedit/util/Fido2CredentialRequestExtensionsTest.kt b/app/src/test/java/com/x8bit/bitwarden/ui/vault/feature/addedit/util/Fido2CredentialRequestExtensionsTest.kt index 03fdbe6392..2d3dd755d1 100644 --- a/app/src/test/java/com/x8bit/bitwarden/ui/vault/feature/addedit/util/Fido2CredentialRequestExtensionsTest.kt +++ b/app/src/test/java/com/x8bit/bitwarden/ui/vault/feature/addedit/util/Fido2CredentialRequestExtensionsTest.kt @@ -59,6 +59,7 @@ class Fido2CredentialRequestExtensionsTest { ), Fido2CreateCredentialRequest( userId = "mockUserId-1", + isUserPreVerified = false, requestData = bundleOf(), ) .toDefaultAddTypeContent( @@ -96,6 +97,7 @@ class Fido2CredentialRequestExtensionsTest { ), Fido2CreateCredentialRequest( userId = "mockUserId-1", + isUserPreVerified = false, requestData = bundleOf(), ) .toDefaultAddTypeContent( diff --git a/app/src/test/java/com/x8bit/bitwarden/ui/vault/feature/itemlisting/VaultItemListingViewModelTest.kt b/app/src/test/java/com/x8bit/bitwarden/ui/vault/feature/itemlisting/VaultItemListingViewModelTest.kt index b08b03cd35..bc7d539706 100644 --- a/app/src/test/java/com/x8bit/bitwarden/ui/vault/feature/itemlisting/VaultItemListingViewModelTest.kt +++ b/app/src/test/java/com/x8bit/bitwarden/ui/vault/feature/itemlisting/VaultItemListingViewModelTest.kt @@ -4,6 +4,8 @@ import android.net.Uri import androidx.core.os.bundleOf import androidx.credentials.CreatePublicKeyCredentialRequest import androidx.credentials.GetPublicKeyCredentialOption +import androidx.credentials.provider.BeginGetCredentialRequest +import androidx.credentials.provider.BeginGetPublicKeyCredentialOption import androidx.credentials.provider.ProviderCreateCredentialRequest import androidx.credentials.provider.ProviderGetCredentialRequest import androidx.lifecycle.SavedStateHandle @@ -37,6 +39,7 @@ import com.x8bit.bitwarden.data.autofill.fido2.model.Fido2ValidateOriginResult import com.x8bit.bitwarden.data.autofill.fido2.model.UserVerificationRequirement import com.x8bit.bitwarden.data.autofill.fido2.model.createMockFido2CreateCredentialRequest import com.x8bit.bitwarden.data.autofill.fido2.model.createMockFido2CredentialAssertionRequest +import com.x8bit.bitwarden.data.autofill.fido2.model.createMockFido2GetCredentialsRequest import com.x8bit.bitwarden.data.autofill.manager.AutofillSelectionManager import com.x8bit.bitwarden.data.autofill.manager.AutofillSelectionManagerImpl import com.x8bit.bitwarden.data.autofill.model.AutofillSaveItem @@ -68,6 +71,7 @@ import com.x8bit.bitwarden.data.vault.repository.model.GenerateTotpResult import com.x8bit.bitwarden.data.vault.repository.model.RemovePasswordSendResult import com.x8bit.bitwarden.data.vault.repository.model.VaultData import com.x8bit.bitwarden.ui.autofill.fido2.manager.model.AssertFido2CredentialResult +import com.x8bit.bitwarden.ui.autofill.fido2.manager.model.GetFido2CredentialsResult import com.x8bit.bitwarden.ui.autofill.fido2.manager.model.RegisterFido2CredentialResult import com.x8bit.bitwarden.ui.platform.base.BaseViewModelTest import com.x8bit.bitwarden.ui.platform.components.model.AccountSummary @@ -217,17 +221,30 @@ class VaultItemListingViewModelTest : BaseViewModelTest() { mockk(relaxed = true), ) } + private val mockBeginGetPublicKeyCredentialOption = + mockk(relaxed = true) + private val mockBeginGetCredentialRequest = mockk(relaxed = true) { + every { + beginGetCredentialOptions + } returns listOf( + mockBeginGetPublicKeyCredentialOption, + ) + } @BeforeEach fun setUp() { mockkObject( ProviderCreateCredentialRequest.Companion, ProviderGetCredentialRequest.Companion, + BeginGetCredentialRequest.Companion, ) every { ProviderCreateCredentialRequest.fromBundle(any()) } returns mockk(relaxed = true) every { ProviderGetCredentialRequest.fromBundle(any()) } returns mockProviderGetCredentialRequest + every { + BeginGetCredentialRequest.fromBundle(any()) + } returns mockBeginGetCredentialRequest } @AfterEach @@ -235,6 +252,7 @@ class VaultItemListingViewModelTest : BaseViewModelTest() { unmockkObject( ProviderCreateCredentialRequest.Companion, ProviderGetCredentialRequest.Companion, + BeginGetCredentialRequest.Companion, ) } @@ -252,7 +270,8 @@ class VaultItemListingViewModelTest : BaseViewModelTest() { fun `initial dialog state should be correct when Fido2CreateCredentialRequest is present`() = runTest { val fido2CreateCredentialRequest = Fido2CreateCredentialRequest( - "mockUserId", + userId = "mockUserId", + isUserPreVerified = false, requestData = bundleOf(), ) specialCircumstanceManager.specialCircumstance = SpecialCircumstance.Fido2Save( @@ -1859,6 +1878,7 @@ class VaultItemListingViewModelTest : BaseViewModelTest() { val fido2CreateCredentialRequest = Fido2CreateCredentialRequest( userId = "activeUserId", + isUserPreVerified = false, requestData = bundleOf(), ) @@ -2553,10 +2573,12 @@ class VaultItemListingViewModelTest : BaseViewModelTest() { ) } + //region FIDO2 process handling @Test fun `Fido2CreateCredentialRequest should be evaluated before observing vault data`() { val fido2CreateCredentialRequest = Fido2CreateCredentialRequest( userId = "mockUserId", + isUserPreVerified = false, requestData = bundleOf(), ) specialCircumstanceManager.specialCircumstance = SpecialCircumstance.Fido2Save( @@ -2590,6 +2612,7 @@ class VaultItemListingViewModelTest : BaseViewModelTest() { fun `Fido2ValidateOriginResult should update dialog state on Unknown error`() = runTest { val mockFido2CreateRequest = Fido2CreateCredentialRequest( userId = "mockUserId", + isUserPreVerified = false, requestData = bundleOf(), ) @@ -2628,6 +2651,7 @@ class VaultItemListingViewModelTest : BaseViewModelTest() { runTest { val fido2CreateCredentialRequest = Fido2CreateCredentialRequest( userId = "mockUserId", + isUserPreVerified = false, requestData = bundleOf(), ) @@ -2666,6 +2690,7 @@ class VaultItemListingViewModelTest : BaseViewModelTest() { runTest { val fido2CreateCredentialRequest = Fido2CreateCredentialRequest( userId = "mockUserId", + isUserPreVerified = false, requestData = bundleOf(), ) @@ -2704,6 +2729,7 @@ class VaultItemListingViewModelTest : BaseViewModelTest() { runTest { val fido2CreateCredentialRequest = Fido2CreateCredentialRequest( userId = "mockUserId", + isUserPreVerified = false, requestData = bundleOf(), ) @@ -2742,6 +2768,7 @@ class VaultItemListingViewModelTest : BaseViewModelTest() { runTest { val fido2CreateCredentialRequest = Fido2CreateCredentialRequest( userId = "mockUserId", + isUserPreVerified = false, requestData = bundleOf(), ) @@ -2780,6 +2807,7 @@ class VaultItemListingViewModelTest : BaseViewModelTest() { runTest { val fido2CreateCredentialRequest = Fido2CreateCredentialRequest( userId = "mockUserId", + isUserPreVerified = false, requestData = bundleOf(), ) @@ -2818,6 +2846,7 @@ class VaultItemListingViewModelTest : BaseViewModelTest() { runTest { val fido2CreateCredentialRequest = Fido2CreateCredentialRequest( userId = "mockUserId", + isUserPreVerified = false, requestData = bundleOf(), ) @@ -2990,6 +3019,261 @@ class VaultItemListingViewModelTest : BaseViewModelTest() { ) } + @Suppress("MaxLineLength") + @Test + fun `Fido2GetCredentialsRequest should validate request and emit CompleteFido2GetCredentialsRequest event`() = + runTest { + setupMockUri() + val mockGetCredentialsRequest = createMockFido2GetCredentialsRequest(number = 1) + val mockFido2CredentialsList = createMockSdkFido2CredentialList(number = 1) + val mockCipherView = createMockCipherView( + number = 1, + fido2Credentials = mockFido2CredentialsList, + ) + specialCircumstanceManager.specialCircumstance = + SpecialCircumstance.Fido2GetCredentials( + mockGetCredentialsRequest, + ) + every { + fido2CredentialManager.getPasskeyAssertionOptionsOrNull(any()) + } returns mockk(relaxed = true) + coEvery { + fido2OriginManager.validateOrigin( + callingAppInfo = any(), + relyingPartyId = any(), + ) + } returns Fido2ValidateOriginResult.Success("mockOrigin") + every { + vaultRepository + .ciphersStateFlow + .value + .data + } returns listOf(mockCipherView) + + val dataState = DataState.Loaded( + data = VaultData( + cipherViewList = listOf(mockCipherView), + folderViewList = listOf(createMockFolderView(number = 1)), + collectionViewList = listOf(createMockCollectionView(number = 1)), + sendViewList = listOf(createMockSendView(number = 1)), + ), + ) + + val viewModel = createVaultItemListingViewModel() + mutableVaultDataStateFlow.value = dataState + + viewModel.eventFlow.test { + assertEquals( + VaultItemListingEvent.CompleteFido2GetCredentialsRequest( + result = GetFido2CredentialsResult.Success( + userId = "mockUserId-1", + option = mockBeginGetPublicKeyCredentialOption, + credentials = emptyList(), + ), + ), + awaitItem(), + ) + } + } + + @Test + fun `Fido2GetCredentialsRequest should display error dialog when request is invalid`() = + runTest { + setupMockUri() + val mockGetCredentialsRequest = createMockFido2GetCredentialsRequest(number = 1) + val mockFido2CredentialsList = createMockSdkFido2CredentialList(number = 1) + val mockCipherView = createMockCipherView( + number = 1, + fido2Credentials = mockFido2CredentialsList, + ) + specialCircumstanceManager.specialCircumstance = + SpecialCircumstance.Fido2GetCredentials( + mockGetCredentialsRequest, + ) + + every { + vaultRepository + .ciphersStateFlow + .value + .data + } returns listOf(mockCipherView) + every { + fido2CredentialManager.getPasskeyAssertionOptionsOrNull(any()) + } returns mockk(relaxed = true) + every { + mockBeginGetCredentialRequest.beginGetCredentialOptions + } returns emptyList() + + val dataState = DataState.Loaded( + data = VaultData( + cipherViewList = listOf(mockCipherView), + folderViewList = listOf(createMockFolderView(number = 1)), + collectionViewList = listOf(createMockCollectionView(number = 1)), + sendViewList = listOf(createMockSendView(number = 1)), + ), + ) + + val viewModel = createVaultItemListingViewModel() + mutableVaultDataStateFlow.value = dataState + + assertEquals( + VaultItemListingState.DialogState.Fido2OperationFail( + title = R.string.an_error_has_occurred.asText(), + message = + R.string.passkey_operation_failed_because_the_request_is_invalid.asText(), + ), + viewModel.stateFlow.value.dialogState, + ) + } + + @Suppress("MaxLineLength") + @Test + fun `Fido2GetCredentialsRequest should display error dialog when relyingParty cannot be identified`() = + runTest { + setupMockUri() + val mockGetCredentialsRequest = createMockFido2GetCredentialsRequest(number = 1) + val mockFido2CredentialsList = createMockSdkFido2CredentialList(number = 1) + val mockCipherView = createMockCipherView( + number = 1, + fido2Credentials = mockFido2CredentialsList, + ) + specialCircumstanceManager.specialCircumstance = + SpecialCircumstance.Fido2GetCredentials( + mockGetCredentialsRequest, + ) + + every { + vaultRepository + .ciphersStateFlow + .value + .data + } returns listOf(mockCipherView) + every { + fido2CredentialManager.getPasskeyAssertionOptionsOrNull(any()) + } returns mockk(relaxed = true) { + every { relyingPartyId } returns null + } + + val dataState = DataState.Loaded( + data = VaultData( + cipherViewList = listOf(mockCipherView), + folderViewList = listOf(createMockFolderView(number = 1)), + collectionViewList = listOf(createMockCollectionView(number = 1)), + sendViewList = listOf(createMockSendView(number = 1)), + ), + ) + + val viewModel = createVaultItemListingViewModel() + mutableVaultDataStateFlow.value = dataState + + assertEquals( + VaultItemListingState.DialogState.Fido2OperationFail( + title = R.string.an_error_has_occurred.asText(), + message = + R.string.passkey_operation_failed_because_relying_party_cannot_be_identified + .asText(), + ), + viewModel.stateFlow.value.dialogState, + ) + } + + @Suppress("MaxLineLength") + @Test + fun `Fido2GetCredentialsRequest should display error dialog when callingApp cannot be verified`() = + runTest { + setupMockUri() + val mockGetCredentialsRequest = createMockFido2GetCredentialsRequest(number = 1) + val mockFido2CredentialsList = createMockSdkFido2CredentialList(number = 1) + val mockCipherView = createMockCipherView( + number = 1, + fido2Credentials = mockFido2CredentialsList, + ) + specialCircumstanceManager.specialCircumstance = + SpecialCircumstance.Fido2GetCredentials( + mockGetCredentialsRequest, + ) + + every { + vaultRepository + .ciphersStateFlow + .value + .data + } returns listOf(mockCipherView) + every { + fido2CredentialManager.getPasskeyAssertionOptionsOrNull(any()) + } returns mockk(relaxed = true) + every { + mockBeginGetCredentialRequest.callingAppInfo + } returns null + + val dataState = DataState.Loaded( + data = VaultData( + cipherViewList = listOf(mockCipherView), + folderViewList = listOf(createMockFolderView(number = 1)), + collectionViewList = listOf(createMockCollectionView(number = 1)), + sendViewList = listOf(createMockSendView(number = 1)), + ), + ) + + val viewModel = createVaultItemListingViewModel() + mutableVaultDataStateFlow.value = dataState + + assertEquals( + VaultItemListingState.DialogState.Fido2OperationFail( + title = R.string.an_error_has_occurred.asText(), + message = + R.string.passkey_operation_failed_because_app_could_not_be_verified + .asText(), + ), + viewModel.stateFlow.value.dialogState, + ) + } + + @Test + fun `Fido2GetCredentialsRequest should display error dialog when origin validation fails`() = + runTest { + setupMockUri() + val mockGetCredentialsRequest = createMockFido2GetCredentialsRequest(number = 1) + val mockFido2CredentialsList = createMockSdkFido2CredentialList(number = 1) + val mockCipherView = createMockCipherView( + number = 1, + fido2Credentials = mockFido2CredentialsList, + ) + specialCircumstanceManager.specialCircumstance = + SpecialCircumstance.Fido2GetCredentials( + mockGetCredentialsRequest, + ) + every { + fido2CredentialManager.getPasskeyAssertionOptionsOrNull(any()) + } returns mockk(relaxed = true) + coEvery { + fido2OriginManager.validateOrigin( + callingAppInfo = any(), + relyingPartyId = any(), + ) + } returns Fido2ValidateOriginResult.Error.Unknown + + val dataState = DataState.Loaded( + data = VaultData( + cipherViewList = listOf(mockCipherView), + folderViewList = listOf(createMockFolderView(number = 1)), + collectionViewList = listOf(createMockCollectionView(number = 1)), + sendViewList = listOf(createMockSendView(number = 1)), + ), + ) + + val viewModel = createVaultItemListingViewModel() + mutableVaultDataStateFlow.value = dataState + + assertEquals( + VaultItemListingState.DialogState.Fido2OperationFail( + title = R.string.an_error_has_occurred.asText(), + message = R.string.generic_error_message.asText(), + ), + viewModel.stateFlow.value.dialogState, + ) + } + @Suppress("MaxLineLength") @Test fun `Fido2AssertionRequest should display loading dialog then request user verification when user is not verified and verification is REQUIRED`() = @@ -3651,6 +3935,28 @@ class VaultItemListingViewModelTest : BaseViewModelTest() { } } + @Suppress("MaxLineLength") + @Test + fun `UserVerificationCancelled should clear dialog state, set isUserVerified to false, and emit CompleteFido2Assertion with cancelled result`() = + runTest { + specialCircumstanceManager.specialCircumstance = SpecialCircumstance.Fido2Assertion( + createMockFido2CredentialAssertionRequest(number = 1), + ) + val viewModel = createVaultItemListingViewModel() + viewModel.trySendAction(VaultItemListingsAction.UserVerificationCancelled) + + verify { fido2CredentialManager.isUserVerified = false } + assertNull(viewModel.stateFlow.value.dialogState) + viewModel.eventFlow.test { + assertEquals( + VaultItemListingEvent.CompleteFido2Assertion( + result = AssertFido2CredentialResult.Cancelled, + ), + awaitItem(), + ) + } + } + @Test fun `UserVerificationFail should display Fido2ErrorDialog and set isUserVerified to false`() { val viewModel = createVaultItemListingViewModel() @@ -4511,6 +4817,8 @@ class VaultItemListingViewModelTest : BaseViewModelTest() { ) } + //endregion FIDO2 process handling + @Test fun `InternetConnectionErrorReceived should show network error if no internet connection`() = runTest { diff --git a/app/src/test/java/com/x8bit/bitwarden/ui/vault/feature/itemlisting/util/VaultItemListingDataExtensionsTest.kt b/app/src/test/java/com/x8bit/bitwarden/ui/vault/feature/itemlisting/util/VaultItemListingDataExtensionsTest.kt index 9c671fc376..194a643068 100644 --- a/app/src/test/java/com/x8bit/bitwarden/ui/vault/feature/itemlisting/util/VaultItemListingDataExtensionsTest.kt +++ b/app/src/test/java/com/x8bit/bitwarden/ui/vault/feature/itemlisting/util/VaultItemListingDataExtensionsTest.kt @@ -887,6 +887,7 @@ class VaultItemListingDataExtensionsTest { autofillSelectionData = null, fido2CreationData = Fido2CreateCredentialRequest( userId = "userId", + isUserPreVerified = false, requestData = bundleOf(), ), fido2CredentialAutofillViews = null,