Save state of approving passwordless logins setting and setup initial UI (#644)

This commit is contained in:
Sean Weiser 2024-01-18 09:43:22 -06:00 committed by Álison Fernandes
parent a12bc47c20
commit fed6b74800
11 changed files with 419 additions and 36 deletions

View file

@ -115,4 +115,17 @@ interface SettingsDiskSource {
userId: String,
blockedAutofillUris: List<String>?,
)
/**
* Gets whether or not the given [userId] has enabled approving passwordless logins.
*/
fun getApprovePasswordlessLoginsEnabled(userId: String): Boolean?
/**
* Stores whether or not [isApprovePasswordlessLoginsEnabled] for the given [userId].
*/
fun storeApprovePasswordlessLoginsEnabled(
userId: String,
isApprovePasswordlessLoginsEnabled: Boolean?,
)
}

View file

@ -20,6 +20,7 @@ private const val BLOCKED_AUTOFILL_URIS_KEY = "$BASE_KEY:autofillBlacklistedUris
private const val VAULT_TIMEOUT_ACTION_KEY = "$BASE_KEY:vaultTimeoutAction"
private const val VAULT_TIME_IN_MINUTES_KEY = "$BASE_KEY:vaultTimeout"
private const val DISABLE_ICON_LOADING_KEY = "$BASE_KEY:disableFavicon"
private const val APPROVE_PASSWORDLESS_LOGINS_KEY = "$BASE_KEY:approvePasswordlessLogins"
/**
* Primary implementation of [SettingsDiskSource].
@ -92,6 +93,10 @@ class SettingsDiskSourceImpl(
storePullToRefreshEnabled(userId = userId, isPullToRefreshEnabled = null)
storeInlineAutofillEnabled(userId = userId, isInlineAutofillEnabled = null)
storeBlockedAutofillUris(userId = userId, blockedAutofillUris = null)
storeApprovePasswordlessLoginsEnabled(
userId = userId,
isApprovePasswordlessLoginsEnabled = null,
)
}
override fun getVaultTimeoutInMinutes(userId: String): Int? =
@ -192,4 +197,18 @@ class SettingsDiskSourceImpl(
mutablePullToRefreshEnabledFlowMap.getOrPut(userId) {
bufferedMutableSharedFlow(replay = 1)
}
override fun getApprovePasswordlessLoginsEnabled(userId: String): Boolean? {
return getBoolean(key = "${APPROVE_PASSWORDLESS_LOGINS_KEY}_$userId")
}
override fun storeApprovePasswordlessLoginsEnabled(
userId: String,
isApprovePasswordlessLoginsEnabled: Boolean?,
) {
putBoolean(
key = "${APPROVE_PASSWORDLESS_LOGINS_KEY}_$userId",
value = isApprovePasswordlessLoginsEnabled,
)
}
}

View file

@ -62,6 +62,11 @@ interface SettingsRepository {
*/
var blockedAutofillUris: List<String>
/**
* Whether or not approving passwordless logins is enabled for the current user.
*/
var isApprovePasswordlessLoginsEnabled: Boolean
/**
* Sets default values for various settings for the given [userId] if necessary. This is
* typically used when logging into a new account.

View file

@ -121,6 +121,20 @@ class SettingsRepositoryImpl(
)
}
override var isApprovePasswordlessLoginsEnabled: Boolean
get() = activeUserId
?.let {
settingsDiskSource.getApprovePasswordlessLoginsEnabled(it)
}
?: false
set(value) {
val userId = activeUserId ?: return
settingsDiskSource.storeApprovePasswordlessLoginsEnabled(
userId = userId,
isApprovePasswordlessLoginsEnabled = value,
)
}
override fun setDefaultsIfNecessary(userId: String) {
// Set Vault Settings defaults
if (!isVaultTimeoutActionSet(userId = userId)) {

View file

@ -139,13 +139,10 @@ fun AccountSecurityScreen(
.fillMaxWidth()
.padding(horizontal = 16.dp),
)
BitwardenWideSwitch(
label = stringResource(
id = R.string.use_this_device_to_approve_login_requests_made_from_other_devices,
),
isChecked = state.isApproveLoginRequestsEnabled,
onCheckedChange = remember(viewModel) {
{ viewModel.trySendAction(AccountSecurityAction.LoginRequestToggle(it)) }
ApprovePasswordlessLoginsRow(
isApproveLoginRequestsEnabled = state.isApproveLoginRequestsEnabled,
onApprovePasswordlessLoginsAction = remember(viewModel) {
{ viewModel.trySendAction(it) }
},
modifier = Modifier
.fillMaxWidth()
@ -617,3 +614,61 @@ private fun FingerPrintPhraseDialog(
containerColor = MaterialTheme.colorScheme.surfaceContainerHigh,
)
}
@Composable
private fun ApprovePasswordlessLoginsRow(
isApproveLoginRequestsEnabled: Boolean,
@Suppress("MaxLineLength")
onApprovePasswordlessLoginsAction: (AccountSecurityAction.ApprovePasswordlessLoginsToggle) -> Unit,
modifier: Modifier = Modifier,
) {
var shouldShowConfirmationDialog by remember { mutableStateOf(false) }
BitwardenWideSwitch(
label = stringResource(
id = R.string.use_this_device_to_approve_login_requests_made_from_other_devices,
),
isChecked = isApproveLoginRequestsEnabled,
onCheckedChange = { isChecked ->
if (isChecked) {
onApprovePasswordlessLoginsAction(
AccountSecurityAction.ApprovePasswordlessLoginsToggle.PendingEnabled,
)
shouldShowConfirmationDialog = true
} else {
onApprovePasswordlessLoginsAction(
AccountSecurityAction.ApprovePasswordlessLoginsToggle.Disabled,
)
}
},
modifier = modifier,
)
if (shouldShowConfirmationDialog) {
BitwardenTwoButtonDialog(
title = stringResource(id = R.string.approve_login_requests),
message = stringResource(
id = R.string.use_this_device_to_approve_login_requests_made_from_other_devices,
),
confirmButtonText = stringResource(id = R.string.yes),
dismissButtonText = stringResource(id = R.string.no),
onConfirmClick = {
onApprovePasswordlessLoginsAction(
AccountSecurityAction.ApprovePasswordlessLoginsToggle.Enabled,
)
shouldShowConfirmationDialog = false
},
onDismissClick = {
onApprovePasswordlessLoginsAction(
AccountSecurityAction.ApprovePasswordlessLoginsToggle.Disabled,
)
shouldShowConfirmationDialog = false
},
onDismissRequest = {
onApprovePasswordlessLoginsAction(
AccountSecurityAction.ApprovePasswordlessLoginsToggle.Disabled,
)
shouldShowConfirmationDialog = false
},
)
}
}

View file

@ -36,7 +36,7 @@ class AccountSecurityViewModel @Inject constructor(
?: AccountSecurityState(
dialog = null,
fingerprintPhrase = "fingerprint-placeholder".asText(),
isApproveLoginRequestsEnabled = false,
isApproveLoginRequestsEnabled = settingsRepository.isApprovePasswordlessLoginsEnabled,
isUnlockWithBiometricsEnabled = false,
isUnlockWithPinEnabled = settingsRepository.isUnlockWithPinEnabled,
vaultTimeout = settingsRepository.vaultTimeout,
@ -59,7 +59,6 @@ class AccountSecurityViewModel @Inject constructor(
AccountSecurityAction.DismissDialog -> handleDismissDialog()
AccountSecurityAction.FingerPrintLearnMoreClick -> handleFingerPrintLearnMoreClick()
AccountSecurityAction.LockNowClick -> handleLockNowClick()
is AccountSecurityAction.LoginRequestToggle -> handleLoginRequestToggle(action)
AccountSecurityAction.LogoutClick -> handleLogoutClick()
AccountSecurityAction.PendingLoginRequestsClick -> handlePendingLoginRequestsClick()
is AccountSecurityAction.VaultTimeoutTypeSelect -> handleVaultTimeoutTypeSelect(action)
@ -74,6 +73,10 @@ class AccountSecurityViewModel @Inject constructor(
}
is AccountSecurityAction.UnlockWithPinToggle -> handleUnlockWithPinToggle(action)
is AccountSecurityAction.ApprovePasswordlessLoginsToggle -> {
handleApprovePasswordlessLoginsToggle(action)
}
}
private fun handleAccountFingerprintPhraseClick() {
@ -108,9 +111,25 @@ class AccountSecurityViewModel @Inject constructor(
vaultRepository.lockVaultForCurrentUser()
}
private fun handleLoginRequestToggle(action: AccountSecurityAction.LoginRequestToggle) {
// TODO BIT-466: Persist pending login requests state
mutableStateFlow.update { it.copy(isApproveLoginRequestsEnabled = action.enabled) }
private fun handleApprovePasswordlessLoginsToggle(
action: AccountSecurityAction.ApprovePasswordlessLoginsToggle,
) {
when (action) {
AccountSecurityAction.ApprovePasswordlessLoginsToggle.Disabled -> {
settingsRepository.isApprovePasswordlessLoginsEnabled = false
mutableStateFlow.update { it.copy(isApproveLoginRequestsEnabled = false) }
}
AccountSecurityAction.ApprovePasswordlessLoginsToggle.Enabled -> {
settingsRepository.isApprovePasswordlessLoginsEnabled = true
mutableStateFlow.update { it.copy(isApproveLoginRequestsEnabled = true) }
}
AccountSecurityAction.ApprovePasswordlessLoginsToggle.PendingEnabled -> {
mutableStateFlow.update { it.copy(isApproveLoginRequestsEnabled = true) }
}
}
// TODO Add permission prompt - BIT-1360
sendEvent(AccountSecurityEvent.ShowToast("Handle Login requests on this device.".asText()))
}
@ -319,13 +338,6 @@ sealed class AccountSecurityAction {
*/
data object LockNowClick : AccountSecurityAction()
/**
* User toggled the login request switch.
*/
data class LoginRequestToggle(
val enabled: Boolean,
) : AccountSecurityAction()
/**
* User clicked log out.
*/
@ -369,6 +381,26 @@ sealed class AccountSecurityAction {
val enabled: Boolean,
) : AccountSecurityAction()
/**
* User toggled the approve passwordless logins switch.
*/
sealed class ApprovePasswordlessLoginsToggle : AccountSecurityAction() {
/**
* The toggle was enabled and confirmed.
*/
data object Enabled : ApprovePasswordlessLoginsToggle()
/**
* The toggle was enabled but not yet confirmed.
*/
data object PendingEnabled : ApprovePasswordlessLoginsToggle()
/**
* The toggle was disabled.
*/
data object Disabled : ApprovePasswordlessLoginsToggle()
}
/**
* User toggled the unlock with pin switch.
*/

