PM-11974: Fix login with device notification for inactive account not…

… switching to that account (#1157)

BitwardenShared/Core/Platform/Models/Domain/PushNotificationData.swift

@@ -103,6 +103,6 @@ struct LoginRequestPushNotification: Codable, Equatable {
     /// How long until the request times out.
     let timeoutInMinutes: Int
 
-    /// The email of the account sending the login request.
-    let userEmail: String
+    /// The user id that sent the login request.
+    let userId: String
 }

BitwardenShared/Core/Platform/Services/NotificationService.swift

@@ -51,6 +51,7 @@ protocol NotificationService {
 
 /// The delegate to handle login request actions originating from notifications.
 ///
+@MainActor
 protocol NotificationServiceDelegate: AnyObject {
     /// Users are logged out, route to landing page.
     ///
@@ -66,10 +67,9 @@ protocol NotificationServiceDelegate: AnyObject {
     ///
     /// - Parameters:
     ///   - account: The account associated with the login request.
-    ///   - loginRequest: The login request to show.
     ///   - showAlert: Whether to show the alert or simply switch the account.
     ///
-    func switchAccounts(to account: Account, for loginRequest: LoginRequest, showAlert: Bool)
+    func switchAccountsForLoginRequest(to account: Account, showAlert: Bool) async
 }
 
 // MARK: - DefaultNotificationService
@@ -279,14 +279,13 @@ class DefaultNotificationService: NotificationService {
         await stateService.setLoginRequest(data)
 
         // Get the email of the account that the login request is coming from.
-        let loginSourceAccount = try await stateService.getAccounts()
-            .first(where: { $0.profile.userId == data.userId })
-        let loginSourceEmail = loginSourceAccount?.profile.email ?? ""
+        let loginSourceAccount = try await stateService.getAccount(userId: data.userId)
+        let loginSourceEmail = loginSourceAccount.profile.email
 
         // Assemble the data to add to the in-app banner notification.
         let loginRequestData = try? JSONEncoder().encode(LoginRequestPushNotification(
             timeoutInMinutes: Constants.loginRequestTimeoutMinutes,
-            userEmail: loginSourceEmail
+            userId: loginSourceAccount.profile.userId
         ))
 
         // Create an in-app banner notification to tell the user about the login request.
@@ -308,14 +307,14 @@ class DefaultNotificationService: NotificationService {
         let request = UNNotificationRequest(identifier: data.id, content: content, trigger: nil)
         try await UNUserNotificationCenter.current().add(request)
 
-        // If the request is for the existing account, show the login request view automatically.
-        guard let loginRequest = try await authService.getPendingLoginRequest(withId: data.id).first
-        else { return }
         if data.userId == userId {
-            delegate?.showLoginRequest(loginRequest)
-        } else if let loginSourceAccount {
+            // If the request is for the existing account, show the login request view automatically.
+            guard let loginRequest = try await authService.getPendingLoginRequest(withId: data.id).first
+            else { return }
+            await delegate?.showLoginRequest(loginRequest)
+        } else {
             // Otherwise, show an alert asking the user if they want to switch accounts.
-            delegate?.switchAccounts(to: loginSourceAccount, for: loginRequest, showAlert: true)
+            await delegate?.switchAccountsForLoginRequest(to: loginSourceAccount, showAlert: true)
         }
     }
 
@@ -361,20 +360,15 @@ class DefaultNotificationService: NotificationService {
     private func handleNotificationTapped(_ loginRequestData: LoginRequestPushNotification) async {
         do {
             // Get the user id of the source of the login request.
-            guard let loginSourceAccount = try await stateService.getAccounts()
-                .first(where: { $0.profile.email == loginRequestData.userEmail })
-            else { return }
+            let loginSourceAccount = try await stateService.getAccount(userId: loginRequestData.userId)
 
             // Get the active account for comparison.
             let activeAccount = try await stateService.getActiveAccount()
 
             // If the notification banner was tapped but it's for a different account, switch
             // to that account automatically.
-            if activeAccount.profile.userId != loginSourceAccount.profile.userId,
-               let loginRequestData = await stateService.getLoginRequest(),
-               let loginRequest = try await authService.getPendingLoginRequest(withId: loginRequestData.id).first {
-                try await stateService.setActiveAccount(userId: loginSourceAccount.profile.userId)
-                delegate?.switchAccounts(to: loginSourceAccount, for: loginRequest, showAlert: false)
+            if activeAccount.profile.userId != loginSourceAccount.profile.userId {
+                await delegate?.switchAccountsForLoginRequest(to: loginSourceAccount, showAlert: false)
             }
         } catch {
             errorReporter.log(error: error)

BitwardenShared/Core/Platform/Services/NotificationServiceTests.swift

@@ -371,6 +371,7 @@ class NotificationServiceTests: BitwardenTestCase { // swiftlint:disable:this ty
 
     /// `messageReceived(_:notificationDismissed:notificationTapped:)` tells
     /// the delegate to show the switch account alert if it's a login request for a non-active account.
+    @MainActor
     func test_messageReceived_loginRequest_differentAccount() async throws {
         // Set up the mock data.
         stateService.setIsAuthenticated()
@@ -379,7 +380,7 @@ class NotificationServiceTests: BitwardenTestCase { // swiftlint:disable:this ty
         authService.getPendingLoginRequestResult = .success([.fixture()])
         let loginRequestNotification = LoginRequestNotification(id: "requestId", userId: "differentUser")
         let notificationData = try JSONEncoder().encode(loginRequestNotification)
-        let message: [AnyHashable: Any] = [
+        nonisolated(unsafe) let message: [AnyHashable: Any] = [
             "data": [
                 "type": NotificationType.authRequest.rawValue,
                 "payload": String(data: notificationData, encoding: .utf8) ?? "",
@@ -392,12 +393,12 @@ class NotificationServiceTests: BitwardenTestCase { // swiftlint:disable:this ty
         // Confirm the results.
         XCTAssertEqual(stateService.loginRequest, loginRequestNotification)
         XCTAssertEqual(delegate.switchAccountsAccount, .fixture(profile: .fixture(userId: "differentUser")))
-        XCTAssertEqual(delegate.switchAccountsLoginRequest, .fixture())
         XCTAssertEqual(delegate.switchAccountsShowAlert, true)
     }
 
     /// `messageReceived(_:notificationDismissed:notificationTapped:)` tells
     /// the delegate to show the login request if it's a login request for the active account.
+    @MainActor
     func test_messageReceived_loginRequest_sameAccount() async throws {
         // Set up the mock data.
         stateService.setIsAuthenticated()
@@ -406,7 +407,7 @@ class NotificationServiceTests: BitwardenTestCase { // swiftlint:disable:this ty
         authService.getPendingLoginRequestResult = .success([.fixture()])
         let loginRequestNotification = LoginRequestNotification(id: "requestId", userId: "1")
         let notificationData = try JSONEncoder().encode(loginRequestNotification)
-        let message: [AnyHashable: Any] = [
+        nonisolated(unsafe) let message: [AnyHashable: Any] = [
             "data": [
                 "type": NotificationType.authRequest.rawValue,
                 "payload": String(data: notificationData, encoding: .utf8) ?? "",
@@ -423,14 +424,15 @@ class NotificationServiceTests: BitwardenTestCase { // swiftlint:disable:this ty
 
     /// `messageReceived(_:notificationDismissed:notificationTapped:)` handles logout requests and will not route
     /// to the landing screen if the logged-out account was not the currently active account.
+    @MainActor
     func test_messageReceived_logout_nonActiveUser() async throws {
         // Set up the mock data.
         stateService.setIsAuthenticated()
         let activeAccount: Account = .fixture()
         let nonActiveAccount: Account = .fixture(profile: .fixture(userId: "b245a33f"))
         stateService.accounts = [activeAccount, nonActiveAccount]
 
-        let message: [AnyHashable: Any] = [
+        nonisolated(unsafe) let message: [AnyHashable: Any] = [
             "data": [
                 "type": NotificationType.logOut.rawValue,
                 "payload": "{\"UserId\":\"\(nonActiveAccount.profile.userId)\"}",
@@ -445,14 +447,15 @@ class NotificationServiceTests: BitwardenTestCase { // swiftlint:disable:this ty
 
     /// `messageReceived(_:notificationDismissed:notificationTapped:)` handles logout requests and will route
     /// to the landing screen if the logged-out account was the currently active account.
+    @MainActor
     func test_messageReceived_logout_activeUser() async throws {
         // Set up the mock data.
         stateService.setIsAuthenticated()
         let activeAccount: Account = .fixture()
         let nonActiveAccount: Account = .fixture(profile: .fixture(userId: "b245a33f"))
         stateService.accounts = [activeAccount, nonActiveAccount]
 
-        let message: [AnyHashable: Any] = [
+        nonisolated(unsafe) let message: [AnyHashable: Any] = [
             "data": [
                 "type": NotificationType.logOut.rawValue,
                 "payload": "{\"UserId\":\"\(activeAccount.profile.userId)\"}",
@@ -466,15 +469,16 @@ class NotificationServiceTests: BitwardenTestCase { // swiftlint:disable:this ty
     }
 
     /// `messageReceived(_:notificationDismissed:notificationTapped:)` handles notifications being dismissed.
+    @MainActor
     func test_messageReceived_notificationDismissed() async throws {
         // Set up the mock data.
         stateService.loginRequest = LoginRequestNotification(id: "1", userId: "2")
         let loginRequest = LoginRequestPushNotification(
             timeoutInMinutes: 15,
-            userEmail: "[email protected]"
+            userId: "2"
         )
         let testData = try JSONEncoder().encode(loginRequest)
-        let message: [AnyHashable: Any] = [
+        nonisolated(unsafe) let message: [AnyHashable: Any] = [
             "notificationData": String(data: testData, encoding: .utf8) ?? "",
         ]
 
@@ -486,6 +490,7 @@ class NotificationServiceTests: BitwardenTestCase { // swiftlint:disable:this ty
     }
 
     /// `messageReceived(_:notificationDismissed:notificationTapped:)` handles notifications being tapped.
+    @MainActor
     func test_messageReceived_notificationTapped() async throws {
         // Set up the mock data.
         stateService.accounts = [.fixture()]
@@ -494,10 +499,10 @@ class NotificationServiceTests: BitwardenTestCase { // swiftlint:disable:this ty
         authService.getPendingLoginRequestResult = .success([.fixture(id: "requestId")])
         let loginRequest = LoginRequestPushNotification(
             timeoutInMinutes: 15,
-            userEmail: Account.fixture().profile.email
+            userId: Account.fixture().profile.userId
         )
         let testData = try JSONEncoder().encode(loginRequest)
-        let message: [AnyHashable: Any] = [
+        nonisolated(unsafe) let message: [AnyHashable: Any] = [
             "notificationData": String(data: testData, encoding: .utf8) ?? "",
         ]
 
@@ -506,31 +511,27 @@ class NotificationServiceTests: BitwardenTestCase { // swiftlint:disable:this ty
 
         // Confirm the results.
         XCTAssertEqual(delegate.switchAccountsAccount, .fixture())
-        XCTAssertEqual(delegate.switchAccountsLoginRequest, .fixture(id: "requestId"))
         XCTAssertEqual(delegate.switchAccountsShowAlert, false)
     }
 
     /// `messageReceived(_:notificationDismissed:notificationTapped:)` handles errors.
+    @MainActor
     func test_messageReceived_notificationTapped_error() async throws {
         // Set up the mock data.
-        stateService.accounts = [.fixture()]
-        stateService.activeAccount = .fixtureAccountLogin()
-        stateService.loginRequest = LoginRequestNotification(id: "requestId", userId: "1")
-        authService.getPendingLoginRequestResult = .failure(BitwardenTestError.example)
         let loginRequest = LoginRequestPushNotification(
             timeoutInMinutes: 15,
-            userEmail: Account.fixture().profile.email
+            userId: Account.fixture().profile.userId
         )
         let testData = try JSONEncoder().encode(loginRequest)
-        let message: [AnyHashable: Any] = [
+        nonisolated(unsafe) let message: [AnyHashable: Any] = [
             "notificationData": String(data: testData, encoding: .utf8) ?? "",
         ]
 
         // Test.
         await subject.messageReceived(message, notificationDismissed: nil, notificationTapped: true)
 
         // Confirm the results.
-        XCTAssertEqual(errorReporter.errors.last as? BitwardenTestError, .example)
+        XCTAssertEqual(errorReporter.errors.last as? StateServiceError, .noAccounts)
     }
 }
 
@@ -542,7 +543,6 @@ class MockNotificationServiceDelegate: NotificationServiceDelegate {
     var showLoginRequestRequest: LoginRequest?
 
     var switchAccountsAccount: Account?
-    var switchAccountsLoginRequest: LoginRequest?
     var switchAccountsShowAlert: Bool?
 
     func routeToLanding() async {
@@ -553,9 +553,8 @@ class MockNotificationServiceDelegate: NotificationServiceDelegate {
         showLoginRequestRequest = loginRequest
     }
 
-    func switchAccounts(to account: Account, for loginRequest: LoginRequest, showAlert: Bool) {
+    func switchAccountsForLoginRequest(to account: Account, showAlert: Bool) {
         switchAccountsAccount = account
-        switchAccountsLoginRequest = loginRequest
         switchAccountsShowAlert = showAlert
     }
 } // swiftlint:disable:this file_length

BitwardenShared/UI/Platform/Application/AppProcessor.swift

@@ -542,33 +542,29 @@ extension AppProcessor: NotificationServiceDelegate {
     ///
     /// - Parameters:
     ///   - account: The account associated with the login request.
-    ///   - loginRequest: The login request to show.
     ///   - showAlert: Whether to show the alert or simply switch the account.
     ///
-    func switchAccounts(to account: Account, for loginRequest: LoginRequest, showAlert: Bool) {
-        DispatchQueue.main.async {
-            if showAlert {
-                self.coordinator?.showAlert(.confirmation(
-                    title: Localizations.logInRequested,
-                    message: Localizations.loginAttemptFromXDoYouWantToSwitchToThisAccount(account.profile.email)
-                ) {
-                    self.switchAccounts(to: account.profile.userId, for: loginRequest)
-                })
-            } else {
-                self.switchAccounts(to: account.profile.userId, for: loginRequest)
-            }
+    func switchAccountsForLoginRequest(to account: Account, showAlert: Bool) async {
+        if showAlert {
+            coordinator?.showAlert(.confirmation(
+                title: Localizations.logInRequested,
+                message: Localizations.loginAttemptFromXDoYouWantToSwitchToThisAccount(account.profile.email)
+            ) {
+                await self.switchAccountsForLoginRequest(to: account.profile.userId)
+            })
+        } else {
+            await switchAccountsForLoginRequest(to: account.profile.userId)
         }
     }
 
-    /// Switch to the specified account and show the login request.
+    /// Switch to the specified account so they can see the login request.
     ///
-    /// - Parameters:
-    ///   - userId: The userId of the account to switch to.
-    ///   - loginRequest: The login request to show.
+    /// - Parameter userId: The user ID of the account to switch to.
     ///
-    private func switchAccounts(to userId: String, for loginRequest: LoginRequest) {
-        (coordinator as? VaultCoordinatorDelegate)?.didTapAccount(userId: userId)
-        coordinator?.navigate(to: .loginRequest(loginRequest))
+    private func switchAccountsForLoginRequest(to userId: String) async {
+        // Switch to the account, the login request will be shown when their vault loads (either
+        // immediately or after vault unlock).
+        await coordinator?.handleEvent(.switchAccounts(userId: userId, isAutomatic: false))
     }
 }
 

BitwardenShared/UI/Platform/Application/AppProcessorTests.swift

@@ -1084,6 +1084,39 @@ class AppProcessorTests: BitwardenTestCase { // swiftlint:disable:this type_body
         XCTAssertEqual(stateService.accountSetupAutofill, ["1": .complete])
     }
 
+    /// `switchAccountsForLoginRequest(to:showAlert:)` has the coordinator switch to the specified
+    /// account without showing a confirmation alert.
+    @MainActor
+    func test_switchAccountsForLoginRequest() async {
+        await subject.switchAccountsForLoginRequest(to: .fixture(), showAlert: false)
+
+        XCTAssertEqual(coordinator.events, [.switchAccounts(userId: "1", isAutomatic: false)])
+    }
+
+    /// `switchAccountsForLoginRequest(to:showAlert:)` shows an alert to confirm the user wants to
+    /// switch to the specified account and then has the coordinator switch accounts.
+    @MainActor
+    func test_switchAccountsForLoginRequest_showAlert() async throws {
+        let account = Account.fixture()
+        await subject.switchAccountsForLoginRequest(to: account, showAlert: true)
+
+        let alert = try XCTUnwrap(coordinator.alertShown.last)
+        XCTAssertEqual(
+            alert,
+            .confirmation(
+                title: Localizations.logInRequested,
+                message: Localizations.loginAttemptFromXDoYouWantToSwitchToThisAccount(account.profile.email),
+                confirmationHandler: {}
+            )
+        )
+
+        try await alert.tapAction(title: Localizations.cancel)
+        XCTAssertTrue(coordinator.events.isEmpty)
+
+        try await alert.tapAction(title: Localizations.yes)
+        XCTAssertEqual(coordinator.events, [.switchAccounts(userId: account.profile.userId, isAutomatic: false)])
+    }
+
     /// `unlockVaultWithNeverlockKey()` unlocks it calling the auth repository.
     func test_unlockVaultWithNeverlockKey() async throws {
         try await subject.unlockVaultWithNeverlockKey()