diff --git a/DuckDuckGo/SyncSettingsViewController+SyncDelegate.swift b/DuckDuckGo/SyncSettingsViewController+SyncDelegate.swift index 15ee213021..6c6343bfe2 100644 --- a/DuckDuckGo/SyncSettingsViewController+SyncDelegate.swift +++ b/DuckDuckGo/SyncSettingsViewController+SyncDelegate.swift @@ -207,20 +207,18 @@ extension SyncSettingsViewController: SyncManagementViewModelDelegate { } func switchAccounts(recoveryKey: SyncCode.RecoveryKey) async { - Task { [weak self] in - do { - try await self?.syncService.disconnect() - } catch { - Pixel.fire(pixel: .syncUserSwitchedLogoutError) - } + do { + try await syncService.disconnect() + } catch { + Pixel.fire(pixel: .syncUserSwitchedLogoutError) + } - do { - try await self?.loginAndShowDeviceConnected(recoveryKey: recoveryKey) - } catch { - Pixel.fire(pixel: .syncUserSwitchedLoginError) - } - Pixel.fire(pixel: .syncUserSwitchedAccount) + do { + try await loginAndShowDeviceConnected(recoveryKey: recoveryKey) + } catch { + Pixel.fire(pixel: .syncUserSwitchedLoginError) } + Pixel.fire(pixel: .syncUserSwitchedAccount) } private func getErrorType(from errorString: String?) -> AsyncErrorType? { diff --git a/DuckDuckGo/SyncSettingsViewController.swift b/DuckDuckGo/SyncSettingsViewController.swift index d34e57c182..0f4ca0f613 100644 --- a/DuckDuckGo/SyncSettingsViewController.swift +++ b/DuckDuckGo/SyncSettingsViewController.swift @@ -361,11 +361,7 @@ extension SyncSettingsViewController: ScanOrPasteCodeViewModelDelegate { return true } catch { if self.rootView.model.isSyncEnabled { - if rootView.model.devices.count > 1 { - promptToSwitchAccounts(recoveryKey: recoveryKey) - } else { - await switchAccounts(recoveryKey: recoveryKey) - } + await handleTwoSyncAccountsFoundDuringRecovery(recoveryKey) } else { handleError(.unableToSyncToServer, error: error, event: .syncLoginError) } @@ -406,6 +402,14 @@ extension SyncSettingsViewController: ScanOrPasteCodeViewModelDelegate { return false } + func handleTwoSyncAccountsFoundDuringRecovery(_ recoveryKey: SyncCode.RecoveryKey) async { + if rootView.model.devices.count > 1 { + promptToSwitchAccounts(recoveryKey: recoveryKey) + } else { + await switchAccounts(recoveryKey: recoveryKey) + } + } + func dismissVCAndShowRecoveryPDF() { self.navigationController?.topViewController?.dismiss(animated: true, completion: self.showRecoveryPDF) } diff --git a/DuckDuckGoTests/MockDDGSyncing.swift b/DuckDuckGoTests/MockDDGSyncing.swift index 3c7e9e4a69..b1bcd046f8 100644 --- a/DuckDuckGoTests/MockDDGSyncing.swift +++ b/DuckDuckGoTests/MockDDGSyncing.swift @@ -71,8 +71,13 @@ final class MockDDGSyncing: DDGSyncing { func createAccount(deviceName: String, deviceType: String) async throws { } + var stubLogin: [RegisteredDevice] = [] + lazy var spyLogin: (SyncCode.RecoveryKey, String, String) throws -> [RegisteredDevice] = { _, _, _ in + return self.stubLogin + } + func login(_ recoveryKey: SyncCode.RecoveryKey, deviceName: String, deviceType: String) async throws -> [RegisteredDevice] { - return [] + return try spyLogin(recoveryKey, deviceName, deviceType) } func remoteConnect() throws -> RemoteConnecting { diff --git a/DuckDuckGoTests/SyncSettingsViewControllerErrorTests.swift b/DuckDuckGoTests/SyncSettingsViewControllerErrorTests.swift index 0233f92598..cab863e20d 100644 --- a/DuckDuckGoTests/SyncSettingsViewControllerErrorTests.swift +++ b/DuckDuckGoTests/SyncSettingsViewControllerErrorTests.swift @@ -21,15 +21,18 @@ import XCTest @testable import DuckDuckGo import Core import Combine -import DDGSync +@testable import DDGSync import Persistence import Common +import SyncUI final class SyncSettingsViewControllerErrorTests: XCTestCase { var cancellables: Set! var vc: SyncSettingsViewController! var errorHandler: CapturingSyncPausedStateManager! + var ddgSyncing: MockDDGSyncing! + var testRecoveryCode = "eyJyZWNvdmVyeSI6eyJ1c2VyX2lkIjoiMDZGODhFNzEtNDFBRS00RTUxLUE2UkRtRkEwOTcwMDE5QkYwIiwicHJpbWFyeV9rZXkiOiI1QTk3U3dsQVI5RjhZakJaU09FVXBzTktnSnJEYnE3aWxtUmxDZVBWazgwPSJ9fQ==" @MainActor override func setUpWithError() throws { @@ -46,7 +49,7 @@ final class SyncSettingsViewControllerErrorTests: XCTestCase { model: model, readOnly: true, options: [:]) - let ddgSyncing = MockDDGSyncing(authState: .active, isSyncInProgress: false) + ddgSyncing = MockDDGSyncing(authState: .active, isSyncInProgress: false) let bookmarksAdapter = SyncBookmarksAdapter( database: database, favoritesDisplayModeStorage: MockFavoritesDisplayModeStoring(), @@ -162,6 +165,93 @@ final class SyncSettingsViewControllerErrorTests: XCTestCase { await fulfillment(of: [expectation], timeout: 5.0) XCTAssertTrue(errorHandler.syncDidTurnOffCalled) } + + func test_syncCodeEntered_accountAlreadyExists_oneDevice_disconnectsThenLogsInAgain() async { + await setUpWithSingleDevice(id: "1") + + var secondLoginCalled = false + + ddgSyncing.spyLogin = { [weak self] _, _, _ in + guard let self else { return [] } + ddgSyncing.spyLogin = { [weak self] _, _, _ in + secondLoginCalled = true + guard let self else { return [] } + // Assert disconnect was called first + XCTAssert(ddgSyncing.disconnectCalled) + return [RegisteredDevice(id: "1", name: "iPhone", type: "iPhone"), RegisteredDevice(id: "2", name: "Macbook Pro", type: "Macbook Pro")] + } + throw SyncError.accountAlreadyExists + } + + _ = await vc.syncCodeEntered(code: testRecoveryCode) + + XCTAssert(secondLoginCalled) + } + + func test_syncCodeEntered_accountAlreadyExists_oneDevice_updatesDevicesWithReturnedDevices() async throws { + await setUpWithSingleDevice(id: "1") + + ddgSyncing.spyLogin = { [weak self] _, _, _ in + self?.ddgSyncing.spyLogin = { _, _, _ in + return [RegisteredDevice(id: "1", name: "iPhone", type: "iPhone"), RegisteredDevice(id: "2", name: "Macbook Pro", type: "Macbook Pro")] + } + throw SyncError.accountAlreadyExists + } + + _ = await vc.syncCodeEntered(code: testRecoveryCode) + + let deviceIDs = await vc.viewModel?.devices.flatMap(\.id) + XCTAssertEqual(deviceIDs, ["1", "2"]) + } + + func test_switchAccounts_disconnectsThenLogsInAgain() async throws { + var loginCalled = false + + ddgSyncing.spyLogin = { [weak self] _, _, _ in + guard let self else { return [] } + // Assert disconnect before returning from login to ensure correct order + XCTAssert(ddgSyncing.disconnectCalled) + loginCalled = true + return [RegisteredDevice(id: "1", name: "iPhone", type: "iPhone"), RegisteredDevice(id: "2", name: "Macbook Pro", type: "Macbook Pro")] + } + + guard let syncCode = try? SyncCode.decodeBase64String(testRecoveryCode), + let recoveryKey = syncCode.recovery else { + XCTFail("Could not create RecoveryKey from code") + return + } + + await vc.switchAccounts(recoveryKey: recoveryKey) + + XCTAssert(loginCalled) + } + + func test_switchAccounts_updatesDevicesWithReturnedDevices() async throws { + ddgSyncing.spyLogin = { [weak self] _, _, _ in + guard let self else { return [] } + // Assert disconnect before returning from login to ensure correct order + XCTAssert(ddgSyncing.disconnectCalled) + return [RegisteredDevice(id: "1", name: "iPhone", type: "iPhone"), RegisteredDevice(id: "2", name: "Macbook Pro", type: "Macbook Pro")] + } + + guard let syncCode = try? SyncCode.decodeBase64String(testRecoveryCode), + let recoveryKey = syncCode.recovery else { + XCTFail("Could not create RecoveryKey from code") + return + } + + await vc.switchAccounts(recoveryKey: recoveryKey) + + let deviceIDs = await vc.viewModel?.devices.flatMap(\.id) + XCTAssertEqual(deviceIDs, ["1", "2"]) + } + + @MainActor + private func setUpWithSingleDevice(id: String) { + ddgSyncing.account = SyncAccount(deviceId: id, deviceName: "iPhone", deviceType: "iPhone", userId: "", primaryKey: Data(), secretKey: Data(), token: nil, state: .active) + ddgSyncing.registeredDevices = [RegisteredDevice(id: id, name: "iPhone", type: "iPhone")] + vc.viewModel?.devices = [SyncSettingsViewModel.Device(id: id, name: "iPhone", type: "iPhone", isThisDevice: true)] + } } class MockFavoritesDisplayModeStoring: MockFavoriteDisplayModeStorage {}