Merge branch 'reduce-risk-for-account-creation-mistakes-droid-1335'

7 files changed, 144 insertions, 25 deletions

diff --git a/android/app/src/main/kotlin/net/mullvad/mullvadvpn/compose/dialog/CreateAccountConfirmationDialog.kt b/android/app/src/main/kotlin/net/mullvad/mullvadvpn/compose/dialog/CreateAccountConfirmationDialog.kt
new file mode 100644
index 0000000000..b670f7d79e
--- /dev/null
+++ b/android/app/src/main/kotlin/net/mullvad/mullvadvpn/compose/dialog/CreateAccountConfirmationDialog.kt
@@ -0,0 +1,54 @@
+package net.mullvad.mullvadvpn.compose.dialog
+
+import androidx.compose.foundation.layout.Spacer
+import androidx.compose.foundation.layout.fillMaxWidth
+import androidx.compose.foundation.layout.height
+import androidx.compose.material3.MaterialTheme
+import androidx.compose.material3.Text
+import androidx.compose.runtime.Composable
+import androidx.compose.ui.Modifier
+import androidx.compose.ui.res.stringResource
+import androidx.compose.ui.tooling.preview.Preview
+import com.ramcosta.composedestinations.annotation.Destination
+import com.ramcosta.composedestinations.annotation.RootGraph
+import com.ramcosta.composedestinations.result.EmptyResultBackNavigator
+import com.ramcosta.composedestinations.result.ResultBackNavigator
+import com.ramcosta.composedestinations.spec.DestinationStyle
+import net.mullvad.mullvadvpn.R
+import net.mullvad.mullvadvpn.compose.dialog.info.InfoConfirmationDialog
+import net.mullvad.mullvadvpn.compose.dialog.info.InfoConfirmationDialogTitleType
+import net.mullvad.mullvadvpn.lib.theme.AppTheme
+import net.mullvad.mullvadvpn.lib.theme.Dimens
+
+@Preview
+@Composable
+private fun PreviewCreateAccountConfirmationDialog() {
+    AppTheme { CreateAccountConfirmation(EmptyResultBackNavigator()) }
+}
+
+@Composable
+@Destination<RootGraph>(style = DestinationStyle.Dialog::class)
+fun CreateAccountConfirmation(navigator: ResultBackNavigator<Boolean>) {
+    InfoConfirmationDialog(
+        navigator = navigator,
+        titleType = InfoConfirmationDialogTitleType.IconOnly,
+        confirmButtonTitle = stringResource(R.string.create_new_account),
+        cancelButtonTitle = stringResource(R.string.cancel),
+    ) {
+        Text(
+            text = stringResource(id = R.string.create_new_account_warning_paragraph1),
+            color = MaterialTheme.colorScheme.onSurface,
+            style = MaterialTheme.typography.bodySmall,
+            modifier = Modifier.fillMaxWidth(),
+        )
+
+        Spacer(modifier = Modifier.height(Dimens.verticalSpace))
+
+        Text(
+            text = stringResource(id = R.string.create_new_account_warning_paragraph2),
+            color = MaterialTheme.colorScheme.onSurface,
+            style = MaterialTheme.typography.bodySmall,
+            modifier = Modifier.fillMaxWidth(),
+        )
+    }
+}
diff --git a/android/app/src/main/kotlin/net/mullvad/mullvadvpn/compose/screen/LoginScreen.kt b/android/app/src/main/kotlin/net/mullvad/mullvadvpn/compose/screen/LoginScreen.kt
index 7a3d338022..a0829b79d5 100644
--- a/android/app/src/main/kotlin/net/mullvad/mullvadvpn/compose/screen/LoginScreen.kt
+++ b/android/app/src/main/kotlin/net/mullvad/mullvadvpn/compose/screen/LoginScreen.kt
@@ -56,11 +56,13 @@ import com.ramcosta.composedestinations.annotation.Destination
 import com.ramcosta.composedestinations.annotation.RootGraph
 import com.ramcosta.composedestinations.generated.NavGraphs
 import com.ramcosta.composedestinations.generated.destinations.ConnectDestination
