bitwarden · brant-livefront · Oct 7, 2024 · Sep 12, 2024 · Sep 12, 2024 · Sep 12, 2024
@@ -174,22 +174,37 @@ actor DefaultAuthenticatorSyncService: NSObject, AuthenticatorSyncService {
         }
     }
 
+    /// If sync has been turned off for all accounts, delete the Authenticator key from the shared keychain.
+    ///
+    private func deleteKeyIfSyncingIsOff() async throws {
+        for account in try await stateService.getAccounts() {
+            let hasAccountWithSync = try await stateService.getSyncToAuthenticator(userId: account.profile.userId)
+            guard !hasAccountWithSync else {
+                return
+            }
+        }
+        try sharedKeychainRepository.deleteAuthenticatorKey()
+    }
+
     /// Determine if the given userId has sync turned on and an unlocked vault. This method serves as the
     /// integration point of both the sync settings subscriber and the vault subscriber. When the user has sync turned
     /// on and the vault unlocked, we can proceed with the sync.
     ///
     /// - Parameter userId: The userId of the user whose sync status is being determined.
     ///
     private func determineSyncForUserId(_ userId: String) async throws {
-        guard try await stateService.getSyncToAuthenticator(userId: userId),
-              !vaultTimeoutService.isLocked(userId: userId) else {
+        if try await !stateService.getSyncToAuthenticator(userId: userId) {
             cipherPublisherTasks[userId]?.cancel()
             cipherPublisherTasks.removeValue(forKey: userId)
-            return
+            try await authBridgeItemService.deleteAllForUserId(userId)
+            try await deleteKeyIfSyncingIsOff()
+        } else if vaultTimeoutService.isLocked(userId: userId) {
+            cipherPublisherTasks[userId]?.cancel()
+            cipherPublisherTasks.removeValue(forKey: userId)
+        } else {
+            try await createAuthenticatorKeyIfNeeded()
+            subscribeToCipherUpdates(userId: userId)
         }
-
-        try await createAuthenticatorKeyIfNeeded()
-        subscribeToCipherUpdates(userId: userId)
     }
 
     /// Create a task for the given userId to listen for Cipher updates and sync to the Authenticator store.

@@ -206,10 +206,107 @@ final class AuthenticatorSyncServiceTests: BitwardenTestCase { // swiftlint:disa
         XCTAssertEqual(item.username, "user@bitwarden.com")
     }
 
-    /// Verifies that the AuthSyncService handles and reports errors when sync is turned On..
+    /// Verifies that the AuthSyncService handles an error when attempting to fetch the accounts to check
+    /// if any are left with sync.
     ///
     @MainActor
-    func test_determineSyncForUserId_error() async throws {
+    func test_deleteKeyIfSyncingIsOff_errorFetchingAccounts() async throws {
+        setupInitialState()
+        await subject.start()
+        stateService.syncToAuthenticatorSubject.send(("1", true))
+
+        waitFor(sharedKeychainRepository.authenticatorKey != nil)
+
+        stateService.accounts = nil
+        stateService.syncToAuthenticatorByUserId["1"] = false
+        stateService.syncToAuthenticatorSubject.send(("1", false))
+
+        waitFor(!errorReporter.errors.isEmpty)
+    }
+
+    /// Verifies that the AuthSyncService handles a keychain error when attempting to remove the Authenticator key.
+    ///
+    @MainActor
+    func test_deleteKeyIfSyncingIsOff_errorInKeychain() async throws {
+        setupInitialState()
+        await subject.start()
+        stateService.syncToAuthenticatorSubject.send(("1", true))
+
+        waitFor(sharedKeychainRepository.authenticatorKey != nil)
+
+        sharedKeychainRepository.errorToThrow = BitwardenTestError.example
+        stateService.syncToAuthenticatorByUserId["1"] = false
+        stateService.syncToAuthenticatorSubject.send(("1", false))
+
+        waitFor(!errorReporter.errors.isEmpty)
+    }
+
+    /// Verifies that the AuthSyncService removes the Authenticator key when the last account to sync is turned off.
+    ///
+    @MainActor
+    func test_deleteKeyIfSyncingIsOff_lastAccountSyncTurnedOff() async throws {
+        setupInitialState()
+        await subject.start()
+        stateService.syncToAuthenticatorSubject.send(("1", true))
+
+        waitFor(sharedKeychainRepository.authenticatorKey != nil)
+        stateService.syncToAuthenticatorByUserId["1"] = false
+        stateService.syncToAuthenticatorSubject.send(("1", false))
+
+        waitFor(sharedKeychainRepository.authenticatorKey == nil)
+    }
+
+    /// Verifies that the AuthSyncService does not removes the Authenticator key there are still
+    /// accounts with sync is turned on.
+    ///
+    @MainActor
+    func test_deleteKeyIfSyncingIsOff_notLastAccount() async throws {
+        setupInitialState()
+        stateService.accounts?.append(.fixture(profile: .fixture(userId: "2")))
+        stateService.syncToAuthenticatorByUserId["2"] = true
+        await subject.start()
+        stateService.syncToAuthenticatorSubject.send(("1", true))
+        waitFor(sharedKeychainRepository.authenticatorKey != nil)
+
+        stateService.syncToAuthenticatorByUserId["1"] = false
+        stateService.syncToAuthenticatorSubject.send(("1", false))
+        try await Task.sleep(nanoseconds: 10_000_000)
+
+        XCTAssertNotNil(sharedKeychainRepository.authenticatorKey)
+    }
+
+    /// Verifies that the AuthSyncService handles and reports errors when sync is turned off and the
+    /// service attempts to delete this account's items from the Store.
+    ///
+    @MainActor
+    func test_determineSyncForUserId_errorFromDeleteAllItems() async throws {
+        setupInitialState()
+        await subject.start()
+
+        authBridgeItemService.errorToThrow = BitwardenTestError.example
+        stateService.syncToAuthenticatorByUserId["1"] = false
+        stateService.syncToAuthenticatorSubject.send(("1", false))
+        waitFor(!errorReporter.errors.isEmpty)
+    }
+
+    /// Verifies that the AuthSyncService handles and reports errors when and there is an error
+    /// thrown while accessing the sync setting for the account.
+    ///
+    @MainActor
+    func test_determineSyncForUserId_errorFromFetchingSyncSetting() async throws {
+        setupInitialState()
+        await subject.start()
+
+        stateService.syncToAuthenticatorResult = .failure(BitwardenTestError.example)
+        stateService.syncToAuthenticatorSubject.send(("1", true))
+        waitFor(!errorReporter.errors.isEmpty)
+    }
+
+    /// Verifies that the AuthSyncService handles and reports errors when sync is turned On and the
+    /// keychain throws an error.
+    ///
+    @MainActor
+    func test_determineSyncForUserId_errorFromKeychain() async throws {
         setupInitialState()
         await subject.start()
         sharedKeychainRepository.errorToThrow = BitwardenTestError.example
@@ -232,14 +329,33 @@ final class AuthenticatorSyncServiceTests: BitwardenTestCase { // swiftlint:disa
         waitFor(!errorReporter.errors.isEmpty)
     }
 
-    /// Verifies that the AuthSyncService stops listening for Cipher updates when the user has sync turned off.
+    /// Verifies that the AuthSyncService stops listening for Cipher updates and removes all data in the shared store
+    /// for a user when the user has sync turned off.
     ///
     @MainActor
-    func test_determineSyncForUserId_syncOff() async throws {
+    func test_determineSyncForUserId_syncTurnedOff() async throws {
         setupInitialState()
         await subject.start()
+
+        // Send initial updates, record in Store
+        stateService.syncToAuthenticatorSubject.send(("1", true))
+        cipherDataStore.cipherSubjectByUserId["1"]?.send([
+            .fixture(
+                id: "1234",
+                login: .fixture(
+                    username: "user@bitwarden.com",
+                    totp: "totp"
+                )
+            ),
+        ])
+        waitFor(authBridgeItemService.storedItems["1"]?.first != nil)
+
+        // Unsubscribe from sync, wait for items to be deleted
         stateService.syncToAuthenticatorByUserId["1"] = false
         stateService.syncToAuthenticatorSubject.send(("1", false))
+        waitFor((authBridgeItemService.storedItems["1"]?.isEmpty) ?? false)
+
+        // Sending additional updates should not appear in Store
         cipherDataStore.cipherSubjectByUserId["1"]?.send([
             .fixture(
                 id: "1234",
@@ -250,9 +366,8 @@ final class AuthenticatorSyncServiceTests: BitwardenTestCase { // swiftlint:disa
             ),
         ])
 
-        try await Task.sleep(nanoseconds: 100_000_000)
-
-        XCTAssertFalse(authBridgeItemService.replaceAllCalled)
+        try await Task.sleep(nanoseconds: 10_000_000)
+        XCTAssertTrue(authBridgeItemService.storedItems["1"]?.isEmpty ?? false)
     }
 
     /// When user "1" has sync turned on and user "2" unlocks their vault, the service should not take
@@ -498,6 +613,7 @@ final class AuthenticatorSyncServiceTests: BitwardenTestCase { // swiftlint:disa
         cipherDataStore.cipherSubjectByUserId["1"] = CurrentValueSubject<[Cipher], Error>([])
         configService.featureFlagsBool[.enableAuthenticatorSync] = true
         stateService.activeAccount = .fixture()
+        stateService.accounts = [.fixture()]
         stateService.syncToAuthenticatorByUserId["1"] = syncOn
         vaultTimeoutService.isClientLocked["1"] = vaultLocked
     }

@@ -3,21 +3,24 @@ import BitwardenShared
 import Combine
 
 class MockAuthenticatorBridgeItemService: AuthenticatorBridgeItemService {
+    var errorToThrow: Error?
     var replaceAllCalled = false
-    var sharedItemsPublisherError: Error?
     var sharedItemsSubject = CurrentValueSubject<[AuthenticatorBridgeItemDataView], Error>([])
     var storedItems: [String: [AuthenticatorBridgeItemDataView]] = [:]
     var syncOn = false
 
     func deleteAllForUserId(_ userId: String) async throws {
+        guard errorToThrow == nil else { throw errorToThrow! }
         storedItems[userId] = []
     }
 
     func fetchAllForUserId(_ userId: String) async throws -> [AuthenticatorBridgeItemDataView] {
-        storedItems[userId] ?? []
+        guard errorToThrow == nil else { throw errorToThrow! }
+        return storedItems[userId] ?? []
     }
 
     func insertItems(_ items: [AuthenticatorBridgeItemDataView], forUserId userId: String) async throws {
+        guard errorToThrow == nil else { throw errorToThrow! }
         storedItems[userId] = items
     }
 
@@ -26,15 +29,15 @@ class MockAuthenticatorBridgeItemService: AuthenticatorBridgeItemService {
     }
 
     func replaceAllItems(with items: [AuthenticatorBridgeItemDataView], forUserId userId: String) async throws {
+        guard errorToThrow == nil else { throw errorToThrow! }
         storedItems[userId] = items
         replaceAllCalled = true
     }
 
     func sharedItemsPublisher() async throws ->
         AnyPublisher<[AuthenticatorBridgeKit.AuthenticatorBridgeItemDataView], any Error> {
-        if let sharedItemsPublisherError {
-            throw sharedItemsPublisherError
-        }
+        guard errorToThrow == nil else { throw errorToThrow! }
+
         return sharedItemsSubject.eraseToAnyPublisher()
     }
 }
@@ -15,6 +15,8 @@ extension MockSharedKeychainRepository: SharedKeychainRepository {
     }
 
     func deleteAuthenticatorKey() throws {
+        guard errorToThrow == nil else { throw errorToThrow! }
+
         authenticatorKey = nil
     }
-Original file line number
+Diff line change
@@ Expand Up @@
         }
         func deleteAuthenticatorKey() throws {
+            guard errorToThrow == nil else { throw errorToThrow! }
             authenticatorKey = nil
         }
@@ Expand Down @@