From ec8e934bf4e4e7e9d5ee98a6a27c3fc67088b77d Mon Sep 17 00:00:00 2001 From: Dave Severns <149429124+dseverns-livefront@users.noreply.github.com> Date: Wed, 20 Nov 2024 15:45:26 -0500 Subject: [PATCH] PM-15062 Checking if the user has a no longer supported biometric as their only way of unlocking their account. (#4338) --- .../auth/datasource/disk/model/AccountJson.kt | 6 ++- .../KeyConnectorUserDecryptionOptionsJson.kt | 6 ++- .../TrustedDeviceUserDecryptionOptionsJson.kt | 18 +++++-- .../model/UserDecryptionOptionsJson.kt | 12 +++-- .../feature/vaultunlock/VaultUnlockScreen.kt | 23 +++++++++ .../vaultunlock/VaultUnlockViewModel.kt | 51 +++++++++++++++++++ app/src/main/res/values/strings.xml | 2 + .../datasource/disk/AuthDiskSourceTest.kt | 20 ++++---- .../vaultunlock/VaultUnlockScreenTest.kt | 46 +++++++++++++++++ .../vaultunlock/VaultUnlockViewModelTest.kt | 30 +++++++++++ 10 files changed, 194 insertions(+), 20 deletions(-) diff --git a/app/src/main/java/com/x8bit/bitwarden/data/auth/datasource/disk/model/AccountJson.kt b/app/src/main/java/com/x8bit/bitwarden/data/auth/datasource/disk/model/AccountJson.kt index d7d1efd42..e7f7f0fc0 100644 --- a/app/src/main/java/com/x8bit/bitwarden/data/auth/datasource/disk/model/AccountJson.kt +++ b/app/src/main/java/com/x8bit/bitwarden/data/auth/datasource/disk/model/AccountJson.kt @@ -2,8 +2,10 @@ package com.x8bit.bitwarden.data.auth.datasource.disk.model import com.x8bit.bitwarden.data.auth.datasource.network.model.KdfTypeJson import com.x8bit.bitwarden.data.auth.datasource.network.model.UserDecryptionOptionsJson +import kotlinx.serialization.ExperimentalSerializationApi import kotlinx.serialization.SerialName import kotlinx.serialization.Serializable +import kotlinx.serialization.json.JsonNames /** * Represents the current account information for a given user. @@ -45,6 +47,7 @@ data class AccountJson( * @property kdfParallelism The number of threads to use when calculating a password hash. * @property userDecryptionOptions The options available to a user for decryption. */ + @OptIn(ExperimentalSerializationApi::class) @Serializable data class Profile( @SerialName("userId") @@ -86,7 +89,8 @@ data class AccountJson( @SerialName("kdfParallelism") val kdfParallelism: Int?, - @SerialName("accountDecryptionOptions") + @SerialName("userDecryptionOptions") + @JsonNames("accountDecryptionOptions") val userDecryptionOptions: UserDecryptionOptionsJson?, ) diff --git a/app/src/main/java/com/x8bit/bitwarden/data/auth/datasource/network/model/KeyConnectorUserDecryptionOptionsJson.kt b/app/src/main/java/com/x8bit/bitwarden/data/auth/datasource/network/model/KeyConnectorUserDecryptionOptionsJson.kt index 221be523e..277678221 100644 --- a/app/src/main/java/com/x8bit/bitwarden/data/auth/datasource/network/model/KeyConnectorUserDecryptionOptionsJson.kt +++ b/app/src/main/java/com/x8bit/bitwarden/data/auth/datasource/network/model/KeyConnectorUserDecryptionOptionsJson.kt @@ -1,15 +1,19 @@ package com.x8bit.bitwarden.data.auth.datasource.network.model +import kotlinx.serialization.ExperimentalSerializationApi import kotlinx.serialization.SerialName import kotlinx.serialization.Serializable +import kotlinx.serialization.json.JsonNames /** * Decryption options related to a user's key connector. * * @property keyConnectorUrl URL to the user's key connector. */ +@OptIn(ExperimentalSerializationApi::class) @Serializable data class KeyConnectorUserDecryptionOptionsJson( - @SerialName("KeyConnectorUrl") + @SerialName("keyConnectorUrl") + @JsonNames("KeyConnectorUrl") val keyConnectorUrl: String, ) diff --git a/app/src/main/java/com/x8bit/bitwarden/data/auth/datasource/network/model/TrustedDeviceUserDecryptionOptionsJson.kt b/app/src/main/java/com/x8bit/bitwarden/data/auth/datasource/network/model/TrustedDeviceUserDecryptionOptionsJson.kt index 42872d9c7..fe5fda629 100644 --- a/app/src/main/java/com/x8bit/bitwarden/data/auth/datasource/network/model/TrustedDeviceUserDecryptionOptionsJson.kt +++ b/app/src/main/java/com/x8bit/bitwarden/data/auth/datasource/network/model/TrustedDeviceUserDecryptionOptionsJson.kt @@ -1,7 +1,9 @@ package com.x8bit.bitwarden.data.auth.datasource.network.model +import kotlinx.serialization.ExperimentalSerializationApi import kotlinx.serialization.SerialName import kotlinx.serialization.Serializable +import kotlinx.serialization.json.JsonNames /** * Decryption options related to a user's trusted device. @@ -13,20 +15,26 @@ import kotlinx.serialization.Serializable * @property hasManageResetPasswordPermission Whether or not the user has manage reset password * permission. */ +@OptIn(ExperimentalSerializationApi::class) @Serializable data class TrustedDeviceUserDecryptionOptionsJson( - @SerialName("EncryptedPrivateKey") + @SerialName("encryptedPrivateKey") + @JsonNames("EncryptedPrivateKey") val encryptedPrivateKey: String?, - @SerialName("EncryptedUserKey") + @SerialName("encryptedUserKey") + @JsonNames("EncryptedUserKey") val encryptedUserKey: String?, - @SerialName("HasAdminApproval") + @SerialName("hasAdminApproval") + @JsonNames("HasAdminApproval") val hasAdminApproval: Boolean, - @SerialName("HasLoginApprovingDevice") + @SerialName("hasLoginApprovingDevice") + @JsonNames("HasLoginApprovingDevice") val hasLoginApprovingDevice: Boolean, - @SerialName("HasManageResetPasswordPermission") + @SerialName("hasManageResetPasswordPermission") + @JsonNames("HasManageResetPasswordPermission") val hasManageResetPasswordPermission: Boolean, ) diff --git a/app/src/main/java/com/x8bit/bitwarden/data/auth/datasource/network/model/UserDecryptionOptionsJson.kt b/app/src/main/java/com/x8bit/bitwarden/data/auth/datasource/network/model/UserDecryptionOptionsJson.kt index d4dcb29e7..3d26912ad 100644 --- a/app/src/main/java/com/x8bit/bitwarden/data/auth/datasource/network/model/UserDecryptionOptionsJson.kt +++ b/app/src/main/java/com/x8bit/bitwarden/data/auth/datasource/network/model/UserDecryptionOptionsJson.kt @@ -1,7 +1,9 @@ package com.x8bit.bitwarden.data.auth.datasource.network.model +import kotlinx.serialization.ExperimentalSerializationApi import kotlinx.serialization.SerialName import kotlinx.serialization.Serializable +import kotlinx.serialization.json.JsonNames /** * The options available to a user for decryption. @@ -12,14 +14,18 @@ import kotlinx.serialization.Serializable * device. * @property keyConnectorUserDecryptionOptions Decryption options related to a user's key connector. */ +@OptIn(ExperimentalSerializationApi::class) @Serializable data class UserDecryptionOptionsJson( - @SerialName("HasMasterPassword") + @SerialName("hasMasterPassword") + @JsonNames("HasMasterPassword") val hasMasterPassword: Boolean, - @SerialName("TrustedDeviceOption") + @SerialName("trustedDeviceOption") + @JsonNames("TrustedDeviceOption") val trustedDeviceUserDecryptionOptions: TrustedDeviceUserDecryptionOptionsJson?, - @SerialName("KeyConnectorOption") + @SerialName("keyConnectorOption") + @JsonNames("KeyConnectorOption") val keyConnectorUserDecryptionOptions: KeyConnectorUserDecryptionOptionsJson?, ) diff --git a/app/src/main/java/com/x8bit/bitwarden/ui/auth/feature/vaultunlock/VaultUnlockScreen.kt b/app/src/main/java/com/x8bit/bitwarden/ui/auth/feature/vaultunlock/VaultUnlockScreen.kt index d2fea85ba..e33e4de84 100644 --- a/app/src/main/java/com/x8bit/bitwarden/ui/auth/feature/vaultunlock/VaultUnlockScreen.kt +++ b/app/src/main/java/com/x8bit/bitwarden/ui/auth/feature/vaultunlock/VaultUnlockScreen.kt @@ -17,6 +17,7 @@ import androidx.compose.material3.Text import androidx.compose.material3.TopAppBarDefaults import androidx.compose.material3.rememberTopAppBarState import androidx.compose.runtime.Composable +import androidx.compose.runtime.LaunchedEffect import androidx.compose.runtime.getValue import androidx.compose.runtime.mutableStateOf import androidx.compose.runtime.remember @@ -85,6 +86,12 @@ fun VaultUnlockScreen( val context = LocalContext.current val resources = context.resources + LaunchedEffect(state.requiresBiometricsLogin) { + if (state.requiresBiometricsLogin && !biometricsManager.isBiometricsSupported) { + viewModel.trySendAction(VaultUnlockAction.BiometricsNoLongerSupported) + } + } + val onBiometricsUnlockSuccess: (cipher: Cipher?) -> Unit = remember(viewModel) { { viewModel.trySendAction(VaultUnlockAction.BiometricsUnlockSuccess(it)) } } @@ -148,6 +155,22 @@ fun VaultUnlockScreen( visibilityState = LoadingDialogState.Shown(R.string.loading.asText()), ) + VaultUnlockState.VaultUnlockDialog.BiometricsNoLongerSupported -> { + BitwardenBasicDialog( + visibilityState = BasicDialogState.Shown( + title = R.string.biometrics_no_longer_supported_title.asText(), + message = R.string.biometrics_no_longer_supported.asText(), + ), + onDismissRequest = remember { + { + viewModel.trySendAction( + VaultUnlockAction.DismissBiometricsNoLongerSupportedDialog, + ) + } + }, + ) + } + null -> Unit } diff --git a/app/src/main/java/com/x8bit/bitwarden/ui/auth/feature/vaultunlock/VaultUnlockViewModel.kt b/app/src/main/java/com/x8bit/bitwarden/ui/auth/feature/vaultunlock/VaultUnlockViewModel.kt index aefc42572..9dfaae1e1 100644 --- a/app/src/main/java/com/x8bit/bitwarden/ui/auth/feature/vaultunlock/VaultUnlockViewModel.kt +++ b/app/src/main/java/com/x8bit/bitwarden/ui/auth/feature/vaultunlock/VaultUnlockViewModel.kt @@ -95,6 +95,7 @@ class VaultUnlockViewModel @Inject constructor( fido2GetCredentialsRequest = null, // TODO: [PM-13076] Handle Fido2CredentialAssertionRequest special circumstance fido2CredentialAssertionRequest = null, + hasMasterPassword = activeAccount.hasMasterPassword, ) }, ) { @@ -138,9 +139,32 @@ class VaultUnlockViewModel @Inject constructor( is VaultUnlockAction.BiometricsUnlockSuccess -> handleBiometricsUnlockSuccess(action) VaultUnlockAction.UnlockClick -> handleUnlockClick() is VaultUnlockAction.Internal -> handleInternalAction(action) + VaultUnlockAction.BiometricsNoLongerSupported -> { + handleBiometricsNoLongerSupported() + } + + VaultUnlockAction.DismissBiometricsNoLongerSupportedDialog -> { + handleDismissBiometricsNoLongerSupportedDialog() + } } } + private fun handleBiometricsNoLongerSupported() { + mutableStateFlow.update { + it.copy( + dialog = VaultUnlockState.VaultUnlockDialog.BiometricsNoLongerSupported, + ) + } + } + + private fun handleDismissBiometricsNoLongerSupportedDialog() { + mutableStateFlow.update { + it.copy(dialog = null) + } + authRepository.logout() + authRepository.hasPendingAccountAddition = true + } + private fun handleAddAccountClick() { authRepository.hasPendingAccountAddition = true } @@ -362,6 +386,7 @@ class VaultUnlockViewModel @Inject constructor( isBiometricEnabled = userState.activeAccount.isBiometricsEnabled, vaultUnlockType = userState.activeAccount.vaultUnlockType, input = "", + hasMasterPassword = userState.activeAccount.hasMasterPassword, ) } @@ -403,6 +428,7 @@ data class VaultUnlockState( val userId: String, val fido2GetCredentialsRequest: Fido2GetCredentialsRequest? = null, val fido2CredentialAssertionRequest: Fido2CredentialAssertionRequest? = null, + private val hasMasterPassword: Boolean, ) : Parcelable { /** @@ -434,6 +460,15 @@ data class VaultUnlockState( val fido2RequestUserId: String? get() = fido2GetCredentialsRequest?.userId ?: fido2CredentialAssertionRequest?.userId + /** + * If the user requires biometrics to be able to unlock the account. + */ + val requiresBiometricsLogin: Boolean + get() = when (vaultUnlockType) { + VaultUnlockType.MASTER_PASSWORD -> !hasMasterPassword && isBiometricEnabled + VaultUnlockType.PIN -> false + } + /** * Represents the various dialogs the vault unlock screen can display. */ @@ -452,6 +487,12 @@ data class VaultUnlockState( */ @Parcelize data object Loading : VaultUnlockDialog() + + /** + * Show dialog for when biometrics the user has is no longer supported. + */ + @Parcelize + data object BiometricsNoLongerSupported : VaultUnlockDialog() } } @@ -496,6 +537,11 @@ sealed class VaultUnlockAction { */ data object DismissDialog : VaultUnlockAction() + /** + * The user has dismissed the biometrics not supported dialog + */ + data object DismissBiometricsNoLongerSupportedDialog : VaultUnlockAction() + /** * The user has clicked on the logout confirmation button. */ @@ -548,6 +594,11 @@ sealed class VaultUnlockAction { */ data object BiometricsLockOut : VaultUnlockAction() + /** + * The user has biometric unlock setup that is no longer valid. + */ + data object BiometricsNoLongerSupported : VaultUnlockAction() + /** * The user has clicked the unlock button. */ diff --git a/app/src/main/res/values/strings.xml b/app/src/main/res/values/strings.xml index 426bcbcce..f9733c142 100644 --- a/app/src/main/res/values/strings.xml +++ b/app/src/main/res/values/strings.xml @@ -1095,4 +1095,6 @@ Do you want to switch to this account? Copy email Copy phone number Copy address + Biometrics are no longer supported on this device + You’ve been logged out because your device’s biometrics don’t meet the latest security requirements. To update settings, log in once again or contact your administrator for access. diff --git a/app/src/test/java/com/x8bit/bitwarden/data/auth/datasource/disk/AuthDiskSourceTest.kt b/app/src/test/java/com/x8bit/bitwarden/data/auth/datasource/disk/AuthDiskSourceTest.kt index 716b0ae6a..e11185540 100644 --- a/app/src/test/java/com/x8bit/bitwarden/data/auth/datasource/disk/AuthDiskSourceTest.kt +++ b/app/src/test/java/com/x8bit/bitwarden/data/auth/datasource/disk/AuthDiskSourceTest.kt @@ -1266,17 +1266,17 @@ private const val USER_STATE_JSON = """ "kdfIterations": 600000, "kdfMemory": 16, "kdfParallelism": 4, - "accountDecryptionOptions": { - "HasMasterPassword": true, - "TrustedDeviceOption": { - "EncryptedPrivateKey": "encryptedPrivateKey", - "EncryptedUserKey": "encryptedUserKey", - "HasAdminApproval": true, - "HasLoginApprovingDevice": true, - "HasManageResetPasswordPermission": true + "userDecryptionOptions": { + "hasMasterPassword": true, + "trustedDeviceOption": { + "encryptedPrivateKey": "encryptedPrivateKey", + "encryptedUserKey": "encryptedUserKey", + "hasAdminApproval": true, + "hasLoginApprovingDevice": true, + "hasManageResetPasswordPermission": true }, - "KeyConnectorOption": { - "KeyConnectorUrl": "keyConnectorUrl" + "keyConnectorOption": { + "keyConnectorUrl": "keyConnectorUrl" } } }, diff --git a/app/src/test/java/com/x8bit/bitwarden/ui/auth/feature/vaultunlock/VaultUnlockScreenTest.kt b/app/src/test/java/com/x8bit/bitwarden/ui/auth/feature/vaultunlock/VaultUnlockScreenTest.kt index ef02ea960..9cdcbc1e7 100644 --- a/app/src/test/java/com/x8bit/bitwarden/ui/auth/feature/vaultunlock/VaultUnlockScreenTest.kt +++ b/app/src/test/java/com/x8bit/bitwarden/ui/auth/feature/vaultunlock/VaultUnlockScreenTest.kt @@ -541,6 +541,51 @@ class VaultUnlockScreenTest : BaseComposeTest() { .assertDoesNotExist() composeTestRule.onNodeWithText("Unlock").assertDoesNotExist() } + + @Test + fun `biometrics not supported dialog shows correctly`() { + mutableStateFlow.update { + it.copy(dialog = VaultUnlockState.VaultUnlockDialog.BiometricsNoLongerSupported) + } + composeTestRule + .onNodeWithText("Biometrics are no longer supported on this device") + .assertIsDisplayed() + } + + @Test + fun `DismissBiometricsNoLongerSupportedDialog should be sent when dialog is dismissed`() { + mutableStateFlow.update { + it.copy(dialog = VaultUnlockState.VaultUnlockDialog.BiometricsNoLongerSupported) + } + composeTestRule + .onNodeWithText("Biometrics are no longer supported on this device") + .assertIsDisplayed() + + composeTestRule + .onNodeWithText("Ok") + .performClick() + + verify { + viewModel.trySendAction(VaultUnlockAction.DismissBiometricsNoLongerSupportedDialog) + } + } + + @Suppress("MaxLineLength") + @Test + fun `when biometric is needed but no longer supported BiometricsNoLongerSupported action is sent`() { + every { biometricsManager.isBiometricsSupported } returns false + mutableStateFlow.update { + it.copy( + isBiometricEnabled = true, + hasMasterPassword = false, + vaultUnlockType = VaultUnlockType.MASTER_PASSWORD, + ) + } + composeTestRule.waitForIdle() + verify { + viewModel.trySendAction(VaultUnlockAction.BiometricsNoLongerSupported) + } + } } private const val DEFAULT_ENVIRONMENT_URL: String = "vault.bitwarden.com" @@ -588,4 +633,5 @@ private val DEFAULT_STATE: VaultUnlockState = VaultUnlockState( showBiometricInvalidatedMessage = false, userId = ACTIVE_ACCOUNT_SUMMARY.userId, vaultUnlockType = VaultUnlockType.MASTER_PASSWORD, + hasMasterPassword = true, ) diff --git a/app/src/test/java/com/x8bit/bitwarden/ui/auth/feature/vaultunlock/VaultUnlockViewModelTest.kt b/app/src/test/java/com/x8bit/bitwarden/ui/auth/feature/vaultunlock/VaultUnlockViewModelTest.kt index 09d69a725..aa44e1839 100644 --- a/app/src/test/java/com/x8bit/bitwarden/ui/auth/feature/vaultunlock/VaultUnlockViewModelTest.kt +++ b/app/src/test/java/com/x8bit/bitwarden/ui/auth/feature/vaultunlock/VaultUnlockViewModelTest.kt @@ -1227,6 +1227,35 @@ class VaultUnlockViewModelTest : BaseViewModelTest() { verify { fido2CredentialManager.isUserVerified = false } } + @Test + fun `on BiometricsNoLongerSupported should show correct dialog state`() { + val viewModel = createViewModel() + viewModel.trySendAction(VaultUnlockAction.BiometricsNoLongerSupported) + assertEquals( + DEFAULT_STATE.copy( + dialog = VaultUnlockState.VaultUnlockDialog.BiometricsNoLongerSupported, + ), + viewModel.stateFlow.value, + ) + } + + @Suppress("MaxLineLength") + @Test + fun `on DismissBiometricsNoLongerSupportedDialog should dismiss dialog state and log the user out`() { + val viewModel = createViewModel() + viewModel.trySendAction(VaultUnlockAction.DismissBiometricsNoLongerSupportedDialog) + assertEquals( + DEFAULT_STATE.copy( + dialog = null, + ), + viewModel.stateFlow.value, + ) + verify(exactly = 1) { + authRepository.logout() + authRepository.hasPendingAccountAddition = true + } + } + private fun createViewModel( state: VaultUnlockState? = null, unlockType: UnlockType = UnlockType.STANDARD, @@ -1275,6 +1304,7 @@ private val DEFAULT_STATE: VaultUnlockState = VaultUnlockState( showBiometricInvalidatedMessage = false, userId = USER_ID, vaultUnlockType = VaultUnlockType.MASTER_PASSWORD, + hasMasterPassword = true, ) private val TRUSTED_DEVICE: UserState.TrustedDevice = UserState.TrustedDevice(