From a706db2b281d94a86841fe019d90d5c192b933d3 Mon Sep 17 00:00:00 2001 From: Sean Weiser <125889608+sean-livefront@users.noreply.github.com> Date: Fri, 19 Jan 2024 09:35:05 -0600 Subject: [PATCH] BIT-1360: Prompt for push notification permission when approving passwordless logins (#677) --- .../accountsecurity/AccountSecurityScreen.kt | 44 ++++++++ .../AccountSecurityViewModel.kt | 20 +++- .../platform/manager/intent/IntentManager.kt | 5 + .../manager/intent/IntentManagerImpl.kt | 6 ++ .../AccountSecurityScreenTest.kt | 102 +++++++++++++++++- .../AccountSecurityViewModelTest.kt | 34 +++--- 6 files changed, 193 insertions(+), 18 deletions(-) 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 6e3c772f66..f1081084a7 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 @@ -30,6 +30,8 @@ import androidx.compose.ui.res.stringResource import androidx.compose.ui.unit.dp import androidx.core.net.toUri import androidx.hilt.navigation.compose.hiltViewModel +import android.Manifest +import android.os.Build import com.x8bit.bitwarden.R import com.x8bit.bitwarden.data.platform.repository.model.VaultTimeout import com.x8bit.bitwarden.data.platform.repository.model.VaultTimeoutAction @@ -48,9 +50,11 @@ import com.x8bit.bitwarden.ui.platform.components.BitwardenTwoButtonDialog import com.x8bit.bitwarden.ui.platform.components.BitwardenWideSwitch import com.x8bit.bitwarden.ui.platform.components.dialog.BitwardenTimePickerDialog import com.x8bit.bitwarden.ui.platform.manager.intent.IntentManager +import com.x8bit.bitwarden.ui.platform.manager.permissions.PermissionsManager import com.x8bit.bitwarden.ui.platform.theme.LocalIntentManager import com.x8bit.bitwarden.ui.platform.theme.LocalNonMaterialColors import com.x8bit.bitwarden.ui.platform.theme.LocalNonMaterialTypography +import com.x8bit.bitwarden.ui.platform.theme.LocalPermissionsManager import com.x8bit.bitwarden.ui.platform.util.displayLabel import com.x8bit.bitwarden.ui.platform.util.toFormattedPattern import java.time.LocalTime @@ -68,6 +72,7 @@ fun AccountSecurityScreen( onNavigateToDeleteAccount: () -> Unit, viewModel: AccountSecurityViewModel = hiltViewModel(), intentManager: IntentManager = LocalIntentManager.current, + permissionsManager: PermissionsManager = LocalPermissionsManager.current, ) { val state by viewModel.stateFlow.collectAsState() val context = LocalContext.current @@ -76,6 +81,10 @@ fun AccountSecurityScreen( when (event) { AccountSecurityEvent.NavigateBack -> onNavigateBack() + AccountSecurityEvent.NavigateToApplicationDataSettings -> { + intentManager.startApplicationDetailsSettingsActivity() + } + AccountSecurityEvent.NavigateToDeleteAccount -> onNavigateToDeleteAccount() AccountSecurityEvent.NavigateToFingerprintPhrase -> { @@ -144,6 +153,10 @@ fun AccountSecurityScreen( onApprovePasswordlessLoginsAction = remember(viewModel) { { viewModel.trySendAction(it) } }, + permissionsManager = permissionsManager, + onPushNotificationConfirm = remember(viewModel) { + { viewModel.trySendAction(AccountSecurityAction.PushNotificationConfirm) } + }, modifier = Modifier .fillMaxWidth() .padding(horizontal = 16.dp), @@ -615,14 +628,18 @@ private fun FingerPrintPhraseDialog( ) } +@Suppress("LongMethod") @Composable private fun ApprovePasswordlessLoginsRow( isApproveLoginRequestsEnabled: Boolean, @Suppress("MaxLineLength") onApprovePasswordlessLoginsAction: (AccountSecurityAction.ApprovePasswordlessLoginsToggle) -> Unit, + onPushNotificationConfirm: () -> Unit, + permissionsManager: PermissionsManager, modifier: Modifier = Modifier, ) { var shouldShowConfirmationDialog by remember { mutableStateOf(false) } + var shouldShowPermissionDialog by remember { mutableStateOf(false) } BitwardenWideSwitch( label = stringResource( id = R.string.use_this_device_to_approve_login_requests_made_from_other_devices, @@ -656,6 +673,12 @@ private fun ApprovePasswordlessLoginsRow( AccountSecurityAction.ApprovePasswordlessLoginsToggle.Enabled, ) shouldShowConfirmationDialog = false + if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.TIRAMISU) { + @Suppress("MaxLineLength") + if (!permissionsManager.checkPermission(Manifest.permission.POST_NOTIFICATIONS)) { + shouldShowPermissionDialog = true + } + } }, onDismissClick = { onApprovePasswordlessLoginsAction( @@ -671,4 +694,25 @@ private fun ApprovePasswordlessLoginsRow( }, ) } + + if (shouldShowPermissionDialog) { + BitwardenTwoButtonDialog( + title = null, + message = stringResource( + id = R.string.receive_push_notifications_for_new_login_requests, + ), + confirmButtonText = stringResource(id = R.string.settings), + dismissButtonText = stringResource(id = R.string.no_thanks), + onConfirmClick = { + shouldShowPermissionDialog = false + onPushNotificationConfirm() + }, + onDismissClick = { + shouldShowPermissionDialog = false + }, + onDismissRequest = { + shouldShowPermissionDialog = false + }, + ) + } } 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 1080d07b92..9e1e50081e 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 @@ -77,6 +77,10 @@ class AccountSecurityViewModel @Inject constructor( is AccountSecurityAction.ApprovePasswordlessLoginsToggle -> { handleApprovePasswordlessLoginsToggle(action) } + + is AccountSecurityAction.PushNotificationConfirm -> { + handlePushNotificationConfirm() + } } private fun handleAccountFingerprintPhraseClick() { @@ -129,8 +133,10 @@ class AccountSecurityViewModel @Inject constructor( mutableStateFlow.update { it.copy(isApproveLoginRequestsEnabled = true) } } } - // TODO Add permission prompt - BIT-1360 - sendEvent(AccountSecurityEvent.ShowToast("Handle Login requests on this device.".asText())) + } + + private fun handlePushNotificationConfirm() { + sendEvent(AccountSecurityEvent.NavigateToApplicationDataSettings) } private fun handleLogoutClick() { @@ -275,6 +281,11 @@ sealed class AccountSecurityEvent { */ data object NavigateBack : AccountSecurityEvent() + /** + * Navigate to the application's settings screen. + */ + data object NavigateToApplicationDataSettings : AccountSecurityEvent() + /** * Navigate to the delete account screen. */ @@ -381,6 +392,11 @@ sealed class AccountSecurityAction { val enabled: Boolean, ) : AccountSecurityAction() + /** + * User confirmed the push notification permission prompt. + */ + data object PushNotificationConfirm : AccountSecurityAction() + /** * User toggled the approve passwordless logins switch. */ diff --git a/app/src/main/java/com/x8bit/bitwarden/ui/platform/manager/intent/IntentManager.kt b/app/src/main/java/com/x8bit/bitwarden/ui/platform/manager/intent/IntentManager.kt index f2cecb4b50..ce4df7f54d 100644 --- a/app/src/main/java/com/x8bit/bitwarden/ui/platform/manager/intent/IntentManager.kt +++ b/app/src/main/java/com/x8bit/bitwarden/ui/platform/manager/intent/IntentManager.kt @@ -32,6 +32,11 @@ interface IntentManager { */ fun startSystemAutofillSettingsActivity(): Boolean + /** + * Starts the application's settings activity. + */ + fun startApplicationDetailsSettingsActivity() + /** * Start an activity to view the given [uri] in an external browser. */ diff --git a/app/src/main/java/com/x8bit/bitwarden/ui/platform/manager/intent/IntentManagerImpl.kt b/app/src/main/java/com/x8bit/bitwarden/ui/platform/manager/intent/IntentManagerImpl.kt index b62d92fcca..4dd5979cc4 100644 --- a/app/src/main/java/com/x8bit/bitwarden/ui/platform/manager/intent/IntentManagerImpl.kt +++ b/app/src/main/java/com/x8bit/bitwarden/ui/platform/manager/intent/IntentManagerImpl.kt @@ -93,6 +93,12 @@ class IntentManagerImpl( false } + override fun startApplicationDetailsSettingsActivity() { + val intent = Intent(Settings.ACTION_APPLICATION_DETAILS_SETTINGS) + intent.data = Uri.parse("package:" + context.packageName) + startActivity(intent = intent) + } + override fun launchUri(uri: Uri) { val newUri = if (uri.scheme == null) { uri.buildUpon().scheme("https").build() 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 803c85bf7a..d3f7101db4 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 @@ -23,6 +23,7 @@ import com.x8bit.bitwarden.data.platform.repository.util.bufferedMutableSharedFl import com.x8bit.bitwarden.ui.platform.base.BaseComposeTest import com.x8bit.bitwarden.ui.platform.base.util.asText import com.x8bit.bitwarden.ui.platform.manager.intent.IntentManager +import com.x8bit.bitwarden.ui.platform.manager.permissions.FakePermissionManager import com.x8bit.bitwarden.ui.util.assertNoDialogExists import io.mockk.every import io.mockk.just @@ -43,7 +44,10 @@ class AccountSecurityScreenTest : BaseComposeTest() { private val intentManager = mockk { every { launchUri(any()) } just runs + every { startActivity(any()) } just runs + every { startApplicationDetailsSettingsActivity() } just runs } + private val permissionsManager = FakePermissionManager() private val mutableEventFlow = bufferedMutableSharedFlow() private val mutableStateFlow = MutableStateFlow(DEFAULT_STATE) private val viewModel = mockk(relaxed = true) { @@ -59,6 +63,7 @@ class AccountSecurityScreenTest : BaseComposeTest() { onNavigateToDeleteAccount = { onNavigateToDeleteAccountCalled = true }, viewModel = viewModel, intentManager = intentManager, + permissionsManager = permissionsManager, ) } } @@ -121,8 +126,10 @@ class AccountSecurityScreenTest : BaseComposeTest() { } } + @Suppress("MaxLineLength") @Test - fun `on approve login requests confirm Yes should send Enabled action and hide dialog`() { + fun `on approve login requests confirm Yes should send Enabled action and hide dialog when permission already granted`() { + permissionsManager.checkPermissionResult = true mutableStateFlow.update { it.copy(isApproveLoginRequestsEnabled = false) } composeTestRule @@ -144,6 +151,42 @@ class AccountSecurityScreenTest : BaseComposeTest() { } } + @Suppress("MaxLineLength") + @Test + fun `on approve login requests confirm Yes should send Enabled action and show permission dialog when permission not granted`() { + permissionsManager.checkPermissionResult = false + 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 + .onAllNodesWithText("Receive push notifications for new login requests") + .filterToOne(hasAnyAncestor(isDialog())) + .assertIsDisplayed() + composeTestRule + .onAllNodesWithText("No thanks") + .filterToOne(hasAnyAncestor(isDialog())) + .assertIsDisplayed() + composeTestRule + .onAllNodesWithText("Settings") + .filterToOne(hasAnyAncestor(isDialog())) + .assertIsDisplayed() + + 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) } @@ -178,6 +221,63 @@ class AccountSecurityScreenTest : BaseComposeTest() { .assertIsOn() } + @Test + fun `on push permission dialog No thanks should hide dialog`() { + permissionsManager.checkPermissionResult = false + 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 + .onAllNodesWithText("No thanks") + .filterToOne(hasAnyAncestor(isDialog())) + .performClick() + + composeTestRule.assertNoDialogExists() + } + + @Test + fun `on push permission dialog Settings should hide dialog and send confirm action`() { + permissionsManager.checkPermissionResult = false + 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 + .onAllNodesWithText("Settings") + .filterToOne(hasAnyAncestor(isDialog())) + .performClick() + + composeTestRule.assertNoDialogExists() + + verify { + viewModel.trySendAction(AccountSecurityAction.PushNotificationConfirm) + } + } + + @Test + fun `on NavigateToApplicationDataSettings should launch the correct intent`() { + mutableEventFlow.tryEmit(AccountSecurityEvent.NavigateToApplicationDataSettings) + + verify { intentManager.startApplicationDetailsSettingsActivity() } + } + @Test fun `on pending login requests click should send PendingLoginRequestsClick`() { composeTestRule 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 99dc400239..3173f2a93c 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 @@ -312,7 +312,7 @@ class AccountSecurityViewModelTest : BaseViewModelTest() { @Suppress("MaxLineLength") @Test - fun `on ApprovePasswordlessLoginsToggle enabled should update settings, set isApprovePasswordlessLoginsEnabled to true, and display toast`() = + fun `on ApprovePasswordlessLoginsToggle enabled should update settings and set isApprovePasswordlessLoginsEnabled to true`() = runTest { val settingsRepository = mockk { every { isApprovePasswordlessLoginsEnabled = true } just runs @@ -324,10 +324,7 @@ class AccountSecurityViewModelTest : BaseViewModelTest() { viewModel.trySendAction( AccountSecurityAction.ApprovePasswordlessLoginsToggle.Enabled, ) - assertEquals( - AccountSecurityEvent.ShowToast("Handle Login requests on this device.".asText()), - awaitItem(), - ) + expectNoEvents() verify(exactly = 1) { settingsRepository.isApprovePasswordlessLoginsEnabled = true } } assertTrue(viewModel.stateFlow.value.isApproveLoginRequestsEnabled) @@ -335,24 +332,21 @@ class AccountSecurityViewModelTest : BaseViewModelTest() { @Suppress("MaxLineLength") @Test - fun `on ApprovePasswordlessLoginsToggle pending enabled should set isApprovePasswordlessLoginsEnabled to true and display toast`() = + fun `on ApprovePasswordlessLoginsToggle pending enabled should set isApprovePasswordlessLoginsEnabled to true`() = runTest { val viewModel = createViewModel() viewModel.eventFlow.test { viewModel.trySendAction( AccountSecurityAction.ApprovePasswordlessLoginsToggle.PendingEnabled, ) - assertEquals( - AccountSecurityEvent.ShowToast("Handle Login requests on this device.".asText()), - awaitItem(), - ) + expectNoEvents() } assertTrue(viewModel.stateFlow.value.isApproveLoginRequestsEnabled) } @Suppress("MaxLineLength") @Test - fun `on ApprovePasswordlessLoginsToggle disabled should update settings, set isApprovePasswordlessLoginsEnabled to false, and display toast`() = + fun `on ApprovePasswordlessLoginsToggle disabled should update settings and set isApprovePasswordlessLoginsEnabled to false`() = runTest { val settingsRepository = mockk { every { isApprovePasswordlessLoginsEnabled = false } just runs @@ -364,10 +358,7 @@ class AccountSecurityViewModelTest : BaseViewModelTest() { viewModel.trySendAction( AccountSecurityAction.ApprovePasswordlessLoginsToggle.Disabled, ) - assertEquals( - AccountSecurityEvent.ShowToast("Handle Login requests on this device.".asText()), - awaitItem(), - ) + expectNoEvents() verify(exactly = 1) { settingsRepository.isApprovePasswordlessLoginsEnabled = false } @@ -375,6 +366,19 @@ class AccountSecurityViewModelTest : BaseViewModelTest() { assertFalse(viewModel.stateFlow.value.isApproveLoginRequestsEnabled) } + @Test + fun `on PushNotificationConfirm should send NavigateToApplicationDataSettings event`() = + runTest { + val viewModel = createViewModel() + viewModel.eventFlow.test { + viewModel.trySendAction(AccountSecurityAction.PushNotificationConfirm) + assertEquals( + AccountSecurityEvent.NavigateToApplicationDataSettings, + awaitItem(), + ) + } + } + private fun createViewModel( initialState: AccountSecurityState? = DEFAULT_STATE, authRepository: AuthRepository = mockk(relaxed = true),