From f2301e15b93fa28b3302dad377dd9ac465b1bfc9 Mon Sep 17 00:00:00 2001 From: Caleb Derosier <125901828+caleb-livefront@users.noreply.github.com> Date: Thu, 11 Apr 2024 14:11:05 -0600 Subject: [PATCH] BIT-2232: Don't require password for PIN setup for users w/o password (#1252) --- .../data/auth/repository/model/UserState.kt | 4 +- .../accountsecurity/AccountSecurityScreen.kt | 19 ++++++++-- .../AccountSecurityViewModel.kt | 7 ++++ .../AccountSecurityScreenTest.kt | 38 ++++++++++++++++++- .../AccountSecurityViewModelTest.kt | 29 +++++++++++++- 5 files changed, 90 insertions(+), 7 deletions(-) diff --git a/app/src/main/java/com/x8bit/bitwarden/data/auth/repository/model/UserState.kt b/app/src/main/java/com/x8bit/bitwarden/data/auth/repository/model/UserState.kt index e38cb1d3ef..d8ce09016c 100644 --- a/app/src/main/java/com/x8bit/bitwarden/data/auth/repository/model/UserState.kt +++ b/app/src/main/java/com/x8bit/bitwarden/data/auth/repository/model/UserState.kt @@ -42,7 +42,9 @@ data class UserState( * @property isVaultUnlocked Whether or not the user's vault is currently unlocked. * @property needsPasswordReset If the user needs to reset their password. * @property needsMasterPassword Indicates whether the user needs to create a password (e.g. - * they logged in using SSO and don't yet have one). + * they logged in using SSO and don't yet have one). NOTE: This should **not** be used to + * determine whether a user has a master password. There are cases in which a user can both + * not have a password but still not need one, such as TDE. * @property organizations List of [Organization]s the user is associated with, if any. * @property isBiometricsEnabled Indicates that the biometrics mechanism for unlocking the * user's vault is enabled. diff --git a/app/src/main/java/com/x8bit/bitwarden/ui/platform/feature/settings/accountsecurity/AccountSecurityScreen.kt b/app/src/main/java/com/x8bit/bitwarden/ui/platform/feature/settings/accountsecurity/AccountSecurityScreen.kt index 5c10527b44..822346226c 100644 --- a/app/src/main/java/com/x8bit/bitwarden/ui/platform/feature/settings/accountsecurity/AccountSecurityScreen.kt +++ b/app/src/main/java/com/x8bit/bitwarden/ui/platform/feature/settings/accountsecurity/AccountSecurityScreen.kt @@ -206,6 +206,7 @@ fun AccountSecurityScreen( .padding(horizontal = 16.dp), ) UnlockWithPinRow( + isUnlockWithPasswordEnabled = state.isUnlockWithPasswordEnabled, isUnlockWithPinEnabled = state.isUnlockWithPinEnabled, onUnlockWithPinToggleAction = remember(viewModel) { { viewModel.trySendAction(it) } @@ -400,6 +401,7 @@ private fun UnlockWithBiometricsRow( @Suppress("LongMethod") @Composable private fun UnlockWithPinRow( + isUnlockWithPasswordEnabled: Boolean, isUnlockWithPinEnabled: Boolean, onUnlockWithPinToggleAction: (AccountSecurityAction.UnlockWithPinToggle) -> Unit, modifier: Modifier = Modifier, @@ -440,10 +442,19 @@ private fun UnlockWithPinRow( onSubmitClick = { if (pin.isNotEmpty()) { shouldShowPinInputDialog = false - shouldShowPinConfirmationDialog = true - onUnlockWithPinToggleAction( - AccountSecurityAction.UnlockWithPinToggle.PendingEnabled, - ) + if (isUnlockWithPasswordEnabled) { + shouldShowPinConfirmationDialog = true + onUnlockWithPinToggleAction( + AccountSecurityAction.UnlockWithPinToggle.PendingEnabled, + ) + } else { + onUnlockWithPinToggleAction( + AccountSecurityAction.UnlockWithPinToggle.Enabled( + pin = pin, + shouldRequireMasterPasswordOnRestart = false, + ), + ) + } } else { shouldShowPinInputDialog = false onUnlockWithPinToggleAction( diff --git a/app/src/main/java/com/x8bit/bitwarden/ui/platform/feature/settings/accountsecurity/AccountSecurityViewModel.kt b/app/src/main/java/com/x8bit/bitwarden/ui/platform/feature/settings/accountsecurity/AccountSecurityViewModel.kt index 9c43778398..e434b026cd 100644 --- a/app/src/main/java/com/x8bit/bitwarden/ui/platform/feature/settings/accountsecurity/AccountSecurityViewModel.kt +++ b/app/src/main/java/com/x8bit/bitwarden/ui/platform/feature/settings/accountsecurity/AccountSecurityViewModel.kt @@ -50,6 +50,12 @@ class AccountSecurityViewModel @Inject constructor( fingerprintPhrase = "".asText(), // This will be filled in dynamically isApproveLoginRequestsEnabled = settingsRepository.isApprovePasswordlessLoginsEnabled, isUnlockWithBiometricsEnabled = settingsRepository.isUnlockWithBiometricsEnabled, + isUnlockWithPasswordEnabled = authRepository + .userStateFlow + .value + ?.activeAccount + ?.trustedDevice + ?.hasMasterPassword != false, isUnlockWithPinEnabled = settingsRepository.isUnlockWithPinEnabled, vaultTimeout = settingsRepository.vaultTimeout, vaultTimeoutAction = settingsRepository.vaultTimeoutAction, @@ -357,6 +363,7 @@ data class AccountSecurityState( val fingerprintPhrase: Text, val isApproveLoginRequestsEnabled: Boolean, val isUnlockWithBiometricsEnabled: Boolean, + val isUnlockWithPasswordEnabled: Boolean, val isUnlockWithPinEnabled: Boolean, val vaultTimeout: VaultTimeout, val vaultTimeoutAction: VaultTimeoutAction, diff --git a/app/src/test/java/com/x8bit/bitwarden/ui/platform/feature/settings/accountsecurity/AccountSecurityScreenTest.kt b/app/src/test/java/com/x8bit/bitwarden/ui/platform/feature/settings/accountsecurity/AccountSecurityScreenTest.kt index dae632dbbe..a2672bc5c3 100644 --- a/app/src/test/java/com/x8bit/bitwarden/ui/platform/feature/settings/accountsecurity/AccountSecurityScreenTest.kt +++ b/app/src/test/java/com/x8bit/bitwarden/ui/platform/feature/settings/accountsecurity/AccountSecurityScreenTest.kt @@ -517,7 +517,7 @@ class AccountSecurityScreenTest : BaseComposeTest() { @Suppress("MaxLineLength") @Test - fun `PIN input dialog Submit click with non-empty pin should show a confirmation dialog and send UnlockWithPinToggle PendingEnabled`() { + fun `PIN input dialog Submit click with non-empty pin and isUnlockWithPasswordEnabled true should show a confirmation dialog and send UnlockWithPinToggle PendingEnabled`() { mutableStateFlow.update { it.copy(isUnlockWithPinEnabled = false) } @@ -558,6 +558,41 @@ class AccountSecurityScreenTest : BaseComposeTest() { verify { viewModel.trySendAction(AccountSecurityAction.UnlockWithPinToggle.PendingEnabled) } } + @Suppress("MaxLineLength") + @Test + fun `PIN input dialog Submit click with non-empty pin and isUnlockWithPasswordEnabled false should show a confirmation dialog and send UnlockWithPinToggle Enabled`() { + mutableStateFlow.update { + it.copy( + isUnlockWithPinEnabled = false, + isUnlockWithPasswordEnabled = false, + ) + } + composeTestRule + .onNodeWithText("Unlock with PIN code") + .performScrollTo() + .performClick() + + composeTestRule + .onAllNodesWithText("PIN") + .filterToOne(hasAnyAncestor(isDialog())) + .performTextInput("1234") + composeTestRule + .onAllNodesWithText("Submit") + .filterToOne(hasAnyAncestor(isDialog())) + .performClick() + + composeTestRule.assertNoDialogExists() + + verify { + viewModel.trySendAction( + AccountSecurityAction.UnlockWithPinToggle.Enabled( + pin = "1234", + shouldRequireMasterPasswordOnRestart = false, + ), + ) + } + } + @Suppress("MaxLineLength") @Test fun `PIN confirmation dialog No click should send UnlockWithPinToggle Enabled and close the dialog`() { @@ -1482,6 +1517,7 @@ class AccountSecurityScreenTest : BaseComposeTest() { fingerprintPhrase = "fingerprint-placeholder".asText(), isApproveLoginRequestsEnabled = false, isUnlockWithBiometricsEnabled = false, + isUnlockWithPasswordEnabled = true, isUnlockWithPinEnabled = false, vaultTimeout = VaultTimeout.ThirtyMinutes, vaultTimeoutAction = VaultTimeoutAction.LOCK, diff --git a/app/src/test/java/com/x8bit/bitwarden/ui/platform/feature/settings/accountsecurity/AccountSecurityViewModelTest.kt b/app/src/test/java/com/x8bit/bitwarden/ui/platform/feature/settings/accountsecurity/AccountSecurityViewModelTest.kt index 107c9e097a..e848b506c1 100644 --- a/app/src/test/java/com/x8bit/bitwarden/ui/platform/feature/settings/accountsecurity/AccountSecurityViewModelTest.kt +++ b/app/src/test/java/com/x8bit/bitwarden/ui/platform/feature/settings/accountsecurity/AccountSecurityViewModelTest.kt @@ -6,6 +6,7 @@ import com.x8bit.bitwarden.R import com.x8bit.bitwarden.data.auth.repository.AuthRepository import com.x8bit.bitwarden.data.auth.repository.model.PolicyInformation import com.x8bit.bitwarden.data.auth.repository.model.UserFingerprintResult +import com.x8bit.bitwarden.data.auth.repository.model.UserState import com.x8bit.bitwarden.data.platform.manager.PolicyManager import com.x8bit.bitwarden.data.platform.repository.EnvironmentRepository import com.x8bit.bitwarden.data.platform.repository.SettingsRepository @@ -28,6 +29,7 @@ import io.mockk.just import io.mockk.mockk import io.mockk.runs import io.mockk.verify +import kotlinx.coroutines.flow.MutableStateFlow import kotlinx.coroutines.test.runTest import kotlinx.serialization.json.Json import kotlinx.serialization.json.encodeToJsonElement @@ -40,7 +42,10 @@ import org.junit.jupiter.api.Test class AccountSecurityViewModelTest : BaseViewModelTest() { private val fakeEnvironmentRepository = FakeEnvironmentRepository() - private val authRepository: AuthRepository = mockk(relaxed = true) + private val mutableUserStateFlow = MutableStateFlow(DEFAULT_USER_STATE) + private val authRepository: AuthRepository = mockk(relaxed = true) { + every { userStateFlow } returns mutableUserStateFlow + } private val vaultRepository: VaultRepository = mockk(relaxed = true) private val settingsRepository: SettingsRepository = mockk { every { isUnlockWithBiometricsEnabled } returns false @@ -559,9 +564,31 @@ private val DEFAULT_STATE: AccountSecurityState = AccountSecurityState( fingerprintPhrase = FINGERPRINT.asText(), isApproveLoginRequestsEnabled = false, isUnlockWithBiometricsEnabled = false, + isUnlockWithPasswordEnabled = true, isUnlockWithPinEnabled = false, vaultTimeout = VaultTimeout.ThirtyMinutes, vaultTimeoutAction = VaultTimeoutAction.LOCK, vaultTimeoutPolicyMinutes = null, vaultTimeoutPolicyAction = null, ) + +private val DEFAULT_USER_STATE = UserState( + activeUserId = "activeUserId", + accounts = listOf( + UserState.Account( + userId = "activeUserId", + name = "Active User", + email = "active@bitwarden.com", + avatarColorHex = "#aa00aa", + environment = Environment.Us, + isPremium = true, + isLoggedIn = true, + isVaultUnlocked = true, + needsPasswordReset = false, + isBiometricsEnabled = false, + organizations = emptyList(), + needsMasterPassword = false, + trustedDevice = null, + ), + ), +)