View file

@ -85,6 +85,10 @@ class SettingsDiskSourceTest {
userId = userId,
blockedAutofillUris = listOf("www.example.com"),
)
settingsDiskSource.storeApprovePasswordlessLoginsEnabled(
userId = userId,
isApprovePasswordlessLoginsEnabled = true,
)
settingsDiskSource.clearData(userId = userId)
@ -93,6 +97,7 @@ class SettingsDiskSourceTest {
assertNull(settingsDiskSource.getPullToRefreshEnabled(userId = userId))
assertNull(settingsDiskSource.getInlineAutofillEnabled(userId = userId))
assertNull(settingsDiskSource.getBlockedAutofillUris(userId = userId))
assertNull(settingsDiskSource.getApprovePasswordlessLoginsEnabled(userId = userId))
}
@Test
@ -507,4 +512,65 @@ class SettingsDiskSourceTest {
json.parseToJsonElement(requireNotNull(actual)),
)
}
@Suppress("MaxLineLength")
@Test
fun `getApprovePasswordlessLoginsEnabled when values are present should pull from SharedPreferences`() {
val approvePasswordlessLoginsBaseKey = "bwPreferencesStorage:approvePasswordlessLogins"
val mockUserId = "mockUserId"
val isEnabled = true
fakeSharedPreferences
.edit {
putBoolean(
"${approvePasswordlessLoginsBaseKey}_$mockUserId",
isEnabled,
)
}
val actual = settingsDiskSource.getApprovePasswordlessLoginsEnabled(userId = mockUserId)
assertEquals(
isEnabled,
actual,
)
}
@Test
fun `getApprovePasswordlessLoginsEnabled when values are absent should return null`() {
val mockUserId = "mockUserId"
assertNull(settingsDiskSource.getApprovePasswordlessLoginsEnabled(userId = mockUserId))
}
@Suppress("MaxLineLength")
@Test
fun `storeApprovePasswordlessLoginsEnabled for non-null values should update SharedPreferences`() {
val approvePasswordlessLoginsBaseKey = "bwPreferencesStorage:approvePasswordlessLogins"
val mockUserId = "mockUserId"
val isEnabled = true
settingsDiskSource.storeApprovePasswordlessLoginsEnabled(
userId = mockUserId,
isApprovePasswordlessLoginsEnabled = isEnabled,
)
val actual = fakeSharedPreferences.getBoolean(
"${approvePasswordlessLoginsBaseKey}_$mockUserId",
false,
)
assertEquals(
isEnabled,
actual,
)
}
@Test
fun `storeApprovePasswordlessLoginsEnabled for null values should clear SharedPreferences`() {
val approvePasswordlessLoginsBaseKey = "bwPreferencesStorage:approvePasswordlessLogins"
val mockUserId = "mockUserId"
val approvePasswordlessLoginsKey = "${approvePasswordlessLoginsBaseKey}_$mockUserId"
fakeSharedPreferences.edit {
putBoolean(approvePasswordlessLoginsKey, true)
}
settingsDiskSource.storeApprovePasswordlessLoginsEnabled(
userId = mockUserId,
isApprovePasswordlessLoginsEnabled = null,
)
assertFalse(fakeSharedPreferences.contains(approvePasswordlessLoginsKey))
}
}

