Ask user to swith account when reading code while signed in (#5271)

Task/Issue URL: https://app.asana.com/0/72649045549333/1208755822305612/f ### Description When a Sync signed in user reads a recovery code, ask them to confirm if they want to switch account. Includes: - Remote FF (disabled by default until implementation is completed) - Logic to handle that scenario when reading a QR code or introducing text manually. ### Steps to test this PR _Feature 1_ - [x] fresh install in 2 devices - [x] Go to Feature flags and enable FF `seamlessAccountSwitching` on both - [x] Create a Sync account in both devices (different ones) - [x] From both devices access Sync settings and select "Sync With Another Device" - [x] From one device, read the QR code from the other device - [x] Ensure it appears a dialog asking the user to switch accounts appears - [x] Click on switch account - [x] Confirm you are switched to the new account - [x] Confirm you see the recovery code screen and Device connected screen after that _Feature 2_ (continuation from previous test) - [x] logout one device, and create a new sync account there - [x] you should have again 2 devices on 2 different sync accounts - [x] From both devices access Sync settings and select "Sync With Another Device" - [x] From one device, click "share copy text" so you have the recovery code copied in the clipboard - [x] Send the code to the other device - [x] In the other device, from that "Sync With Another Device" screen, click on "Manually enter code" - [x] Click on paste code (which should be the code you copied before) - [x] Ensure it appears a dialog asking the user to switch accounts appears - [x] Click on switch account - [x] Confirm you are switched to the new account - [x] Confirm you see the recovery code screen and Device connected screen after that ### UI changes | Before | After | | ------ | ----- | !(Upload before screenshot)|(Upload after screenshot)|
duckduckgo · Nov 22, 2024 · 0a1f156 · 0a1f156
1 parent 83e8be5
commit 0a1f156
Show file tree

Hide file tree

Showing 19 changed files with 678 additions and 74 deletions.
diff --git a/sync/sync-impl/lint-baseline.xml b/sync/sync-impl/lint-baseline.xml
@@ -34,6 +34,17 @@
             message="Defined here, included via layout/activity_sync.xml => layout/view_sync_disabled.xml defines @+id/syncSetupOtherPlatforms"/>
     </issue>
 
+    <issue
+        id="DenyListedApi"
+        message="If you find yourself using this API in production, you&apos;re doing something wrong!!"
+        errorLine1="        this.seamlessAccountSwitching().setRawStoredState(State(true))"
+        errorLine2="        ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~">
+        <location
+            file="src/test/java/com/duckduckgo/sync/impl/AppSyncAccountRepositoryTest.kt"
+            line="100"
+            column="9"/>
+    </issue>
+
     <issue
         id="DenyListedApi"
         message="No one remembers what these constants map to. Use the API level integer value directly since it&apos;s self-defining."
@@ -100,6 +111,28 @@
             column="38"/>
     </issue>
 
+    <issue
+        id="DenyListedApi"
+        message="If you find yourself using this API in production, you&apos;re doing something wrong!!"
+        errorLine1="        this.seamlessAccountSwitching().setRawStoredState(State(true))"
+        errorLine2="        ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~">
+        <location
+            file="src/test/java/com/duckduckgo/sync/impl/ui/EnterCodeViewModelTest.kt"
+            line="63"
+            column="9"/>
+    </issue>
+
+    <issue
+        id="DenyListedApi"
+        message="If you find yourself using this API in production, you&apos;re doing something wrong!!"
+        errorLine1="        syncFeature.seamlessAccountSwitching().setRawStoredState(State(false))"
+        errorLine2="        ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~">
+        <location
+            file="src/test/java/com/duckduckgo/sync/impl/ui/EnterCodeViewModelTest.kt"
+            line="137"
+            column="9"/>
+    </issue>
+
     <issue
         id="DenyListedApi"
         message="No one remembers what these constants map to. Use the API level integer value directly since it&apos;s self-defining."
@@ -375,6 +408,28 @@
             column="16"/>
     </issue>
 
+    <issue
+        id="DenyListedApi"
+        message="If you find yourself using this API in production, you&apos;re doing something wrong!!"
+        errorLine1="        this.seamlessAccountSwitching().setRawStoredState(State(true))"
+        errorLine2="        ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~">
+        <location
+            file="src/test/java/com/duckduckgo/sync/impl/ui/SyncWithAnotherDeviceViewModelTest.kt"
+            line="62"
+            column="9"/>
+    </issue>
+
+    <issue
+        id="DenyListedApi"
+        message="If you find yourself using this API in production, you&apos;re doing something wrong!!"
+        errorLine1="        syncFeature.seamlessAccountSwitching().setRawStoredState(State(false))"
+        errorLine2="        ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~">
+        <location
+            file="src/test/java/com/duckduckgo/sync/impl/ui/SyncWithAnotherDeviceViewModelTest.kt"
+            line="135"
+            column="9"/>
+    </issue>
+
     <issue
         id="VectorRaster"
         message="Limit vector icons sizes to 200×200 to keep icon drawing fast; see https://developer.android.com/studio/write/vector-asset-studio#when for more"

diff --git a/sync/sync-impl/src/main/java/com/duckduckgo/sync/impl/SyncAccountRepository.kt b/sync/sync-impl/src/main/java/com/duckduckgo/sync/impl/SyncAccountRepository.kt
@@ -59,6 +59,7 @@ interface SyncAccountRepository {
     fun getConnectQR(): Result<String>
     fun pollConnectionKeys(): Result<Boolean>
     fun renameDevice(device: ConnectedDevice): Result<Boolean>
+    fun logoutAndJoinNewAccount(stringCode: String): Result<Boolean>
 }
 
 @ContributesBinding(AppScope::class)
@@ -73,8 +74,11 @@ class AppSyncAccountRepository @Inject constructor(
     private val syncPixels: SyncPixels,
     @AppCoroutineScope private val appCoroutineScope: CoroutineScope,
     private val dispatcherProvider: DispatcherProvider,
+    private val syncFeature: SyncFeature,
 ) : SyncAccountRepository {
 
+    private val connectedDevicesCached: MutableList<ConnectedDevice> = mutableListOf()
+
     override fun isSyncSupported(): Boolean {
         return syncStore.isEncryptionSupported()
     }
@@ -107,9 +111,20 @@ class AppSyncAccountRepository @Inject constructor(
     }
 
     private fun login(recoveryCode: RecoveryCode): Result<Boolean> {
+        var wasUserLogout = false
         if (isSignedIn()) {
-            return Error(code = ALREADY_SIGNED_IN.code, reason = "Already signed in")
-                .alsoFireAlreadySignedInErrorPixel()
+            val allowSwitchAccount = syncFeature.seamlessAccountSwitching().isEnabled()
+            val error = Error(code = ALREADY_SIGNED_IN.code, reason = "Already signed in").alsoFireAlreadySignedInErrorPixel()
+            if (allowSwitchAccount && connectedDevicesCached.size == 1) {
+                val thisDeviceId = syncStore.deviceId.orEmpty()
+                val result = logout(thisDeviceId)
+                if (result is Error) {
+                    return result
+                }
+                wasUserLogout = true
+            } else {
+                return error
+            }
         }
 
         val primaryKey = recoveryCode.primaryKey
@@ -119,6 +134,9 @@ class AppSyncAccountRepository @Inject constructor(
 
         return performLogin(userId, deviceId, deviceName, primaryKey).onFailure {
             it.alsoFireLoginErrorPixel()
+            if (wasUserLogout) {
+                syncPixels.fireUserSwitchedLoginError()
+            }
             return it.copy(code = LOGIN_FAILED.code)
         }
     }
@@ -287,6 +305,7 @@ class AppSyncAccountRepository @Inject constructor(
 
         return when (val result = syncApi.getDevices(token)) {
             is Error -> {
+                connectedDevicesCached.clear()
                 result.alsoFireAccountErrorPixel().copy(code = GENERIC_ERROR.code)
             }
 
@@ -314,6 +333,11 @@ class AppSyncAccountRepository @Inject constructor(
                         }
                     }.sortedWith { a, b ->
                         if (a.thisDevice) -1 else 1
+                    }.also {
+                        connectedDevicesCached.apply {
+                            clear()
+                            addAll(it)
+                        }
                     },
                 )
             }
@@ -322,6 +346,23 @@ class AppSyncAccountRepository @Inject constructor(
 
     override fun isSignedIn() = syncStore.isSignedIn()
 
+    override fun logoutAndJoinNewAccount(stringCode: String): Result<Boolean> {
+        val thisDeviceId = syncStore.deviceId.orEmpty()
+        return when (val result = logout(thisDeviceId)) {
+            is Error -> {
+                syncPixels.fireUserSwitchedLogoutError()
+                result
+            }
+            is Result.Success -> {
+                val loginResult = processCode(stringCode)
+                if (loginResult is Error) {
+                    syncPixels.fireUserSwitchedLoginError()
+                }
+                loginResult
+            }
+        }
+    }
+
     private fun performCreateAccount(): Result<Boolean> {
         val userId = syncDeviceIds.userId()
         val account: AccountKeys = kotlin.runCatching {

diff --git a/sync/sync-impl/src/main/java/com/duckduckgo/sync/impl/SyncFeature.kt b/sync/sync-impl/src/main/java/com/duckduckgo/sync/impl/SyncFeature.kt
@@ -19,6 +19,7 @@ package com.duckduckgo.sync.impl
 import com.duckduckgo.anvil.annotations.ContributesRemoteFeature
 import com.duckduckgo.di.scopes.AppScope
 import com.duckduckgo.feature.toggles.api.Toggle
+import com.duckduckgo.feature.toggles.api.Toggle.InternalAlwaysEnabled
 
 @ContributesRemoteFeature(
     scope = AppScope::class,
@@ -43,4 +44,7 @@ interface SyncFeature {
 
     @Toggle.DefaultValue(true)
     fun gzipPatchRequests(): Toggle
+
+    @Toggle.DefaultValue(true)
+    fun seamlessAccountSwitching(): Toggle
 }
diff --git a/sync/sync-impl/src/main/java/com/duckduckgo/sync/impl/pixels/SyncPixelParamRemovalPlugin.kt b/sync/sync-impl/src/main/java/com/duckduckgo/sync/impl/pixels/SyncPixelParamRemovalPlugin.kt
@@ -35,6 +35,13 @@ class SyncPixelParamRemovalPlugin @Inject constructor() : PixelParamRemovalPlugi
             SyncPixelName.SYNC_GET_OTHER_DEVICES_SCREEN_SHOWN.pixelName to PixelParameter.removeAtb(),
             SyncPixelName.SYNC_GET_OTHER_DEVICES_LINK_COPIED.pixelName to PixelParameter.removeAtb(),
             SyncPixelName.SYNC_GET_OTHER_DEVICES_LINK_SHARED.pixelName to PixelParameter.removeAtb(),
+
+            SyncPixelName.SYNC_ASK_USER_TO_SWITCH_ACCOUNT.pixelName to PixelParameter.removeAtb(),
+            SyncPixelName.SYNC_USER_ACCEPTED_SWITCHING_ACCOUNT.pixelName to PixelParameter.removeAtb(),
+            SyncPixelName.SYNC_USER_CANCELLED_SWITCHING_ACCOUNT.pixelName to PixelParameter.removeAtb(),
+            SyncPixelName.SYNC_USER_SWITCHED_ACCOUNT.pixelName to PixelParameter.removeAtb(),
+            SyncPixelName.SYNC_USER_SWITCHED_LOGOUT_ERROR.pixelName to PixelParameter.removeAtb(),
+            SyncPixelName.SYNC_USER_SWITCHED_LOGIN_ERROR.pixelName to PixelParameter.removeAtb(),
         )
     }
 }
diff --git a/sync/sync-impl/src/main/java/com/duckduckgo/sync/impl/pixels/SyncPixels.kt b/sync/sync-impl/src/main/java/com/duckduckgo/sync/impl/pixels/SyncPixels.kt
@@ -81,6 +81,13 @@ interface SyncPixels {
         feature: SyncableType,
         apiError: Error,
     )
+
+    fun fireAskUserToSwitchAccount()
+    fun fireUserAcceptedSwitchingAccount()
+    fun fireUserCancelledSwitchingAccount()
+    fun fireUserSwitchedAccount()
+    fun fireUserSwitchedLogoutError()
+    fun fireUserSwitchedLoginError()
 }
 
 @ContributesBinding(AppScope::class)
@@ -255,6 +262,30 @@ class RealSyncPixels @Inject constructor(
         }
     }
 
+    override fun fireUserSwitchedAccount() {
+        pixel.fire(SyncPixelName.SYNC_USER_SWITCHED_ACCOUNT)
+    }
+
+    override fun fireAskUserToSwitchAccount() {
+        pixel.fire(SyncPixelName.SYNC_ASK_USER_TO_SWITCH_ACCOUNT)
+    }
+
+    override fun fireUserAcceptedSwitchingAccount() {
+        pixel.fire(SyncPixelName.SYNC_USER_ACCEPTED_SWITCHING_ACCOUNT)
+    }
+
+    override fun fireUserCancelledSwitchingAccount() {
+        pixel.fire(SyncPixelName.SYNC_USER_CANCELLED_SWITCHING_ACCOUNT)
+    }
+
+    override fun fireUserSwitchedLoginError() {
+        pixel.fire(SyncPixelName.SYNC_USER_SWITCHED_LOGIN_ERROR)
+    }
+
+    override fun fireUserSwitchedLogoutError() {
+        pixel.fire(SyncPixelName.SYNC_USER_SWITCHED_LOGOUT_ERROR)
+    }
+
     companion object {
         private const val SYNC_PIXELS_PREF_FILE = "com.duckduckgo.sync.pixels.v1"
     }
@@ -302,6 +333,12 @@ enum class SyncPixelName(override val pixelName: String) : Pixel.PixelName {
     SYNC_GET_OTHER_DEVICES_SCREEN_SHOWN("sync_get_other_devices"),
     SYNC_GET_OTHER_DEVICES_LINK_COPIED("sync_get_other_devices_copy"),
     SYNC_GET_OTHER_DEVICES_LINK_SHARED("sync_get_other_devices_share"),
+    SYNC_ASK_USER_TO_SWITCH_ACCOUNT("sync_ask_user_to_switch_account"),
+    SYNC_USER_ACCEPTED_SWITCHING_ACCOUNT("sync_user_accepted_switching_account"),
+    SYNC_USER_CANCELLED_SWITCHING_ACCOUNT("sync_user_cancelled_switching_account"),
+    SYNC_USER_SWITCHED_ACCOUNT("sync_user_switched_account"),
+    SYNC_USER_SWITCHED_LOGOUT_ERROR("sync_user_switched_logout_error"),
+    SYNC_USER_SWITCHED_LOGIN_ERROR("sync_user_switched_login_error"),
 }
 
 object SyncPixelParameters {

diff --git a/sync/sync-impl/src/main/java/com/duckduckgo/sync/impl/ui/EnterCodeActivity.kt b/sync/sync-impl/src/main/java/com/duckduckgo/sync/impl/ui/EnterCodeActivity.kt
@@ -36,8 +36,10 @@ import com.duckduckgo.sync.impl.ui.EnterCodeViewModel.AuthState
 import com.duckduckgo.sync.impl.ui.EnterCodeViewModel.AuthState.Idle
 import com.duckduckgo.sync.impl.ui.EnterCodeViewModel.AuthState.Loading
 import com.duckduckgo.sync.impl.ui.EnterCodeViewModel.Command
-import com.duckduckgo.sync.impl.ui.EnterCodeViewModel.Command.LoginSucess
+import com.duckduckgo.sync.impl.ui.EnterCodeViewModel.Command.AskToSwitchAccount
+import com.duckduckgo.sync.impl.ui.EnterCodeViewModel.Command.LoginSuccess
 import com.duckduckgo.sync.impl.ui.EnterCodeViewModel.Command.ShowError
+import com.duckduckgo.sync.impl.ui.EnterCodeViewModel.Command.SwitchAccountSuccess
 import com.duckduckgo.sync.impl.ui.EnterCodeViewModel.ViewState
 import kotlinx.coroutines.flow.launchIn
 import kotlinx.coroutines.flow.onEach
@@ -96,14 +98,22 @@ class EnterCodeActivity : DuckDuckGoActivity() {
 
     private fun processCommand(command: Command) {
         when (command) {
-            LoginSucess -> {
+            LoginSuccess -> {
                 setResult(RESULT_OK)
                 finish()
             }
 
             is ShowError -> {
                 showError(command)
             }
+
+            is AskToSwitchAccount -> askUserToSwitchAccount(command)
+            SwitchAccountSuccess -> {
+                val resultIntent = Intent()
+                resultIntent.putExtra(EXTRA_USER_SWITCHED_ACCOUNT, true)
+                setResult(RESULT_OK, resultIntent)
+                finish()
+            }
         }
     }
 
@@ -120,6 +130,26 @@ class EnterCodeActivity : DuckDuckGoActivity() {
             ).show()
     }
 
+    private fun askUserToSwitchAccount(it: AskToSwitchAccount) {
+        viewModel.onUserAskedToSwitchAccount()
+        TextAlertDialogBuilder(this)
+            .setTitle(R.string.sync_dialog_switch_account_header)
+            .setMessage(R.string.sync_dialog_switch_account_description)
+            .setPositiveButton(R.string.sync_dialog_switch_account_primary_button)
+            .setNegativeButton(R.string.sync_dialog_switch_account_secondary_button)
+            .addEventListener(
+                object : TextAlertDialogBuilder.EventListener() {
+                    override fun onPositiveButtonClicked() {
+                        viewModel.onUserAcceptedJoiningNewAccount(it.encodedStringCode)
+                    }
+
+                    override fun onNegativeButtonClicked() {
+                        viewModel.onUserCancelledJoiningNewAccount()
+                    }
+                },
+            ).show()
+    }
+
     companion object {
         enum class Code {
             RECOVERY_CODE,
@@ -128,6 +158,8 @@ class EnterCodeActivity : DuckDuckGoActivity() {
 
         private const val EXTRA_CODE_TYPE = "codeType"
 
+        const val EXTRA_USER_SWITCHED_ACCOUNT = "userSwitchedAccount"
+
         internal fun intent(context: Context, codeType: Code): Intent {
             return Intent(context, EnterCodeActivity::class.java).apply {
                 putExtra(EXTRA_CODE_TYPE, codeType)