diff --git a/.swiftpm/xcode/xcshareddata/xcschemes/BrowserServicesKit.xcscheme b/.swiftpm/xcode/xcshareddata/xcschemes/BrowserServicesKit.xcscheme index b3e79c64a..6c05e98e7 100644 --- a/.swiftpm/xcode/xcshareddata/xcschemes/BrowserServicesKit.xcscheme +++ b/.swiftpm/xcode/xcshareddata/xcschemes/BrowserServicesKit.xcscheme @@ -165,15 +165,15 @@ - - - + skipped = "NO"> + + + WebsiteAccount? { + guard let tldPlus1 = tld.eTLDplus1(domain) else { + return nil + } + guard let account = Self.accounts[tldPlus1] else { + return nil + } + + guard account.lastUpdated.isLessThan(minutesAgo: 3) else { + Self.accounts.removeValue(forKey: domain) + return nil + } + + return account + } + + func store(partialAccount: WebsiteAccount, for domain: String) { + guard let tldPlus1 = tld.eTLDplus1(domain) else { + return + } + Self.accounts[tldPlus1] = partialAccount + } + + func removePartialAccount(for domain: String) { + guard let tldPlus1 = tld.eTLDplus1(domain) else { + return + } + Self.accounts.removeValue(forKey: tldPlus1) + } +} diff --git a/Sources/BrowserServicesKit/SecureVault/SecureVaultManager.swift b/Sources/BrowserServicesKit/SecureVault/SecureVaultManager.swift index 2c4be3389..ed9c69168 100644 --- a/Sources/BrowserServicesKit/SecureVault/SecureVaultManager.swift +++ b/Sources/BrowserServicesKit/SecureVault/SecureVaultManager.swift @@ -106,6 +106,8 @@ public class SecureVaultManager { // This property can be removed once all platforms will search for partial account matches as the default expected behaviour. private let includePartialAccountMatches: Bool + private let shouldAllowPartialFormSaves: Bool + public let tld: TLD? // Keeps track of partial account created from autogenerated credentials (Private Email + Pwd) @@ -121,6 +123,7 @@ public class SecureVaultManager { private var autogeneratedCredentials: Bool { return autogeneratedUserName || autogeneratedPassword } + private let partialFormSaveManager: PartialFormSaveManager public lazy var autofillWebsiteAccountMatcher: AutofillWebsiteAccountMatcher? = { guard let tld = tld else { return nil } @@ -131,11 +134,18 @@ public class SecureVaultManager { public init(vault: (any AutofillSecureVault)? = nil, passwordManager: PasswordManager? = nil, includePartialAccountMatches: Bool = false, + shouldAllowPartialFormSaves: Bool = false, tld: TLD? = nil) { self.vault = vault self.passwordManager = passwordManager self.includePartialAccountMatches = includePartialAccountMatches + self.shouldAllowPartialFormSaves = shouldAllowPartialFormSaves self.tld = tld + if let tld { + partialFormSaveManager = PartialFormSaveManager(tld: tld) + } else { + partialFormSaveManager = PartialFormSaveManager(tld: TLD()) + } } } @@ -213,6 +223,19 @@ extension SecureVaultManager: AutofillSecureVaultDelegate { } var autofilldata = data + + if shouldAllowPartialFormSaves, + let partialAccountUsername = partialFormSaveManager.partialAccount(forDomain: domain)?.username, + !partialAccountUsername.isEmpty, + let credentials = autofilldata.credentials, + credentials.username.isNilOrEmpty { + autofilldata.credentials = .init( + username: partialAccountUsername, + password: credentials.password, + autogenerated: credentials.autogenerated + ) + } + let vault = try? self.vault ?? AutofillSecureVaultFactory.makeVault(reporter: self.delegate) var autoSavedCredentials: SecureVaultModels.WebsiteCredentials? @@ -240,17 +263,13 @@ extension SecureVaultManager: AutofillSecureVaultDelegate { autosaveAccount = nil } - if autofilldata.trigger == .emailProtection { + switch autofilldata.trigger { + case .emailProtection: autogeneratedUserName = data.credentials?.autogenerated ?? false - } - - if autofilldata.trigger == .passwordGeneration { + case .passwordGeneration: autogeneratedPassword = data.credentials?.autogenerated ?? false NotificationCenter.default.post(name: .autofillFillEvent, object: nil) - } - - // Account for cases when the user has manually changed an autogenerated password or private email - if autofilldata.trigger == .formSubmission { + case .formSubmission: if autosaveAccount != nil, let credentials = autoSavedCredentials { let existingUsername = credentials.account.username @@ -267,6 +286,13 @@ extension SecureVaultManager: AutofillSecureVaultDelegate { } } + case .partialSave: + guard shouldAllowPartialFormSaves else { return } + let username = data.credentials?.username ?? data.identity?.emailAddress + let partialAccount = SecureVaultModels.WebsiteAccount(username: username, domain: domain) + partialFormSaveManager.store(partialAccount: partialAccount, for: domain) + return + case .none, .userInitiated, .autoprompt: break } autofilldata.credentials?.autogenerated = autogeneratedCredentials @@ -287,6 +313,7 @@ extension SecureVaultManager: AutofillSecureVaultDelegate { delegate?.secureVaultManager(self, promptUserToStoreAutofillData: dataToPrompt, withTrigger: data.trigger) autosaveAccount = nil autosaveAccountCreatedInSession = false + partialFormSaveManager.removePartialAccount(for: domain) } return } else { @@ -304,6 +331,7 @@ extension SecureVaultManager: AutofillSecureVaultDelegate { dataToPrompt.automaticallySavedCredentials = autogeneratedCredentials autosaveAccount = nil autosaveAccountCreatedInSession = false + partialFormSaveManager.removePartialAccount(for: domain) } delegate?.secureVaultManager(self, promptUserToStoreAutofillData: dataToPrompt, withTrigger: autofilldata.trigger) } @@ -676,12 +704,11 @@ extension SecureVaultManager: AutofillSecureVaultDelegate { guard let accounts = try? vault.accountsFor(domain: domain), // Matching account (username) or account with empty username for domain - var account = accounts.first(where: { $0.username == credentials.username || $0.username == "" }) else { + var account = accounts.first(where: { $0.username == credentials.username || $0.username.isNilOrEmpty }) else { // No existing credentials found. This is a new entry let account = SecureVaultModels.WebsiteAccount(username: credentials.username ?? "", domain: domain) return SecureVaultModels.WebsiteCredentials(account: account, password: passwordData) - } guard let existingAccountId = account.id, @@ -702,7 +729,7 @@ extension SecureVaultManager: AutofillSecureVaultDelegate { } // Prompt to update the login on submit when previous username was empty (there was a partial password account) - if existingCredentials.account.username == "" { + if existingCredentials.account.username.isNilOrEmpty { account.username = autofillData.credentials?.username ?? "" return SecureVaultModels.WebsiteCredentials(account: account, password: passwordData) } diff --git a/Sources/Common/Extensions/DateExtension.swift b/Sources/Common/Extensions/DateExtension.swift index 2a4ca74ea..4ae7be7f6 100644 --- a/Sources/Common/Extensions/DateExtension.swift +++ b/Sources/Common/Extensions/DateExtension.swift @@ -139,4 +139,8 @@ public extension Date { self > Date().addingTimeInterval(Double(-days) * 24 * 60 * 60) } + func isLessThan(minutesAgo minutes: Int) -> Bool { + self > Date().addingTimeInterval(Double(-minutes) * 60) + } + } diff --git a/Sources/Common/Extensions/StringExtension.swift b/Sources/Common/Extensions/StringExtension.swift index 9282a43b4..31a2481c5 100644 --- a/Sources/Common/Extensions/StringExtension.swift +++ b/Sources/Common/Extensions/StringExtension.swift @@ -495,3 +495,12 @@ public extension StringProtocol { } } + +public extension Optional where Wrapped == String { + var isNilOrEmpty: Bool { + if case .some(let wrapped) = self { + return wrapped.isEmpty + } + return true + } +} diff --git a/Tests/BrowserServicesKitTests/SecureVault/MockAutofillDatabaseProvider.swift b/Tests/BrowserServicesKitTests/SecureVault/MockAutofillDatabaseProvider.swift index d116f3f94..21623f840 100644 --- a/Tests/BrowserServicesKitTests/SecureVault/MockAutofillDatabaseProvider.swift +++ b/Tests/BrowserServicesKitTests/SecureVault/MockAutofillDatabaseProvider.swift @@ -19,6 +19,7 @@ import Foundation import GRDB import SecureStorage +import Common @testable import BrowserServicesKit @@ -57,8 +58,11 @@ internal class MockAutofillDatabaseProvider: AutofillDatabaseProvider { _credentialsDict[accountID] = credentials return accountID } else { + var credentialsToStore = credentials let id = Int64(_credentialsDict.count + 1) - _credentialsDict[id] = credentials + credentialsToStore.account.id = String(id) + _credentialsDict[id] = credentialsToStore + _accounts.append(credentialsToStore.account) return id } } @@ -68,11 +72,15 @@ internal class MockAutofillDatabaseProvider: AutofillDatabaseProvider { } func websiteCredentialsForDomain(_ domain: String) throws -> [BrowserServicesKit.SecureVaultModels.WebsiteCredentials] { - return _credentialsForDomainDict[domain] ?? [] + return _credentialsForDomainDict[domain] ?? _credentialsDict.values.filter { + $0.account.domain == domain + } } func websiteCredentialsForTopLevelDomain(_ eTLDplus1: String) throws -> [BrowserServicesKit.SecureVaultModels.WebsiteCredentials] { - return _credentialsForDomainDict[eTLDplus1] ?? [] + return _credentialsForDomainDict[eTLDplus1] ?? _credentialsDict.values.filter { + TLD().eTLDplus1($0.account.domain) == eTLDplus1 + } } func websiteAccountsForDomain(_ domain: String) throws -> [SecureVaultModels.WebsiteAccount] { diff --git a/Tests/BrowserServicesKitTests/SecureVault/SecureVaultManagerTests.swift b/Tests/BrowserServicesKitTests/SecureVault/SecureVaultManagerTests.swift index 71e65449a..2da97e0aa 100644 --- a/Tests/BrowserServicesKitTests/SecureVault/SecureVaultManagerTests.swift +++ b/Tests/BrowserServicesKitTests/SecureVault/SecureVaultManagerTests.swift @@ -66,7 +66,7 @@ class SecureVaultManagerTests: XCTestCase { self.testVault = DefaultAutofillSecureVault(providers: providers) self.secureVaultManagerDelegate = MockSecureVaultManagerDelegate() - self.manager = SecureVaultManager(vault: self.testVault) + self.manager = SecureVaultManager(vault: self.testVault, shouldAllowPartialFormSaves: true) self.manager.delegate = secureVaultManagerDelegate } @@ -854,6 +854,94 @@ class SecureVaultManagerTests: XCTestCase { } + func testWhenFormSubmittedWithNilUsername_afterPartialSaveWithUsername_storesAndPromptsWithFullCredentials() { + let partialCredentials = AutofillUserScript.IncomingCredentials(username: "email@example.com", password: nil, autogenerated: false) + let partialData = AutofillUserScript.DetectedAutofillData(identity: nil, credentials: partialCredentials, creditCard: nil, trigger: .partialSave) + manager.autofillUserScript(mockAutofillUserScript, didRequestStoreDataForDomain: "fill.dev", data: partialData) + + var incomingCredentials = AutofillUserScript.IncomingCredentials(username: nil, password: "m4nu4lP4sswOrd", autogenerated: false) + var incomingData = AutofillUserScript.DetectedAutofillData(identity: nil, credentials: incomingCredentials, creditCard: nil, trigger: .formSubmission) + manager.autofillUserScript(mockAutofillUserScript, didRequestStoreDataForDomain: "fill.dev", data: incomingData) + + // Check stored + incomingCredentials = AutofillUserScript.IncomingCredentials(username: "email@example.com", password: "m4nu4lP4sswOrd", autogenerated: false) + incomingData = AutofillUserScript.DetectedAutofillData(identity: nil, credentials: incomingCredentials, creditCard: nil, trigger: .formSubmission) + let entries = try? manager.existingEntries(for: "fill.dev", autofillData: incomingData) + XCTAssertEqual(entries?.credentials?.account.username, "email@example.com") + XCTAssertEqual(entries?.credentials?.password, Data("m4nu4lP4sswOrd".utf8)) + + // Check prompted + XCTAssertNotNil(secureVaultManagerDelegate.promptedAutofillData) + XCTAssertEqual(secureVaultManagerDelegate.promptedAutofillData?.credentials?.account.username, "email@example.com") + XCTAssertEqual(secureVaultManagerDelegate.promptedAutofillData?.credentials?.password, Data("m4nu4lP4sswOrd".utf8)) + } + + func testWhenFormSubmittedWithNilUsername_afterPartialSaveWithUsername_domainsDifferent_onlyStoresTheFormSubmission() { + let partialCredentials = AutofillUserScript.IncomingCredentials(username: "email@example.com", password: nil, autogenerated: false) + let partialData = AutofillUserScript.DetectedAutofillData(identity: nil, credentials: partialCredentials, creditCard: nil, trigger: .partialSave) + manager.autofillUserScript(mockAutofillUserScript, didRequestStoreDataForDomain: "dill.fev", data: partialData) + + var incomingCredentials = AutofillUserScript.IncomingCredentials(username: nil, password: "m4nu4lP4sswOrd", autogenerated: false) + var incomingData = AutofillUserScript.DetectedAutofillData(identity: nil, credentials: incomingCredentials, creditCard: nil, trigger: .formSubmission) + manager.autofillUserScript(mockAutofillUserScript, didRequestStoreDataForDomain: "fill.dev", data: incomingData) + + // Check stored + incomingCredentials = AutofillUserScript.IncomingCredentials(username: nil, password: "m4nu4lP4sswOrd", autogenerated: false) + incomingData = AutofillUserScript.DetectedAutofillData(identity: nil, credentials: incomingCredentials, creditCard: nil, trigger: .formSubmission) + let entries = try? manager.existingEntries(for: "fill.dev", autofillData: incomingData) + XCTAssertNotEqual(entries?.credentials?.account.username, "email@example.com") + XCTAssertEqual(entries?.credentials?.password, Data("m4nu4lP4sswOrd".utf8)) + + // Check prompted + XCTAssertNotNil(secureVaultManagerDelegate.promptedAutofillData) + XCTAssertNotEqual(secureVaultManagerDelegate.promptedAutofillData?.credentials?.account.username, "email@example.com") + XCTAssertEqual(secureVaultManagerDelegate.promptedAutofillData?.credentials?.password, Data("m4nu4lP4sswOrd".utf8)) + } + + func testWhenFormSubmittedWithNilUsername_afterPartialSaveWithUsername_updatesExistingUsernameOnlyStoredData() { + // Check initial stored + let initialCredentials = SecureVaultModels.WebsiteCredentials(account: .init(username: "email@example.com", domain: "fill.dev"), password: nil) + guard let theID = try? testVault.storeWebsiteCredentials(initialCredentials) else { + XCTFail("Couldn't store initial credentials") + return + } + + let partialCredentials = AutofillUserScript.IncomingCredentials(username: "email@example.com", password: nil, autogenerated: false) + let partialData = AutofillUserScript.DetectedAutofillData(identity: nil, credentials: partialCredentials, creditCard: nil, trigger: .partialSave) + manager.autofillUserScript(mockAutofillUserScript, didRequestStoreDataForDomain: "fill.dev", data: partialData) + + let incomingCredentials = AutofillUserScript.IncomingCredentials(username: nil, password: "m4nu4lP4sswOrd", autogenerated: false) + let incomingData = AutofillUserScript.DetectedAutofillData(identity: nil, credentials: incomingCredentials, creditCard: nil, trigger: .formSubmission) + manager.autofillUserScript(mockAutofillUserScript, didRequestStoreDataForDomain: "fill.dev", data: incomingData) + + // Check prompted + XCTAssertNotNil(secureVaultManagerDelegate.promptedAutofillData) + // Prompting with an account with ID will result in an update + XCTAssertEqual(secureVaultManagerDelegate.promptedAutofillData?.credentials?.account.id, String(theID)) + } + + func testWhenFormSubmittedWithNilUsername_afterPartialSaveWithUsername_updatesExistingPasswordOnlyStoredData() { + // Check initial stored + let initialCredentials = SecureVaultModels.WebsiteCredentials(account: .init(username: nil, domain: "fill.dev"), password: "m4nu4lP4sswOrd".data(using: .utf8)) + guard let theID = try? testVault.storeWebsiteCredentials(initialCredentials) else { + XCTFail("Couldn't store initial credentials") + return + } + + let partialCredentials = AutofillUserScript.IncomingCredentials(username: "email@example.com", password: nil, autogenerated: false) + let partialData = AutofillUserScript.DetectedAutofillData(identity: nil, credentials: partialCredentials, creditCard: nil, trigger: .partialSave) + manager.autofillUserScript(mockAutofillUserScript, didRequestStoreDataForDomain: "fill.dev", data: partialData) + + let incomingCredentials = AutofillUserScript.IncomingCredentials(username: nil, password: "m4nu4lP4sswOrd", autogenerated: false) + let incomingData = AutofillUserScript.DetectedAutofillData(identity: nil, credentials: incomingCredentials, creditCard: nil, trigger: .formSubmission) + manager.autofillUserScript(mockAutofillUserScript, didRequestStoreDataForDomain: "fill.dev", data: incomingData) + + // Check prompted + XCTAssertNotNil(secureVaultManagerDelegate.promptedAutofillData) + // Prompting with an account with ID will result in an update + XCTAssertEqual(secureVaultManagerDelegate.promptedAutofillData?.credentials?.account.id, String(theID)) + } + // MARK: - Test Utilities private func identity(id: Int64? = nil, name: (String, String, String), addressStreet: String?) -> SecureVaultModels.Identity {