View file

@ -39,6 +39,8 @@ class FakeSettingsDiskSource : SettingsDiskSource {
private var storedIsIconLoadingDisabled: Boolean? = null
private val storedApprovePasswordLoginsEnabled = mutableMapOf<String, Boolean?>()
override var appLanguage: AppLanguage? = null
override var appTheme: AppTheme
@ -138,6 +140,16 @@ class FakeSettingsDiskSource : SettingsDiskSource {
storedBlockedAutofillUris[userId] = blockedAutofillUris
}
override fun getApprovePasswordlessLoginsEnabled(userId: String): Boolean? =
storedApprovePasswordLoginsEnabled[userId]
override fun storeApprovePasswordlessLoginsEnabled(
userId: String,
isApprovePasswordlessLoginsEnabled: Boolean?,
) {
storedApprovePasswordLoginsEnabled[userId] = isApprovePasswordlessLoginsEnabled
}
//region Private helper functions
private fun getMutableVaultTimeoutActionsFlow(

View file

@ -533,6 +533,32 @@ class SettingsRepositoryTest {
)
}
}
@Test
fun `isApprovePasswordlessLoginsEnabled should properly update SettingsDiskSource`() {
fakeAuthDiskSource.userState = null
assertFalse(settingsRepository.isApprovePasswordlessLoginsEnabled)
val userId = "userId"
fakeAuthDiskSource.userState = MOCK_USER_STATE
// Updates to the disk source change the repository value
fakeSettingsDiskSource.storeApprovePasswordlessLoginsEnabled(
userId = userId,
isApprovePasswordlessLoginsEnabled = true,
)
assertEquals(
true,
settingsRepository.isApprovePasswordlessLoginsEnabled,
)
// Updates to the repository value change the disk source
settingsRepository.isApprovePasswordlessLoginsEnabled = false
assertEquals(
false,
fakeSettingsDiskSource.getApprovePasswordlessLoginsEnabled(userId = userId),
)
}
}
private val MOCK_USER_STATE =

