diff --git a/BitwardenResources/Localizations/en.lproj/Localizable.strings b/BitwardenResources/Localizations/en.lproj/Localizable.strings index 02107ed7d6..fd3c6d5f83 100644 --- a/BitwardenResources/Localizations/en.lproj/Localizable.strings +++ b/BitwardenResources/Localizations/en.lproj/Localizable.strings @@ -571,6 +571,8 @@ "RequestAdminApproval" = "Request admin approval"; "ApproveWithMasterPassword" = "Approve with master password"; "TurnOffUsingPublicDevice" = "Turn off using a public device"; +"TurnOnFillAssist" = "Turn on fill assist"; +"TurnOnFillAssistDescriptionLong" = "Fill Assist improves autofill accuracy on supported sites by using site-specific rules."; "RememberThisDevice" = "Remember this device"; "Passkey" = "Passkey"; "Passkeys" = "Passkeys"; diff --git a/BitwardenShared/Core/Auth/Repositories/AuthRepository.swift b/BitwardenShared/Core/Auth/Repositories/AuthRepository.swift index ef3c48e5ef..e5063e7401 100644 --- a/BitwardenShared/Core/Auth/Repositories/AuthRepository.swift +++ b/BitwardenShared/Core/Auth/Repositories/AuthRepository.swift @@ -471,6 +471,9 @@ class DefaultAuthRepository { /// The service used by the application to report non-fatal errors. private let errorReporter: ErrorReporter + /// The repository used to manage cached Fill Assist targeting rules. + private let fillAssistRepository: FillAssistRepository + /// The service used by the application for recording temporary debug logs. private let flightRecorder: FlightRecorder @@ -523,6 +526,7 @@ class DefaultAuthRepository { /// - configService: The service to get server-specified configuration. /// - environmentService: The service used by the application to manage the environment settings. /// - errorReporter: The service used by the application to report non-fatal errors. + /// - fillAssistRepository: The repository used to manage cached Fill Assist targeting rules. /// - flightRecorder: The service used by the application for recording temporary debug logs. /// - keychainService: The keychain service used by the application. /// - keyConnectorService: The service used by the application to manage Key Connector. @@ -549,6 +553,7 @@ class DefaultAuthRepository { configService: ConfigService, environmentService: EnvironmentService, errorReporter: ErrorReporter, + fillAssistRepository: FillAssistRepository, flightRecorder: FlightRecorder, keychainService: KeychainRepository, keyConnectorService: KeyConnectorService, @@ -573,6 +578,7 @@ class DefaultAuthRepository { self.configService = configService self.environmentService = environmentService self.errorReporter = errorReporter + self.fillAssistRepository = fillAssistRepository self.flightRecorder = flightRecorder self.keychainService = keychainService self.keyConnectorService = keyConnectorService @@ -872,6 +878,7 @@ extension DefaultAuthRepository: AuthRepository { try await stateService.setSyncToAuthenticator(false, userId: userId) try await keychainService.deleteItems(for: userId) try await clientCertificateService.removeCertificate(userId: userId) + try await fillAssistRepository.clearRules(userId: userId) await vaultTimeoutService.remove(userId: userId) if await policyService.policyAppliesToUser(.removeUnlockWithPin) { diff --git a/BitwardenShared/Core/Auth/Repositories/AuthRepositoryTests.swift b/BitwardenShared/Core/Auth/Repositories/AuthRepositoryTests.swift index 6262f13b86..bf078399ea 100644 --- a/BitwardenShared/Core/Auth/Repositories/AuthRepositoryTests.swift +++ b/BitwardenShared/Core/Auth/Repositories/AuthRepositoryTests.swift @@ -26,6 +26,7 @@ class AuthRepositoryTests: BitwardenTestCase { // swiftlint:disable:this type_bo var configService: MockConfigService! var environmentService: MockEnvironmentService! var errorReporter: MockErrorReporter! + var fillAssistRepository: MockFillAssistRepository! var flightRecorder: MockFlightRecorder! var keyConnectorService: MockKeyConnectorService! var keychainService: MockKeychainRepository! @@ -112,6 +113,7 @@ class AuthRepositoryTests: BitwardenTestCase { // swiftlint:disable:this type_bo configService = MockConfigService() environmentService = MockEnvironmentService() errorReporter = MockErrorReporter() + fillAssistRepository = MockFillAssistRepository() flightRecorder = MockFlightRecorder() keyConnectorService = MockKeyConnectorService() keychainService = MockKeychainRepository() @@ -150,6 +152,7 @@ class AuthRepositoryTests: BitwardenTestCase { // swiftlint:disable:this type_bo configService: configService, environmentService: environmentService, errorReporter: errorReporter, + fillAssistRepository: fillAssistRepository, flightRecorder: flightRecorder, keychainService: keychainService, keyConnectorService: keyConnectorService, @@ -181,6 +184,7 @@ class AuthRepositoryTests: BitwardenTestCase { // swiftlint:disable:this type_bo configService = nil environmentService = nil errorReporter = nil + fillAssistRepository = nil keychainService = nil organizationService = nil policyService = nil @@ -2896,6 +2900,8 @@ class AuthRepositoryTests: BitwardenTestCase { // swiftlint:disable:this type_bo XCTAssertEqual(keychainService.deleteItemsCallsCount, 1) XCTAssertEqual(keychainService.deleteItemsReceivedUserId, "1") XCTAssertEqual(clientCertificateService.removeCertificateUserIdReceivedUserId, account.profile.userId) + XCTAssertTrue(fillAssistRepository.clearRulesCalled) + XCTAssertEqual(fillAssistRepository.clearRulesReceivedUserId, account.profile.userId) XCTAssertTrue(stateService.logoutAccountUserInitiated) XCTAssertEqual(vaultTimeoutService.removedIds, [anneAccount.profile.userId]) XCTAssertEqual(stateService.pinProtectedUserKeyValue["1"], "1") diff --git a/BitwardenShared/Core/Autofill/Models/FormsMapResponseModel.swift b/BitwardenShared/Core/Autofill/Models/FormsMapResponseModel.swift index 1f1fd2ca6c..03f1891362 100644 --- a/BitwardenShared/Core/Autofill/Models/FormsMapResponseModel.swift +++ b/BitwardenShared/Core/Autofill/Models/FormsMapResponseModel.swift @@ -21,6 +21,19 @@ struct FormsMapResponseModel: Equatable, JSONResponse { /// The schema version of the map. let schemaVersion: String + // MARK: Initialization + + /// Creates a `FormsMapResponseModel`. + /// + /// - Parameters: + /// - hosts: The host entries keyed by hostname. + /// - schemaVersion: The schema version of the map. + /// + init(hosts: [String: FormsMapHostEntry], schemaVersion: String) { + self.hosts = hosts + self.schemaVersion = schemaVersion + } + // MARK: Codable init(from decoder: any Decoder) throws { @@ -43,6 +56,24 @@ struct FormsMapHostEntry: Codable, Equatable { /// Pathname-specific form descriptions. Null-valued entries are excluded during decoding. @CompactDecodable var pathnames: [String: FormsMapPathnameEntry]? + + /// All form descriptions for this host, pooling top-level and pathname-specific entries. + var allForms: [FormsMapContent] { + (forms ?? []) + (pathnames?.values.flatMap(\.forms) ?? []) + } + + // MARK: Initialization + + /// Creates a `FormsMapHostEntry`. + /// + /// - Parameters: + /// - forms: Site-wide fallback form descriptions. + /// - pathnames: Pathname-specific form descriptions. Defaults to `nil`. + /// + init(forms: [FormsMapContent]?, pathnames: [String: FormsMapPathnameEntry]? = nil) { + self.forms = forms + _pathnames = CompactDecodable(wrappedValue: pathnames) + } } // MARK: - FormsMapPathnameEntry @@ -84,6 +115,18 @@ enum FormsMapSelector: Codable, Equatable { /// An ordered sequence of CSS selectors composing a single field value. case sequence([String]) + // MARK: Computed Properties + + /// The `FillAssistFieldAttributes` derived from this selector, empty if the selector is unsupported. + var attributes: [FillAssistFieldAttributes] { + switch self { + case let .single(css): + FillAssistSelectorParser.parse(css).map { [$0] } ?? [] + case let .sequence(parts): + parts.compactMap { FillAssistSelectorParser.parse($0) } + } + } + // MARK: Codable init(from decoder: any Decoder) throws { diff --git a/BitwardenShared/Core/Autofill/Repositories/FillAssistRepository.swift b/BitwardenShared/Core/Autofill/Repositories/FillAssistRepository.swift new file mode 100644 index 0000000000..a97ada84fb --- /dev/null +++ b/BitwardenShared/Core/Autofill/Repositories/FillAssistRepository.swift @@ -0,0 +1,176 @@ +import BitwardenKit +import Foundation + +// MARK: - FillAssistRepository + +/// A protocol for a repository that manages fetching and caching fill-assist targeting rules. +/// +protocol FillAssistRepository { // sourcery: AutoMockable + /// Fetches and caches fill-assist rules for the active account. + /// + func syncRules() async + + /// Returns the cached fill-assist rules for a given hostname, or `nil` if unavailable. + /// + /// - Parameter hostname: The hostname to look up. + /// - Returns: The cached `FillAssistHostRules`, or `nil`. + /// + func rules(for hostname: String) async -> FillAssistHostRules? + + /// Clears all cached fill-assist data for an account. + /// + /// - Parameter userId: The user ID of the account to clear. Defaults to the active account if `nil`. + /// + func clearRules(userId: String?) async throws +} + +// MARK: - DefaultFillAssistRepository + +/// The default implementation of `FillAssistRepository`. +/// +class DefaultFillAssistRepository: FillAssistRepository { + // MARK: Private Properties + + /// The store for persisting fill-assist cached data. + private let appSettingsStore: AppSettingsStore + + /// The service for checking feature flags and configuration. + private let configService: ConfigService + + /// The service for reporting non-fatal errors. + private let errorReporter: ErrorReporter + + /// The service for accessing environment URLs. + private let environmentService: EnvironmentService + + /// The API service for fetching fill-assist data. + private let fillAssistAPIService: FillAssistAPIService + + /// The service for accessing account state. + private let stateService: StateService + + /// The provider of the current time, used for interval checks. + private let timeProvider: TimeProvider + + // MARK: Initialization + + /// Creates a `DefaultFillAssistRepository`. + /// + /// - Parameters: + /// - appSettingsStore: The store for persisting fill-assist cached data. + /// - configService: The service for checking feature flags and configuration. + /// - environmentService: The service for accessing environment URLs. + /// - errorReporter: The service for reporting non-fatal errors. + /// - fillAssistAPIService: The API service for fetching fill-assist data. + /// - stateService: The service for accessing account state. + /// - timeProvider: The provider of the current time. + /// + init( + appSettingsStore: AppSettingsStore, + configService: ConfigService, + environmentService: EnvironmentService, + errorReporter: ErrorReporter, + fillAssistAPIService: FillAssistAPIService, + stateService: StateService, + timeProvider: TimeProvider, + ) { + self.appSettingsStore = appSettingsStore + self.configService = configService + self.environmentService = environmentService + self.errorReporter = errorReporter + self.fillAssistAPIService = fillAssistAPIService + self.stateService = stateService + self.timeProvider = timeProvider + } + + // MARK: FillAssistRepository + + func syncRules() async { + do { + try await performSync() + } catch { + // Sync failures are non-fatal — existing cache remains available. + errorReporter.log(error: error) + } + } + + func rules(for hostname: String) async -> FillAssistHostRules? { + guard let userId = try? await stateService.getActiveAccountId() else { return nil } + return appSettingsStore.fillAssistCachedData(userId: userId)?.rules[hostname] + } + + func clearRules(userId: String?) async throws { + let resolvedUserId: String = if let userId { + userId + } else { + try await stateService.getActiveAccountId() + } + appSettingsStore.setFillAssistCachedData(nil, userId: resolvedUserId) + appSettingsStore.setFillAssistLastFetchTimestamp(nil, userId: resolvedUserId) + } + + // MARK: Private + + /// Runs the full sync pipeline if sync conditions are met + /// + private func performSync() async throws { + guard await configService.getFeatureFlag(.fillAssistTargetingRules), + try await stateService.getFillAssistEnabled() else { return } + + let sourceUrl = environmentService.fillAssistRulesURL + let userId = try await stateService.getActiveAccountId() + let cached = appSettingsStore.fillAssistCachedData(userId: userId) + let lastFetch = appSettingsStore.fillAssistLastFetchTimestamp(userId: userId) + if let lastFetch, + cached != nil, + timeProvider.presentTime.timeIntervalSince(lastFetch) < Constants.FillAssist.updateInterval { + return + } + + let manifest = try await fillAssistAPIService.getManifest() + + guard let entry = manifest.maps["forms"]?[Constants.FillAssist.formsVersion], + !entry.deprecated + else { return } + + if cached?.cid == entry.cid, cached?.sourceUrl == sourceUrl.absoluteString { + appSettingsStore.setFillAssistLastFetchTimestamp(timeProvider.presentTime, userId: userId) + return + } + + let formsMap = try await fillAssistAPIService.getFormsMap(filename: entry.filename) + + let schemaMajor = formsMap.schemaVersion.split(separator: ".").first.map(String.init) ?? "" + guard schemaMajor == Constants.FillAssist.expectedSchemaMajor else { + appSettingsStore.setFillAssistLastFetchTimestamp(timeProvider.presentTime, userId: userId) + return + } + + let rules = buildRules(from: formsMap) + let data = FillAssistCachedData(cid: entry.cid, rules: rules, sourceUrl: sourceUrl.absoluteString) + appSettingsStore.setFillAssistCachedData(data, userId: userId) + appSettingsStore.setFillAssistLastFetchTimestamp(timeProvider.presentTime, userId: userId) + } + + /// Builds a `[hostname: FillAssistHostRules]` dictionary by pooling all non-null field + /// definitions across each host's top-level forms and every pathname entry. + /// + private func buildRules(from formsMap: FormsMapResponseModel) -> [String: FillAssistHostRules] { + var result = [String: FillAssistHostRules]() + for (hostname, hostEntry) in formsMap.hosts { + var pooled = [String: [FillAssistFieldAttributes]]() + let allForms = hostEntry.allForms + for form in allForms { + for (fieldKey, selectors) in form.fields { + let attrs = selectors.flatMap(\.attributes) + pooled[fieldKey, default: []].append(contentsOf: attrs) + } + } + pooled = pooled.filter { !$0.value.isEmpty } + if !pooled.isEmpty { + result[hostname] = FillAssistHostRules(fields: pooled) + } + } + return result + } +} diff --git a/BitwardenShared/Core/Autofill/Repositories/FillAssistRepositoryTests.Fixtures.swift b/BitwardenShared/Core/Autofill/Repositories/FillAssistRepositoryTests.Fixtures.swift new file mode 100644 index 0000000000..e83bbd04b7 --- /dev/null +++ b/BitwardenShared/Core/Autofill/Repositories/FillAssistRepositoryTests.Fixtures.swift @@ -0,0 +1,79 @@ +// swiftlint:disable:this file_name + +import Foundation + +@testable import BitwardenShared + +// MARK: - FillAssistRepositoryTests Fixtures + +extension FillAssistRepositoryTests { + /// Returns a `FillAssistManifestResponseModel` fixture with the given content ID. + func makeManifest(cid: String) -> FillAssistManifestResponseModel { + let entry = FillAssistManifestEntryModel( + cid: cid, + deprecated: false, + filename: "forms.v1.json", + schema: "forms.v1.schema.json", + ) + return FillAssistManifestResponseModel( + buildId: "v1", + gitSha: "abc", + maps: ["forms": ["v1": entry]], + timestamp: Date(timeIntervalSinceReferenceDate: 0), + ) + } + + /// Returns a `FormsMapResponseModel` fixture with a single `example.com` host. + /// + /// - Parameters: + /// - schemaVersion: The schema version string. Defaults to `"1.0.0"`. + /// - usernameSelector: The CSS selector for the username field. Defaults to `"input#user"`. + /// - hosts: Optional map of hostname to selector string overriding the default host. + /// + func makeFormsMap( + schemaVersion: String = "1.0.0", + usernameSelector: String = "input#user", + hosts: [String: String]? = nil, + ) -> FormsMapResponseModel { + let content = { (selector: String) in + FormsMapContent( + category: "account-login", + container: nil, + fields: ["username": [.single(selector)]], + actions: nil, + ) + } + let resolvedHosts: [String: FormsMapHostEntry] = if let hosts { + hosts.mapValues { FormsMapHostEntry(forms: [content($0)]) } + } else { + ["example.com": FormsMapHostEntry(forms: [content(usernameSelector)])] + } + return FormsMapResponseModel(hosts: resolvedHosts, schemaVersion: schemaVersion) + } + + /// Returns a `FormsMapResponseModel` fixture with both top-level and pathname-specific forms + /// for `example.com`, used to test selector pooling across multiple entry points. + func makeFormsMapWithPathnames() -> FormsMapResponseModel { + let loginContent = FormsMapContent( + category: "account-login", + container: nil, + fields: ["username": [.single("input#user1")]], + actions: nil, + ) + let pathnameContent = FormsMapContent( + category: "account-login", + container: nil, + fields: ["username": [.single("input#user2")]], + actions: nil, + ) + return FormsMapResponseModel( + hosts: [ + "example.com": FormsMapHostEntry( + forms: [loginContent], + pathnames: ["/login": FormsMapPathnameEntry(forms: [pathnameContent])], + ), + ], + schemaVersion: "1.0.0", + ) + } +} diff --git a/BitwardenShared/Core/Autofill/Repositories/FillAssistRepositoryTests.swift b/BitwardenShared/Core/Autofill/Repositories/FillAssistRepositoryTests.swift new file mode 100644 index 0000000000..bbf76fc314 --- /dev/null +++ b/BitwardenShared/Core/Autofill/Repositories/FillAssistRepositoryTests.swift @@ -0,0 +1,364 @@ +import BitwardenKit +import BitwardenKitMocks +import Foundation +import Testing + +@testable import BitwardenShared +@testable import BitwardenSharedMocks + +// MARK: - FillAssistRepositoryTests + +@MainActor +struct FillAssistRepositoryTests { + // MARK: Properties + + let appSettingsStore: MockAppSettingsStore + let configService: MockConfigService + let environmentService: MockEnvironmentService + let errorReporter: MockErrorReporter + let fillAssistAPIService: MockFillAssistAPIService + let stateService: MockStateService + let subject: DefaultFillAssistRepository + let timeProvider: MockTimeProvider + + // MARK: Initialization + + init() { + appSettingsStore = MockAppSettingsStore() + configService = MockConfigService() + environmentService = MockEnvironmentService() + errorReporter = MockErrorReporter() + fillAssistAPIService = MockFillAssistAPIService() + stateService = MockStateService() + stateService.activeAccount = .fixture() + stateService.fillAssistEnabledByUserId["1"] = true + timeProvider = MockTimeProvider(.currentTime) + + subject = DefaultFillAssistRepository( + appSettingsStore: appSettingsStore, + configService: configService, + environmentService: environmentService, + errorReporter: errorReporter, + fillAssistAPIService: fillAssistAPIService, + stateService: stateService, + timeProvider: timeProvider, + ) + } + + // MARK: Tests - syncRules + + /// `syncRules()` makes no network calls when the feature flag is disabled. + @Test + func syncRules_featureFlagDisabled() async { + configService.featureFlagsBool[.fillAssistTargetingRules] = false + + await subject.syncRules() + + #expect(!fillAssistAPIService.getManifestCalled) + } + + /// `syncRules()` makes no network calls when the update interval has not elapsed and the + /// cache is readable. + @Test + func syncRules_withinUpdateInterval() async { + configService.featureFlagsBool[.fillAssistTargetingRules] = true + appSettingsStore.fillAssistCachedDataByUserId["1"] = FillAssistCachedData( + cid: "sha256:abc123", + rules: [:], + sourceUrl: environmentService.fillAssistRulesURL.absoluteString, + ) + appSettingsStore.fillAssistLastFetchTimestampByUserId["1"] = timeProvider.presentTime + + await subject.syncRules() + + #expect(!fillAssistAPIService.getManifestCalled) + } + + /// `syncRules()` still fetches the manifest when the update interval has not elapsed but the + /// cache is unreadable, since a fresh timestamp alone shouldn't mask a missing cache. + @Test + func syncRules_withinUpdateInterval_cacheUnreadable_fetchesManifest() async { + configService.featureFlagsBool[.fillAssistTargetingRules] = true + appSettingsStore.fillAssistLastFetchTimestampByUserId["1"] = timeProvider.presentTime + fillAssistAPIService.getManifestReturnValue = makeManifest(cid: "sha256:abc123") + fillAssistAPIService.getFormsMapReturnValue = makeFormsMap() + + await subject.syncRules() + + #expect(fillAssistAPIService.getManifestCalled) + } + + /// `syncRules()` skips download and updates the timestamp when cid and source URL are unchanged. + @Test + func syncRules_cidUnchanged_updatesTimestampOnly() async { + configService.featureFlagsBool[.fillAssistTargetingRules] = true + let sourceUrl = environmentService.fillAssistRulesURL.absoluteString + + appSettingsStore.fillAssistCachedDataByUserId["1"] = FillAssistCachedData( + cid: "sha256:abc123", + rules: [:], + sourceUrl: sourceUrl, + ) + fillAssistAPIService.getManifestReturnValue = makeManifest(cid: "sha256:abc123") + + await subject.syncRules() + + #expect(fillAssistAPIService.getManifestCalled) + #expect(!fillAssistAPIService.getFormsMapCalled) + #expect(appSettingsStore.fillAssistLastFetchTimestampByUserId["1"] != nil) + } + + /// `syncRules()` downloads, parses, and caches rules when cid changes. + @Test + func syncRules_cidChanged_downloadsAndCaches() async throws { + configService.featureFlagsBool[.fillAssistTargetingRules] = true + + fillAssistAPIService.getManifestReturnValue = makeManifest(cid: "sha256:newcid") + fillAssistAPIService.getFormsMapReturnValue = makeFormsMap() + + await subject.syncRules() + + #expect(fillAssistAPIService.getManifestCalled) + #expect(fillAssistAPIService.getFormsMapCalled) + #expect(fillAssistAPIService.getFormsMapReceivedFilename == "forms.v1.json") + let cached = appSettingsStore.fillAssistCachedDataByUserId["1"] + #expect(cached != nil) + #expect(cached?.cid == "sha256:newcid") + #expect(appSettingsStore.fillAssistLastFetchTimestampByUserId["1"] != nil) + } + + /// `syncRules()` skips storing when the schema major version is unsupported. + @Test + func syncRules_unsupportedSchema_skipsCache() async throws { + configService.featureFlagsBool[.fillAssistTargetingRules] = true + + fillAssistAPIService.getManifestReturnValue = makeManifest(cid: "sha256:newcid") + fillAssistAPIService.getFormsMapReturnValue = makeFormsMap(schemaVersion: "2.0.0") + + await subject.syncRules() + + #expect(appSettingsStore.fillAssistCachedDataByUserId["1"] == nil) + #expect(appSettingsStore.fillAssistLastFetchTimestampByUserId["1"] != nil) + } + + /// `syncRules()` parses CSS selectors into `FillAssistFieldAttributes` (selector pooling). + @Test + func syncRules_parsesSelectorsIntoRules() async throws { + configService.featureFlagsBool[.fillAssistTargetingRules] = true + + fillAssistAPIService.getManifestReturnValue = makeManifest(cid: "sha256:newcid") + fillAssistAPIService.getFormsMapReturnValue = makeFormsMap() + + await subject.syncRules() + + let hostRules = appSettingsStore.fillAssistCachedDataByUserId["1"]?.rules["example.com"] + let usernameAttrs = try #require(hostRules?.fields["username"]?.first) + #expect(usernameAttrs.id == "user") + #expect(usernameAttrs.tagName == "input") + } + + /// `syncRules()` pools selectors from multiple pathname entries into a single host entry. + @Test + func syncRules_poolsSelectorsAcrossPathnames() async throws { + configService.featureFlagsBool[.fillAssistTargetingRules] = true + + fillAssistAPIService.getManifestReturnValue = makeManifest(cid: "sha256:newcid") + fillAssistAPIService.getFormsMapReturnValue = makeFormsMapWithPathnames() + + await subject.syncRules() + + let usernameAttrs = appSettingsStore.fillAssistCachedDataByUserId["1"]? + .rules["example.com"]?.fields["username"] + let count = try #require(usernameAttrs).count + #expect(count == 2) + } + + /// `syncRules()` produces no rules entry when a host has no parseable selectors. + @Test + func syncRules_emptyPooled_excludesHost() async throws { + configService.featureFlagsBool[.fillAssistTargetingRules] = true + + fillAssistAPIService.getManifestReturnValue = makeManifest(cid: "sha256:newcid") + // Shadow DOM selectors are excluded by the parser → pooled stays empty. + fillAssistAPIService.getFormsMapReturnValue = makeFormsMap( + usernameSelector: "div#app >>> input#user", + ) + + await subject.syncRules() + + let rules = appSettingsStore.fillAssistCachedDataByUserId["1"]?.rules + #expect(rules?["example.com"] == nil) + } + + /// `syncRules()` returns an empty rules dict when the forms map has no hosts. + @Test + func syncRules_noHosts_emptyRules() async throws { + configService.featureFlagsBool[.fillAssistTargetingRules] = true + + fillAssistAPIService.getManifestReturnValue = makeManifest(cid: "sha256:newcid") + fillAssistAPIService.getFormsMapReturnValue = makeFormsMap(hosts: [:]) + + await subject.syncRules() + + let rules = appSettingsStore.fillAssistCachedDataByUserId["1"]?.rules + #expect(rules?.isEmpty == true) + } + + /// `syncRules()` caches rules for multiple hosts independently. + @Test + func syncRules_multipleHosts_allCached() async throws { + configService.featureFlagsBool[.fillAssistTargetingRules] = true + + fillAssistAPIService.getManifestReturnValue = makeManifest(cid: "sha256:newcid") + fillAssistAPIService.getFormsMapReturnValue = makeFormsMap( + hosts: ["example.com": "input#user1", "other.com": "input#user2"], + ) + + await subject.syncRules() + + let rules = appSettingsStore.fillAssistCachedDataByUserId["1"]?.rules + #expect(rules?["example.com"] != nil) + #expect(rules?["other.com"] != nil) + #expect(rules?.count == 2) + } + + /// `syncRules()` logs errors but does not rethrow them. + @Test + func syncRules_failure_logsError() async { + configService.featureFlagsBool[.fillAssistTargetingRules] = true + fillAssistAPIService.getManifestThrowableError = URLError(.notConnectedToInternet) + + await subject.syncRules() + + #expect(errorReporter.errors.count == 1) + #expect(appSettingsStore.fillAssistCachedDataByUserId["1"] == nil) + } + + // MARK: Tests - rules(for:) + + /// `rules(for:)` returns cached rules for a known hostname. + @Test + func rules_returnsRulesForHostname() async { + let hostRules = FillAssistHostRules(fields: ["username": []]) + appSettingsStore.fillAssistCachedDataByUserId["1"] = FillAssistCachedData( + cid: "sha256:abc", + rules: ["example.com": hostRules], + sourceUrl: "https://example.com", + ) + + let result = await subject.rules(for: "example.com") + + #expect(result == hostRules) + } + + /// `rules(for:)` returns `nil` for an unknown hostname. + @Test + func rules_returnsNilForUnknownHostname() async { + appSettingsStore.fillAssistCachedDataByUserId["1"] = FillAssistCachedData( + cid: "sha256:abc", + rules: [:], + sourceUrl: "https://example.com", + ) + + let result = await subject.rules(for: "unknown.com") + + #expect(result == nil) + } + + // MARK: Tests - clearRules() + + /// `clearRules()` removes cached data and the last-fetch timestamp for the active account. + @Test + func clearRules_removesCachedData() async throws { + appSettingsStore.fillAssistCachedDataByUserId["1"] = FillAssistCachedData( + cid: "sha256:abc", + rules: [:], + sourceUrl: "https://example.com", + ) + appSettingsStore.fillAssistLastFetchTimestampByUserId["1"] = timeProvider.presentTime + + try await subject.clearRules(userId: nil) + + #expect(appSettingsStore.fillAssistCachedDataByUserId["1"] == nil) + #expect(appSettingsStore.fillAssistLastFetchTimestampByUserId["1"] == nil) + } + + /// `clearRules(userId:)` removes cached data for a specific, non-active account. + @Test + func clearRules_removesCachedData_forSpecificUser() async throws { + appSettingsStore.fillAssistCachedDataByUserId["2"] = FillAssistCachedData( + cid: "sha256:abc", + rules: [:], + sourceUrl: "https://example.com", + ) + appSettingsStore.fillAssistLastFetchTimestampByUserId["2"] = timeProvider.presentTime + + try await subject.clearRules(userId: "2") + + #expect(appSettingsStore.fillAssistCachedDataByUserId["2"] == nil) + #expect(appSettingsStore.fillAssistLastFetchTimestampByUserId["2"] == nil) + } + + // MARK: Tests - FormsMapSelector.attributes + + /// `FormsMapSelector.attributes` parses a single CSS selector. + @Test + func formsMapSelector_single_returnsAttributes() { + let attrs = FormsMapSelector.single("input#user").attributes + #expect(attrs.count == 1) + #expect(attrs.first?.id == "user") + #expect(attrs.first?.tagName == "input") + } + + /// `FormsMapSelector.attributes` parses each selector in a sequence. + @Test + func formsMapSelector_sequence_returnsAttributesForEach() { + let attrs = FormsMapSelector.sequence(["input#user", "input#email"]).attributes + #expect(attrs.count == 2) + #expect(attrs.first?.id == "user") + #expect(attrs.last?.id == "email") + } + + /// `FormsMapSelector.attributes` returns empty for an unsupported selector (shadow DOM). + @Test + func formsMapSelector_unsupportedSelector_returnsEmpty() { + let attrs = FormsMapSelector.single("div >>> input#user").attributes + #expect(attrs.isEmpty) + } +} + +// MARK: - Helpers + +extension FillAssistRepositoryTests { + // MARK: Tests - user toggle guard + + /// `syncRules()` makes no network calls when the feature flag is on but the user toggle is off. + @Test + func syncRules_userToggleOff_skipsSync() async { + configService.featureFlagsBool[.fillAssistTargetingRules] = true + stateService.fillAssistEnabledByUserId["1"] = false + + await subject.syncRules() + + #expect(!fillAssistAPIService.getManifestCalled) + } + + /// `syncRules()` proceeds past the user-toggle guard and fetches the manifest when both + /// feature flag and user toggle are on. + @Test + func syncRules_userToggleOn_fetchesManifest() async throws { + configService.featureFlagsBool[.fillAssistTargetingRules] = true + stateService.fillAssistEnabledByUserId["1"] = true + fillAssistAPIService.getManifestReturnValue = makeManifest(cid: "sha256:abc") + // Pre-cache matching cid so sync stops at the "no changes" short-circuit. + appSettingsStore.fillAssistCachedDataByUserId["1"] = FillAssistCachedData( + cid: "sha256:abc", + rules: [:], + sourceUrl: environmentService.fillAssistRulesURL.absoluteString, + ) + + await subject.syncRules() + + #expect(fillAssistAPIService.getManifestCalled) + } +} diff --git a/BitwardenShared/Core/Autofill/Services/API/FillAssistAPIService.swift b/BitwardenShared/Core/Autofill/Services/API/FillAssistAPIService.swift index 8393691cb5..8e50a23933 100644 --- a/BitwardenShared/Core/Autofill/Services/API/FillAssistAPIService.swift +++ b/BitwardenShared/Core/Autofill/Services/API/FillAssistAPIService.swift @@ -24,14 +24,10 @@ protocol FillAssistAPIService { // sourcery: AutoMockable extension APIService: FillAssistAPIService { func getFormsMap(filename: String) async throws -> FormsMapResponseModel { - let fileURL = try await fillAssistService.download(filename: filename) - let data = try Data(contentsOf: fileURL) - return try JSONDecoder.pascalOrSnakeCaseDecoder.decode(FormsMapResponseModel.self, from: data) + try await fillAssistService.send(FormsMapRequest(filename: filename)) } func getManifest() async throws -> FillAssistManifestResponseModel { - let fileURL = try await fillAssistService.download(filename: Constants.FillAssist.manifestFilename) - let data = try Data(contentsOf: fileURL) - return try JSONDecoder.pascalOrSnakeCaseDecoder.decode(FillAssistManifestResponseModel.self, from: data) + try await fillAssistService.send(FillAssistManifestRequest()) } } diff --git a/BitwardenShared/Core/Autofill/Services/API/FillAssistAPIServiceTests.swift b/BitwardenShared/Core/Autofill/Services/API/FillAssistAPIServiceTests.swift index 872e2bbace..30d1a6f599 100644 --- a/BitwardenShared/Core/Autofill/Services/API/FillAssistAPIServiceTests.swift +++ b/BitwardenShared/Core/Autofill/Services/API/FillAssistAPIServiceTests.swift @@ -23,35 +23,29 @@ struct FillAssistAPIServiceTests { // MARK: Tests - /// `getFormsMap(filename:)` downloads the file at the expected URL and decodes the response. + /// `getFormsMap(filename:)` sends a request to the expected URL and decodes the response. @Test func getFormsMap() async throws { - let tempFile = FileManager.default.temporaryDirectory - .appendingPathComponent("forms.v1.json") - try APITestData.formsMap.data.write(to: tempFile) - client.downloadResults = [.success(tempFile)] + client.results = [.httpSuccess(testData: .formsMap)] _ = try await subject.getFormsMap(filename: "forms.v1.json") - let request = try #require(client.downloadRequests.last) - #expect(request.httpMethod == "GET") - #expect(request.url?.absoluteString == "https://example.com/fill-assist-rules/forms.v1.json") + let request = try #require(client.requests.last) + #expect(request.method == .get) + #expect(request.url.absoluteString == "https://example.com/fill-assist-rules/forms.v1.json") } - /// `getManifest()` downloads the manifest at the constant filename and decodes the response. + /// `getManifest()` sends a request to the manifest URL and decodes the response. @Test func getManifest() async throws { - let tempFile = FileManager.default.temporaryDirectory - .appendingPathComponent(Constants.FillAssist.manifestFilename) - try APITestData.fillAssistManifest.data.write(to: tempFile) - client.downloadResults = [.success(tempFile)] + client.results = [.httpSuccess(testData: .fillAssistManifest)] _ = try await subject.getManifest() - let request = try #require(client.downloadRequests.last) - #expect(request.httpMethod == "GET") + let request = try #require(client.requests.last) + #expect(request.method == .get) #expect( - request.url?.absoluteString + request.url.absoluteString == "https://example.com/fill-assist-rules/\(Constants.FillAssist.manifestFilename)", ) } diff --git a/BitwardenShared/Core/Platform/Services/API/APIService.swift b/BitwardenShared/Core/Platform/Services/API/APIService.swift index 985aa8795e..24a22c56ec 100644 --- a/BitwardenShared/Core/Platform/Services/API/APIService.swift +++ b/BitwardenShared/Core/Platform/Services/API/APIService.swift @@ -61,6 +61,7 @@ class APIService { accountTokenProvider: AccountTokenProvider? = nil, activeAccountStateProvider: ActiveAccountStateProvider, client: HTTPClient = URLSession.shared, + fillAssistClient: HTTPClient = URLSession.shared, environmentService: EnvironmentService, errorReporter: ErrorReporter, flightRecorder: FlightRecorder, @@ -113,8 +114,12 @@ class APIService { identityService = httpServiceBuilder.makeService( baseURLGetter: { environmentService.identityURL }, ) - fillAssistService = httpServiceBuilder.makeService( + // Fill-Assist requests target an external CDN (GitHub Releases) that uses multi-hop 302 + // redirects. CertificateHTTPClient deliberately blocks 302s for SSO, so we default to + // URLSession.shared which follows all redirects. Tests may inject a mock client. + fillAssistService = HTTPService( baseURLGetter: { environmentService.fillAssistRulesURL }, + client: fillAssistClient, ) } diff --git a/BitwardenShared/Core/Platform/Services/API/TestHelpers/APIService+Mocks.swift b/BitwardenShared/Core/Platform/Services/API/TestHelpers/APIService+Mocks.swift index dba85da915..edf46a789d 100644 --- a/BitwardenShared/Core/Platform/Services/API/TestHelpers/APIService+Mocks.swift +++ b/BitwardenShared/Core/Platform/Services/API/TestHelpers/APIService+Mocks.swift @@ -9,6 +9,7 @@ extension APIService { accountTokenProvider: AccountTokenProvider? = nil, activeAccountStateProvider: MockActiveAccountStateProvider = MockActiveAccountStateProvider(), client: HTTPClient, + fillAssistClient: HTTPClient? = nil, environmentService: EnvironmentService = MockEnvironmentService(), errorReporter: ErrorReporter = MockErrorReporter(), flightRecorder: FlightRecorder = MockFlightRecorder(), @@ -25,6 +26,7 @@ extension APIService { accountTokenProvider: accountTokenProvider, activeAccountStateProvider: activeAccountStateProvider, client: client, + fillAssistClient: fillAssistClient ?? client, environmentService: environmentService, errorReporter: errorReporter, flightRecorder: flightRecorder, diff --git a/BitwardenShared/Core/Platform/Services/ServiceContainer.swift b/BitwardenShared/Core/Platform/Services/ServiceContainer.swift index 68396e5be3..ae426f095f 100644 --- a/BitwardenShared/Core/Platform/Services/ServiceContainer.swift +++ b/BitwardenShared/Core/Platform/Services/ServiceContainer.swift @@ -118,6 +118,9 @@ public class ServiceContainer: Services { // swiftlint:disable:this type_body_le /// of the `Fido2UserInterface` from the SDK. let fido2UserInterfaceHelper: Fido2UserInterfaceHelper + /// The repository used by the application to manage fill-assist data. + let fillAssistRepository: FillAssistRepository + /// The service used by the application for recording temporary debug logs. public let flightRecorder: FlightRecorder @@ -351,6 +354,7 @@ public class ServiceContainer: Services { // swiftlint:disable:this type_body_le exportVaultService: ExportVaultService, fido2CredentialStore: Fido2CredentialStore, fido2UserInterfaceHelper: Fido2UserInterfaceHelper, + fillAssistRepository: FillAssistRepository, flightRecorder: FlightRecorder, generatorRepository: GeneratorRepository, importCiphersRepository: ImportCiphersRepository, @@ -421,6 +425,7 @@ public class ServiceContainer: Services { // swiftlint:disable:this type_body_le self.exportVaultService = exportVaultService self.fido2CredentialStore = fido2CredentialStore self.fido2UserInterfaceHelper = fido2UserInterfaceHelper + self.fillAssistRepository = fillAssistRepository self.flightRecorder = flightRecorder self.generatorRepository = generatorRepository self.importCiphersRepository = importCiphersRepository @@ -770,6 +775,16 @@ public class ServiceContainer: Services { // swiftlint:disable:this type_body_le stateService: stateService, ) + let fillAssistRepository = DefaultFillAssistRepository( + appSettingsStore: appSettingsStore, + configService: configService, + environmentService: environmentService, + errorReporter: errorReporter, + fillAssistAPIService: apiService, + stateService: stateService, + timeProvider: timeProvider, + ) + let syncService = DefaultSyncService( accountAPIService: apiService, appContextHelper: appContextHelper, @@ -777,6 +792,7 @@ public class ServiceContainer: Services { // swiftlint:disable:this type_body_le clientService: clientService, collectionService: collectionService, configService: configService, + fillAssistRepository: fillAssistRepository, flightRecorder: flightRecorder, folderService: folderService, keyConnectorService: keyConnectorService, @@ -848,6 +864,7 @@ public class ServiceContainer: Services { // swiftlint:disable:this type_body_le configService: configService, environmentService: environmentService, errorReporter: errorReporter, + fillAssistRepository: fillAssistRepository, flightRecorder: flightRecorder, keychainService: keychainRepository, keyConnectorService: keyConnectorService, @@ -1183,6 +1200,7 @@ public class ServiceContainer: Services { // swiftlint:disable:this type_body_le exportVaultService: exportVaultService, fido2CredentialStore: fido2CredentialStore, fido2UserInterfaceHelper: fido2UserInterfaceHelper, + fillAssistRepository: fillAssistRepository, flightRecorder: flightRecorder, generatorRepository: generatorRepository, importCiphersRepository: importCiphersRepository, diff --git a/BitwardenShared/Core/Platform/Services/Services.swift b/BitwardenShared/Core/Platform/Services/Services.swift index aac8f8c371..dee40e780a 100644 --- a/BitwardenShared/Core/Platform/Services/Services.swift +++ b/BitwardenShared/Core/Platform/Services/Services.swift @@ -39,6 +39,7 @@ typealias Services = HasAPIService & HasFido2UserInterfaceHelper & HasFileAPIService & HasFillAssistAPIService + & HasFillAssistRepository & HasFlightRecorder & HasGeneratorRepository & HasImportCiphersRepository @@ -286,6 +287,13 @@ protocol HasFillAssistAPIService { var fillAssistAPIService: FillAssistAPIService { get } } +/// Protocol for an object that provides a `FillAssistRepository`. +/// +protocol HasFillAssistRepository { + /// The repository used by the application to manage fill-assist data. + var fillAssistRepository: FillAssistRepository { get } +} + /// Protocol for an object that provides a `GeneratorRepository`. /// protocol HasGeneratorRepository { diff --git a/BitwardenShared/Core/Platform/Services/StateService.swift b/BitwardenShared/Core/Platform/Services/StateService.swift index 28a3c48b9c..7e112200fa 100644 --- a/BitwardenShared/Core/Platform/Services/StateService.swift +++ b/BitwardenShared/Core/Platform/Services/StateService.swift @@ -219,6 +219,13 @@ protocol StateService: AnyObject, BillingStateService, DebugStateService { /// func getEvents(userId: String?) async throws -> [EventData] + /// Gets whether the Fill Assist feature is enabled for the specified user. + /// + /// - Parameter userId: The user ID, or `nil` for the active account. + /// - Returns: Whether Fill Assist is enabled. + /// + func getFillAssistEnabled(userId: String?) async throws -> Bool + /// Gets the data for the flight recorder. /// /// - Returns: The flight recorder data. @@ -611,6 +618,14 @@ protocol StateService: AnyObject, BillingStateService, DebugStateService { /// func setEvents(_ events: [EventData], userId: String?) async throws + /// Sets whether the Fill Assist feature is enabled for the specified user. + /// + /// - Parameters: + /// - fillAssistEnabled: Whether Fill Assist is enabled. + /// - userId: The user ID of the account. Defaults to the active account if `nil`. + /// + func setFillAssistEnabled(_ fillAssistEnabled: Bool, userId: String?) async throws + /// Sets the force password reset reason for an account. /// /// - Parameters: @@ -1037,6 +1052,14 @@ extension StateService { try await getEnvironmentURLs(userId: nil) } + /// Gets whether Fill Assist is enabled for the active account. + /// + /// - Returns: Whether Fill Assist is enabled. + /// + func getFillAssistEnabled() async throws -> Bool { + try await getFillAssistEnabled(userId: nil) + } + /// Gets whether a sync has been done successfully after login for the current user. /// This is particular useful to trigger logic that needs to be executed right after login in /// and after the first successful sync. @@ -1293,6 +1316,14 @@ extension StateService { try await setDisableAutoTotpCopy(disableAutoTotpCopy, userId: nil) } + /// Sets whether Fill Assist is enabled for the active account. + /// + /// - Parameter fillAssistEnabled: Whether Fill Assist is enabled. + /// + func setFillAssistEnabled(_ fillAssistEnabled: Bool) async throws { + try await setFillAssistEnabled(fillAssistEnabled, userId: nil) + } + /// Sets the force password reset reason for the active account. /// /// - Parameter reason: The reason why a password reset is required. @@ -1735,6 +1766,11 @@ actor DefaultStateService: StateService, ActiveAccountStateProvider, ConfigState return appSettingsStore.events(userId: userId) } + func getFillAssistEnabled(userId: String?) async throws -> Bool { + let userId = try userId ?? getActiveAccountUserId() + return appSettingsStore.fillAssistEnabled(userId: userId) + } + func getFlightRecorderData() async -> FlightRecorderData? { appSettingsStore.flightRecorderData } @@ -1912,8 +1948,6 @@ actor DefaultStateService: StateService, ActiveAccountStateProvider, ConfigState appSettingsStore.setDefaultUriMatchType(nil, userId: knownUserId) appSettingsStore.setDisableAutoTotpCopy(nil, userId: knownUserId) appSettingsStore.setEncryptedUserKey(key: nil, userId: knownUserId) - appSettingsStore.setFillAssistCachedData(nil, userId: knownUserId) - appSettingsStore.setFillAssistLastFetchTimestamp(nil, userId: knownUserId) appSettingsStore.setHasPerformedSyncAfterLogin(nil, userId: knownUserId) appSettingsStore.setLastSyncTime(nil, userId: knownUserId) appSettingsStore.setMasterPasswordHash(nil, userId: knownUserId) @@ -2086,6 +2120,11 @@ actor DefaultStateService: StateService, ActiveAccountStateProvider, ConfigState appSettingsStore.setEvents(events, userId: userId) } + func setFillAssistEnabled(_ fillAssistEnabled: Bool, userId: String?) async throws { + let userId = try userId ?? getActiveAccountUserId() + appSettingsStore.setFillAssistEnabled(fillAssistEnabled, userId: userId) + } + func setFlightRecorderData(_ data: FlightRecorderData?) async { appSettingsStore.flightRecorderData = data } diff --git a/BitwardenShared/Core/Platform/Services/StateServiceTests.swift b/BitwardenShared/Core/Platform/Services/StateServiceTests.swift index b2ca2509fb..916eb17332 100644 --- a/BitwardenShared/Core/Platform/Services/StateServiceTests.swift +++ b/BitwardenShared/Core/Platform/Services/StateServiceTests.swift @@ -764,6 +764,15 @@ class StateServiceTests: BitwardenTestCase { // swiftlint:disable:this type_body XCTAssertEqual(errorReporter.errors as? [StateServiceError], [.noActiveAccount]) } + /// `getFillAssistEnabled()` returns the Fill Assist enabled value for the active account. + func test_getFillAssistEnabled() async throws { + await subject.addAccount(.fixture()) + appSettingsStore.fillAssistEnabledByUserId["1"] = true + + let value = try await subject.getFillAssistEnabled() + XCTAssertTrue(value) + } + /// `getDisableAutoTotpCopy()` returns the disable auto-copy TOTP value for the active account. func test_getDisableAutoTotpCopy() async throws { await subject.addAccount(.fixture()) @@ -2069,6 +2078,17 @@ class StateServiceTests: BitwardenTestCase { // swiftlint:disable:this type_body XCTAssertEqual(appSettingsStore.defaultUriMatchTypeByUserId["1"], .regularExpression) } + /// `setFillAssistEnabled(_:userId:)` sets the Fill Assist enabled value for a user. + func test_setFillAssistEnabled() async throws { + await subject.addAccount(.fixture(profile: .fixture(userId: "1"))) + + try await subject.setFillAssistEnabled(true, userId: "1") + XCTAssertEqual(appSettingsStore.fillAssistEnabledByUserId["1"], true) + + try await subject.setFillAssistEnabled(false, userId: "1") + XCTAssertEqual(appSettingsStore.fillAssistEnabledByUserId["1"], false) + } + /// `setDisableAutoTotpCopy(_:userId:)` sets the disable auto-copy TOTP value for a user. func test_setDisableAutoTotpCopy() async throws { await subject.addAccount(.fixture(profile: .fixture(userId: "1"))) diff --git a/BitwardenShared/Core/Platform/Services/Stores/AppSettingsStore.swift b/BitwardenShared/Core/Platform/Services/Stores/AppSettingsStore.swift index e3e4fadb57..cface3be69 100644 --- a/BitwardenShared/Core/Platform/Services/Stores/AppSettingsStore.swift +++ b/BitwardenShared/Core/Platform/Services/Stores/AppSettingsStore.swift @@ -193,6 +193,12 @@ protocol AppSettingsStore: AnyObject { /// func fillAssistCachedData(userId: String) -> FillAssistCachedData? + /// Gets whether the Fill Assist feature is enabled for the user ID. + /// + /// - Parameter userId: The user ID associated with the Fill Assist enabled value. + /// + func fillAssistEnabled(userId: String) -> Bool + /// Gets the timestamp of the last successful fill-assist manifest check for the user ID. /// /// - Parameter userId: The user ID associated with the timestamp. @@ -461,6 +467,14 @@ protocol AppSettingsStore: AnyObject { /// func setFillAssistCachedData(_ data: FillAssistCachedData?, userId: String) + /// Sets whether the Fill Assist feature is enabled for the user ID. + /// + /// - Parameters: + /// - fillAssistEnabled: The value to set, or `nil` to clear. + /// - userId: The user ID associated with the Fill Assist enabled value. + /// + func setFillAssistEnabled(_ fillAssistEnabled: Bool?, userId: String) + /// Sets the timestamp of the last successful fill-assist manifest check for the user ID. /// /// - Parameters: @@ -839,6 +853,7 @@ extension DefaultAppSettingsStore: AppSettingsStore, ConfigSettingsStore { case encryptedUserKey(userId: String) case events(userId: String) case fillAssistCachedData(userId: String) + case fillAssistEnabled(userId: String) case fillAssistLastFetchTimestamp(userId: String) case flightRecorderData case hasPerformedSyncAfterLogin(userId: String) @@ -930,6 +945,8 @@ extension DefaultAppSettingsStore: AppSettingsStore, ConfigSettingsStore { "events_\(userId)" case let .fillAssistCachedData(userId): "fillAssistCachedData_\(userId)" + case let .fillAssistEnabled(userId): + "fillAssistEnabled_\(userId)" case let .fillAssistLastFetchTimestamp(userId): "fillAssistLastFetchTimestamp_\(userId)" case .flightRecorderData: @@ -1187,6 +1204,10 @@ extension DefaultAppSettingsStore: AppSettingsStore, ConfigSettingsStore { fetch(for: .fillAssistCachedData(userId: userId)) } + func fillAssistEnabled(userId: String) -> Bool { + fetch(for: .fillAssistEnabled(userId: userId)) + } + func fillAssistLastFetchTimestamp(userId: String) -> Date? { fetch(for: .fillAssistLastFetchTimestamp(userId: userId)) } @@ -1333,6 +1354,10 @@ extension DefaultAppSettingsStore: AppSettingsStore, ConfigSettingsStore { store(data, for: .fillAssistCachedData(userId: userId)) } + func setFillAssistEnabled(_ fillAssistEnabled: Bool?, userId: String) { + store(fillAssistEnabled, for: .fillAssistEnabled(userId: userId)) + } + func setFillAssistLastFetchTimestamp(_ timestamp: Date?, userId: String) { store(timestamp, for: .fillAssistLastFetchTimestamp(userId: userId)) } diff --git a/BitwardenShared/Core/Platform/Services/Stores/AppSettingsStoreTests.swift b/BitwardenShared/Core/Platform/Services/Stores/AppSettingsStoreTests.swift index 68952de6e5..dedbe3e95e 100644 --- a/BitwardenShared/Core/Platform/Services/Stores/AppSettingsStoreTests.swift +++ b/BitwardenShared/Core/Platform/Services/Stores/AppSettingsStoreTests.swift @@ -512,6 +512,23 @@ class AppSettingsStoreTests: BitwardenTestCase { // swiftlint:disable:this type_ XCTAssertNotNil(userDefaults.object(forKey: "bwPreferencesStorage:fillAssistLastFetchTimestamp_2")) } + /// `fillAssistEnabled(userId:)` returns `false` when no value has been set. + func test_fillAssistEnabled_isInitiallyFalse() { + XCTAssertFalse(subject.fillAssistEnabled(userId: "-1")) + } + + /// `fillAssistEnabled(userId:)` and `setFillAssistEnabled(_:userId:)` get and set the value + /// per user and persist under the expected UserDefaults key. + func test_fillAssistEnabled_withValue() { + subject.setFillAssistEnabled(true, userId: "1") + subject.setFillAssistEnabled(false, userId: "2") + + XCTAssertTrue(subject.fillAssistEnabled(userId: "1")) + XCTAssertFalse(subject.fillAssistEnabled(userId: "2")) + XCTAssertNotNil(userDefaults.object(forKey: "bwPreferencesStorage:fillAssistEnabled_1")) + XCTAssertNotNil(userDefaults.object(forKey: "bwPreferencesStorage:fillAssistEnabled_2")) + } + /// `overrideDebugFeatureFlag(name:value:)` and `debugFeatureFlag(name:)` work as expected with correct values. func test_featureFlags() { let featureFlags: [FeatureFlag] = [.testFeatureFlag] diff --git a/BitwardenShared/Core/Platform/Services/Stores/TestHelpers/MockAppSettingsStore.swift b/BitwardenShared/Core/Platform/Services/Stores/TestHelpers/MockAppSettingsStore.swift index 6425f0de48..0f1c09206b 100644 --- a/BitwardenShared/Core/Platform/Services/Stores/TestHelpers/MockAppSettingsStore.swift +++ b/BitwardenShared/Core/Platform/Services/Stores/TestHelpers/MockAppSettingsStore.swift @@ -49,6 +49,7 @@ class MockAppSettingsStore: AppSettingsStore { // swiftlint:disable:this type_bo var eventsByUserId = [String: [EventData]]() var featureFlags = [String: Bool]() var fillAssistCachedDataByUserId = [String: FillAssistCachedData]() + var fillAssistEnabledByUserId = [String: Bool]() var fillAssistLastFetchTimestampByUserId = [String: Date]() var hasPerformedSyncAfterLogin = [String: Bool]() var lastActiveTime = [String: Date]() @@ -156,6 +157,10 @@ class MockAppSettingsStore: AppSettingsStore { // swiftlint:disable:this type_bo fillAssistCachedDataByUserId[userId] } + func fillAssistEnabled(userId: String) -> Bool { + fillAssistEnabledByUserId[userId] ?? false + } + func fillAssistLastFetchTimestamp(userId: String) -> Date? { fillAssistLastFetchTimestampByUserId[userId] } @@ -313,6 +318,10 @@ class MockAppSettingsStore: AppSettingsStore { // swiftlint:disable:this type_bo fillAssistCachedDataByUserId[userId] = data } + func setFillAssistEnabled(_ fillAssistEnabled: Bool?, userId: String) { + fillAssistEnabledByUserId[userId] = fillAssistEnabled + } + func setFillAssistLastFetchTimestamp(_ timestamp: Date?, userId: String) { fillAssistLastFetchTimestampByUserId[userId] = timestamp } diff --git a/BitwardenShared/Core/Platform/Services/TestHelpers/MockStateService.swift b/BitwardenShared/Core/Platform/Services/TestHelpers/MockStateService.swift index 77917f8767..1e3d48b7da 100644 --- a/BitwardenShared/Core/Platform/Services/TestHelpers/MockStateService.swift +++ b/BitwardenShared/Core/Platform/Services/TestHelpers/MockStateService.swift @@ -51,6 +51,9 @@ class MockStateService: StateService, ActiveAccountStateProvider, AutofillStateS var didAccountSwitchInExtensionResult: Result = .success(false) var disableAutoTotpCopyByUserId = [String: Bool]() var doesActiveAccountHavePremiumCalled = false + var fillAssistEnabledByUserId = [String: Bool]() + var getFillAssistEnabledError: Error? + var setFillAssistEnabledError: Error? var doesActiveAccountHavePremiumResult: Bool = true var doesActiveAccountHavePremiumPersonallyCalled = false // swiftlint:disable:this identifier_name var doesActiveAccountHavePremiumPersonallyResult: Bool = true // swiftlint:disable:this identifier_name @@ -324,6 +327,14 @@ class MockStateService: StateService, ActiveAccountStateProvider, AutofillStateS return events[userId] ?? [] } + func getFillAssistEnabled(userId: String?) async throws -> Bool { + if let getFillAssistEnabledError { + throw getFillAssistEnabledError + } + let userId = try unwrapUserId(userId) + return fillAssistEnabledByUserId[userId] ?? false + } + func getFlightRecorderData() async -> FlightRecorderData? { flightRecorderData } @@ -666,6 +677,14 @@ class MockStateService: StateService, ActiveAccountStateProvider, AutofillStateS self.events[userId] = events } + func setFillAssistEnabled(_ fillAssistEnabled: Bool, userId: String?) async throws { + if let setFillAssistEnabledError { + throw setFillAssistEnabledError + } + let userId = try unwrapUserId(userId) + fillAssistEnabledByUserId[userId] = fillAssistEnabled + } + func setFlightRecorderData(_ data: FlightRecorderData?) async { flightRecorderData = data } diff --git a/BitwardenShared/Core/Platform/Services/TestHelpers/ServiceContainer+Mocks.swift b/BitwardenShared/Core/Platform/Services/TestHelpers/ServiceContainer+Mocks.swift index bd5db6df85..1007a47b61 100644 --- a/BitwardenShared/Core/Platform/Services/TestHelpers/ServiceContainer+Mocks.swift +++ b/BitwardenShared/Core/Platform/Services/TestHelpers/ServiceContainer+Mocks.swift @@ -43,6 +43,7 @@ extension ServiceContainer { exportVaultService: ExportVaultService = MockExportVaultService(), fido2CredentialStore: Fido2CredentialStore = MockFido2CredentialStore(), fido2UserInterfaceHelper: Fido2UserInterfaceHelper = MockFido2UserInterfaceHelper(), + fillAssistRepository: FillAssistRepository = MockFillAssistRepository(), flightRecorder: FlightRecorder = MockFlightRecorder(), generatorRepository: GeneratorRepository = MockGeneratorRepository(), importCiphersRepository: ImportCiphersRepository = MockImportCiphersRepository(), @@ -131,6 +132,7 @@ extension ServiceContainer { exportVaultService: exportVaultService, fido2CredentialStore: fido2CredentialStore, fido2UserInterfaceHelper: fido2UserInterfaceHelper, + fillAssistRepository: fillAssistRepository, flightRecorder: flightRecorder, generatorRepository: generatorRepository, importCiphersRepository: importCiphersRepository, diff --git a/BitwardenShared/Core/Platform/Utilities/ExternalLinksConstants.swift b/BitwardenShared/Core/Platform/Utilities/ExternalLinksConstants.swift index 838b33f759..7bc248e847 100644 --- a/BitwardenShared/Core/Platform/Utilities/ExternalLinksConstants.swift +++ b/BitwardenShared/Core/Platform/Utilities/ExternalLinksConstants.swift @@ -23,6 +23,9 @@ extension ExternalLinksConstants { /// A link to the auto fill help page. static let autofillHelp = URL(string: "https://bitwarden.com/help/auto-fill-ios/#keyboard-auto-fill")! + /// A link to Bitwarden's help page for the Fill Assist feature. + static let fillAssistHelp = URL(string: "https://bitwarden.com/help/fill-assist/")! + /// A link to Bitwarden's help page for learning more about the account fingerprint phrase. static let fingerprintPhrase = URL(string: "https://bitwarden.com/help/fingerprint-phrase/")! diff --git a/BitwardenShared/Core/Vault/Services/SyncService.swift b/BitwardenShared/Core/Vault/Services/SyncService.swift index 459bd8434d..4d6d627752 100644 --- a/BitwardenShared/Core/Vault/Services/SyncService.swift +++ b/BitwardenShared/Core/Vault/Services/SyncService.swift @@ -170,6 +170,9 @@ class DefaultSyncService: SyncService { /// The service to get server-specified configuration. private let configService: ConfigService + /// The repository used by the application to manage fill-assist data. + private let fillAssistRepository: FillAssistRepository + /// The service used by the application for recording temporary debug logs. private let flightRecorder: FlightRecorder @@ -240,6 +243,7 @@ class DefaultSyncService: SyncService { clientService: ClientService, collectionService: CollectionService, configService: ConfigService, + fillAssistRepository: FillAssistRepository, flightRecorder: FlightRecorder, folderService: FolderService, keyConnectorService: KeyConnectorService, @@ -259,6 +263,7 @@ class DefaultSyncService: SyncService { self.clientService = clientService self.collectionService = collectionService self.configService = configService + self.fillAssistRepository = fillAssistRepository self.flightRecorder = flightRecorder self.folderService = folderService self.keyConnectorService = keyConnectorService @@ -422,6 +427,8 @@ extension DefaultSyncService { let account = try await stateService.getActiveAccount() let userId = account.profile.userId + await fillAssistRepository.syncRules() + guard try await needsSync(forceSync: forceSync, isPeriodic: isPeriodic, userId: userId) else { return } diff --git a/BitwardenShared/Core/Vault/Services/SyncServiceTests.swift b/BitwardenShared/Core/Vault/Services/SyncServiceTests.swift index b8c89d339e..1e35be8d4f 100644 --- a/BitwardenShared/Core/Vault/Services/SyncServiceTests.swift +++ b/BitwardenShared/Core/Vault/Services/SyncServiceTests.swift @@ -18,6 +18,7 @@ class SyncServiceTests: BitwardenTestCase { // swiftlint:disable:this type_body_ var clientService: MockClientService! var collectionService: MockCollectionService! var configService: MockConfigService! + var fillAssistRepository: MockFillAssistRepository! var flightRecorder: MockFlightRecorder! var folderService: MockFolderService! var keyConnectorService: MockKeyConnectorService! @@ -34,7 +35,7 @@ class SyncServiceTests: BitwardenTestCase { // swiftlint:disable:this type_body_ // MARK: Setup & Teardown - override func setUp() { + override func setUp() { // swiftlint:disable:this function_body_length super.setUp() appContextHelper = MockAppContextHelper() @@ -43,6 +44,7 @@ class SyncServiceTests: BitwardenTestCase { // swiftlint:disable:this type_body_ clientService = MockClientService() collectionService = MockCollectionService() configService = MockConfigService() + fillAssistRepository = MockFillAssistRepository() flightRecorder = MockFlightRecorder() folderService = MockFolderService() keyConnectorService = MockKeyConnectorService() @@ -73,6 +75,7 @@ class SyncServiceTests: BitwardenTestCase { // swiftlint:disable:this type_body_ clientService: clientService, collectionService: collectionService, configService: configService, + fillAssistRepository: fillAssistRepository, flightRecorder: flightRecorder, folderService: folderService, keyConnectorService: keyConnectorService, @@ -98,6 +101,7 @@ class SyncServiceTests: BitwardenTestCase { // swiftlint:disable:this type_body_ clientService = nil collectionService = nil configService = nil + fillAssistRepository = nil flightRecorder = nil folderService = nil keyConnectorService = nil @@ -367,6 +371,31 @@ class SyncServiceTests: BitwardenTestCase { // swiftlint:disable:this type_body_ ) } + /// `fetchSync()` calls `syncRules()` on the fill-assist repository independently of vault sync. + func test_fetchSync_syncsFillAssistRules() async throws { + client.result = .httpSuccess(testData: .syncWithCiphers) + stateService.activeAccount = .fixture() + + try await subject.fetchSync(forceSync: false) + + XCTAssertTrue(fillAssistRepository.syncRulesCalled) + } + + /// `fetchSync()` calls `syncRules()` even when the vault does not need syncing. + func test_fetchSync_syncsFillAssistRules_evenWhenVaultSyncSkipped() async throws { + client.result = .httpSuccess(testData: .syncWithCipher) + stateService.activeAccount = .fixture() + stateService.lastSyncTimeByUserId["1"] = try XCTUnwrap( + timeProvider.presentTime.addingTimeInterval(-(Constants.minimumSyncInterval - 1)), + ) + keyConnectorService.userNeedsMigrationResult = .success(false) + + try await subject.fetchSync(forceSync: false, isPeriodic: true) + + XCTAssertTrue(client.requests.isEmpty) + XCTAssertTrue(fillAssistRepository.syncRulesCalled) + } + /// `fetchSync()` with `forceSync: true` performs the sync API request regardless of the /// account revision or sync interval. func test_fetchSync_failedParse() async throws { diff --git a/BitwardenShared/UI/Platform/Application/TestHelpers/MockAppExtensionDelegate.swift b/BitwardenShared/UI/Platform/Application/TestHelpers/MockAppExtensionDelegate.swift index d5840234f6..daf9b79a45 100644 --- a/BitwardenShared/UI/Platform/Application/TestHelpers/MockAppExtensionDelegate.swift +++ b/BitwardenShared/UI/Platform/Application/TestHelpers/MockAppExtensionDelegate.swift @@ -5,7 +5,7 @@ class MockAppExtensionDelegate: AppExtensionDelegate { var canAutofill = true var didCancelCalled = false var didCompleteAuthCalled = false - var didCompleteAutofillRequestFields: [(String, String)]? + var didCompleteAutofillRequestFields: [(selector: String, value: String)]? var didCompleteAutofillRequestPassword: String? var didCompleteAutofillRequestUsername: String? var isInAppExtension = false diff --git a/BitwardenShared/UI/Platform/Settings/Settings/AutoFill/AutoFillAction.swift b/BitwardenShared/UI/Platform/Settings/Settings/AutoFill/AutoFillAction.swift index 591a5e4da1..d2e4fd587a 100644 --- a/BitwardenShared/UI/Platform/Settings/Settings/AutoFill/AutoFillAction.swift +++ b/BitwardenShared/UI/Platform/Settings/Settings/AutoFill/AutoFillAction.swift @@ -23,6 +23,9 @@ enum AutoFillAction: Equatable { /// The copy TOTP automatically toggle value changed. case toggleCopyTOTPToggle(Bool) + /// The Fill Assist toggle value changed. + case toggleFillAssist(Bool) + /// A toast was shown or hidden. case toastShown(Toast?) } diff --git a/BitwardenShared/UI/Platform/Settings/Settings/AutoFill/AutoFillProcessor.swift b/BitwardenShared/UI/Platform/Settings/Settings/AutoFill/AutoFillProcessor.swift index f8ceca95df..2fe13c682f 100644 --- a/BitwardenShared/UI/Platform/Settings/Settings/AutoFill/AutoFillProcessor.swift +++ b/BitwardenShared/UI/Platform/Settings/Settings/AutoFill/AutoFillProcessor.swift @@ -12,6 +12,7 @@ final class AutoFillProcessor: StateProcessor! var errorReporter: MockErrorReporter! + var fillAssistRepository: MockFillAssistRepository! var settingsRepository: MockSettingsRepository! var stateService: MockStateService! var subject: AutoFillProcessor! @@ -29,8 +31,10 @@ class AutoFillProcessorTests: BitwardenTestCase { configService = MockConfigService() coordinator = MockCoordinator() errorReporter = MockErrorReporter() + fillAssistRepository = MockFillAssistRepository() settingsRepository = MockSettingsRepository() stateService = MockStateService() + stateService.activeAccount = .fixture() subject = AutoFillProcessor( coordinator: coordinator.asAnyCoordinator(), @@ -39,6 +43,7 @@ class AutoFillProcessorTests: BitwardenTestCase { autofillCredentialService: autofillCredentialService, configService: configService, errorReporter: errorReporter, + fillAssistRepository: fillAssistRepository, settingsRepository: settingsRepository, stateService: stateService, ), @@ -54,6 +59,7 @@ class AutoFillProcessorTests: BitwardenTestCase { configService = nil coordinator = nil errorReporter = nil + fillAssistRepository = nil settingsRepository = nil stateService = nil subject = nil @@ -77,6 +83,8 @@ class AutoFillProcessorTests: BitwardenTestCase { /// error occurs. @MainActor func test_perform_dismissSetUpAutofillActionCard_error() async { + stateService.activeAccount = nil + await subject.perform(.dismissSetUpAutofillActionCard) XCTAssertEqual(coordinator.alertShown, [.defaultAlert(title: Localizations.anErrorHasOccurred)]) @@ -89,18 +97,36 @@ class AutoFillProcessorTests: BitwardenTestCase { autofillCredentialService.isAutofillCredentialsEnabled = false settingsRepository.getDefaultUriMatchTypeResult = .exact settingsRepository.getDisableAutoTotpCopyResult = .success(false) + stateService.fillAssistEnabledByUserId["1"] = false await subject.perform(.fetchSettingValues) XCTAssertEqual(subject.state.defaultUriMatchType, .exact) XCTAssertTrue(subject.state.isCopyTOTPToggleOn) XCTAssertTrue(subject.state.shouldShowPasswordAutofill) + XCTAssertFalse(subject.state.isFillAssistEnabled) autofillCredentialService.isAutofillCredentialsEnabled = true settingsRepository.getDefaultUriMatchTypeResult = .regularExpression settingsRepository.getDisableAutoTotpCopyResult = .success(true) + stateService.fillAssistEnabledByUserId["1"] = true await subject.perform(.fetchSettingValues) XCTAssertEqual(subject.state.defaultUriMatchType, .regularExpression) XCTAssertFalse(subject.state.isCopyTOTPToggleOn) XCTAssertFalse(subject.state.shouldShowPasswordAutofill) + XCTAssertTrue(subject.state.isFillAssistEnabled) + } + + /// `perform(_:)` with `.fetchSettingValues` loads fill assist feature flag state. + @MainActor + func test_perform_fetchSettingValues_fillAssistFeatureFlag() async { + settingsRepository.getDisableAutoTotpCopyResult = .success(false) + + configService.featureFlagsBool[.fillAssistTargetingRules] = true + await subject.perform(.fetchSettingValues) + XCTAssertTrue(subject.state.isFillAssistFeatureFlagEnabled) + + configService.featureFlagsBool[.fillAssistTargetingRules] = false + await subject.perform(.fetchSettingValues) + XCTAssertFalse(subject.state.isFillAssistFeatureFlagEnabled) } /// `perform(_:)` with `.fetchSettingValues` logs an error and shows an alert if fetching the values fails. @@ -187,6 +213,8 @@ class AutoFillProcessorTests: BitwardenTestCase { /// `perform(_:)` with `.streamSettingsBadge` logs an error if streaming the settings badge state fails. @MainActor func test_perform_streamSettingsBadge_error() async { + stateService.activeAccount = nil + await subject.perform(.streamSettingsBadge) XCTAssertEqual(errorReporter.errors as? [StateServiceError], [.noActiveAccount]) @@ -235,6 +263,47 @@ class AutoFillProcessorTests: BitwardenTestCase { XCTAssertNil(subject.state.toast) } + /// `.receive(_:)` with `.toggleFillAssist(true)` persists the value and triggers a rules sync. + @MainActor + func test_receive_toggleFillAssist_on() { + subject.state.isFillAssistEnabled = false + subject.receive(.toggleFillAssist(true)) + + XCTAssertTrue(subject.state.isFillAssistEnabled) + waitFor(stateService.fillAssistEnabledByUserId["1"] == true) + XCTAssertEqual(stateService.fillAssistEnabledByUserId["1"], true) + waitFor(fillAssistRepository.syncRulesCalled) + XCTAssertTrue(fillAssistRepository.syncRulesCalled) + } + + /// `.receive(_:)` with `.toggleFillAssist(false)` persists the value and does NOT sync rules. + @MainActor + func test_receive_toggleFillAssist_off() { + subject.state.isFillAssistEnabled = true + subject.receive(.toggleFillAssist(false)) + + XCTAssertFalse(subject.state.isFillAssistEnabled) + waitFor(stateService.fillAssistEnabledByUserId["1"] == false) + XCTAssertEqual(stateService.fillAssistEnabledByUserId["1"], false) + XCTAssertFalse(fillAssistRepository.syncRulesCalled) + } + + /// `.receive(_:)` with `.toggleFillAssist` reverts the toggle and shows an alert if persisting + /// the value fails. + @MainActor + func test_receive_toggleFillAssist_persistFails() { + subject.state.isFillAssistEnabled = false + stateService.setFillAssistEnabledError = BitwardenTestError.example + + subject.receive(.toggleFillAssist(true)) + + waitFor(subject.state.isFillAssistEnabled == false) + XCTAssertFalse(subject.state.isFillAssistEnabled) + XCTAssertFalse(fillAssistRepository.syncRulesCalled) + XCTAssertEqual(errorReporter.errors.last as? BitwardenTestError, .example) + XCTAssertEqual(coordinator.errorAlertsShown.last as? BitwardenTestError, .example) + } + /// `.receive(_:)` with `.toggleCopyTOTPToggle` updates the state. @MainActor func test_receive_toggleCopyTOTPToggle() throws { diff --git a/BitwardenShared/UI/Platform/Settings/Settings/AutoFill/AutoFillState.swift b/BitwardenShared/UI/Platform/Settings/Settings/AutoFill/AutoFillState.swift index bc2a3c11a2..e4d37b81f3 100644 --- a/BitwardenShared/UI/Platform/Settings/Settings/AutoFill/AutoFillState.swift +++ b/BitwardenShared/UI/Platform/Settings/Settings/AutoFill/AutoFillState.swift @@ -18,6 +18,12 @@ struct AutoFillState { /// Whether or not the copy TOTP automatically toggle is on. var isCopyTOTPToggleOn: Bool = false + /// Whether the Fill Assist toggle is on. + var isFillAssistEnabled: Bool = false + + /// Whether the Fill Assist feature flag is enabled. + var isFillAssistFeatureFlagEnabled: Bool = false + /// Whether to show the password autofill row. var shouldShowPasswordAutofill: Bool = false diff --git a/BitwardenShared/UI/Platform/Settings/Settings/AutoFill/AutoFillView.swift b/BitwardenShared/UI/Platform/Settings/Settings/AutoFill/AutoFillView.swift index 507bdbe400..77c169cb74 100644 --- a/BitwardenShared/UI/Platform/Settings/Settings/AutoFill/AutoFillView.swift +++ b/BitwardenShared/UI/Platform/Settings/Settings/AutoFill/AutoFillView.swift @@ -85,6 +85,30 @@ struct AutoFillView: View { /// The additional options section. private var additionalOptionsSection: some View { SectionView(Localizations.additionalOptions, contentSpacing: 8) { + if store.state.isFillAssistFeatureFlagEnabled { + BitwardenToggle( + footer: Localizations.turnOnFillAssistDescriptionLong, + isOn: store.binding( + get: \.isFillAssistEnabled, + send: AutoFillAction.toggleFillAssist, + ), + accessibilityIdentifier: "FillAssistSwitch", + ) { + HStack(spacing: 8) { + Text(Localizations.turnOnFillAssist) + Button { + openURL(ExternalLinksConstants.fillAssistHelp) + } label: { + SharedAsset.Icons.questionCircle16.swiftUIImage + .scaledFrame(width: 16, height: 16) + .accessibilityLabel(Localizations.learnMore) + } + .buttonStyle(.fieldLabelIcon) + } + } + .contentBlock() + } + BitwardenToggle( Localizations.copyTotpAutomatically, footer: Localizations.copyTotpAutomaticallyDescription, diff --git a/BitwardenShared/UI/Platform/Settings/SettingsCoordinator.swift b/BitwardenShared/UI/Platform/Settings/SettingsCoordinator.swift index 187c777a5d..6ced6bf42b 100644 --- a/BitwardenShared/UI/Platform/Settings/SettingsCoordinator.swift +++ b/BitwardenShared/UI/Platform/Settings/SettingsCoordinator.swift @@ -78,6 +78,7 @@ final class SettingsCoordinator: Coordinator, HasStackNavigator { // swiftlint:d & HasEventService & HasExportCXFCiphersRepository & HasExportVaultService + & HasFillAssistRepository & HasFlightRecorder & HasLanguageStateService & HasNotificationCenterService diff --git a/BitwardenShared/UI/Vault/Helpers/AutofillHelper.swift b/BitwardenShared/UI/Vault/Helpers/AutofillHelper.swift index e48f8bd40c..0808fae652 100644 --- a/BitwardenShared/UI/Vault/Helpers/AutofillHelper.swift +++ b/BitwardenShared/UI/Vault/Helpers/AutofillHelper.swift @@ -1,6 +1,7 @@ import BitwardenKit import BitwardenResources @preconcurrency import BitwardenSdk +import Foundation /// A helper class to handle when a cipher is selected for autofill. /// @@ -9,9 +10,12 @@ class AutofillHelper { // MARK: Types typealias Services = HasAuthRepository + & HasConfigService & HasErrorReporter & HasEventService + & HasFillAssistRepository & HasPasteboardService + & HasStateService & HasVaultRepository // MARK: Properties @@ -94,6 +98,36 @@ class AutofillHelper { } } + /// Builds FillAssist-derived `(selector, value)` tuples for username and password fields when + /// the `fillAssistTargetingRules` feature flag is enabled and cached rules exist for the host. + /// + /// - Parameters: + /// - uri: The URI string of the current page. + /// - username: The username value to fill. + /// - password: The password value to fill. + /// - Returns: An array of `(selector, value)` tuples, or an empty array if unavailable. + /// + private func fillAssistFields(for uri: String?, username: String, password: String) async -> [(String, String)] { + guard await services.configService.getFeatureFlag(.fillAssistTargetingRules), + await (try? services.stateService.getFillAssistEnabled()) == true, + let uri, + let url = URL(string: uri), + let lookupHost = url.domain, + let rules = await services.fillAssistRepository.rules(for: lookupHost) else { return [] } + + var result: [(String, String)] = [] + + if let selector = rules.fields["username"]?.compactMap({ $0.id ?? $0.name }).first { + result.append((selector, username)) + } + + if let selector = rules.fields["password"]?.compactMap({ $0.id ?? $0.name }).first { + result.append((selector, password)) + } + + return result + } + /// Handles autofill for a cipher after the master password reprompt has been confirmed, if it's /// required by the cipher. /// @@ -117,7 +151,9 @@ class AutofillHelper { return } - guard appExtensionDelegate?.canAutofill ?? false, + let fillAssistEnabled = await (try? services.stateService.getFillAssistEnabled()) == true + let canAutofill = appExtensionDelegate?.canAutofill ?? false + guard fillAssistEnabled || canAutofill, let username = cipherView.login?.username, !username.isEmpty, let password = cipherView.login?.password, !password.isEmpty else { await handleMissingValueForAutofill(cipherView: cipherView, showToast: showToast) @@ -126,10 +162,18 @@ class AutofillHelper { await copyTotpIfNeeded(cipherView: cipherView) - let fields: [(String, String)]? = cipherView.fields?.compactMap { field in + let cipherFields: [(String, String)] = cipherView.fields?.compactMap { field in guard let name = field.name, let value = field.value else { return nil } return (name, value) - } + } ?? [] + + let assistFields = await fillAssistFields( + for: appExtensionDelegate?.uri, + username: username, + password: password, + ) + + let combinedFields = (assistFields + cipherFields).nilIfEmpty await services.eventService.collect( eventType: .cipherClientAutofilled, @@ -139,7 +183,7 @@ class AutofillHelper { appExtensionDelegate?.completeAutofillRequest( username: username, password: password, - fields: fields, + fields: combinedFields, ) } catch { services.errorReporter.log(error: error) diff --git a/BitwardenShared/UI/Vault/Helpers/AutofillHelperTests.swift b/BitwardenShared/UI/Vault/Helpers/AutofillHelperTests.swift index ae2d08a831..be90863ac5 100644 --- a/BitwardenShared/UI/Vault/Helpers/AutofillHelperTests.swift +++ b/BitwardenShared/UI/Vault/Helpers/AutofillHelperTests.swift @@ -6,15 +6,20 @@ import TestHelpers import XCTest @testable import BitwardenShared +@testable import BitwardenSharedMocks +@MainActor class AutofillHelperTests: BitwardenTestCase { // swiftlint:disable:this type_body_length // MARK: Properties var appExtensionDelegate: MockAppExtensionDelegate! var authRepository: MockAuthRepository! + var configService: MockConfigService! var coordinator: MockCoordinator! var errorReporter: MockErrorReporter! + var fillAssistRepository: MockFillAssistRepository! var pasteboardService: MockPasteboardService! + var stateService: MockStateService! var subject: AutofillHelper! var vaultRepository: MockVaultRepository! @@ -24,10 +29,16 @@ class AutofillHelperTests: BitwardenTestCase { // swiftlint:disable:this type_bo super.setUp() appExtensionDelegate = MockAppExtensionDelegate() + appExtensionDelegate.uri = "https://example.com/login" authRepository = MockAuthRepository() + configService = MockConfigService() coordinator = MockCoordinator() errorReporter = MockErrorReporter() + fillAssistRepository = MockFillAssistRepository() pasteboardService = MockPasteboardService() + stateService = MockStateService() + stateService.activeAccount = .fixture() + stateService.fillAssistEnabledByUserId["1"] = true vaultRepository = MockVaultRepository() subject = AutofillHelper( @@ -35,21 +46,27 @@ class AutofillHelperTests: BitwardenTestCase { // swiftlint:disable:this type_bo coordinator: coordinator.asAnyCoordinator(), services: ServiceContainer.withMocks( authRepository: authRepository, + configService: configService, errorReporter: errorReporter, + fillAssistRepository: fillAssistRepository, pasteboardService: pasteboardService, + stateService: stateService, vaultRepository: vaultRepository, ), ) } - override func tearDown() { - super.tearDown() + override func tearDown() async throws { + try await super.tearDown() appExtensionDelegate = nil authRepository = nil + configService = nil coordinator = nil errorReporter = nil + fillAssistRepository = nil pasteboardService = nil + stateService = nil subject = nil vaultRepository = nil } @@ -76,6 +93,7 @@ class AutofillHelperTests: BitwardenTestCase { // swiftlint:disable:this type_bo @MainActor func test_handleCipherForAutofill_autofillNotSupported() async throws { appExtensionDelegate.canAutofill = false + stateService.fillAssistEnabledByUserId["1"] = false vaultRepository.fetchCipherResult = .success(.fixture( login: .fixture(password: "PASSWORD", username: "user@bitwarden.com"), @@ -466,4 +484,231 @@ class AutofillHelperTests: BitwardenTestCase { // swiftlint:disable:this type_bo XCTAssertNil(pasteboardService.copiedString) } + + // MARK: Tests - FillAssist + + /// `handleCipherForAutofill` does not call `fillAssistRepository` when the feature flag is off. + func test_handleCipherForAutofill_fillAssist_flagOff() async { + configService.featureFlagsBool[.fillAssistTargetingRules] = false + fillAssistRepository.rulesReturnValue = FillAssistHostRules( + fields: ["username": [.init(id: "login-email", name: nil, role: nil, tagName: nil, type: nil)]], + ) + vaultRepository.fetchCipherResult = .success(.fixture( + login: .fixture(password: "PASSWORD", username: "user@bitwarden.com"), + )) + + await subject.handleCipherForAutofill(cipherListView: .fixture(id: "1")) { _ in } + + XCTAssertFalse(fillAssistRepository.rulesCalled) + XCTAssertNil(appExtensionDelegate.didCompleteAutofillRequestFields) + } + + /// `handleCipherForAutofill` calls `completeAutofillRequest` with `nil` fields when no rules + /// exist for the host. + func test_handleCipherForAutofill_fillAssist_noRulesForHost() async { + configService.featureFlagsBool[.fillAssistTargetingRules] = true + fillAssistRepository.rulesReturnValue = nil + vaultRepository.fetchCipherResult = .success(.fixture( + login: .fixture(password: "PASSWORD", username: "user@bitwarden.com"), + )) + + await subject.handleCipherForAutofill(cipherListView: .fixture(id: "1")) { _ in } + + XCTAssertTrue(fillAssistRepository.rulesCalled) + XCTAssertNil(appExtensionDelegate.didCompleteAutofillRequestFields) + } + + /// `handleCipherForAutofill` prepends FillAssist id-based selectors before cipher custom fields. + func test_handleCipherForAutofill_fillAssist_rulesFound_idSelector() async throws { + configService.featureFlagsBool[.fillAssistTargetingRules] = true + fillAssistRepository.rulesReturnValue = FillAssistHostRules(fields: [ + "username": [.init(id: "login-email", name: nil, role: nil, tagName: nil, type: nil)], + "password": [.init(id: "login-pwd", name: nil, role: nil, tagName: nil, type: nil)], + ]) + vaultRepository.fetchCipherResult = .success(.fixture( + login: .fixture(password: "PASSWORD", username: "user@bitwarden.com"), + )) + + await subject.handleCipherForAutofill(cipherListView: .fixture(id: "1")) { _ in } + + XCTAssertEqual(fillAssistRepository.rulesReceivedHostname, "example.com") + let fields = try XCTUnwrap(appExtensionDelegate.didCompleteAutofillRequestFields) + XCTAssertEqual(fields[0].selector, "login-email") + XCTAssertEqual(fields[0].value, "user@bitwarden.com") + XCTAssertEqual(fields[1].selector, "login-pwd") + XCTAssertEqual(fields[1].value, "PASSWORD") + } + + /// `handleCipherForAutofill` falls back to the `name` attribute when `id` is nil. + func test_handleCipherForAutofill_fillAssist_rulesFound_nameFallback() async throws { + configService.featureFlagsBool[.fillAssistTargetingRules] = true + fillAssistRepository.rulesReturnValue = FillAssistHostRules(fields: [ + "username": [.init(id: nil, name: "email", role: nil, tagName: nil, type: nil)], + "password": [.init(id: nil, name: "pass", role: nil, tagName: nil, type: nil)], + ]) + vaultRepository.fetchCipherResult = .success(.fixture( + login: .fixture(password: "PASSWORD", username: "user@bitwarden.com"), + )) + + await subject.handleCipherForAutofill(cipherListView: .fixture(id: "1")) { _ in } + + let fields = try XCTUnwrap(appExtensionDelegate.didCompleteAutofillRequestFields) + XCTAssertEqual(fields[0].selector, "email") + XCTAssertEqual(fields[0].value, "user@bitwarden.com") + XCTAssertEqual(fields[1].selector, "pass") + XCTAssertEqual(fields[1].value, "PASSWORD") + } + + /// `handleCipherForAutofill` does not call `fillAssistRepository` when the URI is nil. + func test_handleCipherForAutofill_fillAssist_nilUri() async { + configService.featureFlagsBool[.fillAssistTargetingRules] = true + appExtensionDelegate.uri = nil + vaultRepository.fetchCipherResult = .success(.fixture( + login: .fixture(password: "PASSWORD", username: "user@bitwarden.com"), + )) + + await subject.handleCipherForAutofill(cipherListView: .fixture(id: "1")) { _ in } + + XCTAssertFalse(fillAssistRepository.rulesCalled) + XCTAssertNil(appExtensionDelegate.didCompleteAutofillRequestFields) + } + + /// `handleCipherForAutofill` prepends FillAssist selectors before cipher custom fields. + func test_handleCipherForAutofill_fillAssist_prependsBeforeCipherFields() async throws { + configService.featureFlagsBool[.fillAssistTargetingRules] = true + fillAssistRepository.rulesReturnValue = FillAssistHostRules(fields: [ + "username": [.init(id: "login-email", name: nil, role: nil, tagName: nil, type: nil)], + "password": [.init(id: "login-pwd", name: nil, role: nil, tagName: nil, type: nil)], + ]) + vaultRepository.fetchCipherResult = .success(.fixture( + fields: [.fixture(name: "custom", value: "val")], + login: .fixture(password: "PASSWORD", username: "user@bitwarden.com"), + )) + + await subject.handleCipherForAutofill(cipherListView: .fixture(id: "1")) { _ in } + + let fields = try XCTUnwrap(appExtensionDelegate.didCompleteAutofillRequestFields) + XCTAssertEqual(fields.count, 3) + XCTAssertEqual(fields[0].selector, "login-email") + XCTAssertEqual(fields[0].value, "user@bitwarden.com") + XCTAssertEqual(fields[1].selector, "login-pwd") + XCTAssertEqual(fields[1].value, "PASSWORD") + XCTAssertEqual(fields[2].selector, "custom") + XCTAssertEqual(fields[2].value, "val") + } + + /// `handleCipherForAutofill` strips the subdomain when looking up FillAssist rules so that a + /// URI like `https://www.example.com` matches rules stored under `example.com`. + func test_handleCipherForAutofill_fillAssist_stripsSubdomain() async throws { + configService.featureFlagsBool[.fillAssistTargetingRules] = true + appExtensionDelegate.uri = "https://www.example.com/login" + fillAssistRepository.rulesReturnValue = FillAssistHostRules(fields: [ + "username": [.init(id: "login-email", name: nil, role: nil, tagName: nil, type: nil)], + "password": [.init(id: "login-pwd", name: nil, role: nil, tagName: nil, type: nil)], + ]) + vaultRepository.fetchCipherResult = .success(.fixture( + login: .fixture(password: "PASSWORD", username: "user@bitwarden.com"), + )) + + await subject.handleCipherForAutofill(cipherListView: .fixture(id: "1")) { _ in } + + // Rules should be looked up under the registered domain, not the full host. + XCTAssertEqual(fillAssistRepository.rulesReceivedHostname, "example.com") + let fields = try XCTUnwrap(appExtensionDelegate.didCompleteAutofillRequestFields) + XCTAssertEqual(fields[0].selector, "login-email") + XCTAssertEqual(fields[0].value, "user@bitwarden.com") + XCTAssertEqual(fields[1].selector, "login-pwd") + XCTAssertEqual(fields[1].value, "PASSWORD") + } + + /// `handleCipherForAutofill` appends only a username selector when rules have no password entry. + func test_handleCipherForAutofill_fillAssist_usernameOnlyRule() async throws { + configService.featureFlagsBool[.fillAssistTargetingRules] = true + fillAssistRepository.rulesReturnValue = FillAssistHostRules(fields: [ + "username": [.init(id: "login-email", name: nil, role: nil, tagName: nil, type: nil)], + ]) + vaultRepository.fetchCipherResult = .success(.fixture( + login: .fixture(password: "PASSWORD", username: "user@bitwarden.com"), + )) + + await subject.handleCipherForAutofill(cipherListView: .fixture(id: "1")) { _ in } + + let fields = try XCTUnwrap(appExtensionDelegate.didCompleteAutofillRequestFields) + XCTAssertEqual(fields.count, 1) + XCTAssertEqual(fields[0].selector, "login-email") + XCTAssertEqual(fields[0].value, "user@bitwarden.com") + } + + /// `handleCipherForAutofill` appends only a password selector when rules have no username entry. + func test_handleCipherForAutofill_fillAssist_passwordOnlyRule() async throws { + configService.featureFlagsBool[.fillAssistTargetingRules] = true + fillAssistRepository.rulesReturnValue = FillAssistHostRules(fields: [ + "password": [.init(id: "login-pwd", name: nil, role: nil, tagName: nil, type: nil)], + ]) + vaultRepository.fetchCipherResult = .success(.fixture( + login: .fixture(password: "PASSWORD", username: "user@bitwarden.com"), + )) + + await subject.handleCipherForAutofill(cipherListView: .fixture(id: "1")) { _ in } + + let fields = try XCTUnwrap(appExtensionDelegate.didCompleteAutofillRequestFields) + XCTAssertEqual(fields.count, 1) + XCTAssertEqual(fields[0].selector, "login-pwd") + XCTAssertEqual(fields[0].value, "PASSWORD") + } + + /// `handleCipherForAutofill` produces no FillAssist fields when rules only contain + /// keys other than "username" and "password". + func test_handleCipherForAutofill_fillAssist_unknownFieldKeys_noFields() async { + configService.featureFlagsBool[.fillAssistTargetingRules] = true + fillAssistRepository.rulesReturnValue = FillAssistHostRules(fields: [ + "email": [.init(id: "email-field", name: nil, role: nil, tagName: nil, type: nil)], + "newPassword": [.init(id: "pass-field", name: nil, role: nil, tagName: nil, type: nil)], + ]) + vaultRepository.fetchCipherResult = .success(.fixture( + login: .fixture(password: "PASSWORD", username: "user@bitwarden.com"), + )) + + await subject.handleCipherForAutofill(cipherListView: .fixture(id: "1")) { _ in } + + XCTAssertNil(appExtensionDelegate.didCompleteAutofillRequestFields) + } + + /// `handleCipherForAutofill` does not use FillAssist when the user toggle is off, + /// even if the feature flag is on and rules are cached. + func test_handleCipherForAutofill_fillAssist_userToggleOff() async { + configService.featureFlagsBool[.fillAssistTargetingRules] = true + stateService.fillAssistEnabledByUserId["1"] = false + fillAssistRepository.rulesReturnValue = FillAssistHostRules(fields: [ + "username": [.init(id: "login-email", name: nil, role: nil, tagName: nil, type: nil)], + "password": [.init(id: "login-pwd", name: nil, role: nil, tagName: nil, type: nil)], + ]) + vaultRepository.fetchCipherResult = .success(.fixture( + login: .fixture(password: "PASSWORD", username: "user@bitwarden.com"), + )) + + await subject.handleCipherForAutofill(cipherListView: .fixture(id: "1")) { _ in } + + XCTAssertFalse(fillAssistRepository.rulesCalled) + XCTAssertNil(appExtensionDelegate.didCompleteAutofillRequestFields) + } + + /// `handleCipherForAutofill` does not use Fill Assist when `getFillAssistEnabled` throws, + /// treating it the same as the toggle being off. + func test_handleCipherForAutofill_fillAssist_getFillAssistEnabledThrows() async { + configService.featureFlagsBool[.fillAssistTargetingRules] = true + stateService.getFillAssistEnabledError = BitwardenTestError.example + fillAssistRepository.rulesReturnValue = FillAssistHostRules(fields: [ + "username": [.init(id: "login-email", name: nil, role: nil, tagName: nil, type: nil)], + "password": [.init(id: "login-pwd", name: nil, role: nil, tagName: nil, type: nil)], + ]) + vaultRepository.fetchCipherResult = .success(.fixture( + login: .fixture(password: "PASSWORD", username: "user@bitwarden.com"), + )) + + await subject.handleCipherForAutofill(cipherListView: .fixture(id: "1")) { _ in } + + XCTAssertFalse(fillAssistRepository.rulesCalled) + XCTAssertNil(appExtensionDelegate.didCompleteAutofillRequestFields) + } } // swiftlint:disable:this file_length diff --git a/BitwardenShared/UI/Vault/Vault/AutofillList/VaultAutofillListProcessor.swift b/BitwardenShared/UI/Vault/Vault/AutofillList/VaultAutofillListProcessor.swift index 6adb648ea0..82dac991d0 100644 --- a/BitwardenShared/UI/Vault/Vault/AutofillList/VaultAutofillListProcessor.swift +++ b/BitwardenShared/UI/Vault/Vault/AutofillList/VaultAutofillListProcessor.swift @@ -24,6 +24,7 @@ class VaultAutofillListProcessor: StateProcessor URL { - let url = baseURL.appendingPathComponent(filename) - return try await client.download(from: URLRequest(url: url)) - } - /// Performs a network request. /// /// - Parameter request: The request to perform. diff --git a/Networking/Tests/NetworkingTests/HTTPServiceTests.swift b/Networking/Tests/NetworkingTests/HTTPServiceTests.swift index 4928543b57..b1f60c08a8 100644 --- a/Networking/Tests/NetworkingTests/HTTPServiceTests.swift +++ b/Networking/Tests/NetworkingTests/HTTPServiceTests.swift @@ -303,21 +303,6 @@ class HTTPServiceTests: XCTestCase { XCTAssertEqual(error as? TestError, .invalidResponse) } } - - /// `download(filename:)` appends the filename to the base URL and calls the client. - func test_download_filename() async throws { - let tempFile = FileManager.default.temporaryDirectory.appendingPathComponent("test.json") - try "{}".write(to: tempFile, atomically: true, encoding: .utf8) - client.downloadResults = [.success(tempFile)] - - let result = try await subject.download(filename: "manifest.json") - - XCTAssertEqual( - client.downloadRequests.last?.url?.absoluteString, - "https://example.com/manifest.json", - ) - XCTAssertEqual(result, tempFile) - } } private struct RequestError: Error {}