diff --git a/app/src/main/java/com/x8bit/bitwarden/ui/platform/components/row/BitwardenTextRow.kt b/app/src/main/java/com/x8bit/bitwarden/ui/platform/components/row/BitwardenTextRow.kt index a93221f828..376f95199a 100644 --- a/app/src/main/java/com/x8bit/bitwarden/ui/platform/components/row/BitwardenTextRow.kt +++ b/app/src/main/java/com/x8bit/bitwarden/ui/platform/components/row/BitwardenTextRow.kt @@ -28,6 +28,8 @@ import androidx.compose.ui.unit.dp * @param onClick The callback when the row is clicked. * @param modifier The modifier to be applied to the layout. * @param description An optional description label to be displayed below the [text]. + * @param isEnabled Indicates if the row is enabled or not, a disabled row will not be clickable + * and it's contents will be dimmed. * @param withDivider Indicates if a divider should be drawn on the bottom of the row, defaults * to `false`. * @param content The content of the [BitwardenTextRow]. @@ -38,6 +40,7 @@ fun BitwardenTextRow( onClick: () -> Unit, modifier: Modifier = Modifier, description: String? = null, + isEnabled: Boolean = true, withDivider: Boolean = false, content: (@Composable () -> Unit)? = null, ) { @@ -45,6 +48,7 @@ fun BitwardenTextRow( contentAlignment = Alignment.BottomCenter, modifier = modifier .clickable( + enabled = isEnabled, interactionSource = remember { MutableInteractionSource() }, indication = rememberRipple(color = MaterialTheme.colorScheme.primary), onClick = onClick, @@ -67,13 +71,17 @@ fun BitwardenTextRow( Text( text = text, style = MaterialTheme.typography.bodyLarge, - color = MaterialTheme.colorScheme.onSurface, + color = MaterialTheme.colorScheme.onSurface.copy( + alpha = if (isEnabled) 1.0f else 0.38f, + ), ) description?.let { Text( text = it, style = MaterialTheme.typography.bodyMedium, - color = MaterialTheme.colorScheme.onSurfaceVariant, + color = MaterialTheme.colorScheme.onSurfaceVariant.copy( + alpha = if (isEnabled) 1.0f else 0.38f, + ), ) } } 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 822346226c..39b68f363c 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 @@ -255,6 +255,7 @@ fun AccountSecurityScreen( ) } SessionTimeoutActionRow( + isEnabled = state.hasUnlockMechanism, vaultTimeoutPolicyAction = state.vaultTimeoutPolicyAction, selectedVaultTimeoutAction = state.vaultTimeoutAction, onVaultTimeoutActionSelect = remember(viewModel) { @@ -293,27 +294,31 @@ fun AccountSecurityScreen( .semantics { testTag = "TwoStepLoginLinkItemView" } .fillMaxWidth(), ) - BitwardenExternalLinkRow( - text = stringResource(id = R.string.change_master_password), - onConfirmClick = remember(viewModel) { - { viewModel.trySendAction(AccountSecurityAction.ChangeMasterPasswordClick) } - }, - withDivider = false, - dialogTitle = stringResource(id = R.string.continue_to_web_app), - dialogMessage = stringResource( - id = R.string.change_master_password_description_long, - ), - modifier = Modifier.fillMaxWidth(), - ) - BitwardenTextRow( - text = stringResource(id = R.string.lock_now), - onClick = remember(viewModel) { - { viewModel.trySendAction(AccountSecurityAction.LockNowClick) } - }, - modifier = Modifier - .semantics { testTag = "LockNowLabel" } - .fillMaxWidth(), - ) + if (state.isUnlockWithPasswordEnabled) { + BitwardenExternalLinkRow( + text = stringResource(id = R.string.change_master_password), + onConfirmClick = remember(viewModel) { + { viewModel.trySendAction(AccountSecurityAction.ChangeMasterPasswordClick) } + }, + withDivider = false, + dialogTitle = stringResource(id = R.string.continue_to_web_app), + dialogMessage = stringResource( + id = R.string.change_master_password_description_long, + ), + modifier = Modifier.fillMaxWidth(), + ) + } + if (state.hasUnlockMechanism) { + BitwardenTextRow( + text = stringResource(id = R.string.lock_now), + onClick = remember(viewModel) { + { viewModel.trySendAction(AccountSecurityAction.LockNowClick) } + }, + modifier = Modifier + .semantics { testTag = "LockNowLabel" } + .fillMaxWidth(), + ) + } BitwardenTextRow( text = stringResource(id = R.string.log_out), onClick = remember(viewModel) { @@ -695,6 +700,7 @@ private fun SessionCustomTimeoutRow( @Suppress("LongMethod") @Composable private fun SessionTimeoutActionRow( + isEnabled: Boolean, vaultTimeoutPolicyAction: String?, selectedVaultTimeoutAction: VaultTimeoutAction, onVaultTimeoutActionSelect: (VaultTimeoutAction) -> Unit, @@ -703,7 +709,12 @@ private fun SessionTimeoutActionRow( var shouldShowSelectionDialog by rememberSaveable { mutableStateOf(false) } var shouldShowLogoutActionConfirmationDialog by rememberSaveable { mutableStateOf(false) } BitwardenTextRow( + isEnabled = isEnabled, text = stringResource(id = R.string.session_timeout_action), + description = stringResource( + id = R.string.set_up_an_unlock_option_to_change_your_vault_timeout_action, + ) + .takeUnless { isEnabled }, onClick = { // The option is not selectable if there's a policy in place. if (vaultTimeoutPolicyAction != null) return@BitwardenTextRow @@ -714,7 +725,9 @@ private fun SessionTimeoutActionRow( Text( text = selectedVaultTimeoutAction.displayLabel(), style = MaterialTheme.typography.labelSmall, - color = MaterialTheme.colorScheme.onSurfaceVariant, + color = MaterialTheme.colorScheme.onSurfaceVariant.copy( + alpha = if (isEnabled) 1.0f else 0.38f, + ), modifier = Modifier.semantics { testTag = "SessionTimeoutActionStatusLabel" }, ) } @@ -731,11 +744,11 @@ private fun SessionTimeoutActionRow( isSelected = option == selectedVaultTimeoutAction, onClick = { shouldShowSelectionDialog = false - val seletedAction = vaultTimeoutActionOptions.first { it == option } - if (seletedAction == VaultTimeoutAction.LOGOUT) { + val selectedAction = vaultTimeoutActionOptions.first { it == option } + if (selectedAction == VaultTimeoutAction.LOGOUT) { shouldShowLogoutActionConfirmationDialog = true } else { - onVaultTimeoutActionSelect(seletedAction) + onVaultTimeoutActionSelect(selectedAction) } }, ) 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 e434b026cd..d21ac3851d 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 @@ -231,13 +231,7 @@ class AccountSecurityViewModel @Inject constructor( private fun handleVaultTimeoutActionSelect( action: AccountSecurityAction.VaultTimeoutActionSelect, ) { - val vaultTimeoutAction = action.vaultTimeoutAction - mutableStateFlow.update { - it.copy( - vaultTimeoutAction = action.vaultTimeoutAction, - ) - } - settingsRepository.vaultTimeoutAction = vaultTimeoutAction + setVaultTimeoutAction(action.vaultTimeoutAction) } private fun handleTwoStepLoginClick() { @@ -261,6 +255,7 @@ class AccountSecurityViewModel @Inject constructor( } else { settingsRepository.clearBiometricsKey() mutableStateFlow.update { it.copy(isUnlockWithBiometricsEnabled = false) } + validateVaultTimeoutAction() } } @@ -273,6 +268,7 @@ class AccountSecurityViewModel @Inject constructor( AccountSecurityAction.UnlockWithPinToggle.PendingEnabled -> Unit AccountSecurityAction.UnlockWithPinToggle.Disabled -> { settingsRepository.clearUnlockPin() + validateVaultTimeoutAction() } is AccountSecurityAction.UnlockWithPinToggle.Enabled -> { @@ -352,6 +348,17 @@ class AccountSecurityViewModel @Inject constructor( ) } } + + private fun validateVaultTimeoutAction() { + if (!state.hasUnlockMechanism) { + setVaultTimeoutAction(VaultTimeoutAction.LOGOUT) + } + } + + private fun setVaultTimeoutAction(vaultTimeoutAction: VaultTimeoutAction) { + mutableStateFlow.update { it.copy(vaultTimeoutAction = vaultTimeoutAction) } + settingsRepository.vaultTimeoutAction = vaultTimeoutAction + } } /** @@ -369,7 +376,15 @@ data class AccountSecurityState( val vaultTimeoutAction: VaultTimeoutAction, val vaultTimeoutPolicyMinutes: Int?, val vaultTimeoutPolicyAction: String?, -) : Parcelable +) : Parcelable { + /** + * Indicates that there is a mechanism for unlocking your vault in place. + */ + val hasUnlockMechanism: Boolean + get() = isUnlockWithPasswordEnabled || + isUnlockWithPinEnabled || + isUnlockWithBiometricsEnabled +} /** * Representation of the dialogs that can be displayed on account security screen. 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 a2672bc5c3..06e3a5f947 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 @@ -1511,6 +1511,62 @@ class AccountSecurityScreenTest : BaseComposeTest() { .assert(hasAnyAncestor(isDialog())) } + @Test + fun `change master password row should be displayed according to state`() { + val rowText = "Change master password" + composeTestRule.onNodeWithText(rowText).performScrollTo().assertIsDisplayed() + + mutableStateFlow.update { it.copy(isUnlockWithPasswordEnabled = false) } + + composeTestRule.onNodeWithText(rowText).assertDoesNotExist() + } + + @Test + fun `lock now row should be displayed according to state`() { + val rowText = "Lock now" + composeTestRule.onNodeWithText(rowText).performScrollTo().assertIsDisplayed() + + mutableStateFlow.update { + it.copy( + isUnlockWithBiometricsEnabled = true, + isUnlockWithPasswordEnabled = false, + isUnlockWithPinEnabled = false, + ) + } + + composeTestRule.onNodeWithText(rowText).performScrollTo().assertIsDisplayed() + + mutableStateFlow.update { + it.copy( + isUnlockWithBiometricsEnabled = false, + isUnlockWithPasswordEnabled = true, + isUnlockWithPinEnabled = false, + ) + } + + composeTestRule.onNodeWithText(rowText).performScrollTo().assertIsDisplayed() + + mutableStateFlow.update { + it.copy( + isUnlockWithBiometricsEnabled = false, + isUnlockWithPasswordEnabled = false, + isUnlockWithPinEnabled = true, + ) + } + + composeTestRule.onNodeWithText(rowText).performScrollTo().assertIsDisplayed() + + mutableStateFlow.update { + it.copy( + isUnlockWithBiometricsEnabled = false, + isUnlockWithPasswordEnabled = false, + isUnlockWithPinEnabled = false, + ) + } + + composeTestRule.onNodeWithText(rowText).assertDoesNotExist() + } + companion object { private val DEFAULT_STATE = AccountSecurityState( dialog = null, 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 e848b506c1..bfa888a6a4 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 @@ -329,6 +329,35 @@ class AccountSecurityViewModelTest : BaseViewModelTest() { } } + @Suppress("MaxLineLength") + @Test + fun `on UnlockWithBiometricToggle false should call clearBiometricsKey, reset the vaultTimeoutAction, and update the state`() = + runTest { + val initialState = DEFAULT_STATE.copy( + isUnlockWithPasswordEnabled = false, + isUnlockWithBiometricsEnabled = true, + ) + every { settingsRepository.isUnlockWithBiometricsEnabled } returns true + every { settingsRepository.clearBiometricsKey() } just runs + every { settingsRepository.vaultTimeoutAction = VaultTimeoutAction.LOGOUT } just runs + val viewModel = createViewModel(initialState) + assertEquals(initialState, viewModel.stateFlow.value) + + viewModel.trySendAction(AccountSecurityAction.UnlockWithBiometricToggle(false)) + + assertEquals( + initialState.copy( + isUnlockWithBiometricsEnabled = false, + vaultTimeoutAction = VaultTimeoutAction.LOGOUT, + ), + viewModel.stateFlow.value, + ) + verify(exactly = 1) { + settingsRepository.clearBiometricsKey() + settingsRepository.vaultTimeoutAction = VaultTimeoutAction.LOGOUT + } + } + @Suppress("MaxLineLength") @Test fun `on UnlockWithBiometricToggle true and setupBiometricsKey error should call update the state accordingly`() = @@ -408,6 +437,31 @@ class AccountSecurityViewModelTest : BaseViewModelTest() { } @Suppress("MaxLineLength") + @Test + fun `on UnlockWithPinToggle Disabled should set pin unlock to false, reset the vaultTimeoutAction, and clear the PIN in settings`() { + val initialState = DEFAULT_STATE.copy( + isUnlockWithPasswordEnabled = false, + isUnlockWithPinEnabled = true, + ) + every { settingsRepository.clearUnlockPin() } just runs + every { settingsRepository.vaultTimeoutAction = VaultTimeoutAction.LOGOUT } just runs + val viewModel = createViewModel(initialState = initialState) + viewModel.trySendAction( + AccountSecurityAction.UnlockWithPinToggle.Disabled, + ) + assertEquals( + initialState.copy( + vaultTimeoutAction = VaultTimeoutAction.LOGOUT, + isUnlockWithPinEnabled = false, + ), + viewModel.stateFlow.value, + ) + verify { + settingsRepository.clearUnlockPin() + settingsRepository.vaultTimeoutAction = VaultTimeoutAction.LOGOUT + } + } + @Test fun `on UnlockWithPinToggle PendingEnabled should set pin unlock to true`() { val initialState = DEFAULT_STATE.copy(