View file

@ -69,13 +69,102 @@ class AccountSecurityScreenTest : BaseComposeTest() {
verify { viewModel.trySendAction(AccountSecurityAction.LogoutClick) }
}
@Suppress("MaxLineLength")
@Test
fun `on approve login requests toggle should send LoginRequestToggle`() {
fun `on approve login requests toggle on should send PendingEnabled action and display dialog`() {
composeTestRule.assertNoDialogExists()
composeTestRule
.onNodeWithText("Use this device to approve login requests made from other devices")
.performScrollTo()
.performClick()
verify { viewModel.trySendAction(AccountSecurityAction.LoginRequestToggle(true)) }
composeTestRule
.onAllNodesWithText("Approve login requests")
.filterToOne(hasAnyAncestor(isDialog()))
.assertIsDisplayed()
composeTestRule
.onAllNodesWithText(
"Use this device to approve login requests made from other devices",
)
.filterToOne(hasAnyAncestor(isDialog()))
.assertIsDisplayed()
composeTestRule
.onAllNodesWithText("No")
.filterToOne(hasAnyAncestor(isDialog()))
.assertIsDisplayed()
composeTestRule
.onAllNodesWithText("Yes")
.filterToOne(hasAnyAncestor(isDialog()))
.assertIsDisplayed()
verify {
viewModel.trySendAction(
AccountSecurityAction.ApprovePasswordlessLoginsToggle.PendingEnabled,
)
}
}
@Test
fun `on approve login requests toggle off should send Disabled action`() {
mutableStateFlow.update { it.copy(isApproveLoginRequestsEnabled = true) }
composeTestRule
.onNodeWithText("Use this device to approve login requests made from other devices")
.performScrollTo()
.performClick()
verify {
viewModel.trySendAction(
AccountSecurityAction.ApprovePasswordlessLoginsToggle.Disabled,
)
}
}
@Test
fun `on approve login requests confirm Yes should send Enabled action and hide dialog`() {
mutableStateFlow.update { it.copy(isApproveLoginRequestsEnabled = false) }
composeTestRule
.onNodeWithText("Use this device to approve login requests made from other devices")
.performScrollTo()
.performClick()
composeTestRule
.onAllNodesWithText("Yes")
.filterToOne(hasAnyAncestor(isDialog()))
.performClick()
composeTestRule.assertNoDialogExists()
verify {
viewModel.trySendAction(
AccountSecurityAction.ApprovePasswordlessLoginsToggle.Enabled,
)
}
}
@Test
fun `on approve login requests confirm No should send Disabled action and hide dialog`() {
mutableStateFlow.update { it.copy(isApproveLoginRequestsEnabled = false) }
composeTestRule
.onNodeWithText("Use this device to approve login requests made from other devices")
.performScrollTo()
.performClick()
composeTestRule
.onAllNodesWithText("No")
.filterToOne(hasAnyAncestor(isDialog()))
.performClick()
composeTestRule.assertNoDialogExists()
verify {
viewModel.trySendAction(
AccountSecurityAction.ApprovePasswordlessLoginsToggle.Disabled,
)
}
}
@Test

View file

@ -16,6 +16,7 @@ import io.mockk.runs
import io.mockk.verify
import kotlinx.coroutines.test.runTest
import org.junit.jupiter.api.Assertions.assertEquals
import org.junit.jupiter.api.Assertions.assertFalse
import org.junit.jupiter.api.Assertions.assertTrue
import org.junit.jupiter.api.Test
@ -33,6 +34,7 @@ class AccountSecurityViewModelTest : BaseViewModelTest() {
every { isUnlockWithPinEnabled } returns true
every { vaultTimeout } returns VaultTimeout.ThirtyMinutes
every { vaultTimeoutAction } returns VaultTimeoutAction.LOCK
every { isApprovePasswordlessLoginsEnabled } returns false
}
val viewModel = createViewModel(
initialState = null,
@ -112,21 +114,6 @@ class AccountSecurityViewModelTest : BaseViewModelTest() {
verify { vaultRepository.lockVaultForCurrentUser() }
}
@Test
fun `on LoginRequestToggle should emit ShowToast`() = runTest {
val viewModel = createViewModel()
viewModel.eventFlow.test {
viewModel.trySendAction(AccountSecurityAction.LoginRequestToggle(true))
assertEquals(
AccountSecurityEvent.ShowToast("Handle Login requests on this device.".asText()),
awaitItem(),
)
}
viewModel.stateFlow.test {
assertTrue(awaitItem().isApproveLoginRequestsEnabled)
}
}
@Test
fun `on PendingLoginRequestsClick should emit ShowToast`() = runTest {
val viewModel = createViewModel()
@ -323,6 +310,71 @@ class AccountSecurityViewModelTest : BaseViewModelTest() {
assertEquals(DEFAULT_STATE.copy(dialog = null), viewModel.stateFlow.value)
}
@Suppress("MaxLineLength")
@Test
fun `on ApprovePasswordlessLoginsToggle enabled should update settings, set isApprovePasswordlessLoginsEnabled to true, and display toast`() =
runTest {
val settingsRepository = mockk<SettingsRepository> {
every { isApprovePasswordlessLoginsEnabled = true } just runs
}
val viewModel = createViewModel(
settingsRepository = settingsRepository,
)
viewModel.eventFlow.test {
viewModel.trySendAction(
AccountSecurityAction.ApprovePasswordlessLoginsToggle.Enabled,
)
assertEquals(
AccountSecurityEvent.ShowToast("Handle Login requests on this device.".asText()),
awaitItem(),
)
verify(exactly = 1) { settingsRepository.isApprovePasswordlessLoginsEnabled = true }
}
assertTrue(viewModel.stateFlow.value.isApproveLoginRequestsEnabled)
}
@Suppress("MaxLineLength")
@Test
fun `on ApprovePasswordlessLoginsToggle pending enabled should set isApprovePasswordlessLoginsEnabled to true and display toast`() =
runTest {
val viewModel = createViewModel()
viewModel.eventFlow.test {
viewModel.trySendAction(
AccountSecurityAction.ApprovePasswordlessLoginsToggle.PendingEnabled,
)
assertEquals(
AccountSecurityEvent.ShowToast("Handle Login requests on this device.".asText()),
awaitItem(),
)
}
assertTrue(viewModel.stateFlow.value.isApproveLoginRequestsEnabled)
}
@Suppress("MaxLineLength")
@Test
fun `on ApprovePasswordlessLoginsToggle disabled should update settings, set isApprovePasswordlessLoginsEnabled to false, and display toast`() =
runTest {
val settingsRepository = mockk<SettingsRepository> {
every { isApprovePasswordlessLoginsEnabled = false } just runs
}
val viewModel = createViewModel(
settingsRepository = settingsRepository,
)
viewModel.eventFlow.test {
viewModel.trySendAction(
AccountSecurityAction.ApprovePasswordlessLoginsToggle.Disabled,
)
assertEquals(
AccountSecurityEvent.ShowToast("Handle Login requests on this device.".asText()),
awaitItem(),
)
verify(exactly = 1) {
settingsRepository.isApprovePasswordlessLoginsEnabled = false
}
}
assertFalse(viewModel.stateFlow.value.isApproveLoginRequestsEnabled)
}
private fun createViewModel(
initialState: AccountSecurityState? = DEFAULT_STATE,
authRepository: AuthRepository = mockk(relaxed = true),