Add FXIOS-7781 [v121] Verification logic for logins syncing (#17355)

Providers/RustSyncManager.swift

@@ -328,6 +328,22 @@ public class RustSyncManager: NSObject, SyncManager {
         public let description = "Failed to get sync engine and key data."
     }
 
+    func shouldSyncLogins(completion: @escaping (Bool) -> Void) {
+        if !(self.prefs.boolForKey(PrefsKeys.HasVerifiedRustLogins) ?? false) {
+            // We should only sync logins when the verify logins step hasn't been successfully
+            // completed. Otherwise logins could exist in the database that can't be decrypted
+            // and would prevent logins from syncing if they're not removed.
+
+            self.profile?.logins.verifyLogins { successfullyVerified in
+                self.prefs.setBool(successfullyVerified, forKey: PrefsKeys.HasVerifiedRustLogins)
+                completion(successfullyVerified)
+            }
+        } else {
+            // Successful logins verification already occurred so login syncing can proceed
+            completion(true)
+        }
+    }
+
     func getEnginesAndKeys(engines: [RustSyncManagerAPI.TogglableEngine],
                            completion: @escaping (([String], [String: String])) -> Void) {
         var localEncryptionKeys: [String: String] = [:]
@@ -341,10 +357,12 @@ public class RustSyncManager: NSObject, SyncManager {
                     self.profile?.tabs.registerWithSyncManager()
                     rustEngines.append(engine.rawValue)
                 case .passwords:
-                    self.profile?.logins.registerWithSyncManager()
-                    if let key = loginKey {
-                        localEncryptionKeys[engine.rawValue] = key
-                        rustEngines.append(engine.rawValue)
+                    self.shouldSyncLogins { shouldSync in
+                        if shouldSync, let key = loginKey {
+                            self.profile?.logins.registerWithSyncManager()
+                            localEncryptionKeys[engine.rawValue] = key
+                            rustEngines.append(engine.rawValue)
+                        }
                     }
                 case .creditcards:
                     if self.creditCardAutofillEnabled {

Shared/Prefs.swift

@@ -12,6 +12,7 @@ public struct PrefsKeys {
 
     // Global sync state for rust sync manager
     public static let RustSyncManagerPersistedState = "rustSyncManagerPersistedStateKey"
+    public static let HasVerifiedRustLogins = "hasVerifiedRustLogins"
 
     public static let KeyLastSyncFinishTime = "lastSyncFinishTime"
     public static let KeyDefaultHomePageURL = "KeyDefaultHomePageURL"

Storage/Rust/RustLogins.swift

@@ -547,6 +547,72 @@ public class RustLogins {
         }
     }
 
+    private func deleteInvalidLogins(key: String,
+                                     logins: [EncryptedLogin],
+                                     completion: @escaping (Bool) -> Void) {
+        // Create a list of IDs from saved logins that can't be decrypted
+        let loginsToDelete = logins
+            .filter { (try? decryptLogin(login: $0, encryptionKey: key)) == nil }
+            .map { $0.record.id }
+
+        // Delete all the logins that can't be decrypted
+        self.deleteLogins(ids: loginsToDelete).upon { deleteResults in
+            var verified = true
+            for result in deleteResults {
+                if case let .failure(err) = result {
+                    let errMsg = "Login could not be deleted during verification"
+                    self.logger.log(errMsg,
+                                    level: .warning,
+                                    category: .storage,
+                                    description: err.localizedDescription)
+                    verified = false
+                }
+            }
+            completion(verified)
+        }
+    }
+
+    public func verifyLogins(completion: @escaping (Bool) -> Void) {
+        queue.async {
+            self.listLogins().upon { loginResult in
+                switch loginResult {
+                case let .failure(error):
+                    self.logger.log("Logins could not be retrieved for verification",
+                                    level: .warning,
+                                    category: .storage,
+                                    description: error.localizedDescription)
+                    completion(false)
+                    return
+                case let .success(logins):
+                    guard !logins.isEmpty else {
+                        // If there are no logins we don't need to go through this verification
+                        // process in the future so we return true.
+                        completion(true)
+                        return
+                    }
+
+                    let rustKeys = RustLoginEncryptionKeys()
+                    guard let key = rustKeys.keychain.string(forKey: rustKeys.loginPerFieldKeychainKey) else {
+                        // If the key is missing during the verification process, we wipe the database and
+                        // recreate the key.
+                        self.resetLoginsAndKey(rustKeys: rustKeys) { resetResult in
+                            if case let .failure(error) = resetResult {
+                                self.logger.log("Logins and key could not be reset during verification",
+                                                level: .warning,
+                                                category: .storage,
+                                                description: error.localizedDescription)
+                            }
+                            return
+                        }
+                        completion(false)
+                        return
+                    }
+                    self.deleteInvalidLogins(key: key, logins: logins, completion: completion)
+                }
+            }
+        }
+    }
+
     private func close() -> NSError? {
         storage = nil
         isOpen = false

firefox-ios/Tests/StorageTests/RustLoginsTests.swift

@@ -54,11 +54,33 @@ class MockRustLogins: RustLogins {
     }
 }
 
+class MockListLoginsFailure: RustLogins {
+    override func listLogins() -> Deferred<Maybe<[EncryptedLogin]>> {
+        let deferred = Deferred<Maybe<[EncryptedLogin]>>()
+        queue.async {
+            let error = LoginsStoreError.UnexpectedLoginsApiError(reason: "Database is closed")
+            deferred.fill(Maybe(failure: error as MaybeErrorType))
+        }
+        return deferred
+    }
+}
+
+class MockListLoginsEmpty: RustLogins {
+    override func listLogins() -> Deferred<Maybe<[EncryptedLogin]>> {
+        let deferred = Deferred<Maybe<[EncryptedLogin]>>()
+        queue.async {
+            deferred.fill(Maybe(success: [EncryptedLogin]()))
+        }
+        return deferred
+    }
+}
+
 class RustLoginsTests: XCTestCase {
     var files: FileAccessor!
     var logins: RustLogins!
     var mockLogins: MockRustLogins!
-    var encryptionKey: String!
+    var mockListLoginsFailure: MockListLoginsFailure!
+    var mockListLoginsEmpty: MockListLoginsEmpty!
 
     let keychain = MZKeychainWrapper.sharedClientAppContainerKeychain
     let canaryPhraseKey = "canaryPhrase"
@@ -75,16 +97,13 @@ class RustLoginsTests: XCTestCase {
             let databasePath = URL(fileURLWithPath: rootDirectory, isDirectory: true).appendingPathComponent("testLoginsPerField.db").path
             try? files.remove("testLoginsPerField.db")
 
-            if let key = try? createKey() {
-                encryptionKey = key
-            } else {
-                XCTFail("Encryption key wasn't created")
-            }
-
             logins = RustLogins(sqlCipherDatabasePath: sqlCipherDatabasePath, databasePath: databasePath)
             _ = logins.reopenIfClosed()
 
             mockLogins = MockRustLogins(sqlCipherDatabasePath: sqlCipherDatabasePath, databasePath: databasePath)
+
+            self.keychain.removeObject(forKey: self.canaryPhraseKey, withAccessibility: .afterFirstUnlock)
+            self.keychain.removeObject(forKey: self.loginKeychainKey, withAccessibility: .afterFirstUnlock)
         } else {
             XCTFail("Could not retrieve root directory")
         }
@@ -96,6 +115,42 @@ class RustLoginsTests: XCTestCase {
         self.keychain.removeObject(forKey: self.loginKeychainKey, withAccessibility: .afterFirstUnlock)
     }
 
+    func setUpMockListLoginsFailure() {
+        files = MockFiles()
+
+        if let rootDirectory = try? files.getAndEnsureDirectory() {
+            let sqlCipherDatabasePath = URL(fileURLWithPath: rootDirectory, isDirectory: true).appendingPathComponent("testlogins.db").path
+            try? files.remove("testlogins.db")
+
+            let databasePath = URL(fileURLWithPath: rootDirectory, isDirectory: true).appendingPathComponent("testLoginsPerField.db").path
+            try? files.remove("testLoginsPerField.db")
+
+            mockListLoginsFailure = MockListLoginsFailure(sqlCipherDatabasePath: sqlCipherDatabasePath,
+                                                          databasePath: databasePath)
+            _ = mockListLoginsFailure.reopenIfClosed()
+        } else {
+            XCTFail("Could not retrieve root directory")
+        }
+    }
+
+    func setUpMockListLoginsEmpty() {
+        files = MockFiles()
+
+        if let rootDirectory = try? files.getAndEnsureDirectory() {
+            let sqlCipherDatabasePath = URL(fileURLWithPath: rootDirectory, isDirectory: true).appendingPathComponent("testlogins.db").path
+            try? files.remove("testlogins.db")
+
+            let databasePath = URL(fileURLWithPath: rootDirectory, isDirectory: true).appendingPathComponent("testLoginsPerField.db").path
+            try? files.remove("testLoginsPerField.db")
+
+            mockListLoginsEmpty = MockListLoginsEmpty(sqlCipherDatabasePath: sqlCipherDatabasePath,
+                                                      databasePath: databasePath)
+            _ = mockListLoginsEmpty.reopenIfClosed()
+        } else {
+            XCTFail("Could not retrieve root directory")
+        }
+    }
+
     func addLogin() -> Deferred<Maybe<String>> {
         let login = LoginEntry(fromJSONDict: [
             "hostname": "https://example.com",
@@ -229,4 +284,31 @@ class RustLoginsTests: XCTestCase {
         }
         waitForExpectations(timeout: 5)
     }
+
+    func testVerifyWithFailedList() {
+        // Mocking a failed call to listLogins within verifyLogins
+        setUpMockListLoginsFailure()
+
+        let exp = expectation(description: "\(#function)\(#line)")
+        mockListLoginsFailure.verifyLogins { result in
+            exp.fulfill()
+
+            // Check that verification failed
+            XCTAssertFalse(result)
+        }
+        waitForExpectations(timeout: 5)
+    }
+
+    func testVerifyWithNoLogins() {
+        setUpMockListLoginsEmpty()
+
+        let exp = expectation(description: "\(#function)\(#line)")
+        mockListLoginsEmpty.verifyLogins { result in
+            exp.fulfill()
+
+            // Check that verification succeeds when there are no saved logins
+            XCTAssertTrue(result)
+        }
+        waitForExpectations(timeout: 5)
+    }
 }