From 793971c3a39fffafecc8a5fc0943394adee96cfc Mon Sep 17 00:00:00 2001 From: Patrick Honkonen <1883101+SaintPatrck@users.noreply.github.com> Date: Thu, 25 Jul 2024 10:33:14 -0400 Subject: [PATCH] [PM-9409] Complete FIDO 2 assertion with appropriate response (#3615) --- .../model/Fido2CredentialAssertionResult.kt | 22 +++ .../fido2/manager/Fido2CompletionManager.kt | 6 + .../manager/Fido2CompletionManagerImpl.kt | 47 ++++- ...ido2CompletionManagerUnsupportedApiImpl.kt | 14 ++ .../composition/LocalManagerProvider.kt | 11 +- .../manager/Fido2CompletionManagerTest.kt | 165 ++++++++++++++++++ 6 files changed, 260 insertions(+), 5 deletions(-) create mode 100644 app/src/main/java/com/x8bit/bitwarden/data/autofill/fido2/model/Fido2CredentialAssertionResult.kt create mode 100644 app/src/main/java/com/x8bit/bitwarden/ui/autofill/fido2/manager/Fido2CompletionManagerUnsupportedApiImpl.kt create mode 100644 app/src/test/java/com/x8bit/bitwarden/ui/autofill/fido2/manager/Fido2CompletionManagerTest.kt diff --git a/app/src/main/java/com/x8bit/bitwarden/data/autofill/fido2/model/Fido2CredentialAssertionResult.kt b/app/src/main/java/com/x8bit/bitwarden/data/autofill/fido2/model/Fido2CredentialAssertionResult.kt new file mode 100644 index 0000000000..17418a66dc --- /dev/null +++ b/app/src/main/java/com/x8bit/bitwarden/data/autofill/fido2/model/Fido2CredentialAssertionResult.kt @@ -0,0 +1,22 @@ +package com.x8bit.bitwarden.data.autofill.fido2.model + +/** + * Represents possible outcomes of a FIDO 2 credential assertion request. + */ +sealed class Fido2CredentialAssertionResult { + + /** + * Indicates the assertion request completed and [responseJson] was successfully generated. + */ + data class Success(val responseJson: String) : Fido2CredentialAssertionResult() + + /** + * Indicates there was an error and the assertion was not successful. + */ + data object Error : Fido2CredentialAssertionResult() + + /** + * Indicates assertion was cancelled by the user. + */ + data object Cancelled : Fido2CredentialAssertionResult() +} diff --git a/app/src/main/java/com/x8bit/bitwarden/ui/autofill/fido2/manager/Fido2CompletionManager.kt b/app/src/main/java/com/x8bit/bitwarden/ui/autofill/fido2/manager/Fido2CompletionManager.kt index 3fa548a443..1f71a5fb6c 100644 --- a/app/src/main/java/com/x8bit/bitwarden/ui/autofill/fido2/manager/Fido2CompletionManager.kt +++ b/app/src/main/java/com/x8bit/bitwarden/ui/autofill/fido2/manager/Fido2CompletionManager.kt @@ -1,5 +1,6 @@ package com.x8bit.bitwarden.ui.autofill.fido2.manager +import com.x8bit.bitwarden.data.autofill.fido2.model.Fido2CredentialAssertionResult import com.x8bit.bitwarden.data.autofill.fido2.model.Fido2RegisterCredentialResult /** @@ -11,4 +12,9 @@ interface Fido2CompletionManager { * Completes the FIDO 2 registration process with the provided [result]. */ fun completeFido2Registration(result: Fido2RegisterCredentialResult) + + /** + * Complete the FIDO 2 credential assertion process with the provided [result]. + */ + fun completeFido2Assertion(result: Fido2CredentialAssertionResult) } diff --git a/app/src/main/java/com/x8bit/bitwarden/ui/autofill/fido2/manager/Fido2CompletionManagerImpl.kt b/app/src/main/java/com/x8bit/bitwarden/ui/autofill/fido2/manager/Fido2CompletionManagerImpl.kt index 5cc0d8cac2..3354018b77 100644 --- a/app/src/main/java/com/x8bit/bitwarden/ui/autofill/fido2/manager/Fido2CompletionManagerImpl.kt +++ b/app/src/main/java/com/x8bit/bitwarden/ui/autofill/fido2/manager/Fido2CompletionManagerImpl.kt @@ -5,21 +5,25 @@ import android.content.Intent import android.os.Build import androidx.annotation.RequiresApi import androidx.credentials.CreatePublicKeyCredentialResponse +import androidx.credentials.GetCredentialResponse +import androidx.credentials.PublicKeyCredential import androidx.credentials.exceptions.CreateCredentialCancellationException import androidx.credentials.exceptions.CreateCredentialUnknownException +import androidx.credentials.exceptions.GetCredentialCancellationException +import androidx.credentials.exceptions.GetCredentialUnknownException import androidx.credentials.provider.PendingIntentHandler +import com.x8bit.bitwarden.data.autofill.fido2.model.Fido2CredentialAssertionResult import com.x8bit.bitwarden.data.autofill.fido2.model.Fido2RegisterCredentialResult -import com.x8bit.bitwarden.data.platform.annotation.OmitFromCoverage /** - * Primary implementation of [Fido2CompletionManager]. + * Primary implementation of [Fido2CompletionManager] when the build version is + * UPSIDE_DOWN_CAKE (34) or above. */ -@OmitFromCoverage +@RequiresApi(Build.VERSION_CODES.UPSIDE_DOWN_CAKE) class Fido2CompletionManagerImpl( private val activity: Activity, ) : Fido2CompletionManager { - @RequiresApi(Build.VERSION_CODES.UPSIDE_DOWN_CAKE) override fun completeFido2Registration(result: Fido2RegisterCredentialResult) { activity.also { val intent = Intent() @@ -54,4 +58,39 @@ class Fido2CompletionManagerImpl( it.finish() } } + + override fun completeFido2Assertion(result: Fido2CredentialAssertionResult) { + activity.also { + val intent = Intent() + when (result) { + Fido2CredentialAssertionResult.Cancelled -> { + PendingIntentHandler + .setGetCredentialException( + intent = intent, + exception = GetCredentialCancellationException(), + ) + } + + Fido2CredentialAssertionResult.Error -> { + PendingIntentHandler + .setGetCredentialException( + intent = intent, + exception = GetCredentialUnknownException(), + ) + } + + is Fido2CredentialAssertionResult.Success -> { + PendingIntentHandler + .setGetCredentialResponse( + intent = intent, + response = GetCredentialResponse( + credential = PublicKeyCredential(result.responseJson), + ), + ) + } + } + it.setResult(Activity.RESULT_OK, intent) + it.finish() + } + } } diff --git a/app/src/main/java/com/x8bit/bitwarden/ui/autofill/fido2/manager/Fido2CompletionManagerUnsupportedApiImpl.kt b/app/src/main/java/com/x8bit/bitwarden/ui/autofill/fido2/manager/Fido2CompletionManagerUnsupportedApiImpl.kt new file mode 100644 index 0000000000..07cdf8bc64 --- /dev/null +++ b/app/src/main/java/com/x8bit/bitwarden/ui/autofill/fido2/manager/Fido2CompletionManagerUnsupportedApiImpl.kt @@ -0,0 +1,14 @@ +package com.x8bit.bitwarden.ui.autofill.fido2.manager + +import com.x8bit.bitwarden.data.autofill.fido2.model.Fido2CredentialAssertionResult +import com.x8bit.bitwarden.data.autofill.fido2.model.Fido2RegisterCredentialResult + +/** + * A no-op implementation of [Fido2CompletionManagerImpl] provided when the build version is below + * UPSIDE_DOWN_CAKE (34). These versions do not support [androidx.credentials.CredentialProvider]. + */ +object Fido2CompletionManagerUnsupportedApiImpl : Fido2CompletionManager { + override fun completeFido2Registration(result: Fido2RegisterCredentialResult) = Unit + + override fun completeFido2Assertion(result: Fido2CredentialAssertionResult) = Unit +} diff --git a/app/src/main/java/com/x8bit/bitwarden/ui/platform/composition/LocalManagerProvider.kt b/app/src/main/java/com/x8bit/bitwarden/ui/platform/composition/LocalManagerProvider.kt index 6d13b82b79..7b6daba7a9 100644 --- a/app/src/main/java/com/x8bit/bitwarden/ui/platform/composition/LocalManagerProvider.kt +++ b/app/src/main/java/com/x8bit/bitwarden/ui/platform/composition/LocalManagerProvider.kt @@ -3,6 +3,7 @@ package com.x8bit.bitwarden.ui.platform.composition import android.app.Activity +import android.os.Build import androidx.compose.runtime.Composable import androidx.compose.runtime.CompositionLocal import androidx.compose.runtime.CompositionLocalProvider @@ -10,8 +11,10 @@ import androidx.compose.runtime.ProvidableCompositionLocal import androidx.compose.runtime.compositionLocalOf import androidx.compose.ui.platform.LocalContext import com.x8bit.bitwarden.data.platform.annotation.OmitFromCoverage +import com.x8bit.bitwarden.data.platform.util.isBuildVersionBelow import com.x8bit.bitwarden.ui.autofill.fido2.manager.Fido2CompletionManager import com.x8bit.bitwarden.ui.autofill.fido2.manager.Fido2CompletionManagerImpl +import com.x8bit.bitwarden.ui.autofill.fido2.manager.Fido2CompletionManagerUnsupportedApiImpl import com.x8bit.bitwarden.ui.platform.manager.biometrics.BiometricsManager import com.x8bit.bitwarden.ui.platform.manager.biometrics.BiometricsManagerImpl import com.x8bit.bitwarden.ui.platform.manager.exit.ExitManager @@ -29,13 +32,19 @@ import com.x8bit.bitwarden.ui.platform.manager.permissions.PermissionsManagerImp @Composable fun LocalManagerProvider(content: @Composable () -> Unit) { val activity = LocalContext.current as Activity + val fido2CompletionManager = + if (isBuildVersionBelow(Build.VERSION_CODES.UPSIDE_DOWN_CAKE)) { + Fido2CompletionManagerUnsupportedApiImpl + } else { + Fido2CompletionManagerImpl(activity) + } CompositionLocalProvider( LocalPermissionsManager provides PermissionsManagerImpl(activity), LocalIntentManager provides IntentManagerImpl(activity), LocalExitManager provides ExitManagerImpl(activity), LocalBiometricsManager provides BiometricsManagerImpl(activity), LocalNfcManager provides NfcManagerImpl(activity), - LocalFido2CompletionManager provides Fido2CompletionManagerImpl(activity), + LocalFido2CompletionManager provides fido2CompletionManager, ) { content() } diff --git a/app/src/test/java/com/x8bit/bitwarden/ui/autofill/fido2/manager/Fido2CompletionManagerTest.kt b/app/src/test/java/com/x8bit/bitwarden/ui/autofill/fido2/manager/Fido2CompletionManagerTest.kt new file mode 100644 index 0000000000..ac51d84c4d --- /dev/null +++ b/app/src/test/java/com/x8bit/bitwarden/ui/autofill/fido2/manager/Fido2CompletionManagerTest.kt @@ -0,0 +1,165 @@ +package com.x8bit.bitwarden.ui.autofill.fido2.manager + +import android.app.Activity +import android.content.Intent +import androidx.credentials.provider.PendingIntentHandler +import com.x8bit.bitwarden.data.autofill.fido2.model.Fido2CredentialAssertionResult +import com.x8bit.bitwarden.data.autofill.fido2.model.Fido2RegisterCredentialResult +import io.mockk.Called +import io.mockk.MockKVerificationScope +import io.mockk.Ordering +import io.mockk.every +import io.mockk.just +import io.mockk.mockk +import io.mockk.mockkConstructor +import io.mockk.mockkObject +import io.mockk.runs +import io.mockk.unmockkConstructor +import io.mockk.unmockkObject +import io.mockk.verify +import org.junit.jupiter.api.AfterEach +import org.junit.jupiter.api.BeforeEach +import org.junit.jupiter.api.Nested +import org.junit.jupiter.api.Test + +class Fido2CompletionManagerTest { + + private val mockActivity = mockk { + every { setResult(Activity.RESULT_OK, any()) } just runs + every { finish() } just runs + } + private lateinit var fido2CompletionManager: Fido2CompletionManager + + @Nested + inner class NoOpImplementation { + @BeforeEach + fun setUp() { + fido2CompletionManager = Fido2CompletionManagerUnsupportedApiImpl + } + + @Test + fun `completeFido2Registration should perform no operations`() { + val mockRegistrationResult = mockk() + fido2CompletionManager.completeFido2Registration(mockRegistrationResult) + verify { + mockRegistrationResult wasNot Called + mockActivity wasNot Called + } + } + + @Test + fun `completeFido2Assertion should perform no operations`() { + val mockAssertionResult = mockk() + fido2CompletionManager.completeFido2Assertion(mockAssertionResult) + verify { + mockAssertionResult wasNot Called + mockActivity wasNot Called + } + } + } + + @Nested + inner class DefaultImplementation { + + @BeforeEach + fun setUp() { + fido2CompletionManager = Fido2CompletionManagerImpl(mockActivity) + mockkConstructor(Intent::class) + mockkObject(PendingIntentHandler.Companion) + every { + PendingIntentHandler.setCreateCredentialException(any(), any()) + } just runs + } + + @AfterEach + fun tearDown() { + unmockkConstructor(Intent::class) + unmockkObject(PendingIntentHandler.Companion) + } + + @Suppress("MaxLineLength") + @Test + fun `completeFido2Registration should set CreateCredentialResponse, set activity result, then finish activity when result is Success`() { + fido2CompletionManager + .completeFido2Registration( + Fido2RegisterCredentialResult.Success( + registrationResponse = "registrationResponse", + ), + ) + + verifyActivityResultIsSetAndFinishedAfter { + PendingIntentHandler.setCreateCredentialResponse(any(), any()) + } + } + + @Suppress("MaxLineLength") + @Test + fun `completeFido2Registration should set CreateCredentialException, set activity result, then finish activity when result is Error`() { + fido2CompletionManager + .completeFido2Registration(Fido2RegisterCredentialResult.Error) + + verifyActivityResultIsSetAndFinishedAfter { + PendingIntentHandler.setCreateCredentialException(any(), any()) + } + } + + @Suppress("MaxLineLength") + @Test + fun `completeFido2Registration should set CreateCredentialException, set activity result, then finish activity when result is Canclled`() { + fido2CompletionManager + .completeFido2Registration(Fido2RegisterCredentialResult.Cancelled) + + verifyActivityResultIsSetAndFinishedAfter { + PendingIntentHandler.setCreateCredentialException(any(), any()) + } + } + + @Suppress("MaxLineLength") + @Test + fun `completeFido2Assertion should set GetCredentialResponse, set activity result, then finish activity when result is Success`() { + fido2CompletionManager + .completeFido2Assertion(Fido2CredentialAssertionResult.Success("responseJson")) + + verifyActivityResultIsSetAndFinishedAfter { + PendingIntentHandler.setGetCredentialResponse(any(), any()) + } + } + + @Suppress("MaxLineLength") + @Test + fun `completeFido2Assertion should set GetCredentialException, set activity result, then finish activity when result is Error`() { + fido2CompletionManager + .completeFido2Assertion(Fido2CredentialAssertionResult.Error) + + verifyActivityResultIsSetAndFinishedAfter { + PendingIntentHandler.setGetCredentialException(any(), any()) + } + } + + @Suppress("MaxLineLength") + @Test + fun `completeFido2Assertion should set cancellation exception, set activity result, then finish activity when result is Cancelled`() { + fido2CompletionManager + .completeFido2Assertion(Fido2CredentialAssertionResult.Cancelled) + + verifyActivityResultIsSetAndFinishedAfter { + PendingIntentHandler.setGetCredentialException(any(), any()) + } + } + + /** + * Convenience function to ensure the given [calls] are performed before setting the + * [mockActivity] result and calling finish. This sequence is expected to be performed for + * all FIDO 2 operations triggered by [androidx.credentials.CredentialProvider] APIs. + */ + private fun verifyActivityResultIsSetAndFinishedAfter( + calls: MockKVerificationScope.() -> Unit, + ) { + verify(Ordering.SEQUENCE) { + calls() + mockActivity.setResult(Activity.RESULT_OK, any()) + mockActivity.finish() + } + } + } +}