+import com.ramcosta.composedestinations.generated.destinations.CreateAccountConfirmationDestination
 import com.ramcosta.composedestinations.generated.destinations.DeviceListDestination
 import com.ramcosta.composedestinations.generated.destinations.OutOfTimeDestination
 import com.ramcosta.composedestinations.generated.destinations.SettingsDestination
 import com.ramcosta.composedestinations.generated.destinations.WelcomeDestination
 import com.ramcosta.composedestinations.navigation.DestinationsNavigator
+import com.ramcosta.composedestinations.result.ResultRecipient
 import net.mullvad.mullvadvpn.R
 import net.mullvad.mullvadvpn.compose.button.PrimaryButton
 import net.mullvad.mullvadvpn.compose.button.VariantButton
@@ -78,6 +80,7 @@ import net.mullvad.mullvadvpn.compose.test.LOGIN_TITLE_TEST_TAG
 import net.mullvad.mullvadvpn.compose.textfield.mullvadWhiteTextFieldColors
 import net.mullvad.mullvadvpn.compose.transitions.LoginTransition
 import net.mullvad.mullvadvpn.compose.util.CollectSideEffectWithLifecycle
+import net.mullvad.mullvadvpn.compose.util.OnNavResultValue
 import net.mullvad.mullvadvpn.compose.util.accountNumberVisualTransformation
 import net.mullvad.mullvadvpn.compose.util.showSnackbarImmediately
 import net.mullvad.mullvadvpn.lib.theme.AppTheme
@@ -86,7 +89,7 @@ import net.mullvad.mullvadvpn.viewmodel.LoginUiSideEffect
 import net.mullvad.mullvadvpn.viewmodel.LoginViewModel
 import org.koin.androidx.compose.koinViewModel
 
