From 64b5123ff0fe7ea8a258f295742626de8d6fba13 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andre=CC=81=20Bispo?= Date: Fri, 19 Jun 2026 18:17:33 +0100 Subject: [PATCH 01/23] [PM-38443] feat: Add FillAssistRepository and sync integration --- .../Repositories/FillAssistRepository.swift | 210 ++++++++++++++++++ .../Platform/Services/ServiceContainer.swift | 16 ++ .../Core/Platform/Services/Services.swift | 8 + .../TestHelpers/ServiceContainer+Mocks.swift | 2 + .../Core/Vault/Services/SyncService.swift | 8 + .../Vault/Services/SyncServiceTests.swift | 6 +- 6 files changed, 249 insertions(+), 1 deletion(-) create mode 100644 BitwardenShared/Core/Autofill/Repositories/FillAssistRepository.swift diff --git a/BitwardenShared/Core/Autofill/Repositories/FillAssistRepository.swift b/BitwardenShared/Core/Autofill/Repositories/FillAssistRepository.swift new file mode 100644 index 0000000000..2e1fe35b81 --- /dev/null +++ b/BitwardenShared/Core/Autofill/Repositories/FillAssistRepository.swift @@ -0,0 +1,210 @@ +import BitwardenKit +import CryptoKit +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 syncFillAssistRules() 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 fillAssistRules(for hostname: String) async -> FillAssistHostRules? + + /// Clears all cached fill-assist data for the active account. + /// + func clearFillAssistRules() async throws +} + +// MARK: - DefaultFillAssistRepository + +/// The default implementation of `FillAssistRepository`. +/// +class DefaultFillAssistRepository: FillAssistRepository { + // MARK: Types + + typealias Services = HasConfigService + & HasEnvironmentService + & HasFillAssistAPIService + & HasStateService + + // 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 + + // 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. + /// + init( + appSettingsStore: AppSettingsStore, + configService: ConfigService, + environmentService: EnvironmentService, + errorReporter: ErrorReporter, + fillAssistAPIService: FillAssistAPIService, + stateService: StateService, + ) { + self.appSettingsStore = appSettingsStore + self.configService = configService + self.environmentService = environmentService + self.errorReporter = errorReporter + self.fillAssistAPIService = fillAssistAPIService + self.stateService = stateService + } + + // MARK: FillAssistRepository + + func syncFillAssistRules() async { + do { + try await performSync() + } catch { + // Sync failures are non-fatal — existing cache remains available. + errorReporter.log(error: error) + } + } + + func fillAssistRules(for hostname: String) async -> FillAssistHostRules? { + guard let userId = try? await stateService.getActiveAccountId() else { return nil } + return appSettingsStore.fillAssistCachedData(userId: userId)?.rules[hostname] + } + + func clearFillAssistRules() async throws { + let userId = try await stateService.getActiveAccountId() + appSettingsStore.setFillAssistCachedData(nil, userId: userId) + } + + // MARK: Private + + /// Runs the full sync pipeline if sync conditions are met + /// + private func performSync() async throws { + // 1. Feature flag guard. + guard await configService.getFeatureFlag(.fillAssistTargetingRules) else { return } + + // 2. Time-interval guard — skip if last fetch was less than fillAssistUpdateInterval ago. + let sourceUrl = environmentService.fillAssistRulesURL + let userId = try await stateService.getActiveAccountId() + let lastFetch = appSettingsStore.fillAssistLastFetchTimestamp(userId: userId) + if let lastFetch, Date().timeIntervalSince(lastFetch) < Constants.fillAssistUpdateInterval { + return + } + + // 3. Fetch manifest. + let manifest = try await fillAssistAPIService.getManifest() + + // 4. Resolve the non-deprecated entry for the current forms version. + guard let entry = manifest.maps["forms"]?[Constants.fillAssistFormsVersion], + !entry.deprecated + else { return } + + // 5. If cid and source URL are unchanged, update timestamp and skip download. + let cached = appSettingsStore.fillAssistCachedData(userId: userId) + if cached?.cid == entry.cid, cached?.sourceUrl == sourceUrl.absoluteString { + appSettingsStore.setFillAssistLastFetchTimestamp(Date(), userId: userId) + return + } + + // 6. Download forms file. + let formsMap = try await fillAssistAPIService.getFormsMap(filename: entry.filename) + + // 7. Validate schema major version; update timestamp and skip if unsupported. + let schemaMajor = formsMap.schemaVersion.split(separator: ".").first.map(String.init) ?? "" + guard schemaMajor == Constants.fillAssistExpectedSchemaMajor else { + appSettingsStore.setFillAssistLastFetchTimestamp(Date(), userId: userId) + return + } + + // 8. SHA-256 integrity check (strip "sha256:" prefix from manifest cid). + let cidHex = entry.cid.hasPrefix("sha256:") ? String(entry.cid.dropFirst(7)) : entry.cid + try verifyIntegrity(of: formsMap, expectedCidHex: cidHex) + + // 9. Parse, cache, and update timestamp. + let rules = buildRules(from: formsMap) + let data = FillAssistCachedData(cid: entry.cid, rules: rules, sourceUrl: sourceUrl.absoluteString) + appSettingsStore.setFillAssistCachedData(data, userId: userId) + appSettingsStore.setFillAssistLastFetchTimestamp(Date(), userId: userId) + } + + /// Verifies the SHA-256 hash of the serialised `FormsMapResponseModel` against the expected hex. + /// + private func verifyIntegrity(of formsMap: FormsMapResponseModel, expectedCidHex: String) throws { + let data = try JSONEncoder().encode(formsMap) + let actualHex = SHA256.hash(data: data).map { String(format: "%02hhx", $0) }.joined() + guard actualHex == expectedCidHex else { + throw FillAssistRepositoryError.integrityCheckFailed + } + } + + /// 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.forms ?? []) + + (hostEntry.pathnames?.values.compactMap(\.self).flatMap(\.forms) ?? []) + for form in allForms { + for (fieldKey, selectors) in form.fields { + let attrs = selectors.flatMap { parseSelector($0) } + pooled[fieldKey, default: []].append(contentsOf: attrs) + } + } + if !pooled.isEmpty { + result[hostname] = FillAssistHostRules(fields: pooled) + } + } + return result + } + + /// Converts a `FormsMapSelector` into zero or more `FillAssistFieldAttributes`. + /// + private func parseSelector(_ selector: FormsMapSelector) -> [FillAssistFieldAttributes] { + switch selector { + case let .single(css): + FillAssistSelectorParser.parse(css).map { [$0] } ?? [] + case let .sequence(parts): + parts.compactMap { FillAssistSelectorParser.parse($0) } + } + } +} + +// MARK: - FillAssistRepositoryError + +/// Errors thrown internally by `DefaultFillAssistRepository`. +/// +enum FillAssistRepositoryError: Error { + /// The downloaded forms file did not match the expected SHA-256 hash. + case integrityCheckFailed +} diff --git a/BitwardenShared/Core/Platform/Services/ServiceContainer.swift b/BitwardenShared/Core/Platform/Services/ServiceContainer.swift index ba37781e13..1a13cdf610 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 @@ -348,6 +351,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, @@ -418,6 +422,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 @@ -767,6 +772,15 @@ 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, + ) + let syncService = DefaultSyncService( accountAPIService: apiService, appContextHelper: appContextHelper, @@ -774,6 +788,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, @@ -1179,6 +1194,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 309bfa0d33..d5e03ff485 100644 --- a/BitwardenShared/Core/Platform/Services/Services.swift +++ b/BitwardenShared/Core/Platform/Services/Services.swift @@ -38,6 +38,7 @@ typealias Services = HasAPIService & HasFido2UserInterfaceHelper & HasFileAPIService & HasFillAssistAPIService + & HasFillAssistRepository & HasFlightRecorder & HasGeneratorRepository & HasImportCiphersRepository @@ -285,6 +286,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/TestHelpers/ServiceContainer+Mocks.swift b/BitwardenShared/Core/Platform/Services/TestHelpers/ServiceContainer+Mocks.swift index 7c8639e0f6..2e051e402e 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/Vault/Services/SyncService.swift b/BitwardenShared/Core/Vault/Services/SyncService.swift index 459bd8434d..5177a7d298 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 @@ -474,6 +479,9 @@ extension DefaultSyncService { ) } + // Sync fill-assist rules alongside vault data. Failures must not block vault sync. + await fillAssistRepository.syncFillAssistRules() + await delegate?.onFetchSyncSucceeded(userId: userId) } diff --git a/BitwardenShared/Core/Vault/Services/SyncServiceTests.swift b/BitwardenShared/Core/Vault/Services/SyncServiceTests.swift index b8c89d339e..6534e6b10a 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 From 42e8f20e2a0278b18272cb9c988a2bbfc233b763 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andre=CC=81=20Bispo?= Date: Sat, 20 Jun 2026 19:38:26 +0100 Subject: [PATCH 02/23] [PM-38443] fix: Remove SHA-256 integrity check and use URLSession.shared for fill-assist CDN --- .../Repositories/FillAssistRepository.swift | 26 +- .../FillAssistRepositoryTests.swift | 227 ++++++++++++++++++ .../Platform/Services/API/APIService.swift | 6 +- 3 files changed, 233 insertions(+), 26 deletions(-) create mode 100644 BitwardenShared/Core/Autofill/Repositories/FillAssistRepositoryTests.swift diff --git a/BitwardenShared/Core/Autofill/Repositories/FillAssistRepository.swift b/BitwardenShared/Core/Autofill/Repositories/FillAssistRepository.swift index 2e1fe35b81..ce65cdb799 100644 --- a/BitwardenShared/Core/Autofill/Repositories/FillAssistRepository.swift +++ b/BitwardenShared/Core/Autofill/Repositories/FillAssistRepository.swift @@ -1,5 +1,4 @@ import BitwardenKit -import CryptoKit import Foundation // MARK: - FillAssistRepository @@ -145,27 +144,13 @@ class DefaultFillAssistRepository: FillAssistRepository { return } - // 8. SHA-256 integrity check (strip "sha256:" prefix from manifest cid). - let cidHex = entry.cid.hasPrefix("sha256:") ? String(entry.cid.dropFirst(7)) : entry.cid - try verifyIntegrity(of: formsMap, expectedCidHex: cidHex) - - // 9. Parse, cache, and update timestamp. + // 8. Parse, cache, and update timestamp. let rules = buildRules(from: formsMap) let data = FillAssistCachedData(cid: entry.cid, rules: rules, sourceUrl: sourceUrl.absoluteString) appSettingsStore.setFillAssistCachedData(data, userId: userId) appSettingsStore.setFillAssistLastFetchTimestamp(Date(), userId: userId) } - /// Verifies the SHA-256 hash of the serialised `FormsMapResponseModel` against the expected hex. - /// - private func verifyIntegrity(of formsMap: FormsMapResponseModel, expectedCidHex: String) throws { - let data = try JSONEncoder().encode(formsMap) - let actualHex = SHA256.hash(data: data).map { String(format: "%02hhx", $0) }.joined() - guard actualHex == expectedCidHex else { - throw FillAssistRepositoryError.integrityCheckFailed - } - } - /// Builds a `[hostname: FillAssistHostRules]` dictionary by pooling all non-null field /// definitions across each host's top-level forms and every pathname entry. /// @@ -199,12 +184,3 @@ class DefaultFillAssistRepository: FillAssistRepository { } } } - -// MARK: - FillAssistRepositoryError - -/// Errors thrown internally by `DefaultFillAssistRepository`. -/// -enum FillAssistRepositoryError: Error { - /// The downloaded forms file did not match the expected SHA-256 hash. - case integrityCheckFailed -} diff --git a/BitwardenShared/Core/Autofill/Repositories/FillAssistRepositoryTests.swift b/BitwardenShared/Core/Autofill/Repositories/FillAssistRepositoryTests.swift new file mode 100644 index 0000000000..5f2984394b --- /dev/null +++ b/BitwardenShared/Core/Autofill/Repositories/FillAssistRepositoryTests.swift @@ -0,0 +1,227 @@ +import BitwardenKit +import BitwardenKitMocks +import XCTest + +@testable import BitwardenShared +@testable import BitwardenSharedMocks + +// MARK: - FillAssistRepositoryTests + +class FillAssistRepositoryTests: BitwardenTestCase { + // MARK: Properties + + var appSettingsStore: MockAppSettingsStore! + var configService: MockConfigService! + var environmentService: MockEnvironmentService! + var errorReporter: MockErrorReporter! + var fillAssistAPIService: MockFillAssistAPIService! + var stateService: MockStateService! + var subject: DefaultFillAssistRepository! + + // MARK: Setup & Teardown + + override func setUp() { + super.setUp() + + appSettingsStore = MockAppSettingsStore() + configService = MockConfigService() + environmentService = MockEnvironmentService() + errorReporter = MockErrorReporter() + fillAssistAPIService = MockFillAssistAPIService() + stateService = MockStateService() + + subject = DefaultFillAssistRepository( + appSettingsStore: appSettingsStore, + configService: configService, + environmentService: environmentService, + errorReporter: errorReporter, + fillAssistAPIService: fillAssistAPIService, + stateService: stateService, + ) + } + + override func tearDown() { + super.tearDown() + + appSettingsStore = nil + configService = nil + environmentService = nil + errorReporter = nil + fillAssistAPIService = nil + stateService = nil + subject = nil + } + + // MARK: Tests - syncFillAssistRules + + /// `syncFillAssistRules()` makes no network calls when the feature flag is disabled. + func test_syncFillAssistRules_featureFlagDisabled() async { + configService.featureFlagsBool[.fillAssistTargetingRules] = false + stateService.activeAccount = .fixture() + + await subject.syncFillAssistRules() + + XCTAssertFalse(fillAssistAPIService.getManifestCalled) + } + + /// `syncFillAssistRules()` makes no network calls when the update interval has not elapsed. + func test_syncFillAssistRules_withinUpdateInterval() async { + configService.featureFlagsBool[.fillAssistTargetingRules] = true + stateService.activeAccount = .fixture() + appSettingsStore.fillAssistLastFetchTimestampByUserId["1"] = Date() + + await subject.syncFillAssistRules() + + XCTAssertFalse(fillAssistAPIService.getManifestCalled) + } + + /// `syncFillAssistRules()` skips download and updates the timestamp when cid and source URL are unchanged. + func test_syncFillAssistRules_cidUnchanged_updatesTimestampOnly() async throws { + configService.featureFlagsBool[.fillAssistTargetingRules] = true + stateService.activeAccount = .fixture() + let sourceUrl = environmentService.fillAssistRulesURL.absoluteString + + appSettingsStore.fillAssistCachedDataByUserId["1"] = FillAssistCachedData( + cid: "sha256:abc123", + rules: [:], + sourceUrl: sourceUrl, + ) + fillAssistAPIService.getManifestReturnValue = try makeManifest(cid: "sha256:abc123") + + await subject.syncFillAssistRules() + + XCTAssertTrue(fillAssistAPIService.getManifestCalled) + XCTAssertFalse(fillAssistAPIService.getFormsMapCalled) + XCTAssertNotNil(appSettingsStore.fillAssistLastFetchTimestampByUserId["1"]) + } + + /// `syncFillAssistRules()` downloads, parses, and caches rules when cid changes. + func test_syncFillAssistRules_cidChanged_downloadsAndCaches() async throws { + configService.featureFlagsBool[.fillAssistTargetingRules] = true + stateService.activeAccount = .fixture() + + fillAssistAPIService.getManifestReturnValue = try makeManifest(cid: "sha256:newcid") + fillAssistAPIService.getFormsMapReturnValue = try makeFormsMap() + + await subject.syncFillAssistRules() + + XCTAssertTrue(fillAssistAPIService.getManifestCalled) + XCTAssertTrue(fillAssistAPIService.getFormsMapCalled) + XCTAssertEqual(fillAssistAPIService.getFormsMapReceivedFilename, "forms.v1.json") + let cached = appSettingsStore.fillAssistCachedDataByUserId["1"] + XCTAssertNotNil(cached) + XCTAssertEqual(cached?.cid, "sha256:newcid") + XCTAssertNotNil(appSettingsStore.fillAssistLastFetchTimestampByUserId["1"]) + } + + /// `syncFillAssistRules()` skips storing when the schema major version is unsupported. + func test_syncFillAssistRules_unsupportedSchema_skipsCache() async throws { + configService.featureFlagsBool[.fillAssistTargetingRules] = true + stateService.activeAccount = .fixture() + + fillAssistAPIService.getManifestReturnValue = try makeManifest(cid: "sha256:newcid") + fillAssistAPIService.getFormsMapReturnValue = try makeFormsMap(schemaVersion: "2.0.0") + + await subject.syncFillAssistRules() + + XCTAssertNil(appSettingsStore.fillAssistCachedDataByUserId["1"]) + XCTAssertNotNil(appSettingsStore.fillAssistLastFetchTimestampByUserId["1"]) + } + + /// `syncFillAssistRules()` logs errors but does not rethrow them. + func test_syncFillAssistRules_failure_logsError() async { + configService.featureFlagsBool[.fillAssistTargetingRules] = true + stateService.activeAccount = .fixture() + fillAssistAPIService.getManifestThrowableError = URLError(.notConnectedToInternet) + + await subject.syncFillAssistRules() + + XCTAssertEqual(errorReporter.errors.count, 1) + XCTAssertNil(appSettingsStore.fillAssistCachedDataByUserId["1"]) + } + + // MARK: Tests - fillAssistRules(for:) + + /// `fillAssistRules(for:)` returns rules for a cached hostname. + func test_fillAssistRules_returnsRulesForHostname() async { + stateService.activeAccount = .fixture() + let hostRules = FillAssistHostRules(fields: ["username": []]) + appSettingsStore.fillAssistCachedDataByUserId["1"] = FillAssistCachedData( + cid: "sha256:abc", + rules: ["example.com": hostRules], + sourceUrl: "https://example.com", + ) + + let result = await subject.fillAssistRules(for: "example.com") + + XCTAssertEqual(result, hostRules) + } + + /// `fillAssistRules(for:)` returns nil for an unknown hostname. + func test_fillAssistRules_returnsNilForUnknownHostname() async { + stateService.activeAccount = .fixture() + appSettingsStore.fillAssistCachedDataByUserId["1"] = FillAssistCachedData( + cid: "sha256:abc", + rules: [:], + sourceUrl: "https://example.com", + ) + + let result = await subject.fillAssistRules(for: "unknown.com") + + XCTAssertNil(result) + } + + // MARK: Tests - clearFillAssistRules() + + /// `clearFillAssistRules()` removes cached data for the active account. + func test_clearFillAssistRules_removesCachedData() async throws { + stateService.activeAccount = .fixture() + appSettingsStore.fillAssistCachedDataByUserId["1"] = FillAssistCachedData( + cid: "sha256:abc", + rules: [:], + sourceUrl: "https://example.com", + ) + + try await subject.clearFillAssistRules() + + XCTAssertNil(appSettingsStore.fillAssistCachedDataByUserId["1"]) + } + + // MARK: Helpers + + private func makeManifest(cid: String) throws -> FillAssistManifestResponseModel { + let json = """ + { + "buildId": "v1", + "gitSha": "abc", + "maps": { "forms": { "v1": { + "cid": "\(cid)", + "filename": "forms.v1.json", + "schema": "forms.v1.schema.json" + }}}, + "timestamp": "2026-06-11T14:29:32.184Z" + } + """ + return try JSONDecoder.pascalOrSnakeCaseDecoder.decode( + FillAssistManifestResponseModel.self, + from: Data(json.utf8), + ) + } + + private func makeFormsMap(schemaVersion: String = "1.0.0") throws -> FormsMapResponseModel { + let json = """ + { + "schemaVersion": "\(schemaVersion)", + "hosts": { + "example.com": { + "forms": [{ "category": "account-login", "fields": { "username": ["input#user"] } }] + } + } + } + """ + return try JSONDecoder.pascalOrSnakeCaseDecoder.decode( + FormsMapResponseModel.self, + from: Data(json.utf8), + ) + } +} diff --git a/BitwardenShared/Core/Platform/Services/API/APIService.swift b/BitwardenShared/Core/Platform/Services/API/APIService.swift index 985aa8795e..028f38ea69 100644 --- a/BitwardenShared/Core/Platform/Services/API/APIService.swift +++ b/BitwardenShared/Core/Platform/Services/API/APIService.swift @@ -113,8 +113,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 construct + // a plain HTTPService with URLSession.shared which follows all redirects. + fillAssistService = HTTPService( baseURLGetter: { environmentService.fillAssistRulesURL }, + client: URLSession.shared, ) } From 5d2329662766806a0c56fae6fcb8aa79cc190088 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andre=CC=81=20Bispo?= Date: Tue, 23 Jun 2026 17:09:09 +0100 Subject: [PATCH 03/23] [PM-38443] fix: Add @MainActor to FillAssistRepositoryTests to fix test build --- .../Autofill/Repositories/FillAssistRepositoryTests.swift | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/BitwardenShared/Core/Autofill/Repositories/FillAssistRepositoryTests.swift b/BitwardenShared/Core/Autofill/Repositories/FillAssistRepositoryTests.swift index 5f2984394b..76e55e1862 100644 --- a/BitwardenShared/Core/Autofill/Repositories/FillAssistRepositoryTests.swift +++ b/BitwardenShared/Core/Autofill/Repositories/FillAssistRepositoryTests.swift @@ -7,6 +7,7 @@ import XCTest // MARK: - FillAssistRepositoryTests +@MainActor class FillAssistRepositoryTests: BitwardenTestCase { // MARK: Properties @@ -40,8 +41,8 @@ class FillAssistRepositoryTests: BitwardenTestCase { ) } - override func tearDown() { - super.tearDown() + override func tearDown() async throws { + try await super.tearDown() appSettingsStore = nil configService = nil From 333a8ccb2e80088e1d8c4633cdbd9ccd77008a5b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andre=CC=81=20Bispo?= Date: Thu, 25 Jun 2026 18:27:25 +0100 Subject: [PATCH 04/23] [PM-38443] fix: Remove unused Services typealias and add selector pooling tests to FillAssistRepository --- .../Repositories/FillAssistRepository.swift | 7 --- .../FillAssistRepositoryTests.swift | 56 +++++++++++++++++++ 2 files changed, 56 insertions(+), 7 deletions(-) diff --git a/BitwardenShared/Core/Autofill/Repositories/FillAssistRepository.swift b/BitwardenShared/Core/Autofill/Repositories/FillAssistRepository.swift index ce65cdb799..4cdf116aa6 100644 --- a/BitwardenShared/Core/Autofill/Repositories/FillAssistRepository.swift +++ b/BitwardenShared/Core/Autofill/Repositories/FillAssistRepository.swift @@ -27,13 +27,6 @@ protocol FillAssistRepository { // sourcery: AutoMockable /// The default implementation of `FillAssistRepository`. /// class DefaultFillAssistRepository: FillAssistRepository { - // MARK: Types - - typealias Services = HasConfigService - & HasEnvironmentService - & HasFillAssistAPIService - & HasStateService - // MARK: Private Properties /// The store for persisting fill-assist cached data. diff --git a/BitwardenShared/Core/Autofill/Repositories/FillAssistRepositoryTests.swift b/BitwardenShared/Core/Autofill/Repositories/FillAssistRepositoryTests.swift index 76e55e1862..6612c04e4d 100644 --- a/BitwardenShared/Core/Autofill/Repositories/FillAssistRepositoryTests.swift +++ b/BitwardenShared/Core/Autofill/Repositories/FillAssistRepositoryTests.swift @@ -129,6 +129,40 @@ class FillAssistRepositoryTests: BitwardenTestCase { XCTAssertNotNil(appSettingsStore.fillAssistLastFetchTimestampByUserId["1"]) } + /// `syncFillAssistRules()` parses CSS selectors from the forms map into `FillAssistFieldAttributes` + /// and stores them in the cached rules (selector pooling). + func test_syncFillAssistRules_parsesSelectorsIntoRules() async throws { + configService.featureFlagsBool[.fillAssistTargetingRules] = true + stateService.activeAccount = .fixture() + + fillAssistAPIService.getManifestReturnValue = try makeManifest(cid: "sha256:newcid") + fillAssistAPIService.getFormsMapReturnValue = try makeFormsMap() + + await subject.syncFillAssistRules() + + let hostRules = appSettingsStore.fillAssistCachedDataByUserId["1"]?.rules["example.com"] + let usernameAttrs = try XCTUnwrap(hostRules?.fields["username"]?.first) + XCTAssertEqual(usernameAttrs.id, "user") + XCTAssertEqual(usernameAttrs.tagName, "input") + } + + /// `syncFillAssistRules()` pools selectors from multiple pathname entries into a single + /// `FillAssistHostRules` for the same host. + func test_syncFillAssistRules_poolsSelectorsAcrossPathnames() async throws { + configService.featureFlagsBool[.fillAssistTargetingRules] = true + stateService.activeAccount = .fixture() + + fillAssistAPIService.getManifestReturnValue = try makeManifest(cid: "sha256:newcid") + fillAssistAPIService.getFormsMapReturnValue = try makeFormsMapWithPathnames() + + await subject.syncFillAssistRules() + + let hostRules = appSettingsStore.fillAssistCachedDataByUserId["1"]?.rules["example.com"] + // Both the top-level form and the pathname-specific form contribute username selectors. + let usernameAttrs = try XCTUnwrap(hostRules?.fields["username"]) + XCTAssertEqual(usernameAttrs.count, 2) + } + /// `syncFillAssistRules()` logs errors but does not rethrow them. func test_syncFillAssistRules_failure_logsError() async { configService.featureFlagsBool[.fillAssistTargetingRules] = true @@ -209,6 +243,28 @@ class FillAssistRepositoryTests: BitwardenTestCase { ) } + private func makeFormsMapWithPathnames() throws -> FormsMapResponseModel { + let json = """ + { + "schemaVersion": "1.0.0", + "hosts": { + "example.com": { + "forms": [{ "category": "account-login", "fields": { "username": ["input#user1"] } }], + "pathnames": { + "/login": { + "forms": [{ "category": "account-login", "fields": { "username": ["input#user2"] } }] + } + } + } + } + } + """ + return try JSONDecoder.pascalOrSnakeCaseDecoder.decode( + FormsMapResponseModel.self, + from: Data(json.utf8), + ) + } + private func makeFormsMap(schemaVersion: String = "1.0.0") throws -> FormsMapResponseModel { let json = """ { From 1decad0f622aea031b92d0ff3156ac15cd15f146 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andre=CC=81=20Bispo?= Date: Fri, 26 Jun 2026 18:02:24 +0100 Subject: [PATCH 05/23] [PM-38443] fix: Update FillAssistRepository to use Constants.FillAssist --- .../Core/Autofill/Repositories/FillAssistRepository.swift | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/BitwardenShared/Core/Autofill/Repositories/FillAssistRepository.swift b/BitwardenShared/Core/Autofill/Repositories/FillAssistRepository.swift index 4cdf116aa6..97d0e894ab 100644 --- a/BitwardenShared/Core/Autofill/Repositories/FillAssistRepository.swift +++ b/BitwardenShared/Core/Autofill/Repositories/FillAssistRepository.swift @@ -108,7 +108,7 @@ class DefaultFillAssistRepository: FillAssistRepository { let sourceUrl = environmentService.fillAssistRulesURL let userId = try await stateService.getActiveAccountId() let lastFetch = appSettingsStore.fillAssistLastFetchTimestamp(userId: userId) - if let lastFetch, Date().timeIntervalSince(lastFetch) < Constants.fillAssistUpdateInterval { + if let lastFetch, Date().timeIntervalSince(lastFetch) < Constants.FillAssist.updateInterval { return } @@ -116,7 +116,7 @@ class DefaultFillAssistRepository: FillAssistRepository { let manifest = try await fillAssistAPIService.getManifest() // 4. Resolve the non-deprecated entry for the current forms version. - guard let entry = manifest.maps["forms"]?[Constants.fillAssistFormsVersion], + guard let entry = manifest.maps["forms"]?[Constants.FillAssist.formsVersion], !entry.deprecated else { return } @@ -132,7 +132,7 @@ class DefaultFillAssistRepository: FillAssistRepository { // 7. Validate schema major version; update timestamp and skip if unsupported. let schemaMajor = formsMap.schemaVersion.split(separator: ".").first.map(String.init) ?? "" - guard schemaMajor == Constants.fillAssistExpectedSchemaMajor else { + guard schemaMajor == Constants.FillAssist.expectedSchemaMajor else { appSettingsStore.setFillAssistLastFetchTimestamp(Date(), userId: userId) return } From 993de63531fb663f8378f91c19f3131682531d7c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andre=CC=81=20Bispo?= Date: Fri, 26 Jun 2026 18:31:34 +0100 Subject: [PATCH 06/23] [PM-38443] fix: Rename FillAssistRepository methods, inject TimeProvider, move helpers to model layer --- .../Models/FormsMapResponseModel.swift | 19 +++++++++ .../Repositories/FillAssistRepository.swift | 42 ++++++++----------- .../Platform/Services/ServiceContainer.swift | 1 + .../Core/Vault/Services/SyncService.swift | 3 +- 4 files changed, 39 insertions(+), 26 deletions(-) diff --git a/BitwardenShared/Core/Autofill/Models/FormsMapResponseModel.swift b/BitwardenShared/Core/Autofill/Models/FormsMapResponseModel.swift index 1f1fd2ca6c..c69453f298 100644 --- a/BitwardenShared/Core/Autofill/Models/FormsMapResponseModel.swift +++ b/BitwardenShared/Core/Autofill/Models/FormsMapResponseModel.swift @@ -43,6 +43,13 @@ struct FormsMapHostEntry: Codable, Equatable { /// Pathname-specific form descriptions. Null-valued entries are excluded during decoding. @CompactDecodable var pathnames: [String: FormsMapPathnameEntry]? + + // MARK: Computed Properties + + /// All form descriptions for this host, pooling top-level and pathname-specific entries. + var allForms: [FormsMapContent] { + (forms ?? []) + (pathnames?.values.flatMap(\.forms) ?? []) + } } // MARK: - FormsMapPathnameEntry @@ -84,6 +91,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 index 97d0e894ab..87b8281e1b 100644 --- a/BitwardenShared/Core/Autofill/Repositories/FillAssistRepository.swift +++ b/BitwardenShared/Core/Autofill/Repositories/FillAssistRepository.swift @@ -8,18 +8,18 @@ import Foundation protocol FillAssistRepository { // sourcery: AutoMockable /// Fetches and caches fill-assist rules for the active account. /// - func syncFillAssistRules() async + 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 fillAssistRules(for hostname: String) async -> FillAssistHostRules? + func rules(for hostname: String) async -> FillAssistHostRules? /// Clears all cached fill-assist data for the active account. /// - func clearFillAssistRules() async throws + func clearRules() async throws } // MARK: - DefaultFillAssistRepository @@ -47,6 +47,9 @@ class DefaultFillAssistRepository: FillAssistRepository { /// 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`. @@ -58,6 +61,7 @@ class DefaultFillAssistRepository: FillAssistRepository { /// - 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, @@ -66,6 +70,7 @@ class DefaultFillAssistRepository: FillAssistRepository { errorReporter: ErrorReporter, fillAssistAPIService: FillAssistAPIService, stateService: StateService, + timeProvider: TimeProvider, ) { self.appSettingsStore = appSettingsStore self.configService = configService @@ -73,11 +78,12 @@ class DefaultFillAssistRepository: FillAssistRepository { self.errorReporter = errorReporter self.fillAssistAPIService = fillAssistAPIService self.stateService = stateService + self.timeProvider = timeProvider } // MARK: FillAssistRepository - func syncFillAssistRules() async { + func syncRules() async { do { try await performSync() } catch { @@ -86,12 +92,12 @@ class DefaultFillAssistRepository: FillAssistRepository { } } - func fillAssistRules(for hostname: String) async -> FillAssistHostRules? { + 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 clearFillAssistRules() async throws { + func clearRules() async throws { let userId = try await stateService.getActiveAccountId() appSettingsStore.setFillAssistCachedData(nil, userId: userId) } @@ -108,7 +114,7 @@ class DefaultFillAssistRepository: FillAssistRepository { let sourceUrl = environmentService.fillAssistRulesURL let userId = try await stateService.getActiveAccountId() let lastFetch = appSettingsStore.fillAssistLastFetchTimestamp(userId: userId) - if let lastFetch, Date().timeIntervalSince(lastFetch) < Constants.FillAssist.updateInterval { + if let lastFetch, timeProvider.presentTime.timeIntervalSince(lastFetch) < Constants.FillAssist.updateInterval { return } @@ -123,7 +129,7 @@ class DefaultFillAssistRepository: FillAssistRepository { // 5. If cid and source URL are unchanged, update timestamp and skip download. let cached = appSettingsStore.fillAssistCachedData(userId: userId) if cached?.cid == entry.cid, cached?.sourceUrl == sourceUrl.absoluteString { - appSettingsStore.setFillAssistLastFetchTimestamp(Date(), userId: userId) + appSettingsStore.setFillAssistLastFetchTimestamp(timeProvider.presentTime, userId: userId) return } @@ -133,7 +139,7 @@ class DefaultFillAssistRepository: FillAssistRepository { // 7. Validate schema major version; update timestamp and skip if unsupported. let schemaMajor = formsMap.schemaVersion.split(separator: ".").first.map(String.init) ?? "" guard schemaMajor == Constants.FillAssist.expectedSchemaMajor else { - appSettingsStore.setFillAssistLastFetchTimestamp(Date(), userId: userId) + appSettingsStore.setFillAssistLastFetchTimestamp(timeProvider.presentTime, userId: userId) return } @@ -141,7 +147,7 @@ class DefaultFillAssistRepository: FillAssistRepository { let rules = buildRules(from: formsMap) let data = FillAssistCachedData(cid: entry.cid, rules: rules, sourceUrl: sourceUrl.absoluteString) appSettingsStore.setFillAssistCachedData(data, userId: userId) - appSettingsStore.setFillAssistLastFetchTimestamp(Date(), userId: userId) + appSettingsStore.setFillAssistLastFetchTimestamp(timeProvider.presentTime, userId: userId) } /// Builds a `[hostname: FillAssistHostRules]` dictionary by pooling all non-null field @@ -151,11 +157,10 @@ class DefaultFillAssistRepository: FillAssistRepository { var result = [String: FillAssistHostRules]() for (hostname, hostEntry) in formsMap.hosts { var pooled = [String: [FillAssistFieldAttributes]]() - let allForms = (hostEntry.forms ?? []) - + (hostEntry.pathnames?.values.compactMap(\.self).flatMap(\.forms) ?? []) + let allForms = hostEntry.allForms for form in allForms { for (fieldKey, selectors) in form.fields { - let attrs = selectors.flatMap { parseSelector($0) } + let attrs = selectors.flatMap(\.attributes) pooled[fieldKey, default: []].append(contentsOf: attrs) } } @@ -165,15 +170,4 @@ class DefaultFillAssistRepository: FillAssistRepository { } return result } - - /// Converts a `FormsMapSelector` into zero or more `FillAssistFieldAttributes`. - /// - private func parseSelector(_ selector: FormsMapSelector) -> [FillAssistFieldAttributes] { - switch selector { - case let .single(css): - FillAssistSelectorParser.parse(css).map { [$0] } ?? [] - case let .sequence(parts): - parts.compactMap { FillAssistSelectorParser.parse($0) } - } - } } diff --git a/BitwardenShared/Core/Platform/Services/ServiceContainer.swift b/BitwardenShared/Core/Platform/Services/ServiceContainer.swift index 1a13cdf610..8ad4b70987 100644 --- a/BitwardenShared/Core/Platform/Services/ServiceContainer.swift +++ b/BitwardenShared/Core/Platform/Services/ServiceContainer.swift @@ -779,6 +779,7 @@ public class ServiceContainer: Services { // swiftlint:disable:this type_body_le errorReporter: errorReporter, fillAssistAPIService: apiService, stateService: stateService, + timeProvider: timeProvider, ) let syncService = DefaultSyncService( diff --git a/BitwardenShared/Core/Vault/Services/SyncService.swift b/BitwardenShared/Core/Vault/Services/SyncService.swift index 5177a7d298..4df2d3c5eb 100644 --- a/BitwardenShared/Core/Vault/Services/SyncService.swift +++ b/BitwardenShared/Core/Vault/Services/SyncService.swift @@ -479,8 +479,7 @@ extension DefaultSyncService { ) } - // Sync fill-assist rules alongside vault data. Failures must not block vault sync. - await fillAssistRepository.syncFillAssistRules() + await fillAssistRepository.syncRules() await delegate?.onFetchSyncSucceeded(userId: userId) } From 10bd65aae2469229f454f7a1e941070f91b5a5e7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andre=CC=81=20Bispo?= Date: Fri, 26 Jun 2026 18:32:39 +0100 Subject: [PATCH 07/23] [PM-38443] test: Convert FillAssistRepositoryTests to Swift Testing and add coverage for edge cases --- .../FillAssistRepositoryTests.swift | 346 +++++++++++------- .../Vault/Services/SyncServiceTests.swift | 10 + 2 files changed, 217 insertions(+), 139 deletions(-) diff --git a/BitwardenShared/Core/Autofill/Repositories/FillAssistRepositoryTests.swift b/BitwardenShared/Core/Autofill/Repositories/FillAssistRepositoryTests.swift index 6612c04e4d..688406a7a7 100644 --- a/BitwardenShared/Core/Autofill/Repositories/FillAssistRepositoryTests.swift +++ b/BitwardenShared/Core/Autofill/Repositories/FillAssistRepositoryTests.swift @@ -1,35 +1,36 @@ import BitwardenKit import BitwardenKitMocks -import XCTest +import Foundation +import Testing @testable import BitwardenShared @testable import BitwardenSharedMocks // MARK: - FillAssistRepositoryTests -@MainActor -class FillAssistRepositoryTests: BitwardenTestCase { +struct FillAssistRepositoryTests { // MARK: Properties - var appSettingsStore: MockAppSettingsStore! - var configService: MockConfigService! - var environmentService: MockEnvironmentService! - var errorReporter: MockErrorReporter! - var fillAssistAPIService: MockFillAssistAPIService! - var stateService: MockStateService! - var subject: DefaultFillAssistRepository! + 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: Setup & Teardown - - override func setUp() { - super.setUp() + // MARK: Initialization + init() { appSettingsStore = MockAppSettingsStore() configService = MockConfigService() environmentService = MockEnvironmentService() errorReporter = MockErrorReporter() fillAssistAPIService = MockFillAssistAPIService() stateService = MockStateService() + stateService.activeAccount = .fixture() + timeProvider = MockTimeProvider(.currentTime) subject = DefaultFillAssistRepository( appSettingsStore: appSettingsStore, @@ -38,48 +39,37 @@ class FillAssistRepositoryTests: BitwardenTestCase { errorReporter: errorReporter, fillAssistAPIService: fillAssistAPIService, stateService: stateService, + timeProvider: timeProvider, ) } - override func tearDown() async throws { - try await super.tearDown() - - appSettingsStore = nil - configService = nil - environmentService = nil - errorReporter = nil - fillAssistAPIService = nil - stateService = nil - subject = nil - } - - // MARK: Tests - syncFillAssistRules + // MARK: Tests - syncRules - /// `syncFillAssistRules()` makes no network calls when the feature flag is disabled. - func test_syncFillAssistRules_featureFlagDisabled() async { + /// `syncRules()` makes no network calls when the feature flag is disabled. + @Test + func syncRules_featureFlagDisabled() async { configService.featureFlagsBool[.fillAssistTargetingRules] = false - stateService.activeAccount = .fixture() - await subject.syncFillAssistRules() + await subject.syncRules() - XCTAssertFalse(fillAssistAPIService.getManifestCalled) + #expect(!fillAssistAPIService.getManifestCalled) } - /// `syncFillAssistRules()` makes no network calls when the update interval has not elapsed. - func test_syncFillAssistRules_withinUpdateInterval() async { + /// `syncRules()` makes no network calls when the update interval has not elapsed. + @Test + func syncRules_withinUpdateInterval() async { configService.featureFlagsBool[.fillAssistTargetingRules] = true - stateService.activeAccount = .fixture() - appSettingsStore.fillAssistLastFetchTimestampByUserId["1"] = Date() + appSettingsStore.fillAssistLastFetchTimestampByUserId["1"] = timeProvider.presentTime - await subject.syncFillAssistRules() + await subject.syncRules() - XCTAssertFalse(fillAssistAPIService.getManifestCalled) + #expect(!fillAssistAPIService.getManifestCalled) } - /// `syncFillAssistRules()` skips download and updates the timestamp when cid and source URL are unchanged. - func test_syncFillAssistRules_cidUnchanged_updatesTimestampOnly() async throws { + /// `syncRules()` skips download and updates the timestamp when cid and source URL are unchanged. + @Test + func syncRules_cidUnchanged_updatesTimestampOnly() async { configService.featureFlagsBool[.fillAssistTargetingRules] = true - stateService.activeAccount = .fixture() let sourceUrl = environmentService.fillAssistRulesURL.absoluteString appSettingsStore.fillAssistCachedDataByUserId["1"] = FillAssistCachedData( @@ -87,99 +77,146 @@ class FillAssistRepositoryTests: BitwardenTestCase { rules: [:], sourceUrl: sourceUrl, ) - fillAssistAPIService.getManifestReturnValue = try makeManifest(cid: "sha256:abc123") + fillAssistAPIService.getManifestReturnValue = makeManifest(cid: "sha256:abc123") - await subject.syncFillAssistRules() + await subject.syncRules() - XCTAssertTrue(fillAssistAPIService.getManifestCalled) - XCTAssertFalse(fillAssistAPIService.getFormsMapCalled) - XCTAssertNotNil(appSettingsStore.fillAssistLastFetchTimestampByUserId["1"]) + #expect(fillAssistAPIService.getManifestCalled) + #expect(!fillAssistAPIService.getFormsMapCalled) + #expect(appSettingsStore.fillAssistLastFetchTimestampByUserId["1"] != nil) } - /// `syncFillAssistRules()` downloads, parses, and caches rules when cid changes. - func test_syncFillAssistRules_cidChanged_downloadsAndCaches() async throws { + /// `syncRules()` downloads, parses, and caches rules when cid changes. + @Test + func syncRules_cidChanged_downloadsAndCaches() async throws { configService.featureFlagsBool[.fillAssistTargetingRules] = true - stateService.activeAccount = .fixture() - fillAssistAPIService.getManifestReturnValue = try makeManifest(cid: "sha256:newcid") + fillAssistAPIService.getManifestReturnValue = makeManifest(cid: "sha256:newcid") fillAssistAPIService.getFormsMapReturnValue = try makeFormsMap() - await subject.syncFillAssistRules() + await subject.syncRules() - XCTAssertTrue(fillAssistAPIService.getManifestCalled) - XCTAssertTrue(fillAssistAPIService.getFormsMapCalled) - XCTAssertEqual(fillAssistAPIService.getFormsMapReceivedFilename, "forms.v1.json") + #expect(fillAssistAPIService.getManifestCalled) + #expect(fillAssistAPIService.getFormsMapCalled) + #expect(fillAssistAPIService.getFormsMapReceivedFilename == "forms.v1.json") let cached = appSettingsStore.fillAssistCachedDataByUserId["1"] - XCTAssertNotNil(cached) - XCTAssertEqual(cached?.cid, "sha256:newcid") - XCTAssertNotNil(appSettingsStore.fillAssistLastFetchTimestampByUserId["1"]) + #expect(cached != nil) + #expect(cached?.cid == "sha256:newcid") + #expect(appSettingsStore.fillAssistLastFetchTimestampByUserId["1"] != nil) } - /// `syncFillAssistRules()` skips storing when the schema major version is unsupported. - func test_syncFillAssistRules_unsupportedSchema_skipsCache() async throws { + /// `syncRules()` skips storing when the schema major version is unsupported. + @Test + func syncRules_unsupportedSchema_skipsCache() async throws { configService.featureFlagsBool[.fillAssistTargetingRules] = true - stateService.activeAccount = .fixture() - fillAssistAPIService.getManifestReturnValue = try makeManifest(cid: "sha256:newcid") + fillAssistAPIService.getManifestReturnValue = makeManifest(cid: "sha256:newcid") fillAssistAPIService.getFormsMapReturnValue = try makeFormsMap(schemaVersion: "2.0.0") - await subject.syncFillAssistRules() + await subject.syncRules() - XCTAssertNil(appSettingsStore.fillAssistCachedDataByUserId["1"]) - XCTAssertNotNil(appSettingsStore.fillAssistLastFetchTimestampByUserId["1"]) + #expect(appSettingsStore.fillAssistCachedDataByUserId["1"] == nil) + #expect(appSettingsStore.fillAssistLastFetchTimestampByUserId["1"] != nil) } - /// `syncFillAssistRules()` parses CSS selectors from the forms map into `FillAssistFieldAttributes` - /// and stores them in the cached rules (selector pooling). - func test_syncFillAssistRules_parsesSelectorsIntoRules() async throws { + /// `syncRules()` parses CSS selectors into `FillAssistFieldAttributes` (selector pooling). + @Test + func syncRules_parsesSelectorsIntoRules() async throws { configService.featureFlagsBool[.fillAssistTargetingRules] = true - stateService.activeAccount = .fixture() - fillAssistAPIService.getManifestReturnValue = try makeManifest(cid: "sha256:newcid") + fillAssistAPIService.getManifestReturnValue = makeManifest(cid: "sha256:newcid") fillAssistAPIService.getFormsMapReturnValue = try makeFormsMap() - await subject.syncFillAssistRules() + await subject.syncRules() let hostRules = appSettingsStore.fillAssistCachedDataByUserId["1"]?.rules["example.com"] - let usernameAttrs = try XCTUnwrap(hostRules?.fields["username"]?.first) - XCTAssertEqual(usernameAttrs.id, "user") - XCTAssertEqual(usernameAttrs.tagName, "input") + let usernameAttrs = try #require(hostRules?.fields["username"]?.first) + #expect(usernameAttrs.id == "user") + #expect(usernameAttrs.tagName == "input") } - /// `syncFillAssistRules()` pools selectors from multiple pathname entries into a single - /// `FillAssistHostRules` for the same host. - func test_syncFillAssistRules_poolsSelectorsAcrossPathnames() async throws { + /// `syncRules()` pools selectors from multiple pathname entries into a single host entry. + @Test + func syncRules_poolsSelectorsAcrossPathnames() async throws { configService.featureFlagsBool[.fillAssistTargetingRules] = true - stateService.activeAccount = .fixture() - fillAssistAPIService.getManifestReturnValue = try makeManifest(cid: "sha256:newcid") + fillAssistAPIService.getManifestReturnValue = makeManifest(cid: "sha256:newcid") fillAssistAPIService.getFormsMapReturnValue = try makeFormsMapWithPathnames() - await subject.syncFillAssistRules() + await subject.syncRules() - let hostRules = appSettingsStore.fillAssistCachedDataByUserId["1"]?.rules["example.com"] - // Both the top-level form and the pathname-specific form contribute username selectors. - let usernameAttrs = try XCTUnwrap(hostRules?.fields["username"]) - XCTAssertEqual(usernameAttrs.count, 2) + let usernameAttrs = appSettingsStore.fillAssistCachedDataByUserId["1"]? + .rules["example.com"]?.fields["username"] + let count = try #require(usernameAttrs).count + #expect(count == 2) } - /// `syncFillAssistRules()` logs errors but does not rethrow them. - func test_syncFillAssistRules_failure_logsError() async { + /// `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 = try 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 = try 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 = try 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 - stateService.activeAccount = .fixture() fillAssistAPIService.getManifestThrowableError = URLError(.notConnectedToInternet) - await subject.syncFillAssistRules() + await subject.syncRules() - XCTAssertEqual(errorReporter.errors.count, 1) - XCTAssertNil(appSettingsStore.fillAssistCachedDataByUserId["1"]) + #expect(errorReporter.errors.count == 1) + #expect(appSettingsStore.fillAssistCachedDataByUserId["1"] == nil) } - // MARK: Tests - fillAssistRules(for:) + // MARK: Tests - rules(for:) - /// `fillAssistRules(for:)` returns rules for a cached hostname. - func test_fillAssistRules_returnsRulesForHostname() async { - stateService.activeAccount = .fixture() + /// `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", @@ -187,77 +224,105 @@ class FillAssistRepositoryTests: BitwardenTestCase { sourceUrl: "https://example.com", ) - let result = await subject.fillAssistRules(for: "example.com") + let result = await subject.rules(for: "example.com") - XCTAssertEqual(result, hostRules) + #expect(result == hostRules) } - /// `fillAssistRules(for:)` returns nil for an unknown hostname. - func test_fillAssistRules_returnsNilForUnknownHostname() async { - stateService.activeAccount = .fixture() + /// `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.fillAssistRules(for: "unknown.com") + let result = await subject.rules(for: "unknown.com") - XCTAssertNil(result) + #expect(result == nil) } - // MARK: Tests - clearFillAssistRules() + // MARK: Tests - clearRules() - /// `clearFillAssistRules()` removes cached data for the active account. - func test_clearFillAssistRules_removesCachedData() async throws { - stateService.activeAccount = .fixture() + /// `clearRules()` removes cached data for the active account. + @Test + func clearRules_removesCachedData() async throws { appSettingsStore.fillAssistCachedDataByUserId["1"] = FillAssistCachedData( cid: "sha256:abc", rules: [:], sourceUrl: "https://example.com", ) - try await subject.clearFillAssistRules() + try await subject.clearRules() + + #expect(appSettingsStore.fillAssistCachedDataByUserId["1"] == 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") + } - XCTAssertNil(appSettingsStore.fillAssistCachedDataByUserId["1"]) + /// `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 - private func makeManifest(cid: String) throws -> FillAssistManifestResponseModel { - let json = """ - { - "buildId": "v1", - "gitSha": "abc", - "maps": { "forms": { "v1": { - "cid": "\(cid)", - "filename": "forms.v1.json", - "schema": "forms.v1.schema.json" - }}}, - "timestamp": "2026-06-11T14:29:32.184Z" - } - """ - return try JSONDecoder.pascalOrSnakeCaseDecoder.decode( - FillAssistManifestResponseModel.self, - from: Data(json.utf8), + private 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), ) } - private func makeFormsMapWithPathnames() throws -> FormsMapResponseModel { - let json = """ - { - "schemaVersion": "1.0.0", - "hosts": { - "example.com": { - "forms": [{ "category": "account-login", "fields": { "username": ["input#user1"] } }], - "pathnames": { - "/login": { - "forms": [{ "category": "account-login", "fields": { "username": ["input#user2"] } }] - } - } - } - } + private func makeFormsMap( + schemaVersion: String = "1.0.0", + usernameSelector: String = "input#user", + hosts: [String: String]? = nil, + ) throws -> FormsMapResponseModel { + let hostsJSON: String + if let hosts { + let entries = hosts.map { hostname, selector in + """ + "\(hostname)": {"forms": [{"category": "account-login", "fields": {"username": ["\(selector)"]}}]} + """ + }.joined(separator: ",") + hostsJSON = "{\(entries)}" + } else { + hostsJSON = """ + {"example.com": {"forms": [{"category": "account-login", "fields": {"username": ["\(usernameSelector)"]}}]}} + """ } + let json = """ + {"schemaVersion": "\(schemaVersion)", "hosts": \(hostsJSON)} """ return try JSONDecoder.pascalOrSnakeCaseDecoder.decode( FormsMapResponseModel.self, @@ -265,13 +330,16 @@ class FillAssistRepositoryTests: BitwardenTestCase { ) } - private func makeFormsMap(schemaVersion: String = "1.0.0") throws -> FormsMapResponseModel { + private func makeFormsMapWithPathnames() throws -> FormsMapResponseModel { let json = """ { - "schemaVersion": "\(schemaVersion)", + "schemaVersion": "1.0.0", "hosts": { "example.com": { - "forms": [{ "category": "account-login", "fields": { "username": ["input#user"] } }] + "forms": [{"category": "account-login", "fields": {"username": ["input#user1"]}}], + "pathnames": { + "/login": {"forms": [{"category": "account-login", "fields": {"username": ["input#user2"]}}]} + } } } } diff --git a/BitwardenShared/Core/Vault/Services/SyncServiceTests.swift b/BitwardenShared/Core/Vault/Services/SyncServiceTests.swift index 6534e6b10a..386e80d4df 100644 --- a/BitwardenShared/Core/Vault/Services/SyncServiceTests.swift +++ b/BitwardenShared/Core/Vault/Services/SyncServiceTests.swift @@ -371,6 +371,16 @@ class SyncServiceTests: BitwardenTestCase { // swiftlint:disable:this type_body_ ) } + /// `fetchSync()` calls `syncRules()` on the fill-assist repository alongside 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()` with `forceSync: true` performs the sync API request regardless of the /// account revision or sync interval. func test_fetchSync_failedParse() async throws { From c9fe9f6c8d510f69968f98e716f1ef987b21285f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andre=CC=81=20Bispo?= Date: Mon, 29 Jun 2026 14:32:06 +0100 Subject: [PATCH 08/23] [PM-38443] fix: Add @MainActor and extract helpers to fix FillAssistRepositoryTests - Add @MainActor so featureFlagsBool mutations compile in Swift Testing - Move helper methods to a private extension to satisfy type_body_length --- .../Autofill/Repositories/FillAssistRepositoryTests.swift | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/BitwardenShared/Core/Autofill/Repositories/FillAssistRepositoryTests.swift b/BitwardenShared/Core/Autofill/Repositories/FillAssistRepositoryTests.swift index 688406a7a7..3ea5f3de90 100644 --- a/BitwardenShared/Core/Autofill/Repositories/FillAssistRepositoryTests.swift +++ b/BitwardenShared/Core/Autofill/Repositories/FillAssistRepositoryTests.swift @@ -8,6 +8,7 @@ import Testing // MARK: - FillAssistRepositoryTests +@MainActor struct FillAssistRepositoryTests { // MARK: Properties @@ -285,10 +286,12 @@ struct FillAssistRepositoryTests { let attrs = FormsMapSelector.single("div >>> input#user").attributes #expect(attrs.isEmpty) } +} - // MARK: Helpers +// MARK: - Helpers - private func makeManifest(cid: String) -> FillAssistManifestResponseModel { +private extension FillAssistRepositoryTests { + func makeManifest(cid: String) -> FillAssistManifestResponseModel { let entry = FillAssistManifestEntryModel( cid: cid, deprecated: false, From 4d9842c479a07fd46d6af7eb95716a24044d3e33 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andre=CC=81=20Bispo?= Date: Mon, 29 Jun 2026 15:40:44 +0100 Subject: [PATCH 09/23] [PM-38443] fix: Filter empty field arrays from pooled rules and inject fillAssistClient for tests - Filter pooled entries with empty attribute arrays before writing host rules, so hosts with only unsupported selectors (e.g. shadow DOM) are correctly excluded - Add fillAssistClient parameter to APIService.init (defaults to URLSession.shared) so FillAssistAPIServiceTests can inject MockHTTPClient for download calls --- .../Core/Autofill/Repositories/FillAssistRepository.swift | 1 + .../Core/Platform/Services/API/APIService.swift | 7 ++++--- .../Services/API/TestHelpers/APIService+Mocks.swift | 2 ++ 3 files changed, 7 insertions(+), 3 deletions(-) diff --git a/BitwardenShared/Core/Autofill/Repositories/FillAssistRepository.swift b/BitwardenShared/Core/Autofill/Repositories/FillAssistRepository.swift index 87b8281e1b..7f4fa878dc 100644 --- a/BitwardenShared/Core/Autofill/Repositories/FillAssistRepository.swift +++ b/BitwardenShared/Core/Autofill/Repositories/FillAssistRepository.swift @@ -164,6 +164,7 @@ class DefaultFillAssistRepository: FillAssistRepository { pooled[fieldKey, default: []].append(contentsOf: attrs) } } + pooled = pooled.filter { !$0.value.isEmpty } if !pooled.isEmpty { result[hostname] = FillAssistHostRules(fields: pooled) } diff --git a/BitwardenShared/Core/Platform/Services/API/APIService.swift b/BitwardenShared/Core/Platform/Services/API/APIService.swift index 028f38ea69..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, @@ -114,11 +115,11 @@ class APIService { baseURLGetter: { environmentService.identityURL }, ) // Fill-Assist requests target an external CDN (GitHub Releases) that uses multi-hop 302 - // redirects. CertificateHTTPClient deliberately blocks 302s for SSO, so we construct - // a plain HTTPService with URLSession.shared which follows all redirects. + // 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: URLSession.shared, + 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, From 7738dc2d9af9ab6748527755e189c12d89e59ef8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andre=CC=81=20Bispo?= Date: Mon, 29 Jun 2026 23:23:43 +0100 Subject: [PATCH 10/23] [PM-38443] refactor: Replace JSON-string test helpers with direct struct initialization Add memberwise inits to FormsMapResponseModel and FormsMapHostEntry so test helpers can construct fixtures without JSON string construction. --- .../Models/FormsMapResponseModel.swift | 16 +++- .../FillAssistRepositoryTests.swift | 84 +++++++++---------- 2 files changed, 55 insertions(+), 45 deletions(-) diff --git a/BitwardenShared/Core/Autofill/Models/FormsMapResponseModel.swift b/BitwardenShared/Core/Autofill/Models/FormsMapResponseModel.swift index c69453f298..7054ac8278 100644 --- a/BitwardenShared/Core/Autofill/Models/FormsMapResponseModel.swift +++ b/BitwardenShared/Core/Autofill/Models/FormsMapResponseModel.swift @@ -21,6 +21,13 @@ struct FormsMapResponseModel: Equatable, JSONResponse { /// The schema version of the map. let schemaVersion: String + // MARK: Initialization + + init(hosts: [String: FormsMapHostEntry], schemaVersion: String) { + self.hosts = hosts + self.schemaVersion = schemaVersion + } + // MARK: Codable init(from decoder: any Decoder) throws { @@ -44,12 +51,17 @@ struct FormsMapHostEntry: Codable, Equatable { /// Pathname-specific form descriptions. Null-valued entries are excluded during decoding. @CompactDecodable var pathnames: [String: FormsMapPathnameEntry]? - // MARK: Computed Properties - /// All form descriptions for this host, pooling top-level and pathname-specific entries. var allForms: [FormsMapContent] { (forms ?? []) + (pathnames?.values.flatMap(\.forms) ?? []) } + + // MARK: Initialization + + init(forms: [FormsMapContent]?, pathnames: [String: FormsMapPathnameEntry]? = nil) { + self.forms = forms + _pathnames = CompactDecodable(wrappedValue: pathnames) + } } // MARK: - FormsMapPathnameEntry diff --git a/BitwardenShared/Core/Autofill/Repositories/FillAssistRepositoryTests.swift b/BitwardenShared/Core/Autofill/Repositories/FillAssistRepositoryTests.swift index 3ea5f3de90..a887b79b8f 100644 --- a/BitwardenShared/Core/Autofill/Repositories/FillAssistRepositoryTests.swift +++ b/BitwardenShared/Core/Autofill/Repositories/FillAssistRepositoryTests.swift @@ -93,7 +93,7 @@ struct FillAssistRepositoryTests { configService.featureFlagsBool[.fillAssistTargetingRules] = true fillAssistAPIService.getManifestReturnValue = makeManifest(cid: "sha256:newcid") - fillAssistAPIService.getFormsMapReturnValue = try makeFormsMap() + fillAssistAPIService.getFormsMapReturnValue = makeFormsMap() await subject.syncRules() @@ -112,7 +112,7 @@ struct FillAssistRepositoryTests { configService.featureFlagsBool[.fillAssistTargetingRules] = true fillAssistAPIService.getManifestReturnValue = makeManifest(cid: "sha256:newcid") - fillAssistAPIService.getFormsMapReturnValue = try makeFormsMap(schemaVersion: "2.0.0") + fillAssistAPIService.getFormsMapReturnValue = makeFormsMap(schemaVersion: "2.0.0") await subject.syncRules() @@ -126,7 +126,7 @@ struct FillAssistRepositoryTests { configService.featureFlagsBool[.fillAssistTargetingRules] = true fillAssistAPIService.getManifestReturnValue = makeManifest(cid: "sha256:newcid") - fillAssistAPIService.getFormsMapReturnValue = try makeFormsMap() + fillAssistAPIService.getFormsMapReturnValue = makeFormsMap() await subject.syncRules() @@ -142,7 +142,7 @@ struct FillAssistRepositoryTests { configService.featureFlagsBool[.fillAssistTargetingRules] = true fillAssistAPIService.getManifestReturnValue = makeManifest(cid: "sha256:newcid") - fillAssistAPIService.getFormsMapReturnValue = try makeFormsMapWithPathnames() + fillAssistAPIService.getFormsMapReturnValue = makeFormsMapWithPathnames() await subject.syncRules() @@ -159,7 +159,7 @@ struct FillAssistRepositoryTests { fillAssistAPIService.getManifestReturnValue = makeManifest(cid: "sha256:newcid") // Shadow DOM selectors are excluded by the parser → pooled stays empty. - fillAssistAPIService.getFormsMapReturnValue = try makeFormsMap( + fillAssistAPIService.getFormsMapReturnValue = makeFormsMap( usernameSelector: "div#app >>> input#user", ) @@ -175,7 +175,7 @@ struct FillAssistRepositoryTests { configService.featureFlagsBool[.fillAssistTargetingRules] = true fillAssistAPIService.getManifestReturnValue = makeManifest(cid: "sha256:newcid") - fillAssistAPIService.getFormsMapReturnValue = try makeFormsMap(hosts: [:]) + fillAssistAPIService.getFormsMapReturnValue = makeFormsMap(hosts: [:]) await subject.syncRules() @@ -189,7 +189,7 @@ struct FillAssistRepositoryTests { configService.featureFlagsBool[.fillAssistTargetingRules] = true fillAssistAPIService.getManifestReturnValue = makeManifest(cid: "sha256:newcid") - fillAssistAPIService.getFormsMapReturnValue = try makeFormsMap( + fillAssistAPIService.getFormsMapReturnValue = makeFormsMap( hosts: ["example.com": "input#user1", "other.com": "input#user2"], ) @@ -310,46 +310,44 @@ private extension FillAssistRepositoryTests { schemaVersion: String = "1.0.0", usernameSelector: String = "input#user", hosts: [String: String]? = nil, - ) throws -> FormsMapResponseModel { - let hostsJSON: String - if let hosts { - let entries = hosts.map { hostname, selector in - """ - "\(hostname)": {"forms": [{"category": "account-login", "fields": {"username": ["\(selector)"]}}]} - """ - }.joined(separator: ",") - hostsJSON = "{\(entries)}" + ) -> 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 { - hostsJSON = """ - {"example.com": {"forms": [{"category": "account-login", "fields": {"username": ["\(usernameSelector)"]}}]}} - """ + ["example.com": FormsMapHostEntry(forms: [content(usernameSelector)])] } - let json = """ - {"schemaVersion": "\(schemaVersion)", "hosts": \(hostsJSON)} - """ - return try JSONDecoder.pascalOrSnakeCaseDecoder.decode( - FormsMapResponseModel.self, - from: Data(json.utf8), - ) + return FormsMapResponseModel(hosts: resolvedHosts, schemaVersion: schemaVersion) } - private func makeFormsMapWithPathnames() throws -> FormsMapResponseModel { - let json = """ - { - "schemaVersion": "1.0.0", - "hosts": { - "example.com": { - "forms": [{"category": "account-login", "fields": {"username": ["input#user1"]}}], - "pathnames": { - "/login": {"forms": [{"category": "account-login", "fields": {"username": ["input#user2"]}}]} - } - } - } - } - """ - return try JSONDecoder.pascalOrSnakeCaseDecoder.decode( - FormsMapResponseModel.self, - from: Data(json.utf8), + private 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", ) } } From 67d82cf3f4855cdfc950caeae35bc1ae6bf0a1ae Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andre=CC=81=20Bispo?= Date: Wed, 1 Jul 2026 22:14:11 +0100 Subject: [PATCH 11/23] [PM-38443] fix: Add missing DocC comments to FormsMapResponseModel inits and test helpers --- .../Core/Autofill/Models/FormsMapResponseModel.swift | 12 ++++++++++++ .../Repositories/FillAssistRepositoryTests.swift | 10 ++++++++++ 2 files changed, 22 insertions(+) diff --git a/BitwardenShared/Core/Autofill/Models/FormsMapResponseModel.swift b/BitwardenShared/Core/Autofill/Models/FormsMapResponseModel.swift index 7054ac8278..03f1891362 100644 --- a/BitwardenShared/Core/Autofill/Models/FormsMapResponseModel.swift +++ b/BitwardenShared/Core/Autofill/Models/FormsMapResponseModel.swift @@ -23,6 +23,12 @@ struct FormsMapResponseModel: Equatable, JSONResponse { // 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 @@ -58,6 +64,12 @@ struct FormsMapHostEntry: Codable, Equatable { // 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) diff --git a/BitwardenShared/Core/Autofill/Repositories/FillAssistRepositoryTests.swift b/BitwardenShared/Core/Autofill/Repositories/FillAssistRepositoryTests.swift index a887b79b8f..de554f9fad 100644 --- a/BitwardenShared/Core/Autofill/Repositories/FillAssistRepositoryTests.swift +++ b/BitwardenShared/Core/Autofill/Repositories/FillAssistRepositoryTests.swift @@ -291,6 +291,7 @@ struct FillAssistRepositoryTests { // MARK: - Helpers private extension FillAssistRepositoryTests { + /// Returns a `FillAssistManifestResponseModel` fixture with the given content ID. func makeManifest(cid: String) -> FillAssistManifestResponseModel { let entry = FillAssistManifestEntryModel( cid: cid, @@ -306,6 +307,13 @@ private extension FillAssistRepositoryTests { ) } + /// 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. + /// private func makeFormsMap( schemaVersion: String = "1.0.0", usernameSelector: String = "input#user", @@ -327,6 +335,8 @@ private extension FillAssistRepositoryTests { 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. private func makeFormsMapWithPathnames() -> FormsMapResponseModel { let loginContent = FormsMapContent( category: "account-login", From 6e9e6e721d25bf7db08c9264f38c5d077f7ce2a8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A9=20Bispo?= Date: Thu, 2 Jul 2026 11:45:08 +0100 Subject: [PATCH 12/23] [PM-38444] feat: Use FillAssist cached rules to guide DOM field targeting in Action Extension (#2830) --- .../en.lproj/Localizable.strings | 2 + .../Repositories/FillAssistRepository.swift | 11 +- .../FillAssistRepositoryTests.swift | 33 +++ .../Core/Platform/Services/StateService.swift | 41 +++ .../Platform/Services/StateServiceTests.swift | 20 ++ .../Services/Stores/AppSettingsStore.swift | 25 ++ .../Stores/AppSettingsStoreTests.swift | 17 ++ .../TestHelpers/MockAppSettingsStore.swift | 9 + .../TestHelpers/MockStateService.swift | 15 ++ .../Utilities/ExternalLinksConstants.swift | 3 + .../MockAppExtensionDelegate.swift | 2 +- .../Settings/AutoFill/AutoFillAction.swift | 3 + .../Settings/AutoFill/AutoFillProcessor.swift | 31 +++ .../AutoFill/AutoFillProcessorTests.swift | 52 ++++ .../Settings/AutoFill/AutoFillState.swift | 6 + .../Settings/AutoFill/AutoFillView.swift | 24 ++ .../Settings/SettingsCoordinator.swift | 1 + .../UI/Vault/Helpers/AutofillHelper.swift | 48 +++- .../Vault/Helpers/AutofillHelperTests.swift | 248 +++++++++++++++++- .../VaultAutofillListProcessor.swift | 1 + .../UI/Vault/Vault/VaultCoordinator.swift | 1 + 21 files changed, 578 insertions(+), 15 deletions(-) diff --git a/BitwardenResources/Localizations/en.lproj/Localizable.strings b/BitwardenResources/Localizations/en.lproj/Localizable.strings index 6f973f0208..4875af354a 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/Autofill/Repositories/FillAssistRepository.swift b/BitwardenShared/Core/Autofill/Repositories/FillAssistRepository.swift index 7f4fa878dc..1f04b722b7 100644 --- a/BitwardenShared/Core/Autofill/Repositories/FillAssistRepository.swift +++ b/BitwardenShared/Core/Autofill/Repositories/FillAssistRepository.swift @@ -107,10 +107,9 @@ class DefaultFillAssistRepository: FillAssistRepository { /// Runs the full sync pipeline if sync conditions are met /// private func performSync() async throws { - // 1. Feature flag guard. - guard await configService.getFeatureFlag(.fillAssistTargetingRules) else { return } + guard await configService.getFeatureFlag(.fillAssistTargetingRules), + try await stateService.getFillAssistEnabled() else { return } - // 2. Time-interval guard — skip if last fetch was less than fillAssistUpdateInterval ago. let sourceUrl = environmentService.fillAssistRulesURL let userId = try await stateService.getActiveAccountId() let lastFetch = appSettingsStore.fillAssistLastFetchTimestamp(userId: userId) @@ -118,32 +117,26 @@ class DefaultFillAssistRepository: FillAssistRepository { return } - // 3. Fetch manifest. let manifest = try await fillAssistAPIService.getManifest() - // 4. Resolve the non-deprecated entry for the current forms version. guard let entry = manifest.maps["forms"]?[Constants.FillAssist.formsVersion], !entry.deprecated else { return } - // 5. If cid and source URL are unchanged, update timestamp and skip download. let cached = appSettingsStore.fillAssistCachedData(userId: userId) if cached?.cid == entry.cid, cached?.sourceUrl == sourceUrl.absoluteString { appSettingsStore.setFillAssistLastFetchTimestamp(timeProvider.presentTime, userId: userId) return } - // 6. Download forms file. let formsMap = try await fillAssistAPIService.getFormsMap(filename: entry.filename) - // 7. Validate schema major version; update timestamp and skip if unsupported. let schemaMajor = formsMap.schemaVersion.split(separator: ".").first.map(String.init) ?? "" guard schemaMajor == Constants.FillAssist.expectedSchemaMajor else { appSettingsStore.setFillAssistLastFetchTimestamp(timeProvider.presentTime, userId: userId) return } - // 8. Parse, cache, and update timestamp. let rules = buildRules(from: formsMap) let data = FillAssistCachedData(cid: entry.cid, rules: rules, sourceUrl: sourceUrl.absoluteString) appSettingsStore.setFillAssistCachedData(data, userId: userId) diff --git a/BitwardenShared/Core/Autofill/Repositories/FillAssistRepositoryTests.swift b/BitwardenShared/Core/Autofill/Repositories/FillAssistRepositoryTests.swift index de554f9fad..3287ac1631 100644 --- a/BitwardenShared/Core/Autofill/Repositories/FillAssistRepositoryTests.swift +++ b/BitwardenShared/Core/Autofill/Repositories/FillAssistRepositoryTests.swift @@ -31,6 +31,7 @@ struct FillAssistRepositoryTests { fillAssistAPIService = MockFillAssistAPIService() stateService = MockStateService() stateService.activeAccount = .fixture() + stateService.fillAssistEnabledByUserId["1"] = true timeProvider = MockTimeProvider(.currentTime) subject = DefaultFillAssistRepository( @@ -360,4 +361,36 @@ private extension FillAssistRepositoryTests { schemaVersion: "1.0.0", ) } + + // 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/Platform/Services/StateService.swift b/BitwardenShared/Core/Platform/Services/StateService.swift index 412b106e1a..b6ab04f19e 100644 --- a/BitwardenShared/Core/Platform/Services/StateService.swift +++ b/BitwardenShared/Core/Platform/Services/StateService.swift @@ -219,6 +219,13 @@ protocol StateService: AnyObject, BillingStateService { /// 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. @@ -623,6 +630,14 @@ protocol StateService: AnyObject, BillingStateService { /// 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: @@ -1049,6 +1064,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. @@ -1305,6 +1328,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. @@ -1760,6 +1791,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 } @@ -2116,6 +2152,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 5d7e65465e..eb3be283cf 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()) @@ -2097,6 +2106,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 e1e4e0df14..3cf8545137 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. @@ -454,6 +460,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: @@ -824,6 +838,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) @@ -914,6 +929,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: @@ -1169,6 +1186,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)) } @@ -1311,6 +1332,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 976fff6618..27a38ccca3 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]() @@ -155,6 +156,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] } @@ -308,6 +313,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 94e9ecfd85..4a70581d69 100644 --- a/BitwardenShared/Core/Platform/Services/TestHelpers/MockStateService.swift +++ b/BitwardenShared/Core/Platform/Services/TestHelpers/MockStateService.swift @@ -45,6 +45,8 @@ 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 doesActiveAccountHavePremiumResult: Bool = true var doesActiveAccountHavePremiumPersonallyCalled = false // swiftlint:disable:this identifier_name var doesActiveAccountHavePremiumPersonallyResult: Bool = true // swiftlint:disable:this identifier_name @@ -313,6 +315,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 } @@ -646,6 +656,11 @@ class MockStateService: StateService, ActiveAccountStateProvider, AutofillStateS self.events[userId] = events } + func setFillAssistEnabled(_ fillAssistEnabled: Bool, userId: String?) async throws { + let userId = try unwrapUserId(userId) + fillAssistEnabledByUserId[userId] = fillAssistEnabled + } + func setFlightRecorderData(_ data: FlightRecorderData?) async { flightRecorderData = data } 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/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 +30,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 +42,7 @@ class AutoFillProcessorTests: BitwardenTestCase { autofillCredentialService: autofillCredentialService, configService: configService, errorReporter: errorReporter, + fillAssistRepository: fillAssistRepository, settingsRepository: settingsRepository, stateService: stateService, ), @@ -54,6 +58,7 @@ class AutoFillProcessorTests: BitwardenTestCase { configService = nil coordinator = nil errorReporter = nil + fillAssistRepository = nil settingsRepository = nil stateService = nil subject = nil @@ -77,6 +82,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 +96,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 +212,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 +262,31 @@ 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 `.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..5a18e5d267 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. /// @@ -126,10 +160,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 +181,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..cb5846778c 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 } @@ -466,4 +483,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 Date: Fri, 3 Jul 2026 11:46:10 +0100 Subject: [PATCH 13/23] [PM-39851] fix: Skip Fill Assist sync only when cache is readable and timestamp is fresh --- .../Autofill/Repositories/FillAssistRepository.swift | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/BitwardenShared/Core/Autofill/Repositories/FillAssistRepository.swift b/BitwardenShared/Core/Autofill/Repositories/FillAssistRepository.swift index 1f04b722b7..25502f0beb 100644 --- a/BitwardenShared/Core/Autofill/Repositories/FillAssistRepository.swift +++ b/BitwardenShared/Core/Autofill/Repositories/FillAssistRepository.swift @@ -112,10 +112,12 @@ class DefaultFillAssistRepository: FillAssistRepository { 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, timeProvider.presentTime.timeIntervalSince(lastFetch) < Constants.FillAssist.updateInterval { - return - } + let cacheIsStale = lastFetch.map { timestamp in + timeProvider.presentTime.timeIntervalSince(timestamp) >= Constants.FillAssist.updateInterval + } ?? true + guard cached == nil || cacheIsStale else { return } let manifest = try await fillAssistAPIService.getManifest() @@ -123,7 +125,6 @@ class DefaultFillAssistRepository: FillAssistRepository { !entry.deprecated else { return } - let cached = appSettingsStore.fillAssistCachedData(userId: userId) if cached?.cid == entry.cid, cached?.sourceUrl == sourceUrl.absoluteString { appSettingsStore.setFillAssistLastFetchTimestamp(timeProvider.presentTime, userId: userId) return From 124ff1359a1f052a4cf99e2abcc1481630d49181 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andre=CC=81=20Bispo?= Date: Fri, 3 Jul 2026 11:49:38 +0100 Subject: [PATCH 14/23] [PM-39851] fix: Skip Fill Assist sync only when cache is readable and timestamp is fresh --- .../Autofill/Repositories/FillAssistRepository.swift | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/BitwardenShared/Core/Autofill/Repositories/FillAssistRepository.swift b/BitwardenShared/Core/Autofill/Repositories/FillAssistRepository.swift index 25502f0beb..b7cd8e55bc 100644 --- a/BitwardenShared/Core/Autofill/Repositories/FillAssistRepository.swift +++ b/BitwardenShared/Core/Autofill/Repositories/FillAssistRepository.swift @@ -114,10 +114,11 @@ class DefaultFillAssistRepository: FillAssistRepository { let userId = try await stateService.getActiveAccountId() let cached = appSettingsStore.fillAssistCachedData(userId: userId) let lastFetch = appSettingsStore.fillAssistLastFetchTimestamp(userId: userId) - let cacheIsStale = lastFetch.map { timestamp in - timeProvider.presentTime.timeIntervalSince(timestamp) >= Constants.FillAssist.updateInterval - } ?? true - guard cached == nil || cacheIsStale else { return } + if let lastFetch, + cached != nil, + timeProvider.presentTime.timeIntervalSince(lastFetch) < Constants.FillAssist.updateInterval { + return + } let manifest = try await fillAssistAPIService.getManifest() From 3511feed1cf12084a138320223f6172320884836 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andre=CC=81=20Bispo?= Date: Fri, 3 Jul 2026 12:31:37 +0100 Subject: [PATCH 15/23] [PM-39851] refactor: Use send(_:) instead of download for Fill Assist API requests --- .../Core/Autofill/Services/API/FillAssistAPIService.swift | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) 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()) } } From 35fcc50e58e1b61d7974083f674ed8cd10e95262 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andre=CC=81=20Bispo?= Date: Fri, 3 Jul 2026 12:32:52 +0100 Subject: [PATCH 16/23] [PM-39851] refactor: Remove unused HTTPService.download(filename:) method and its test --- Networking/Sources/Networking/HTTPService.swift | 13 ------------- .../Tests/NetworkingTests/HTTPServiceTests.swift | 15 --------------- 2 files changed, 28 deletions(-) diff --git a/Networking/Sources/Networking/HTTPService.swift b/Networking/Sources/Networking/HTTPService.swift index af3a8bd436..f8e922cfe3 100644 --- a/Networking/Sources/Networking/HTTPService.swift +++ b/Networking/Sources/Networking/HTTPService.swift @@ -100,19 +100,6 @@ public final class HTTPService: Sendable { try await client.download(from: urlRequest) } - /// Downloads the file at `filename` appended to the service's base URL. - /// - /// Uses `URLSession` download tasks which follow HTTP redirects automatically, making this - /// suitable for external CDN URLs that redirect before serving the final content. - /// - /// - Parameter filename: The filename to append to the base URL (e.g. `"manifest.json"`). - /// - Returns: The temporary `URL` of the downloaded file. - /// - public func download(filename: String) async throws -> 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 {} From 6859eab3b37ab6a2b1b4dc3f99ed87df6057cccb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andre=CC=81=20Bispo?= Date: Fri, 3 Jul 2026 12:32:56 +0100 Subject: [PATCH 17/23] [PM-39851] fix: Run Fill Assist sync independently of vault revision date check --- .../Core/Vault/Services/SyncService.swift | 4 ++-- .../Core/Vault/Services/SyncServiceTests.swift | 17 ++++++++++++++++- 2 files changed, 18 insertions(+), 3 deletions(-) diff --git a/BitwardenShared/Core/Vault/Services/SyncService.swift b/BitwardenShared/Core/Vault/Services/SyncService.swift index 4df2d3c5eb..4d6d627752 100644 --- a/BitwardenShared/Core/Vault/Services/SyncService.swift +++ b/BitwardenShared/Core/Vault/Services/SyncService.swift @@ -427,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 } @@ -479,8 +481,6 @@ extension DefaultSyncService { ) } - await fillAssistRepository.syncRules() - await delegate?.onFetchSyncSucceeded(userId: userId) } diff --git a/BitwardenShared/Core/Vault/Services/SyncServiceTests.swift b/BitwardenShared/Core/Vault/Services/SyncServiceTests.swift index 386e80d4df..1e35be8d4f 100644 --- a/BitwardenShared/Core/Vault/Services/SyncServiceTests.swift +++ b/BitwardenShared/Core/Vault/Services/SyncServiceTests.swift @@ -371,7 +371,7 @@ class SyncServiceTests: BitwardenTestCase { // swiftlint:disable:this type_body_ ) } - /// `fetchSync()` calls `syncRules()` on the fill-assist repository alongside vault sync. + /// `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() @@ -381,6 +381,21 @@ class SyncServiceTests: BitwardenTestCase { // swiftlint:disable:this type_body_ 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 { From aefc828efdc1a812b40fcd7493f7713b7484f203 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andre=CC=81=20Bispo?= Date: Fri, 10 Jul 2026 16:18:07 +0100 Subject: [PATCH 18/23] [PM-39851] test: Fix FillAssistRepositoryTests for cache-readable sync-skip behavior syncRules_withinUpdateInterval now also stubs cached data to match the updated skip condition, and a new test covers a fresh timestamp with no readable cache. Fixture builders moved to a separate file to stay under the file length limit. --- .../FillAssistRepositoryTests.Fixtures.swift | 79 ++++++++++++++++ .../FillAssistRepositoryTests.swift | 94 +++++-------------- 2 files changed, 101 insertions(+), 72 deletions(-) create mode 100644 BitwardenShared/Core/Autofill/Repositories/FillAssistRepositoryTests.Fixtures.swift 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 index 3287ac1631..495c2bb3bb 100644 --- a/BitwardenShared/Core/Autofill/Repositories/FillAssistRepositoryTests.swift +++ b/BitwardenShared/Core/Autofill/Repositories/FillAssistRepositoryTests.swift @@ -57,10 +57,16 @@ struct FillAssistRepositoryTests { #expect(!fillAssistAPIService.getManifestCalled) } - /// `syncRules()` makes no network calls when the update interval has not elapsed. + /// `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() @@ -68,6 +74,20 @@ struct FillAssistRepositoryTests { #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 { @@ -291,77 +311,7 @@ struct FillAssistRepositoryTests { // MARK: - Helpers -private 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. - /// - private 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. - private 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", - ) - } - +extension FillAssistRepositoryTests { // MARK: Tests - user toggle guard /// `syncRules()` makes no network calls when the feature flag is on but the user toggle is off. From 72516fc534a982f32d6e3511818ca47168988faa Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andre=CC=81=20Bispo?= Date: Fri, 10 Jul 2026 16:41:17 +0100 Subject: [PATCH 19/23] [PM-38443] test: Add error-path coverage for the Fill Assist toggle Adds setFillAssistEnabledError to MockStateService and a test verifying AutoFillProcessor reverts the toggle and shows an alert when persisting the preference fails. --- .../Services/TestHelpers/MockStateService.swift | 4 ++++ .../AutoFill/AutoFillProcessorTests.swift | 17 +++++++++++++++++ 2 files changed, 21 insertions(+) diff --git a/BitwardenShared/Core/Platform/Services/TestHelpers/MockStateService.swift b/BitwardenShared/Core/Platform/Services/TestHelpers/MockStateService.swift index a542828f7d..1e3d48b7da 100644 --- a/BitwardenShared/Core/Platform/Services/TestHelpers/MockStateService.swift +++ b/BitwardenShared/Core/Platform/Services/TestHelpers/MockStateService.swift @@ -53,6 +53,7 @@ class MockStateService: StateService, ActiveAccountStateProvider, AutofillStateS 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 @@ -677,6 +678,9 @@ class MockStateService: StateService, ActiveAccountStateProvider, AutofillStateS } func setFillAssistEnabled(_ fillAssistEnabled: Bool, userId: String?) async throws { + if let setFillAssistEnabledError { + throw setFillAssistEnabledError + } let userId = try unwrapUserId(userId) fillAssistEnabledByUserId[userId] = fillAssistEnabled } diff --git a/BitwardenShared/UI/Platform/Settings/Settings/AutoFill/AutoFillProcessorTests.swift b/BitwardenShared/UI/Platform/Settings/Settings/AutoFill/AutoFillProcessorTests.swift index 31de682901..2e16ec9082 100644 --- a/BitwardenShared/UI/Platform/Settings/Settings/AutoFill/AutoFillProcessorTests.swift +++ b/BitwardenShared/UI/Platform/Settings/Settings/AutoFill/AutoFillProcessorTests.swift @@ -1,6 +1,7 @@ import BitwardenKit import BitwardenKitMocks import BitwardenResources +import TestHelpers import XCTest @testable import BitwardenShared @@ -287,6 +288,22 @@ class AutoFillProcessorTests: BitwardenTestCase { 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 { From a34f5e55160d490adca59cac357db5618edeb70d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andre=CC=81=20Bispo?= Date: Fri, 10 Jul 2026 18:16:14 +0100 Subject: [PATCH 20/23] [PM-39851] test: Fix FillAssistAPIServiceTests for send(_:)-based requests The tests still mocked the old download(filename:)-based HTTPClient API, but the production code was migrated to fillAssistService.send(_:) in an earlier commit. Update the mock setup and assertions to match. --- .../API/FillAssistAPIServiceTests.swift | 26 +++++++------------ 1 file changed, 10 insertions(+), 16 deletions(-) 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)", ) } From a486c51685d9720ac4232cf91afe50ec8cabf865 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andre=CC=81=20Bispo?= Date: Mon, 13 Jul 2026 18:00:49 +0100 Subject: [PATCH 21/23] [PM-38443] fix: Allow autofill reprompt flow when Fill Assist is enabled The reprompt handler only proceeded when the app extension's canAutofill was true, blocking Fill Assist-driven autofill from the main app context. --- BitwardenShared/UI/Vault/Helpers/AutofillHelper.swift | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/BitwardenShared/UI/Vault/Helpers/AutofillHelper.swift b/BitwardenShared/UI/Vault/Helpers/AutofillHelper.swift index 5a18e5d267..0808fae652 100644 --- a/BitwardenShared/UI/Vault/Helpers/AutofillHelper.swift +++ b/BitwardenShared/UI/Vault/Helpers/AutofillHelper.swift @@ -151,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) From 9f164cb73c159cc5330b217e3343478864e2965a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andre=CC=81=20Bispo?= Date: Wed, 15 Jul 2026 11:12:22 +0100 Subject: [PATCH 22/23] [PM-38443] refactor: Delegate Fill Assist cache cleanup on logout to clearRules() --- .../Auth/Repositories/AuthRepository.swift | 7 ++++++ .../Repositories/AuthRepositoryTests.swift | 6 +++++ .../Repositories/FillAssistRepository.swift | 17 +++++++++----- .../FillAssistRepositoryTests.swift | 22 +++++++++++++++++-- .../Platform/Services/ServiceContainer.swift | 1 + .../Core/Platform/Services/StateService.swift | 2 -- 6 files changed, 46 insertions(+), 9 deletions(-) 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/Repositories/FillAssistRepository.swift b/BitwardenShared/Core/Autofill/Repositories/FillAssistRepository.swift index b7cd8e55bc..a97ada84fb 100644 --- a/BitwardenShared/Core/Autofill/Repositories/FillAssistRepository.swift +++ b/BitwardenShared/Core/Autofill/Repositories/FillAssistRepository.swift @@ -17,9 +17,11 @@ protocol FillAssistRepository { // sourcery: AutoMockable /// func rules(for hostname: String) async -> FillAssistHostRules? - /// Clears all cached fill-assist data for the active account. + /// Clears all cached fill-assist data for an account. /// - func clearRules() async throws + /// - 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 @@ -97,9 +99,14 @@ class DefaultFillAssistRepository: FillAssistRepository { return appSettingsStore.fillAssistCachedData(userId: userId)?.rules[hostname] } - func clearRules() async throws { - let userId = try await stateService.getActiveAccountId() - appSettingsStore.setFillAssistCachedData(nil, userId: userId) + 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 diff --git a/BitwardenShared/Core/Autofill/Repositories/FillAssistRepositoryTests.swift b/BitwardenShared/Core/Autofill/Repositories/FillAssistRepositoryTests.swift index 495c2bb3bb..bbf76fc314 100644 --- a/BitwardenShared/Core/Autofill/Repositories/FillAssistRepositoryTests.swift +++ b/BitwardenShared/Core/Autofill/Repositories/FillAssistRepositoryTests.swift @@ -267,7 +267,7 @@ struct FillAssistRepositoryTests { // MARK: Tests - clearRules() - /// `clearRules()` removes cached data for the active account. + /// `clearRules()` removes cached data and the last-fetch timestamp for the active account. @Test func clearRules_removesCachedData() async throws { appSettingsStore.fillAssistCachedDataByUserId["1"] = FillAssistCachedData( @@ -275,10 +275,28 @@ struct FillAssistRepositoryTests { rules: [:], sourceUrl: "https://example.com", ) + appSettingsStore.fillAssistLastFetchTimestampByUserId["1"] = timeProvider.presentTime - try await subject.clearRules() + 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 diff --git a/BitwardenShared/Core/Platform/Services/ServiceContainer.swift b/BitwardenShared/Core/Platform/Services/ServiceContainer.swift index b27df7454f..ae426f095f 100644 --- a/BitwardenShared/Core/Platform/Services/ServiceContainer.swift +++ b/BitwardenShared/Core/Platform/Services/ServiceContainer.swift @@ -864,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, diff --git a/BitwardenShared/Core/Platform/Services/StateService.swift b/BitwardenShared/Core/Platform/Services/StateService.swift index ba331941a7..7e112200fa 100644 --- a/BitwardenShared/Core/Platform/Services/StateService.swift +++ b/BitwardenShared/Core/Platform/Services/StateService.swift @@ -1948,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) From b2286ccde6d7242ebe8db2b654da4fabbe23a3ed Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andre=CC=81=20Bispo?= Date: Wed, 15 Jul 2026 13:44:23 +0100 Subject: [PATCH 23/23] [PM-38443] test: Disable Fill Assist in the autofill-not-supported test scenario --- BitwardenShared/UI/Vault/Helpers/AutofillHelperTests.swift | 1 + 1 file changed, 1 insertion(+) diff --git a/BitwardenShared/UI/Vault/Helpers/AutofillHelperTests.swift b/BitwardenShared/UI/Vault/Helpers/AutofillHelperTests.swift index cb5846778c..be90863ac5 100644 --- a/BitwardenShared/UI/Vault/Helpers/AutofillHelperTests.swift +++ b/BitwardenShared/UI/Vault/Helpers/AutofillHelperTests.swift @@ -93,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"),