Skip to content

Commit

Permalink
PM-15062 prevent account lockout when biometrics is only unlock metho…
Browse files Browse the repository at this point in the history
…d and no longer supported class. (#4341)
  • Loading branch information
dseverns-livefront authored Nov 20, 2024
1 parent 1321034 commit a1881ce
Show file tree
Hide file tree
Showing 10 changed files with 195 additions and 20 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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")
Expand Down Expand Up @@ -86,7 +89,8 @@ data class AccountJson(
@SerialName("kdfParallelism")
val kdfParallelism: Int?,

@SerialName("accountDecryptionOptions")
@SerialName("userDecryptionOptions")
@JsonNames("accountDecryptionOptions")
val userDecryptionOptions: UserDecryptionOptionsJson?,
)

Expand Down
Original file line number Diff line number Diff line change
@@ -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,
)
Original file line number Diff line number Diff line change
@@ -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.
Expand All @@ -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,
)
Original file line number Diff line number Diff line change
@@ -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.
Expand All @@ -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?,
)
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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)) }
}
Expand Down Expand Up @@ -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
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -95,6 +95,7 @@ class VaultUnlockViewModel @Inject constructor(
fido2GetCredentialsRequest = null,
// TODO: [PM-13076] Handle Fido2CredentialAssertionRequest special circumstance
fido2CredentialAssertionRequest = null,
hasMasterPassword = activeAccount.hasMasterPassword,
)
},
) {
Expand Down Expand Up @@ -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
}
Expand Down Expand Up @@ -362,6 +386,7 @@ class VaultUnlockViewModel @Inject constructor(
isBiometricEnabled = userState.activeAccount.isBiometricsEnabled,
vaultUnlockType = userState.activeAccount.vaultUnlockType,
input = "",
hasMasterPassword = userState.activeAccount.hasMasterPassword,
)
}

Expand Down Expand Up @@ -403,6 +428,7 @@ data class VaultUnlockState(
val userId: String,
val fido2GetCredentialsRequest: Fido2GetCredentialsRequest? = null,
val fido2CredentialAssertionRequest: Fido2CredentialAssertionRequest? = null,
private val hasMasterPassword: Boolean,
) : Parcelable {

/**
Expand Down Expand Up @@ -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.
*/
Expand All @@ -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()
}
}

Expand Down Expand Up @@ -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.
*/
Expand Down Expand Up @@ -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.
*/
Expand Down
3 changes: 3 additions & 0 deletions app/src/main/res/values/strings.xml
Original file line number Diff line number Diff line change
Expand Up @@ -1087,4 +1087,7 @@ Do you want to switch to this account?</string>
<string name="bitwarden_can_notify_you_each_time_you_receive_a_new_login_request_from_another_device">Bitwarden can notify you each time you receive a new login request from another device.</string>
<string name="skip_for_now">Skip for now</string>
<string name="done_text">Done</string>
<string name="biometrics_no_longer_supported_title">Biometrics are no longer supported on this device</string>
<string name="biometrics_no_longer_supported">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.</string>

</resources>
Original file line number Diff line number Diff line change
Expand Up @@ -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"
}
}
},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -588,4 +633,5 @@ private val DEFAULT_STATE: VaultUnlockState = VaultUnlockState(
showBiometricInvalidatedMessage = false,
userId = ACTIVE_ACCOUNT_SUMMARY.userId,
vaultUnlockType = VaultUnlockType.MASTER_PASSWORD,
hasMasterPassword = true,
)
Loading

0 comments on commit a1881ce

Please sign in to comment.