-@Preview("Default|Loading.LogginIn|Loading.CreatingAccount|LoginError|Success")
+@Preview("Default|Loading.LoggingIn|Loading.CreatingAccount|LoginError|Success")
 @Composable
 private fun PreviewLoginScreen(
     @PreviewParameter(LoginUiStatePreviewParameterProvider::class) state: LoginUiState
@@ -103,6 +106,8 @@ fun Login(
     navigator: DestinationsNavigator,
     accountNumber: String? = null,
     vm: LoginViewModel = koinViewModel(),
+    createAccountConfirmationDialogResult:
+        ResultRecipient<CreateAccountConfirmationDestination, Boolean>,
 ) {
     val state by vm.uiState.collectAsStateWithLifecycle()
 
@@ -114,6 +119,12 @@ fun Login(
         }
     }
 
+    createAccountConfirmationDialogResult.OnNavResultValue { createAccount ->
+        if (createAccount) {
+            vm.onCreateAccountConfirmed()
+        }
+    }
+
     val context = LocalContext.current
     val snackbarHostState = remember { SnackbarHostState() }
     CollectSideEffectWithLifecycle(vm.uiSideEffect) {
@@ -137,6 +148,8 @@ fun Login(
                     launchSingleTop = true
                     popUpTo(NavGraphs.root) { inclusive = true }
                 }
+            LoginUiSideEffect.NavigateToCreateAccountConfirmation ->
+                navigator.navigate(CreateAccountConfirmationDestination)
             LoginUiSideEffect.GenericError ->
                 snackbarHostState.showSnackbarImmediately(
                     message = context.getString(R.string.error_occurred)
@@ -147,7 +160,7 @@ fun Login(
         state = state,
         snackbarHostState = snackbarHostState,
         onLoginClick = vm::login,
-        onCreateAccountClick = vm::createAccount,
+        onCreateAccountClick = vm::onCreateAccountClick,
         onDeleteHistoryClick = vm::clearAccountHistory,
         onAccountNumberChange = vm::onAccountNumberChange,
         onSettingsClick = dropUnlessResumed { navigator.navigate(SettingsDestination) },
diff --git a/android/app/src/main/kotlin/net/mullvad/mullvadvpn/viewmodel/LoginViewModel.kt b/android/app/src/main/kotlin/net/mullvad/mullvadvpn/viewmodel/LoginViewModel.kt
index 3481a8d1e7..25a8a47da3 100644
--- a/android/app/src/main/kotlin/net/mullvad/mullvadvpn/viewmodel/LoginViewModel.kt
+++ b/android/app/src/main/kotlin/net/mullvad/mullvadvpn/viewmodel/LoginViewModel.kt
@@ -41,6 +41,8 @@ sealed interface LoginUiSideEffect {
 
     data object NavigateToOutOfTime : LoginUiSideEffect
 
+    data object NavigateToCreateAccountConfirmation : LoginUiSideEffect
+
     data class TooManyDevices(val accountNumber: AccountNumber) : LoginUiSideEffect
 
     data object GenericError : LoginUiSideEffect
@@ -58,10 +60,8 @@ class LoginViewModel(
     private val _uiSideEffect = Channel<LoginUiSideEffect>()
     val uiSideEffect = _uiSideEffect.receiveAsFlow()
 
-    private val _mutableAccountHistory: MutableStateFlow<AccountNumber?> = MutableStateFlow(null)
-
     private val _uiState =
-        combine(_loginInput, _mutableAccountHistory, _loginState) {
+        combine(_loginInput, accountRepository.accountHistory, _loginState) {
             loginInput,
             historyAccountNumber,
             loginState ->
@@ -70,27 +70,31 @@ class LoginViewModel(
 
     val uiState: StateFlow<LoginUiState> =
         _uiState
-            .onStart {
-                viewModelScope.launch {
-                    _mutableAccountHistory.update { accountRepository.fetchAccountHistory() }
-                }
-            }
+            .onStart { viewModelScope.launch { accountRepository.fetchAccountHistory() } }
             .stateIn(viewModelScope, SharingStarted.WhileSubscribed(), LoginUiState.INITIAL)
 
     fun clearAccountHistory() =
         viewModelScope.launch {
-            accountRepository
-                .clearAccountHistory()
-                .fold(
-                    { _uiSideEffect.send(LoginUiSideEffect.GenericError) },
-                    {
-                        _mutableAccountHistory.update { null }
-                        _mutableAccountHistory.update { accountRepository.fetchAccountHistory() }
-                    },
-                )
+            accountRepository.clearAccountHistory().onLeft {
+                _uiSideEffect.send(LoginUiSideEffect.GenericError)
+            }
         }
 
-    fun createAccount() {
+    fun onCreateAccountClick() {
+        if (hasPreviouslyCreatedAccount()) {
+            viewModelScope.launch {
+                _uiSideEffect.send(LoginUiSideEffect.NavigateToCreateAccountConfirmation)
+            }
+        } else {
+            createAccount()
+        }
+    }
+
+    fun onCreateAccountConfirmed() {
+        createAccount()
+    }
+
+    private fun createAccount() {
         _loginState.value = Loading.CreatingAccount
         viewModelScope.launch(dispatcher) {
             accountRepository
@@ -127,7 +131,7 @@ class LoginViewModel(
         }
     }
 
-    private suspend fun onSuccessfulLogin() {
+    private fun onSuccessfulLogin() {
         newDeviceRepository.newDeviceCreated()
 
         viewModelScope.launch(dispatcher) {
@@ -169,6 +173,8 @@ class LoginViewModel(
         return internetAvailableUseCase()
     }
 
+    private fun hasPreviouslyCreatedAccount(): Boolean = uiState.value.lastUsedAccount != null
+
     companion object {
         private const val SHOW_SUCCESSFUL_LOGIN_MILLIS = 1000L
     }
diff --git a/android/app/src/test/kotlin/net/mullvad/mullvadvpn/viewmodel/LoginViewModelTest.kt b/android/app/src/test/kotlin/net/mullvad/mullvadvpn/viewmodel/LoginViewModelTest.kt
index ba636197f4..1a80597066 100644
--- a/android/app/src/test/kotlin/net/mullvad/mullvadvpn/viewmodel/LoginViewModelTest.kt
+++ b/android/app/src/test/kotlin/net/mullvad/mullvadvpn/viewmodel/LoginViewModelTest.kt
@@ -43,13 +43,16 @@ class LoginViewModelTest {
     @MockK private lateinit var mockedAccountRepository: AccountRepository
 
     private lateinit var loginViewModel: LoginViewModel
+    private lateinit var accountHistoryFlow: MutableStateFlow<AccountNumber?>
 
     @BeforeEach
     fun setup() {
         Dispatchers.setMain(UnconfinedTestDispatcher())
         MockKAnnotations.init(this, relaxUnitFun = true)
+        accountHistoryFlow = MutableStateFlow(null)
         every { connectivityUseCase() } returns true
         coEvery { mockedAccountRepository.fetchAccountHistory() } returns null
+        coEvery { mockedAccountRepository.accountHistory } returns accountHistoryFlow
 
         loginViewModel =
             LoginViewModel(
@@ -96,13 +99,33 @@ class LoginViewModelTest {
 
             // Act, Assert
             uiStates.skipDefaultItem()
-            loginViewModel.createAccount()
+            loginViewModel.onCreateAccountConfirmed()
             assertEquals(Loading.CreatingAccount, uiStates.awaitItem().loginState)
             assertEquals(LoginUiSideEffect.NavigateToWelcome, sideEffects.awaitItem())
         }
     }
 
     @Test
+    fun `when creating a new account the confirmation dialog should be shown when an account exists in the history`() =
+        runTest {
+            turbineScope {
+                // Arrange
+                val uiStates = loginViewModel.uiState.testIn(backgroundScope)
+                val sideEffects = loginViewModel.uiSideEffect.testIn(backgroundScope)
+
+                // Act, Assert
+                uiStates.skipDefaultItem()
+                accountHistoryFlow.value = DUMMY_ACCOUNT_NUMBER
+                loginViewModel.onCreateAccountClick()
+                assertEquals(Idle(null), uiStates.awaitItem().loginState)
+                assertEquals(
+                    LoginUiSideEffect.NavigateToCreateAccountConfirmation,
+                    sideEffects.awaitItem(),
+                )
+            }
+        }
+
+    @Test
     fun `given valid account when logging in then navigate to connect view`() = runTest {
         turbineScope {
             // Arrange
@@ -197,7 +220,7 @@ class LoginViewModelTest {
     fun `on new accountHistory emission uiState should include lastUsedAccount matching accountHistory`() =
         runTest {
             // Arrange
-            coEvery { mockedAccountRepository.fetchAccountHistory() } returns DUMMY_ACCOUNT_NUMBER
+            accountHistoryFlow.value = DUMMY_ACCOUNT_NUMBER
 
             // Act, Assert
             loginViewModel.uiState.test {
diff --git a/android/lib/resource/src/main/res/values/strings.xml b/android/lib/resource/src/main/res/values/strings.xml
index e3a591257d..693ac3a789 100644
--- a/android/lib/resource/src/main/res/values/strings.xml
+++ b/android/lib/resource/src/main/res/values/strings.xml
@@ -18,6 +18,8 @@
     <string name="login_fail_description">Invalid account number</string>
     <string name="dont_have_an_account">Don’t have an account number?</string>
     <string name="create_account">Create account</string>
+    <string name="create_new_account_warning_paragraph1">You already have a saved account number, by creating a new account the saved account number will be removed from this device. This cannot be undone.</string>
+    <string name="create_new_account_warning_paragraph2">Do you want to create a new account?</string>
     <string name="creating_new_account">Creating account...</string>
     <string name="failed_to_create_account">Failed to create account</string>
     <string name="account_created">Account created</string>
@@ -282,6 +284,7 @@
     <string name="all_locations">All locations</string>
     <string name="edit_lists">Edit lists</string>
     <string name="create_new_list">Create new list</string>
+    <string name="create_new_account">Create new account</string>
     <string name="create">Create</string>
     <string name="no_locations_found">No locations found</string>
     <string name="add_locations">Add locations</string>
diff --git a/android/lib/shared/src/main/kotlin/net/mullvad/mullvadvpn/lib/shared/AccountRepository.kt b/android/lib/shared/src/main/kotlin/net/mullvad/mullvadvpn/lib/shared/AccountRepository.kt
index b605bf7ac0..f6b146ecd3 100644
--- a/android/lib/shared/src/main/kotlin/net/mullvad/mullvadvpn/lib/shared/AccountRepository.kt
+++ b/android/lib/shared/src/main/kotlin/net/mullvad/mullvadvpn/lib/shared/AccountRepository.kt
@@ -16,6 +16,7 @@ import kotlinx.coroutines.flow.update
 import net.mullvad.mullvadvpn.lib.daemon.grpc.ManagementService
 import net.mullvad.mullvadvpn.lib.model.AccountData
 import net.mullvad.mullvadvpn.lib.model.AccountNumber
+import net.mullvad.mullvadvpn.lib.model.ClearAccountHistoryError
 import net.mullvad.mullvadvpn.lib.model.CreateAccountError
 import net.mullvad.mullvadvpn.lib.model.DeviceState
 import net.mullvad.mullvadvpn.lib.model.LoginAccountError
@@ -31,7 +32,13 @@ class AccountRepository(
     private val _mutableAccountDataCache: MutableSharedFlow<AccountData> = MutableSharedFlow()
 
     private val _isNewAccount: MutableStateFlow<Boolean> = MutableStateFlow(false)
+
+    private val _mutableAccountHistory: MutableStateFlow<AccountNumber?> = MutableStateFlow(null)
+
     val isNewAccount: StateFlow<Boolean> = _isNewAccount
+
+    val accountHistory: StateFlow<AccountNumber?> = _mutableAccountHistory
+
     val accountData: StateFlow<AccountData?> =
         merge(
                 managementService.deviceState.filterNotNull().map { deviceState ->
@@ -58,9 +65,13 @@ class AccountRepository(
         managementService.logoutAccount().onRight { _isNewAccount.update { false } }
 
     suspend fun fetchAccountHistory(): AccountNumber? =
-        managementService.getAccountHistory().getOrNull()
+        managementService
+            .getAccountHistory()
+            .onRight { _mutableAccountHistory.value = it }
+            .getOrNull()
 
-    suspend fun clearAccountHistory() = managementService.clearAccountHistory()
+    suspend fun clearAccountHistory(): Either<ClearAccountHistoryError, Unit> =
+        managementService.clearAccountHistory().onRight { _mutableAccountHistory.value = null }
 
     suspend fun getAccountData(): AccountData? = nullable {
         val deviceState = ensureNotNull(deviceRepository.deviceState.value as? DeviceState.LoggedIn)
diff --git a/gui/locales/messages.pot b/gui/locales/messages.pot
index a85ec843a2..1c746a90a5 100644
--- a/gui/locales/messages.pot
+++ b/gui/locales/messages.pot
@@ -2341,6 +2341,9 @@ msgstr ""
 msgid "Create"
 msgstr ""
 
+msgid "Create new account"
+msgstr ""
+
 msgid "Create new list"
 msgstr ""
 
@@ -2389,6 +2392,9 @@ msgstr ""
 msgid "Dismiss"
 msgstr ""
 
+msgid "Do you want to create a new account?"
+msgstr ""
+
 msgid "Edit custom lists"
 msgstr ""
 
@@ -2725,6 +2731,9 @@ msgstr ""
 msgid "YOU MIGHT BE LEAKING NETWORK TRAFFIC"
 msgstr ""
 
+msgid "You already have a saved account number, by creating a new account the saved account number will be removed from this device. This cannot be undone."
+msgstr ""
+
 msgid "You are running an unsupported app version."
 msgstr ""