From ff606d7b640ae79257fddadad1ec93259b2dc803 Mon Sep 17 00:00:00 2001 From: bwaresiak Date: Fri, 6 Dec 2024 21:51:39 +0100 Subject: [PATCH 01/12] Fix threading issue between using a Semaphore and async/await (#1115) Required: Task/Issue URL: https://app.asana.com/0/856498667320406/1208887821349124/f iOS PR: tba macOS PR:tba What kind of version bump will this require?: Patch Description: Fix threading issue between using a Semaphore and async/await --- .../ContentBlockerRulesManager.swift | 38 +++++----------- ...kingRulesLastCompiledRulesLookupTask.swift | 45 +++++++++++++------ .../ContentBlockingRulesLookupTask.swift | 44 +++++++++++------- 3 files changed, 72 insertions(+), 55 deletions(-) diff --git a/Sources/BrowserServicesKit/ContentBlocking/ContentBlockerRulesManager.swift b/Sources/BrowserServicesKit/ContentBlocking/ContentBlockerRulesManager.swift index c2de45764..c7cb04bd9 100644 --- a/Sources/BrowserServicesKit/ContentBlocking/ContentBlockerRulesManager.swift +++ b/Sources/BrowserServicesKit/ContentBlocking/ContentBlockerRulesManager.swift @@ -242,26 +242,20 @@ public class ContentBlockerRulesManager: CompiledRuleListsSource { Logger.contentBlocking.debug("Lookup compiled rules") prepareSourceManagers() let initialCompilationTask = LookupRulesTask(sourceManagers: Array(sourceManagers.values)) - let mutex = DispatchSemaphore(value: 0) - Task { - do { - try await initialCompilationTask.lookupCachedRulesLists() - } catch { - Logger.contentBlocking.debug("❌ Lookup failed: \(error.localizedDescription, privacy: .public)") - } - mutex.signal() - } - // We want to confine Compilation work to WorkQueue, so we wait to come back from async Task - mutex.wait() + let result: [LookupRulesTask.LookupResult] + + do { + result = try initialCompilationTask.lookupCachedRulesLists() - if let result = initialCompilationTask.result { let rules = result.map(Rules.init(compilationResult:)) - Logger.contentBlocking.debug("🟩 Found \(rules.count, privacy: .public) rules") + Logger.contentBlocking.debug("🟩 Lookup Found \(rules.count, privacy: .public) rules") applyRules(rules) return true + } catch { + Logger.contentBlocking.debug("❌ Lookup failed: \(error.localizedDescription, privacy: .public)") + return false } - return false } /* @@ -273,18 +267,10 @@ public class ContentBlockerRulesManager: CompiledRuleListsSource { let initialCompilationTask = LastCompiledRulesLookupTask(sourceRules: rulesSource.contentBlockerRulesLists, lastCompiledRules: lastCompiledRules) - let mutex = DispatchSemaphore(value: 0) - Task { - try? await initialCompilationTask.fetchCachedRulesLists() - mutex.signal() - } - // We want to confine Compilation work to WorkQueue, so we wait to come back from async Task - mutex.wait() - - let rulesFound = initialCompilationTask.getFetchedRules() + let rules = initialCompilationTask.fetchCachedRulesLists() - if let rulesFound { - applyRules(rulesFound) + if let rules { + applyRules(rules) } else { lock.lock() state = .idle @@ -294,7 +280,7 @@ public class ContentBlockerRulesManager: CompiledRuleListsSource { // No matter if rules were found or not, we need to schedule recompilation, after all scheduleCompilation() - return rulesFound != nil + return rules != nil } private func prepareSourceManagers() { diff --git a/Sources/BrowserServicesKit/ContentBlocking/ContentBlockingRulesLastCompiledRulesLookupTask.swift b/Sources/BrowserServicesKit/ContentBlocking/ContentBlockingRulesLastCompiledRulesLookupTask.swift index 021e058ed..1399e6202 100644 --- a/Sources/BrowserServicesKit/ContentBlocking/ContentBlockingRulesLastCompiledRulesLookupTask.swift +++ b/Sources/BrowserServicesKit/ContentBlocking/ContentBlockingRulesLastCompiledRulesLookupTask.swift @@ -41,32 +41,49 @@ extension ContentBlockerRulesManager { self.lastCompiledRules = lastCompiledRules } - func fetchCachedRulesLists() async throws { + func fetchCachedRulesLists() -> [Rules]? { let sourceRulesNames = sourceRules.map { $0.name } let filteredBySourceLastCompiledRules = lastCompiledRules.filter { sourceRulesNames.contains($0.name) } guard filteredBySourceLastCompiledRules.count == sourceRules.count else { // We should only load rule lists from cache, in case we can match every one of these - throw WKError(.contentRuleListStoreLookUpFailed) + return nil } var result: [CachedRulesList] = [] + let group = DispatchGroup() + for rules in filteredBySourceLastCompiledRules { - guard let ruleList = try await Task(operation: { @MainActor in - try await WKContentRuleListStore.default().contentRuleList(forIdentifier: rules.identifier.stringValue) - }).value else { throw WKError(.contentRuleListStoreLookUpFailed) } - - result.append(CachedRulesList(name: rules.name, - rulesList: ruleList, - tds: rules.trackerData, - rulesIdentifier: rules.identifier)) + group.enter() + + DispatchQueue.main.async { + // This needs to be called from the main thread. + WKContentRuleListStore.default().lookUpContentRuleList(forIdentifier: rules.identifier.stringValue) { ruleList, error in + guard let ruleList, error == nil else { + group.leave() + return + } + + result.append(CachedRulesList(name: rules.name, + rulesList: ruleList, + tds: rules.trackerData, + rulesIdentifier: rules.identifier)) + group.leave() + } + } } - self.result = result + + let operationResult = group.wait(timeout: .now() + 6) + + guard operationResult == .success, result.count == filteredBySourceLastCompiledRules.count else { + return nil + } + + return getRules(from: result) } - public func getFetchedRules() -> [Rules]? { - guard let result else { return nil } - return result.map { + public func getRules(from cached: [CachedRulesList]) -> [Rules] { + return cached.map { let surrogateTDS = ContentBlockerRulesManager.extractSurrogates(from: $0.tds) let encodedData = try? JSONEncoder().encode(surrogateTDS) let encodedTrackerData = String(data: encodedData!, encoding: .utf8)! diff --git a/Sources/BrowserServicesKit/ContentBlocking/ContentBlockingRulesLookupTask.swift b/Sources/BrowserServicesKit/ContentBlocking/ContentBlockingRulesLookupTask.swift index 974c984ac..fadfc6fbf 100644 --- a/Sources/BrowserServicesKit/ContentBlocking/ContentBlockingRulesLookupTask.swift +++ b/Sources/BrowserServicesKit/ContentBlocking/ContentBlockingRulesLookupTask.swift @@ -28,31 +28,45 @@ extension ContentBlockerRulesManager { private let sourceManagers: [ContentBlockerRulesSourceManager] - public private(set) var result: [LookupResult]? - init(sourceManagers: [ContentBlockerRulesSourceManager]) { self.sourceManagers = sourceManagers } - func lookupCachedRulesLists() async throws { + func lookupCachedRulesLists() throws -> [LookupResult] { + + let models = sourceManagers.compactMap { $0.makeModel() } + if models.count != sourceManagers.count { + // We should only load rule lists, in case we can match every one of the expected ones + throw WKError(.contentRuleListStoreLookUpFailed) + } var result = [LookupResult]() - for sourceManager in sourceManagers { - guard let model = sourceManager.makeModel() else { - throw WKError(.contentRuleListStoreLookUpFailed) - } + let group = DispatchGroup() + + for model in models { + group.enter() - guard let ruleList = try await Task(operation: { @MainActor in - try await WKContentRuleListStore.default().contentRuleList(forIdentifier: model.rulesIdentifier.stringValue) - }).value else { - // All lists must be found for this to be considered successful - throw WKError(.contentRuleListStoreLookUpFailed) + DispatchQueue.main.async { + // This needs to be called from the main thread. + WKContentRuleListStore.default().lookUpContentRuleList(forIdentifier: model.rulesIdentifier.stringValue) { ruleList, error in + guard let ruleList, error == nil else { + group.leave() + return + } + + result.append((ruleList, model)) + group.leave() + } } + } + + let operationResult = group.wait(timeout: .now() + 6) - result.append((ruleList, model)) + guard operationResult == .success, result.count == models.count else { + throw WKError(.contentRuleListStoreLookUpFailed) } - self.result = result - } + return result + } } } From ad28b07d2df9f2ec45ab0306053126e2c8b237ca Mon Sep 17 00:00:00 2001 From: Sam Symons Date: Fri, 6 Dec 2024 12:52:01 -0800 Subject: [PATCH 02/12] Update RMF version matching to ignore build number (#1118) Required: Task/Issue URL: https://app.asana.com/0/1207619243206445/1208908529122506/f iOS PR: duckduckgo/iOS#3686 macOS PR: duckduckgo/macos-browser#3635 What kind of version bump will this require?: Patch Description: This PR updates the RMF version matching logic to ignore build number. Previously, when you tried to match a version like 1.110.0, it would fail because you needed to know the build number. --- .../Matchers/AppAttributeMatcher.swift | 2 +- .../Model/MatchingAttributes.swift | 37 +++++++++++++++++-- .../CommonAppAttributeMatcherTests.swift | 25 ++++++++----- 3 files changed, 50 insertions(+), 14 deletions(-) diff --git a/Sources/RemoteMessaging/Matchers/AppAttributeMatcher.swift b/Sources/RemoteMessaging/Matchers/AppAttributeMatcher.swift index 1805ebbd0..677ca9f3a 100644 --- a/Sources/RemoteMessaging/Matchers/AppAttributeMatcher.swift +++ b/Sources/RemoteMessaging/Matchers/AppAttributeMatcher.swift @@ -81,7 +81,7 @@ public struct CommonAppAttributeMatcher: AttributeMatching { assertionFailure("BundleIdentifier should not be empty") } self.init(bundleId: AppVersion.shared.identifier, - appVersion: AppVersion.shared.versionAndBuildNumber, + appVersion: AppVersion.shared.versionNumber, isInternalUser: isInternalUser, statisticsStore: statisticsStore, variantManager: variantManager) diff --git a/Sources/RemoteMessaging/Model/MatchingAttributes.swift b/Sources/RemoteMessaging/Model/MatchingAttributes.swift index becc8280e..4e2cf04a3 100644 --- a/Sources/RemoteMessaging/Model/MatchingAttributes.swift +++ b/Sources/RemoteMessaging/Model/MatchingAttributes.swift @@ -49,12 +49,27 @@ struct AppIdMatchingAttribute: SingleValueMatching { } struct AppVersionMatchingAttribute: StringRangeMatching { - static let defaultMaxValue: String = AppVersion.shared.versionAndBuildNumber - var min: String = MatchingAttributeDefaults.stringDefaultValue - var max: String = AppVersion.shared.versionAndBuildNumber - var value: String = MatchingAttributeDefaults.stringDefaultValue + static let defaultMaxValue: String = AppVersion.shared.versionNumber + + var min: String + var max: String + var value: String var fallback: Bool? + + // Legacy versions of the app require a build number in the version string in order to match correctly. + // To allow message authors to include a build number for backwards compatibility, while also allowing new clients to use the simpler version + // string, this initializer trims the build number before storing it. + init(min: String = MatchingAttributeDefaults.stringDefaultValue, + max: String = AppVersion.shared.versionNumber, + value: String = MatchingAttributeDefaults.stringDefaultValue, + fallback: Bool?) { + self.min = min.trimmingBuildNumber + self.max = max.trimmingBuildNumber + self.value = value.trimmingBuildNumber + self.fallback = fallback + } + } struct AtbMatchingAttribute: SingleValueMatching { @@ -306,3 +321,17 @@ struct RangeStringNumericMatchingAttribute: Equatable { return version + String(repeating: ".0", count: matchComponents.count - versionComponents.count) } } + +private extension String { + + var trimmingBuildNumber: String { + let components = self.split(separator: ".") + + if components.count == 4 { + return components.dropLast().joined(separator: ".") + } else { + return self + } + } + +} diff --git a/Tests/RemoteMessagingTests/Matchers/CommonAppAttributeMatcherTests.swift b/Tests/RemoteMessagingTests/Matchers/CommonAppAttributeMatcherTests.swift index 7c9d87672..b792cae83 100644 --- a/Tests/RemoteMessagingTests/Matchers/CommonAppAttributeMatcherTests.swift +++ b/Tests/RemoteMessagingTests/Matchers/CommonAppAttributeMatcherTests.swift @@ -26,6 +26,7 @@ import XCTest class CommonAppAttributeMatcherTests: XCTestCase { private var matcher: CommonAppAttributeMatcher! + private let versionNumber = "3.2.1" override func setUpWithError() throws { try super.setUpWithError() @@ -36,7 +37,13 @@ class CommonAppAttributeMatcherTests: XCTestCase { mockStatisticsStore.searchRetentionAtb = "v105-88" let manager = MockVariantManager(isSupportedReturns: true, currentVariant: MockVariant(name: "zo", weight: 44, isIncluded: { return true }, features: [.dummy])) - matcher = CommonAppAttributeMatcher(statisticsStore: mockStatisticsStore, variantManager: manager) + matcher = CommonAppAttributeMatcher( + bundleId: AppVersion.shared.identifier, + appVersion: versionNumber, + isInternalUser: true, + statisticsStore: mockStatisticsStore, + variantManager: manager + ) } override func tearDownWithError() throws { @@ -66,7 +73,7 @@ class CommonAppAttributeMatcherTests: XCTestCase { } func testWhenAppVersionEqualOrLowerThanMaxThenReturnMatch() throws { - let appVersionComponents = AppVersion.shared.versionAndBuildNumber.components(separatedBy: ".").map { $0 } + let appVersionComponents = versionNumber.components(separatedBy: ".").map { $0 } let appMajorVersion = appVersionComponents[0] let appMinorVersion = appVersionComponents.suffix(from: 1).joined(separator: ".") @@ -82,7 +89,7 @@ class CommonAppAttributeMatcherTests: XCTestCase { } func testWhenAppVersionGreaterThanMaxThenReturnFail() throws { - let appVersionComponents = AppVersion.shared.versionAndBuildNumber.components(separatedBy: ".").map { $0 } + let appVersionComponents = versionNumber.components(separatedBy: ".").map { $0 } let appMajorVersion = appVersionComponents[0] let lessThanMax = String(Int(appMajorVersion)! - 1) @@ -91,12 +98,12 @@ class CommonAppAttributeMatcherTests: XCTestCase { } func testWhenAppVersionEqualOrGreaterThanMinThenReturnMatch() throws { - XCTAssertEqual(matcher.evaluate(matchingAttribute: AppVersionMatchingAttribute(min: AppVersion.shared.versionAndBuildNumber, fallback: nil)), + XCTAssertEqual(matcher.evaluate(matchingAttribute: AppVersionMatchingAttribute(min: versionNumber, fallback: nil)), .match) } func testWhenAppVersionLowerThanMinThenReturnFail() throws { - let appVersionComponents = AppVersion.shared.versionAndBuildNumber.components(separatedBy: ".").map { $0 } + let appVersionComponents = versionNumber.components(separatedBy: ".").map { $0 } let (major, minor, patch) = (appVersionComponents[0], appVersionComponents[1], appVersionComponents[2]) let majorBumped = String(Int(major)! + 1) let patchBumped = [major, minor, String(Int(patch)! + 1)].joined(separator: ".") @@ -105,7 +112,7 @@ class CommonAppAttributeMatcherTests: XCTestCase { } func testWhenAppVersionInRangeThenReturnMatch() throws { - let appVersionComponents = AppVersion.shared.versionAndBuildNumber.components(separatedBy: ".").map { $0 } + let appVersionComponents = versionNumber.components(separatedBy: ".").map { $0 } let (major, minor, patch) = (appVersionComponents[0], appVersionComponents[1], appVersionComponents[2]) let majorBumped = String(Int(major)! + 1) let patchDecremented = [major, minor, String(Int(patch)! - 1)].joined(separator: ".") @@ -115,7 +122,7 @@ class CommonAppAttributeMatcherTests: XCTestCase { } func testWhenAppVersionNotInRangeThenReturnFail() throws { - let appVersionComponents = AppVersion.shared.versionAndBuildNumber.components(separatedBy: ".").map { $0 } + let appVersionComponents = versionNumber.components(separatedBy: ".").map { $0 } let appMajorVersion = appVersionComponents[0] let greaterThanMax = String(Int(appMajorVersion)! + 1) @@ -124,12 +131,12 @@ class CommonAppAttributeMatcherTests: XCTestCase { } func testWhenAppVersionSameAsDeviceThenReturnMatch() throws { - XCTAssertEqual(matcher.evaluate(matchingAttribute: AppVersionMatchingAttribute(min: AppVersion.shared.versionAndBuildNumber, max: AppVersion.shared.versionAndBuildNumber, fallback: nil)), + XCTAssertEqual(matcher.evaluate(matchingAttribute: AppVersionMatchingAttribute(min: versionNumber, max: versionNumber, fallback: nil)), .match) } func testWhenAppVersionDifferentToDeviceThenReturnFail() throws { - let appVersionComponents = AppVersion.shared.versionAndBuildNumber.components(separatedBy: ".").map { $0 } + let appVersionComponents = versionNumber.components(separatedBy: ".").map { $0 } let (major, minor, patch) = (appVersionComponents[0], appVersionComponents[1], appVersionComponents[2]) let patchDecremented = [major, minor, String(Int(patch)! - 1)].joined(separator: ".") From a404b05cc0ec6b7b3e346804c1e0970dd91c0634 Mon Sep 17 00:00:00 2001 From: Sam Symons Date: Fri, 6 Dec 2024 13:09:37 -0800 Subject: [PATCH 03/12] Update local network routing (#1117) Required: Task/Issue URL: https://app.asana.com/0/1199230911884351/1208918011157080/f iOS PR: TODO macOS PR: TODO What kind of version bump will this require?: Patch Optional: Tech Design URL: CC: Description: This PR updates local network routing. When including local networks, 10.0.0.0/8 will also go through the tunnel. --- Sources/NetworkProtection/Routing/VPNRoutingRange.swift | 7 ++++++- .../Routing/VPNRoutingTableResolver.swift | 2 +- 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/Sources/NetworkProtection/Routing/VPNRoutingRange.swift b/Sources/NetworkProtection/Routing/VPNRoutingRange.swift index d72f63628..4c94dc51c 100644 --- a/Sources/NetworkProtection/Routing/VPNRoutingRange.swift +++ b/Sources/NetworkProtection/Routing/VPNRoutingRange.swift @@ -34,8 +34,13 @@ public enum VPNRoutingRange { "::1/128", /* loopback */ ] + public static let localNetworkRangeWithoutDNS: [NetworkProtection.IPAddressRange] = [ + "172.16.0.0/12", /* 255.240.0.0 */ + "192.168.0.0/16", /* 255.255.0.0 */ + ] + public static let localNetworkRange: [NetworkProtection.IPAddressRange] = [ - // "10.0.0.0/8", /* 255.0.0.0 */ + "10.0.0.0/8", /* 255.0.0.0 */ "172.16.0.0/12", /* 255.240.0.0 */ "192.168.0.0/16", /* 255.255.0.0 */ ] diff --git a/Sources/NetworkProtection/Routing/VPNRoutingTableResolver.swift b/Sources/NetworkProtection/Routing/VPNRoutingTableResolver.swift index 505aa455a..429cd4c8a 100644 --- a/Sources/NetworkProtection/Routing/VPNRoutingTableResolver.swift +++ b/Sources/NetworkProtection/Routing/VPNRoutingTableResolver.swift @@ -43,7 +43,7 @@ struct VPNRoutingTableResolver { var routes = VPNRoutingRange.alwaysExcludedIPv4Range if excludeLocalNetworks { - routes += VPNRoutingRange.localNetworkRange + routes += VPNRoutingRange.localNetworkRangeWithoutDNS } return routes From a82c14b991f8cbef46d4b7d8e613762f1592fcd2 Mon Sep 17 00:00:00 2001 From: Dax Mobile <44842493+daxmobile@users.noreply.github.com> Date: Mon, 9 Dec 2024 20:01:22 +1100 Subject: [PATCH 04/12] Update autofill to 16.0.0 (#1122) Task/Issue URL: https://app.asana.com/0/1208923931185505/1208923931185505 Autofill Release: https://github.com/duckduckgo/duckduckgo-autofill/releases/tag/16.0.0 ## Description Updates Autofill to version [16.0.0](https://github.com/duckduckgo/duckduckgo-autofill/releases/tag/16.0.0). ### Autofill 16.0.0 release notes ## What's Changed This PR introduces breaking changes for Android/Windows causing save autofill prompt to get triggered in username only fields. * Upgrade to shared eslint config + Adopt Prettier by @muodov in https://github.com/duckduckgo/duckduckgo-autofill/pull/695 * Ignore lint PR in git blame by @muodov in https://github.com/duckduckgo/duckduckgo-autofill/pull/699 * Update password-related json files (2024-11-18) by @daxmobile in https://github.com/duckduckgo/duckduckgo-autofill/pull/706 * [Form] Always scan shadow elements when categorizing the form inputs by @dbajpeyi in https://github.com/duckduckgo/duckduckgo-autofill/pull/703 * Bump ts-to-zod from 3.1.3 to 3.14.0 by @dependabot in https://github.com/duckduckgo/duckduckgo-autofill/pull/710 * [FormAnalyzer] Fix paperlesspost.com login form by @dbajpeyi in https://github.com/duckduckgo/duckduckgo-autofill/pull/711 * [FormAnalyzer] Tweak regex to match forgot password attribute by @dbajpeyi in https://github.com/duckduckgo/duckduckgo-autofill/pull/712 * [Form] Trigger partialSave on username/email only form submit by @dbajpeyi in https://github.com/duckduckgo/duckduckgo-autofill/pull/702 **Full Changelog**: https://github.com/duckduckgo/duckduckgo-autofill/compare/15.1.0...16.0.0 ## Steps to test This release has been tested during autofill development. For smoke test steps see [this task](https://app.asana.com/0/1198964220583541/1200583647142330/f). Co-authored-by: dbajpeyi <3018923+dbajpeyi@users.noreply.github.com> --- Package.resolved | 4 ++-- Package.swift | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/Package.resolved b/Package.resolved index 18584326a..dd982ae8a 100644 --- a/Package.resolved +++ b/Package.resolved @@ -23,8 +23,8 @@ "kind" : "remoteSourceControl", "location" : "https://github.com/duckduckgo/duckduckgo-autofill.git", "state" : { - "revision" : "c992041d16ec10d790e6204dce9abf9966d1363c", - "version" : "15.1.0" + "revision" : "88982a3802ac504e2f1a118a73bfdf2d8f4a7735", + "version" : "16.0.0" } }, { diff --git a/Package.swift b/Package.swift index 6b5dc6f81..7750c3930 100644 --- a/Package.swift +++ b/Package.swift @@ -50,7 +50,7 @@ let package = Package( .library(name: "PrivacyStats", targets: ["PrivacyStats"]), ], dependencies: [ - .package(url: "https://github.com/duckduckgo/duckduckgo-autofill.git", exact: "15.1.0"), + .package(url: "https://github.com/duckduckgo/duckduckgo-autofill.git", exact: "16.0.0"), .package(url: "https://github.com/duckduckgo/GRDB.swift.git", exact: "2.4.2"), .package(url: "https://github.com/duckduckgo/TrackerRadarKit", exact: "3.0.0"), .package(url: "https://github.com/duckduckgo/sync_crypto", exact: "0.3.0"), From cbfeb62093a03a22589e6fc4a52c4a6756727a05 Mon Sep 17 00:00:00 2001 From: Dominik Kapusta Date: Mon, 9 Dec 2024 17:06:45 +0100 Subject: [PATCH 05/12] Add PrivacyStatsError.failedToClearPrivacyStats (#1128) Task/Issue URL: https://app.asana.com/0/1201048563534612/1208936504720914/f Description: This change fixes PrivacyStats error reporting to include a dedicated error case for indicating failure to clear stats. --- Sources/PrivacyStats/PrivacyStats.swift | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/Sources/PrivacyStats/PrivacyStats.swift b/Sources/PrivacyStats/PrivacyStats.swift index c298f60fc..168b04ff7 100644 --- a/Sources/PrivacyStats/PrivacyStats.swift +++ b/Sources/PrivacyStats/PrivacyStats.swift @@ -30,6 +30,7 @@ import TrackerRadarKit public enum PrivacyStatsError: CustomNSError { case failedToFetchPrivacyStatsSummary(Error) case failedToStorePrivacyStats(Error) + case failedToClearPrivacyStats(Error) case failedToLoadCurrentPrivacyStats(Error) public static let errorDomain: String = "PrivacyStatsError" @@ -42,6 +43,8 @@ public enum PrivacyStatsError: CustomNSError { return 2 case .failedToLoadCurrentPrivacyStats: return 3 + case .failedToClearPrivacyStats: + return 4 } } @@ -49,7 +52,8 @@ public enum PrivacyStatsError: CustomNSError { switch self { case .failedToFetchPrivacyStatsSummary(let error), .failedToStorePrivacyStats(let error), - .failedToLoadCurrentPrivacyStats(let error): + .failedToLoadCurrentPrivacyStats(let error), + .failedToClearPrivacyStats(let error): return error } } @@ -165,7 +169,7 @@ public final class PrivacyStats: PrivacyStatsCollecting { Logger.privacyStats.debug("Deleted outdated entries") } catch { Logger.privacyStats.error("Save error: \(error)") - errorEvents?.fire(.failedToFetchPrivacyStatsSummary(error)) + errorEvents?.fire(.failedToClearPrivacyStats(error)) } continuation.resume() } From 20df9e22d5a69acfc25204fb0228a9d6f79b721f Mon Sep 17 00:00:00 2001 From: Graeme Arthur Date: Mon, 9 Dec 2024 18:36:43 +0100 Subject: [PATCH 06/12] Increase ratio of complete form saves (#1124) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Please review the release process for BrowserServicesKit [here](https://app.asana.com/0/1200194497630846/1200837094583426). **Required**: Task/Issue URL: https://app.asana.com/0/0/1206048666874235/f iOS PR: https://github.com/duckduckgo/iOS/pull/3698 macOS PR: https://github.com/duckduckgo/macos-browser/pull/3649 What kind of version bump will this require?: Major **Description**: Increase ratio of complete credentials saves (i.e. that include the username) by checking for recently filled emails/usernames when a form save event is incomplete. Approach is to have a new form save events for username/email-only forms that stays in memory for 3 minutes. If a password-only form save is detected for the same domain, we can complete the form with the previous partial data. This would also cover mobile where we don't have identities autofill – and in general in all cases where the user inputs data manually. **Steps to test this PR**: https://app.asana.com/0/1202926619870900/1208866651723703/f **OS Testing**: * [ ] iOS 14 * [ ] iOS 15 * [ ] iOS 16 * [ ] macOS 10.15 * [ ] macOS 11 * [ ] macOS 12 --- ###### Internal references: [Software Engineering Expectations](https://app.asana.com/0/59792373528535/199064865822552) [Technical Design Template](https://app.asana.com/0/59792373528535/184709971311943) --- .../xcschemes/BrowserServicesKit.xcscheme | 18 ++-- .../AutofillUserScript+SecureVault.swift | 1 + .../Features/PrivacyFeature.swift | 1 + .../SecureVault/PartialFormSaveManager.swift | 62 +++++++++++++ .../SecureVault/SecureVaultManager.swift | 49 +++++++--- Sources/Common/Extensions/DateExtension.swift | 4 + .../Common/Extensions/StringExtension.swift | 9 ++ .../MockAutofillDatabaseProvider.swift | 14 ++- .../SecureVault/SecureVaultManagerTests.swift | 90 ++++++++++++++++++- 9 files changed, 224 insertions(+), 24 deletions(-) create mode 100644 Sources/BrowserServicesKit/SecureVault/PartialFormSaveManager.swift diff --git a/.swiftpm/xcode/xcshareddata/xcschemes/BrowserServicesKit.xcscheme b/.swiftpm/xcode/xcshareddata/xcschemes/BrowserServicesKit.xcscheme index b3e79c64a..6c05e98e7 100644 --- a/.swiftpm/xcode/xcshareddata/xcschemes/BrowserServicesKit.xcscheme +++ b/.swiftpm/xcode/xcshareddata/xcschemes/BrowserServicesKit.xcscheme @@ -165,15 +165,15 @@ - - - + skipped = "NO"> + + + WebsiteAccount? { + guard let tldPlus1 = tld.eTLDplus1(domain) else { + return nil + } + guard let account = Self.accounts[tldPlus1] else { + return nil + } + + guard account.lastUpdated.isLessThan(minutesAgo: 3) else { + Self.accounts.removeValue(forKey: domain) + return nil + } + + return account + } + + func store(partialAccount: WebsiteAccount, for domain: String) { + guard let tldPlus1 = tld.eTLDplus1(domain) else { + return + } + Self.accounts[tldPlus1] = partialAccount + } + + func removePartialAccount(for domain: String) { + guard let tldPlus1 = tld.eTLDplus1(domain) else { + return + } + Self.accounts.removeValue(forKey: tldPlus1) + } +} diff --git a/Sources/BrowserServicesKit/SecureVault/SecureVaultManager.swift b/Sources/BrowserServicesKit/SecureVault/SecureVaultManager.swift index 2c4be3389..ed9c69168 100644 --- a/Sources/BrowserServicesKit/SecureVault/SecureVaultManager.swift +++ b/Sources/BrowserServicesKit/SecureVault/SecureVaultManager.swift @@ -106,6 +106,8 @@ public class SecureVaultManager { // This property can be removed once all platforms will search for partial account matches as the default expected behaviour. private let includePartialAccountMatches: Bool + private let shouldAllowPartialFormSaves: Bool + public let tld: TLD? // Keeps track of partial account created from autogenerated credentials (Private Email + Pwd) @@ -121,6 +123,7 @@ public class SecureVaultManager { private var autogeneratedCredentials: Bool { return autogeneratedUserName || autogeneratedPassword } + private let partialFormSaveManager: PartialFormSaveManager public lazy var autofillWebsiteAccountMatcher: AutofillWebsiteAccountMatcher? = { guard let tld = tld else { return nil } @@ -131,11 +134,18 @@ public class SecureVaultManager { public init(vault: (any AutofillSecureVault)? = nil, passwordManager: PasswordManager? = nil, includePartialAccountMatches: Bool = false, + shouldAllowPartialFormSaves: Bool = false, tld: TLD? = nil) { self.vault = vault self.passwordManager = passwordManager self.includePartialAccountMatches = includePartialAccountMatches + self.shouldAllowPartialFormSaves = shouldAllowPartialFormSaves self.tld = tld + if let tld { + partialFormSaveManager = PartialFormSaveManager(tld: tld) + } else { + partialFormSaveManager = PartialFormSaveManager(tld: TLD()) + } } } @@ -213,6 +223,19 @@ extension SecureVaultManager: AutofillSecureVaultDelegate { } var autofilldata = data + + if shouldAllowPartialFormSaves, + let partialAccountUsername = partialFormSaveManager.partialAccount(forDomain: domain)?.username, + !partialAccountUsername.isEmpty, + let credentials = autofilldata.credentials, + credentials.username.isNilOrEmpty { + autofilldata.credentials = .init( + username: partialAccountUsername, + password: credentials.password, + autogenerated: credentials.autogenerated + ) + } + let vault = try? self.vault ?? AutofillSecureVaultFactory.makeVault(reporter: self.delegate) var autoSavedCredentials: SecureVaultModels.WebsiteCredentials? @@ -240,17 +263,13 @@ extension SecureVaultManager: AutofillSecureVaultDelegate { autosaveAccount = nil } - if autofilldata.trigger == .emailProtection { + switch autofilldata.trigger { + case .emailProtection: autogeneratedUserName = data.credentials?.autogenerated ?? false - } - - if autofilldata.trigger == .passwordGeneration { + case .passwordGeneration: autogeneratedPassword = data.credentials?.autogenerated ?? false NotificationCenter.default.post(name: .autofillFillEvent, object: nil) - } - - // Account for cases when the user has manually changed an autogenerated password or private email - if autofilldata.trigger == .formSubmission { + case .formSubmission: if autosaveAccount != nil, let credentials = autoSavedCredentials { let existingUsername = credentials.account.username @@ -267,6 +286,13 @@ extension SecureVaultManager: AutofillSecureVaultDelegate { } } + case .partialSave: + guard shouldAllowPartialFormSaves else { return } + let username = data.credentials?.username ?? data.identity?.emailAddress + let partialAccount = SecureVaultModels.WebsiteAccount(username: username, domain: domain) + partialFormSaveManager.store(partialAccount: partialAccount, for: domain) + return + case .none, .userInitiated, .autoprompt: break } autofilldata.credentials?.autogenerated = autogeneratedCredentials @@ -287,6 +313,7 @@ extension SecureVaultManager: AutofillSecureVaultDelegate { delegate?.secureVaultManager(self, promptUserToStoreAutofillData: dataToPrompt, withTrigger: data.trigger) autosaveAccount = nil autosaveAccountCreatedInSession = false + partialFormSaveManager.removePartialAccount(for: domain) } return } else { @@ -304,6 +331,7 @@ extension SecureVaultManager: AutofillSecureVaultDelegate { dataToPrompt.automaticallySavedCredentials = autogeneratedCredentials autosaveAccount = nil autosaveAccountCreatedInSession = false + partialFormSaveManager.removePartialAccount(for: domain) } delegate?.secureVaultManager(self, promptUserToStoreAutofillData: dataToPrompt, withTrigger: autofilldata.trigger) } @@ -676,12 +704,11 @@ extension SecureVaultManager: AutofillSecureVaultDelegate { guard let accounts = try? vault.accountsFor(domain: domain), // Matching account (username) or account with empty username for domain - var account = accounts.first(where: { $0.username == credentials.username || $0.username == "" }) else { + var account = accounts.first(where: { $0.username == credentials.username || $0.username.isNilOrEmpty }) else { // No existing credentials found. This is a new entry let account = SecureVaultModels.WebsiteAccount(username: credentials.username ?? "", domain: domain) return SecureVaultModels.WebsiteCredentials(account: account, password: passwordData) - } guard let existingAccountId = account.id, @@ -702,7 +729,7 @@ extension SecureVaultManager: AutofillSecureVaultDelegate { } // Prompt to update the login on submit when previous username was empty (there was a partial password account) - if existingCredentials.account.username == "" { + if existingCredentials.account.username.isNilOrEmpty { account.username = autofillData.credentials?.username ?? "" return SecureVaultModels.WebsiteCredentials(account: account, password: passwordData) } diff --git a/Sources/Common/Extensions/DateExtension.swift b/Sources/Common/Extensions/DateExtension.swift index 2a4ca74ea..4ae7be7f6 100644 --- a/Sources/Common/Extensions/DateExtension.swift +++ b/Sources/Common/Extensions/DateExtension.swift @@ -139,4 +139,8 @@ public extension Date { self > Date().addingTimeInterval(Double(-days) * 24 * 60 * 60) } + func isLessThan(minutesAgo minutes: Int) -> Bool { + self > Date().addingTimeInterval(Double(-minutes) * 60) + } + } diff --git a/Sources/Common/Extensions/StringExtension.swift b/Sources/Common/Extensions/StringExtension.swift index 9282a43b4..31a2481c5 100644 --- a/Sources/Common/Extensions/StringExtension.swift +++ b/Sources/Common/Extensions/StringExtension.swift @@ -495,3 +495,12 @@ public extension StringProtocol { } } + +public extension Optional where Wrapped == String { + var isNilOrEmpty: Bool { + if case .some(let wrapped) = self { + return wrapped.isEmpty + } + return true + } +} diff --git a/Tests/BrowserServicesKitTests/SecureVault/MockAutofillDatabaseProvider.swift b/Tests/BrowserServicesKitTests/SecureVault/MockAutofillDatabaseProvider.swift index d116f3f94..21623f840 100644 --- a/Tests/BrowserServicesKitTests/SecureVault/MockAutofillDatabaseProvider.swift +++ b/Tests/BrowserServicesKitTests/SecureVault/MockAutofillDatabaseProvider.swift @@ -19,6 +19,7 @@ import Foundation import GRDB import SecureStorage +import Common @testable import BrowserServicesKit @@ -57,8 +58,11 @@ internal class MockAutofillDatabaseProvider: AutofillDatabaseProvider { _credentialsDict[accountID] = credentials return accountID } else { + var credentialsToStore = credentials let id = Int64(_credentialsDict.count + 1) - _credentialsDict[id] = credentials + credentialsToStore.account.id = String(id) + _credentialsDict[id] = credentialsToStore + _accounts.append(credentialsToStore.account) return id } } @@ -68,11 +72,15 @@ internal class MockAutofillDatabaseProvider: AutofillDatabaseProvider { } func websiteCredentialsForDomain(_ domain: String) throws -> [BrowserServicesKit.SecureVaultModels.WebsiteCredentials] { - return _credentialsForDomainDict[domain] ?? [] + return _credentialsForDomainDict[domain] ?? _credentialsDict.values.filter { + $0.account.domain == domain + } } func websiteCredentialsForTopLevelDomain(_ eTLDplus1: String) throws -> [BrowserServicesKit.SecureVaultModels.WebsiteCredentials] { - return _credentialsForDomainDict[eTLDplus1] ?? [] + return _credentialsForDomainDict[eTLDplus1] ?? _credentialsDict.values.filter { + TLD().eTLDplus1($0.account.domain) == eTLDplus1 + } } func websiteAccountsForDomain(_ domain: String) throws -> [SecureVaultModels.WebsiteAccount] { diff --git a/Tests/BrowserServicesKitTests/SecureVault/SecureVaultManagerTests.swift b/Tests/BrowserServicesKitTests/SecureVault/SecureVaultManagerTests.swift index 71e65449a..2da97e0aa 100644 --- a/Tests/BrowserServicesKitTests/SecureVault/SecureVaultManagerTests.swift +++ b/Tests/BrowserServicesKitTests/SecureVault/SecureVaultManagerTests.swift @@ -66,7 +66,7 @@ class SecureVaultManagerTests: XCTestCase { self.testVault = DefaultAutofillSecureVault(providers: providers) self.secureVaultManagerDelegate = MockSecureVaultManagerDelegate() - self.manager = SecureVaultManager(vault: self.testVault) + self.manager = SecureVaultManager(vault: self.testVault, shouldAllowPartialFormSaves: true) self.manager.delegate = secureVaultManagerDelegate } @@ -854,6 +854,94 @@ class SecureVaultManagerTests: XCTestCase { } + func testWhenFormSubmittedWithNilUsername_afterPartialSaveWithUsername_storesAndPromptsWithFullCredentials() { + let partialCredentials = AutofillUserScript.IncomingCredentials(username: "email@example.com", password: nil, autogenerated: false) + let partialData = AutofillUserScript.DetectedAutofillData(identity: nil, credentials: partialCredentials, creditCard: nil, trigger: .partialSave) + manager.autofillUserScript(mockAutofillUserScript, didRequestStoreDataForDomain: "fill.dev", data: partialData) + + var incomingCredentials = AutofillUserScript.IncomingCredentials(username: nil, password: "m4nu4lP4sswOrd", autogenerated: false) + var incomingData = AutofillUserScript.DetectedAutofillData(identity: nil, credentials: incomingCredentials, creditCard: nil, trigger: .formSubmission) + manager.autofillUserScript(mockAutofillUserScript, didRequestStoreDataForDomain: "fill.dev", data: incomingData) + + // Check stored + incomingCredentials = AutofillUserScript.IncomingCredentials(username: "email@example.com", password: "m4nu4lP4sswOrd", autogenerated: false) + incomingData = AutofillUserScript.DetectedAutofillData(identity: nil, credentials: incomingCredentials, creditCard: nil, trigger: .formSubmission) + let entries = try? manager.existingEntries(for: "fill.dev", autofillData: incomingData) + XCTAssertEqual(entries?.credentials?.account.username, "email@example.com") + XCTAssertEqual(entries?.credentials?.password, Data("m4nu4lP4sswOrd".utf8)) + + // Check prompted + XCTAssertNotNil(secureVaultManagerDelegate.promptedAutofillData) + XCTAssertEqual(secureVaultManagerDelegate.promptedAutofillData?.credentials?.account.username, "email@example.com") + XCTAssertEqual(secureVaultManagerDelegate.promptedAutofillData?.credentials?.password, Data("m4nu4lP4sswOrd".utf8)) + } + + func testWhenFormSubmittedWithNilUsername_afterPartialSaveWithUsername_domainsDifferent_onlyStoresTheFormSubmission() { + let partialCredentials = AutofillUserScript.IncomingCredentials(username: "email@example.com", password: nil, autogenerated: false) + let partialData = AutofillUserScript.DetectedAutofillData(identity: nil, credentials: partialCredentials, creditCard: nil, trigger: .partialSave) + manager.autofillUserScript(mockAutofillUserScript, didRequestStoreDataForDomain: "dill.fev", data: partialData) + + var incomingCredentials = AutofillUserScript.IncomingCredentials(username: nil, password: "m4nu4lP4sswOrd", autogenerated: false) + var incomingData = AutofillUserScript.DetectedAutofillData(identity: nil, credentials: incomingCredentials, creditCard: nil, trigger: .formSubmission) + manager.autofillUserScript(mockAutofillUserScript, didRequestStoreDataForDomain: "fill.dev", data: incomingData) + + // Check stored + incomingCredentials = AutofillUserScript.IncomingCredentials(username: nil, password: "m4nu4lP4sswOrd", autogenerated: false) + incomingData = AutofillUserScript.DetectedAutofillData(identity: nil, credentials: incomingCredentials, creditCard: nil, trigger: .formSubmission) + let entries = try? manager.existingEntries(for: "fill.dev", autofillData: incomingData) + XCTAssertNotEqual(entries?.credentials?.account.username, "email@example.com") + XCTAssertEqual(entries?.credentials?.password, Data("m4nu4lP4sswOrd".utf8)) + + // Check prompted + XCTAssertNotNil(secureVaultManagerDelegate.promptedAutofillData) + XCTAssertNotEqual(secureVaultManagerDelegate.promptedAutofillData?.credentials?.account.username, "email@example.com") + XCTAssertEqual(secureVaultManagerDelegate.promptedAutofillData?.credentials?.password, Data("m4nu4lP4sswOrd".utf8)) + } + + func testWhenFormSubmittedWithNilUsername_afterPartialSaveWithUsername_updatesExistingUsernameOnlyStoredData() { + // Check initial stored + let initialCredentials = SecureVaultModels.WebsiteCredentials(account: .init(username: "email@example.com", domain: "fill.dev"), password: nil) + guard let theID = try? testVault.storeWebsiteCredentials(initialCredentials) else { + XCTFail("Couldn't store initial credentials") + return + } + + let partialCredentials = AutofillUserScript.IncomingCredentials(username: "email@example.com", password: nil, autogenerated: false) + let partialData = AutofillUserScript.DetectedAutofillData(identity: nil, credentials: partialCredentials, creditCard: nil, trigger: .partialSave) + manager.autofillUserScript(mockAutofillUserScript, didRequestStoreDataForDomain: "fill.dev", data: partialData) + + let incomingCredentials = AutofillUserScript.IncomingCredentials(username: nil, password: "m4nu4lP4sswOrd", autogenerated: false) + let incomingData = AutofillUserScript.DetectedAutofillData(identity: nil, credentials: incomingCredentials, creditCard: nil, trigger: .formSubmission) + manager.autofillUserScript(mockAutofillUserScript, didRequestStoreDataForDomain: "fill.dev", data: incomingData) + + // Check prompted + XCTAssertNotNil(secureVaultManagerDelegate.promptedAutofillData) + // Prompting with an account with ID will result in an update + XCTAssertEqual(secureVaultManagerDelegate.promptedAutofillData?.credentials?.account.id, String(theID)) + } + + func testWhenFormSubmittedWithNilUsername_afterPartialSaveWithUsername_updatesExistingPasswordOnlyStoredData() { + // Check initial stored + let initialCredentials = SecureVaultModels.WebsiteCredentials(account: .init(username: nil, domain: "fill.dev"), password: "m4nu4lP4sswOrd".data(using: .utf8)) + guard let theID = try? testVault.storeWebsiteCredentials(initialCredentials) else { + XCTFail("Couldn't store initial credentials") + return + } + + let partialCredentials = AutofillUserScript.IncomingCredentials(username: "email@example.com", password: nil, autogenerated: false) + let partialData = AutofillUserScript.DetectedAutofillData(identity: nil, credentials: partialCredentials, creditCard: nil, trigger: .partialSave) + manager.autofillUserScript(mockAutofillUserScript, didRequestStoreDataForDomain: "fill.dev", data: partialData) + + let incomingCredentials = AutofillUserScript.IncomingCredentials(username: nil, password: "m4nu4lP4sswOrd", autogenerated: false) + let incomingData = AutofillUserScript.DetectedAutofillData(identity: nil, credentials: incomingCredentials, creditCard: nil, trigger: .formSubmission) + manager.autofillUserScript(mockAutofillUserScript, didRequestStoreDataForDomain: "fill.dev", data: incomingData) + + // Check prompted + XCTAssertNotNil(secureVaultManagerDelegate.promptedAutofillData) + // Prompting with an account with ID will result in an update + XCTAssertEqual(secureVaultManagerDelegate.promptedAutofillData?.credentials?.account.id, String(theID)) + } + // MARK: - Test Utilities private func identity(id: Int64? = nil, name: (String, String, String), addressStreet: String?) -> SecureVaultModels.Identity { From 6c335fb4355e7128dd9fb9fd25ac3de8942846ad Mon Sep 17 00:00:00 2001 From: amddg44 Date: Mon, 9 Dec 2024 20:02:10 +0100 Subject: [PATCH 07/12] iOS System level credential provider (#1127) Task/Issue URL: https://app.asana.com/0/72649045549333/1207512172220035/f iOS PR: duckduckgo/iOS#3699 macOS PR: duckduckgo/macos-browser#3628 What kind of version bump will this require?: Minor Description: Adds support for system level credential provider changes on iOS --- .../ASCredentialIdentityStoring.swift | 37 +++ ...tofillCredentialIdentityStoreManager.swift | 272 ++++++++++++++++++ .../AutofillDatabaseProvider.swift | 51 +++- .../AutofillKeyStoreProvider.swift | 132 +++++++-- .../SecureVault/FileStorageManaging.swift | 94 ++++++ .../SecureVault/SecureVaultModels.swift | 32 +++ .../MockKeystoreProvider.swift | 13 + .../Credentials/CredentialsProvider.swift | 16 +- .../internal/CredentialsResponseHandler.swift | 19 +- .../AppGroupFileStorageManagerTests.swift | 116 ++++++++ ...lCredentialIdentityStoreManagerTests.swift | 207 +++++++++++++ .../AutofillKeyStoreProviderTests.swift | 95 +++++- .../SecureVault/DatabaseProviderTests.swift | 7 + .../MockASCredentialIdentityStore.swift | 125 ++++++++ .../SecureVault/SecureVaultModelTests.swift | 155 ++++++++++ .../SecureVaultSyncableCredentialsTests.swift | 7 + .../CredentialsProviderTestsBase.swift | 3 +- 17 files changed, 1350 insertions(+), 31 deletions(-) create mode 100644 Sources/BrowserServicesKit/SecureVault/ASCredentialIdentityStoring.swift create mode 100644 Sources/BrowserServicesKit/SecureVault/AutofillCredentialIdentityStoreManager.swift create mode 100644 Sources/BrowserServicesKit/SecureVault/FileStorageManaging.swift create mode 100644 Tests/BrowserServicesKitTests/SecureVault/AppGroupFileStorageManagerTests.swift create mode 100644 Tests/BrowserServicesKitTests/SecureVault/AutofillCredentialIdentityStoreManagerTests.swift create mode 100644 Tests/BrowserServicesKitTests/SecureVault/MockASCredentialIdentityStore.swift diff --git a/Sources/BrowserServicesKit/SecureVault/ASCredentialIdentityStoring.swift b/Sources/BrowserServicesKit/SecureVault/ASCredentialIdentityStoring.swift new file mode 100644 index 000000000..a8523d9b9 --- /dev/null +++ b/Sources/BrowserServicesKit/SecureVault/ASCredentialIdentityStoring.swift @@ -0,0 +1,37 @@ +// +// ASCredentialIdentityStoring.swift +// +// Copyright © 2024 DuckDuckGo. All rights reserved. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +// + +import Foundation +import AuthenticationServices + +// This is used to abstract the ASCredentialIdentityStore for testing purposes +public protocol ASCredentialIdentityStoring { + func state() async -> ASCredentialIdentityStoreState + func saveCredentialIdentities(_ credentials: [ASPasswordCredentialIdentity]) async throws + func removeCredentialIdentities(_ credentials: [ASPasswordCredentialIdentity]) async throws + func replaceCredentialIdentities(with newCredentials: [ASPasswordCredentialIdentity]) async throws + + @available(iOS 17.0, macOS 14.0, *) + func saveCredentialIdentities(_ credentials: [ASCredentialIdentity]) async throws + @available(iOS 17.0, macOS 14.0, *) + func removeCredentialIdentities(_ credentials: [ASCredentialIdentity]) async throws + @available(iOS 17.0, macOS 14.0, *) + func replaceCredentialIdentities(_ newCredentials: [ASCredentialIdentity]) async throws +} + +extension ASCredentialIdentityStore: ASCredentialIdentityStoring {} diff --git a/Sources/BrowserServicesKit/SecureVault/AutofillCredentialIdentityStoreManager.swift b/Sources/BrowserServicesKit/SecureVault/AutofillCredentialIdentityStoreManager.swift new file mode 100644 index 000000000..7ecdc5012 --- /dev/null +++ b/Sources/BrowserServicesKit/SecureVault/AutofillCredentialIdentityStoreManager.swift @@ -0,0 +1,272 @@ +// +// AutofillCredentialIdentityStoreManager.swift +// +// Copyright © 2024 DuckDuckGo. All rights reserved. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +// + +import Foundation +import AuthenticationServices +import Common +import os.log + +public protocol AutofillCredentialIdentityStoreManaging { + func credentialStoreStateEnabled() async -> Bool + func populateCredentialStore() async + func replaceCredentialStore(with accounts: [SecureVaultModels.WebsiteAccount]) async + func updateCredentialStore(for domain: String) async + func updateCredentialStoreWith(updatedAccounts: [SecureVaultModels.WebsiteAccount], deletedAccounts: [SecureVaultModels.WebsiteAccount]) async +} + +final public class AutofillCredentialIdentityStoreManager: AutofillCredentialIdentityStoreManaging { + + private let credentialStore: ASCredentialIdentityStoring + private let vault: (any AutofillSecureVault)? + private let tld: TLD + + public init(credentialStore: ASCredentialIdentityStoring = ASCredentialIdentityStore.shared, + vault: (any AutofillSecureVault)?, + tld: TLD) { + self.credentialStore = credentialStore + self.vault = vault + self.tld = tld + } + + // MARK: - Credential Store State + + public func credentialStoreStateEnabled() async -> Bool { + let state = await credentialStore.state() + return state.isEnabled + } + + // MARK: - Credential Store Operations + + public func populateCredentialStore() async { + guard await credentialStoreStateEnabled() else { return } + + do { + let accounts = try fetchAccounts() + try await generateAndSaveCredentialIdentities(from: accounts) + } catch { + Logger.autofill.error("Failed to populate credential store: \(error.localizedDescription, privacy: .public)") + } + } + + public func replaceCredentialStore(with accounts: [SecureVaultModels.WebsiteAccount]) async { + guard await credentialStoreStateEnabled() else { return } + + do { + if #available(iOS 17, macOS 14.0, *) { + let credentialIdentities = try await generateCredentialIdentities(from: accounts) as [any ASCredentialIdentity] + try await replaceCredentialStoreIdentities(credentialIdentities) + } else { + let credentialIdentities = try await generateCredentialIdentities(from: accounts) as [ASPasswordCredentialIdentity] + try await replaceCredentialStoreIdentities(with: credentialIdentities) + } + } catch { + Logger.autofill.error("Failed to replace credential store: \(error.localizedDescription, privacy: .public)") + } + } + + public func updateCredentialStore(for domain: String) async { + guard await credentialStoreStateEnabled() else { return } + + do { + if await storeSupportsIncrementalUpdates() { + let accounts = try fetchAccountsFor(domain: domain) + try await generateAndSaveCredentialIdentities(from: accounts) + } else { + await replaceCredentialStore() + } + } catch { + Logger.autofill.error("Failed to update credential store \(error.localizedDescription, privacy: .public)") + } + } + + public func updateCredentialStoreWith(updatedAccounts: [SecureVaultModels.WebsiteAccount], deletedAccounts: [SecureVaultModels.WebsiteAccount]) async { + guard await credentialStoreStateEnabled() else { return } + + do { + if await storeSupportsIncrementalUpdates() { + if !updatedAccounts.isEmpty { + try await generateAndSaveCredentialIdentities(from: updatedAccounts) + } + + if !deletedAccounts.isEmpty { + try await generateAndDeleteCredentialIdentities(from: deletedAccounts) + } + } else { + await replaceCredentialStore() + } + } catch { + Logger.autofill.error("Failed to update credential store with updated / deleted accounts \(error.localizedDescription, privacy: .public)") + } + + } + + // MARK: - Private Store Operations + + private func storeSupportsIncrementalUpdates() async -> Bool { + let state = await credentialStore.state() + return state.supportsIncrementalUpdates + } + + private func replaceCredentialStore() async { + guard await credentialStoreStateEnabled() else { return } + + do { + let accounts = try fetchAccounts() + + Task { + await replaceCredentialStore(with: accounts) + } + } catch { + Logger.autofill.error("Failed to replace credential store: \(error.localizedDescription, privacy: .public)") + } + } + + private func generateAndSaveCredentialIdentities(from accounts: [SecureVaultModels.WebsiteAccount]) async throws { + if #available(iOS 17, macOS 14.0, *) { + let credentialIdentities = try await generateCredentialIdentities(from: accounts) as [any ASCredentialIdentity] + try await saveToCredentialStore(credentials: credentialIdentities) + } else { + let credentialIdentities = try await generateCredentialIdentities(from: accounts) as [ASPasswordCredentialIdentity] + try await saveToCredentialStore(credentials: credentialIdentities) + } + } + + private func generateAndDeleteCredentialIdentities(from accounts: [SecureVaultModels.WebsiteAccount]) async throws { + if #available(iOS 17, macOS 14.0, *) { + let credentialIdentities = try await generateCredentialIdentities(from: accounts) as [any ASCredentialIdentity] + try await removeCredentialStoreIdentities(credentialIdentities) + } else { + let credentialIdentities = try await generateCredentialIdentities(from: accounts) as [ASPasswordCredentialIdentity] + try await removeCredentialStoreIdentities(credentialIdentities) + } + } + + private func saveToCredentialStore(credentials: [ASPasswordCredentialIdentity]) async throws { + do { + try await credentialStore.saveCredentialIdentities(credentials) + } catch { + Logger.autofill.error("Failed to save credentials to ASCredentialIdentityStore: \(error.localizedDescription, privacy: .public)") + throw error + } + } + + @available(iOS 17.0, macOS 14.0, *) + private func saveToCredentialStore(credentials: [ASCredentialIdentity]) async throws { + do { + try await credentialStore.saveCredentialIdentities(credentials) + } catch { + Logger.autofill.error("Failed to save credentials to ASCredentialIdentityStore: \(error.localizedDescription, privacy: .public)") + throw error + } + } + + private func replaceCredentialStoreIdentities(with credentials: [ASPasswordCredentialIdentity]) async throws { + do { + try await credentialStore.replaceCredentialIdentities(with: credentials) + } catch { + Logger.autofill.error("Failed to replace credentials in ASCredentialIdentityStore: \(error.localizedDescription, privacy: .public)") + throw error + } + } + + @available(iOS 17.0, macOS 14.0, *) + private func replaceCredentialStoreIdentities(_ credentials: [ASCredentialIdentity]) async throws { + do { + try await credentialStore.replaceCredentialIdentities(credentials) + } catch { + Logger.autofill.error("Failed to replace credentials in ASCredentialIdentityStore: \(error.localizedDescription, privacy: .public)") + throw error + } + } + + private func removeCredentialStoreIdentities(_ credentials: [ASPasswordCredentialIdentity]) async throws { + do { + try await credentialStore.removeCredentialIdentities(credentials) + } catch { + Logger.autofill.error("Failed to remove credentials from ASCredentialIdentityStore: \(error.localizedDescription, privacy: .public)") + throw error + } + } + + @available(iOS 17.0, macOS 14.0, *) + private func removeCredentialStoreIdentities(_ credentials: [ASCredentialIdentity]) async throws { + do { + try await credentialStore.removeCredentialIdentities(credentials) + } catch { + Logger.autofill.error("Failed to remove credentials from ASCredentialIdentityStore: \(error.localizedDescription, privacy: .public)") + throw error + } + } + + private func generateCredentialIdentities(from accounts: [SecureVaultModels.WebsiteAccount]) async throws -> [ASPasswordCredentialIdentity] { + let sortedAndDedupedAccounts = accounts.sortedAndDeduplicated(tld: tld) + let groupedAccounts = Dictionary(grouping: sortedAndDedupedAccounts, by: { $0.domain ?? "" }) + var credentialIdentities: [ASPasswordCredentialIdentity] = [] + + for (_, accounts) in groupedAccounts { + // Since accounts are sorted, ranking can be assigned based on index + // but first need to be reversed as highest ranking should apply to the most recently used account + for (rank, account) in accounts.reversed().enumerated() { + let credentialIdentity = createPasswordCredentialIdentity(from: account) + credentialIdentity.rank = rank + credentialIdentities.append(credentialIdentity) + } + } + + return credentialIdentities + } + + private func createPasswordCredentialIdentity(from account: SecureVaultModels.WebsiteAccount) -> ASPasswordCredentialIdentity { + let serviceIdentifier = ASCredentialServiceIdentifier(identifier: account.domain ?? "", type: .domain) + return ASPasswordCredentialIdentity(serviceIdentifier: serviceIdentifier, + user: account.username ?? "", + recordIdentifier: account.id) + } + + // MARK: - Private Secure Vault Operations + + private func fetchAccounts() throws -> [SecureVaultModels.WebsiteAccount] { + guard let vault = vault else { + Logger.autofill.error("Vault not created") + return [] + } + + do { + return try vault.accounts() + } catch { + Logger.autofill.error("Failed to fetch accounts \(error.localizedDescription, privacy: .public)") + throw error + } + + } + + private func fetchAccountsFor(domain: String) throws -> [SecureVaultModels.WebsiteAccount] { + guard let vault = vault else { + Logger.autofill.error("Vault not created") + return [] + } + + do { + return try vault.accountsFor(domain: domain) + } catch { + Logger.autofill.error("Failed to fetch accounts \(error.localizedDescription, privacy: .public)") + throw error + } + + } +} diff --git a/Sources/BrowserServicesKit/SecureVault/AutofillDatabaseProvider.swift b/Sources/BrowserServicesKit/SecureVault/AutofillDatabaseProvider.swift index b145523ea..dae41804d 100644 --- a/Sources/BrowserServicesKit/SecureVault/AutofillDatabaseProvider.swift +++ b/Sources/BrowserServicesKit/SecureVault/AutofillDatabaseProvider.swift @@ -20,6 +20,7 @@ import Common import Foundation import GRDB import SecureStorage +import os.log public protocol AutofillDatabaseProvider: SecureStorageDatabaseProvider { @@ -82,14 +83,35 @@ public protocol AutofillDatabaseProvider: SecureStorageDatabaseProvider { public final class DefaultAutofillDatabaseProvider: GRDBSecureStorageDatabaseProvider, AutofillDatabaseProvider { + struct Constants { + static let dbDirectoryName = "Vault" + static let dbFileName = "Vault.db" + } + public static func defaultDatabaseURL() -> URL { - return DefaultAutofillDatabaseProvider.databaseFilePath(directoryName: "Vault", fileName: "Vault.db") + return DefaultAutofillDatabaseProvider.databaseFilePath(directoryName: Constants.dbDirectoryName, fileName: Constants.dbFileName, appGroupIdentifier: nil) + } + + public static func defaultSharedDatabaseURL() -> URL { + return DefaultAutofillDatabaseProvider.databaseFilePath(directoryName: Constants.dbDirectoryName, fileName: Constants.dbFileName, appGroupIdentifier: Bundle.main.appGroupPrefix + ".vault") } public init(file: URL = DefaultAutofillDatabaseProvider.defaultDatabaseURL(), key: Data, + fileStorageManager: FileStorageManaging = AppGroupFileStorageManager(), customMigrations: ((inout DatabaseMigrator) -> Void)? = nil) throws { - try super.init(file: file, key: key, writerType: .queue) { migrator in + let databaseURL: URL + +#if os(iOS) + databaseURL = Self.migrateDatabaseToSharedGroupIfNeeded(using: fileStorageManager, + from: Self.defaultDatabaseURL(), + to: Self.defaultSharedDatabaseURL()) +#else + // macOS stays in its sandbox location + databaseURL = file +#endif + + try super.init(file: databaseURL, key: key, writerType: .queue) { migrator in if let customMigrations { customMigrations(&migrator) } else { @@ -110,6 +132,17 @@ public final class DefaultAutofillDatabaseProvider: GRDBSecureStorageDatabasePro } } + private static func migrateDatabaseToSharedGroupIfNeeded(using fileStorageManager: FileStorageManaging = AppGroupFileStorageManager(), + from databaseURL: URL, + to sharedDatabaseURL: URL) -> URL { + do { + return try fileStorageManager.migrateDatabaseToSharedStorageIfNeeded(from: databaseURL, to: sharedDatabaseURL) + } catch { + Logger.secureStorage.error("Failed to migrate database to shared storage: \(error.localizedDescription)") + return databaseURL + } + } + public func inTransaction(_ block: @escaping (GRDB.Database) throws -> Void) throws { try db.write { database in try block(database) @@ -1412,3 +1445,17 @@ extension SecureVaultModels.Identity: PersistableRecord, FetchableRecord { public static var databaseTableName: String = "identities" } + +private extension Bundle { + var appGroupPrefix: String { + let groupIdPrefixKey = "DuckDuckGoGroupIdentifierPrefix" + guard let groupIdPrefix = Bundle.main.object(forInfoDictionaryKey: groupIdPrefixKey) as? String else { + #if DEBUG && os(iOS) + return "group.com.duckduckgo.test" + #else + fatalError("Info.plist must contain a \"\(groupIdPrefixKey)\" entry with a string value") + #endif + } + return groupIdPrefix + } +} diff --git a/Sources/BrowserServicesKit/SecureVault/AutofillKeyStoreProvider.swift b/Sources/BrowserServicesKit/SecureVault/AutofillKeyStoreProvider.swift index 0a22e1f0a..b3eed9b99 100644 --- a/Sources/BrowserServicesKit/SecureVault/AutofillKeyStoreProvider.swift +++ b/Sources/BrowserServicesKit/SecureVault/AutofillKeyStoreProvider.swift @@ -21,12 +21,55 @@ import Foundation import SecureStorage import os.log +protocol KeyStorePlatformProviding { + var keychainServiceName: String { get } + func keychainIdentifier(for rawValue: String) -> String + var keychainSecurityGroup: String { get } +} + +struct iOSKeyStorePlatformProvider: KeyStorePlatformProviding { + private let appGroupName: String + + // Using appGroupName in the initializer, allowing injection for tests + init(appGroupName: String = Bundle.main.appGroupName) { + self.appGroupName = appGroupName + } + + var keychainServiceName: String { + return AutofillKeyStoreProvider.Constants.v4ServiceName + } + + func keychainIdentifier(for rawValue: String) -> String { + return appGroupName + rawValue + } + + var keychainSecurityGroup: String { + return appGroupName + } +} + +struct macOSKeyStorePlatformProvider: KeyStorePlatformProviding { + var keychainServiceName: String { + return AutofillKeyStoreProvider.Constants.v3ServiceName + } + + func keychainIdentifier(for rawValue: String) -> String { + return (Bundle.main.bundleIdentifier ?? "com.duckduckgo") + rawValue + } + + var keychainSecurityGroup: String { + return "" + } + +} + final class AutofillKeyStoreProvider: SecureStorageKeyStoreProvider { struct Constants { static let v1ServiceName = "DuckDuckGo Secure Vault" static let v2ServiceName = "DuckDuckGo Secure Vault v2" static let v3ServiceName = "DuckDuckGo Secure Vault v3" + static let v4ServiceName = "DuckDuckGo Secure Vault v4" } // DO NOT CHANGE except if you want to deliberately invalidate all users's vaults. @@ -38,7 +81,12 @@ final class AutofillKeyStoreProvider: SecureStorageKeyStoreProvider { case l2Key = "A5711F4D-7AA5-4F0C-9E4F-BE553F1EA299" // `keychainIdentifier` should be used as Keychain Account names, as app variants (e.g App Store, DMG) should have separate entries - var keychainIdentifier: String { + func keychainIdentifier(using platformProvider: KeyStorePlatformProviding) -> String { + return platformProvider.keychainIdentifier(for: self.rawValue) + } + + // `legacyKeychainIdentifier` is the Keychain Account name pre migration to shared app groups (currently only on iOS) + var legacyKeychainIdentifier: String { (Bundle.main.bundleIdentifier ?? "com.duckduckgo") + rawValue } @@ -53,13 +101,13 @@ final class AutofillKeyStoreProvider: SecureStorageKeyStoreProvider { } } - init?(_ keyValue: String) { + init?(_ keyValue: String, using platformProvider: KeyStorePlatformProviding) { switch keyValue { - case EntryName.generatedPassword.keychainIdentifier: + case platformProvider.keychainIdentifier(for: EntryName.generatedPassword.rawValue), EntryName.generatedPassword.legacyKeychainIdentifier: self = .generatedPassword - case EntryName.l1Key.keychainIdentifier: + case platformProvider.keychainIdentifier(for: EntryName.l1Key.rawValue), EntryName.l1Key.legacyKeychainIdentifier: self = .l1Key - case EntryName.l2Key.keychainIdentifier: + case platformProvider.keychainIdentifier(for: EntryName.l2Key.rawValue), EntryName.l2Key.legacyKeychainIdentifier: self = .l2Key default: return nil @@ -69,27 +117,40 @@ final class AutofillKeyStoreProvider: SecureStorageKeyStoreProvider { let keychainService: KeychainService private var reporter: SecureVaultReporting? + private let platformProvider: KeyStorePlatformProviding init(keychainService: KeychainService = DefaultKeychainService(), - reporter: SecureVaultReporting? = nil) { + reporter: SecureVaultReporting? = nil, + platformProvider: KeyStorePlatformProviding? = nil) { self.keychainService = keychainService self.reporter = reporter + + // Use default platform provider based on the platform. + if let platformProvider = platformProvider { + self.platformProvider = platformProvider + } else { +#if os(iOS) + self.platformProvider = iOSKeyStorePlatformProvider() +#else + self.platformProvider = macOSKeyStorePlatformProvider() +#endif + } } var keychainServiceName: String { - return Constants.v3ServiceName + return platformProvider.keychainServiceName } var generatedPasswordEntryName: String { - return EntryName.generatedPassword.keychainIdentifier + return EntryName.generatedPassword.keychainIdentifier(using: platformProvider) } var l1KeyEntryName: String { - return EntryName.l1Key.keychainIdentifier + return EntryName.l1Key.keychainIdentifier(using: platformProvider) } var l2KeyEntryName: String { - return EntryName.l2Key.keychainIdentifier + return EntryName.l2Key.keychainIdentifier(using: platformProvider) } func readData(named name: String, serviceName: String) throws -> Data? { @@ -103,15 +164,19 @@ final class AutofillKeyStoreProvider: SecureStorageKeyStoreProvider { /// - Returns: Optional data private func readOrMigrate(named name: String, serviceName: String) throws -> Data? { if let data = try read(named: name, serviceName: serviceName) { - Logger.autofill.debug("Autofill Keystore data retrieved") + Logger.autofill.debug("Autofill Keystore \(serviceName) data retrieved") return data } else { - guard let entryName = EntryName(name) else { return nil } + guard let entryName = EntryName(name, using: platformProvider) else { return nil } reporter?.secureVaultKeyStoreEvent(entryName.keyStoreMigrationEvent) + // If V4 migration, look for items in V3 vault (i.e pre-shared Keychain storage) + if isPostV3(serviceName), let data = try migrateEntry(entryName: entryName, serviceName: Constants.v3ServiceName) { + Logger.autofill.debug("Migrated V3 Autofill Keystore data") + return data // Look for items in V2 vault (i.e pre-bundle-specifc Keychain storage) - if let data = try migrateEntry(entryName: entryName, serviceName: Constants.v2ServiceName) { + } else if let data = try migrateEntry(entryName: entryName, serviceName: Constants.v2ServiceName) { Logger.autofill.debug("Migrated V2 Autofill Keystore data") return data // Look for items in V1 vault @@ -120,6 +185,7 @@ final class AutofillKeyStoreProvider: SecureStorageKeyStoreProvider { return data } + Logger.autofill.debug("Keychain migration failed for \(name)") return nil } } @@ -139,7 +205,7 @@ final class AutofillKeyStoreProvider: SecureStorageKeyStoreProvider { let status = keychainService.itemMatching(query, &item) switch status { case errSecSuccess: - if isPostV1(serviceName) { + if isPostV1(serviceName) || isPostV3(serviceName) { guard let itemData = item as? Data, let itemString = String(data: itemData, encoding: .utf8), let decodedData = Data(base64Encoded: itemString) else { @@ -162,14 +228,15 @@ final class AutofillKeyStoreProvider: SecureStorageKeyStoreProvider { /// Migrates an entry to new bundle-specific Keychain storage /// - Parameters: - /// - entryName: Entry to migrate. It's `rawValue` is used when reading from old storage, and it's `keyValue` is used when writing to storage + /// - entryName: Entry to migrate. It's `rawValue` is used when reading from old storage pre-V2, while its `legacyKeychainIdentifier` is used post V2, and it's `keyValue` is used when writing to storage /// - serviceName: Service name to use when querying Keychain for the entry /// - Returns: Optional data private func migrateEntry(entryName: EntryName, serviceName: String) throws -> Data? { - guard let data = try read(named: entryName.rawValue, serviceName: serviceName) else { + let name = serviceName == Constants.v3ServiceName ? entryName.legacyKeychainIdentifier : entryName.rawValue + guard let data = try read(named: name, serviceName: serviceName) else { return nil } - try writeData(data, named: entryName.keychainIdentifier, serviceName: keychainServiceName) + try writeData(data, named: entryName.keychainIdentifier(using: platformProvider), serviceName: keychainServiceName) return data } @@ -177,11 +244,17 @@ final class AutofillKeyStoreProvider: SecureStorageKeyStoreProvider { [Constants.v2ServiceName, Constants.v3ServiceName].contains(serviceName) } + private func isPostV3(_ serviceName: String) -> Bool { + [Constants.v4ServiceName].contains(serviceName) + } + // MARK: - Autofill Attributes func attributesForEntry(named name: String, serviceName: String) -> [String: Any] { if isPostV1(serviceName) { return defaultAttributesForEntry(named: name) + } else if isPostV3(serviceName) { + return defaultAttributesForSharedEntry(named: name) } else { return legacyAttributesForEntry(named: name) } @@ -205,4 +278,29 @@ final class AutofillKeyStoreProvider: SecureStorageKeyStoreProvider { ] as [String: Any] } + private func defaultAttributesForSharedEntry(named name: String) -> [String: Any] { + return [ + kSecClass: kSecClassGenericPassword, + kSecUseDataProtectionKeychain: false, + kSecAttrSynchronizable: false, + kSecAttrAccount: name, + kSecAttrAccessGroup: platformProvider.keychainSecurityGroup + ] as [String: Any] + } +} + +fileprivate extension Bundle { + + static let vaultAppGroupName = "VAULT_APP_GROUP" + + var appGroupName: String { + guard let appGroup = object(forInfoDictionaryKey: Bundle.vaultAppGroupName) as? String else { + #if DEBUG && os(iOS) + return "com.duckduckgo.vault.test" + #else + fatalError("Info.plist is missing \(Bundle.vaultAppGroupName)") + #endif + } + return appGroup + } } diff --git a/Sources/BrowserServicesKit/SecureVault/FileStorageManaging.swift b/Sources/BrowserServicesKit/SecureVault/FileStorageManaging.swift new file mode 100644 index 000000000..fdcf6e210 --- /dev/null +++ b/Sources/BrowserServicesKit/SecureVault/FileStorageManaging.swift @@ -0,0 +1,94 @@ +// +// FileStorageManaging.swift +// +// Copyright © 2024 DuckDuckGo. All rights reserved. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +// + +import Foundation +import os.log + +public protocol FileStorageManaging { + func migrateDatabaseToSharedStorageIfNeeded(from databaseURL: URL, to sharedDatabaseURL: URL) throws -> URL +} + +final public class AppGroupFileStorageManager: FileStorageManaging { + + private let fileManager: FileManager + + public init(fileManager: FileManager = FileManager.default) { + self.fileManager = fileManager + } + + private func fileExists(at url: URL) -> Bool { + return fileManager.fileExists(atPath: url.path) + } + + private func createDirectoryIfNeeded(at url: URL) throws { + let directoryPath = url.path + if !fileManager.fileExists(atPath: directoryPath) { + do { + try fileManager.createDirectory(at: url, withIntermediateDirectories: true, attributes: nil) + Logger.secureStorage.info("Created directory at \(directoryPath)") + } catch { + Logger.secureStorage.error("Failed to create directory: \(error.localizedDescription)") + throw error + } + } + } + + private func copyFile(from sourceURL: URL, to destinationURL: URL) throws { + do { + try fileManager.copyItem(at: sourceURL, to: destinationURL) + } catch { + Logger.secureStorage.error("Error moving file: \(error.localizedDescription)") + throw error + } + } + + private func removeFile(at url: URL) throws { + do { + try fileManager.removeItem(at: url) + Logger.secureStorage.info("Removed file at \(url.path)") + } catch { + Logger.secureStorage.error("Error removing file: \(error.localizedDescription)") + throw error + } + } + + public func migrateDatabaseToSharedStorageIfNeeded(from databaseURL: URL, to sharedDatabaseURL: URL) throws -> URL { + if fileExists(at: sharedDatabaseURL) { + return sharedDatabaseURL + } + + do { + // Ensure the shared group directory exists + try createDirectoryIfNeeded(at: sharedDatabaseURL.deletingLastPathComponent()) + + if fileExists(at: databaseURL) { + try copyFile(from: databaseURL, to: sharedDatabaseURL) + + // If the copy was successful, delete the original file + try removeFile(at: databaseURL) + + return sharedDatabaseURL + } + } catch { + Logger.secureStorage.error("Failed to migrate Vault.db: \(error.localizedDescription)") + throw error + } + + return sharedDatabaseURL + } +} diff --git a/Sources/BrowserServicesKit/SecureVault/SecureVaultModels.swift b/Sources/BrowserServicesKit/SecureVault/SecureVaultModels.swift index 688cc77cc..2ba635f1f 100644 --- a/Sources/BrowserServicesKit/SecureVault/SecureVaultModels.swift +++ b/Sources/BrowserServicesKit/SecureVault/SecureVaultModels.swift @@ -569,6 +569,38 @@ extension Array where Element == SecureVaultModels.WebsiteAccount { return (removeDuplicates ? result.removeDuplicates() : result).filter { $0.domain?.isEmpty == false } } + public func sortedAndDeduplicated(tld: TLD, urlMatcher: AutofillDomainNameUrlMatcher = AutofillDomainNameUrlMatcher()) -> [SecureVaultModels.WebsiteAccount] { + + let groupedBySignature = Dictionary(grouping: self) { $0.signature ?? "" } + + let deduplicatedAccounts = groupedBySignature + .flatMap { (signature, accounts) -> [SecureVaultModels.WebsiteAccount] in + + // no need to dedupe accounts with no signature, or where a signature group only has 1 account + if signature.isEmpty || accounts.count == 1 { + return accounts + } + + // This set is required as accounts can have duplicate signatures but different domains if the domain has a SLD + TLD like `co.uk` + // e.g. accounts with the same username & password for `example.co.uk` and `domain.co.uk` will have the same signature + var uniqueHosts = Set() + + for account in accounts { + if let domain = account.domain, + let urlComponents = urlMatcher.normalizeSchemeForAutofill(domain), + let host = urlComponents.eTLDplus1(tld: tld) ?? urlComponents.host { + uniqueHosts.insert(host) + } + } + + return uniqueHosts.flatMap { host in + accounts.sortedForDomain(host, tld: tld, removeDuplicates: true) + } + } + + return deduplicatedAccounts.sorted { compareAccount($0, $1) } + } + private func extractTLD(domain: String, tld: TLD, urlMatcher: AutofillDomainNameUrlMatcher) -> String? { guard var urlComponents = urlMatcher.normalizeSchemeForAutofill(domain) else { return nil } guard urlComponents.host != .localhost else { return domain } diff --git a/Sources/SecureStorageTestsUtils/MockKeystoreProvider.swift b/Sources/SecureStorageTestsUtils/MockKeystoreProvider.swift index 689641386..dc47eec1f 100644 --- a/Sources/SecureStorageTestsUtils/MockKeystoreProvider.swift +++ b/Sources/SecureStorageTestsUtils/MockKeystoreProvider.swift @@ -23,6 +23,7 @@ public final class MockKeychainService: KeychainService { public enum Mode { case nothingFound + case v4Found case v3Found case v2Found case v1Found @@ -55,9 +56,21 @@ public final class MockKeychainService: KeychainService { switch mode { case .nothingFound: return errSecItemNotFound + case .v4Found: + setResult() + return errSecSuccess case .v3Found: +#if os(iOS) + if itemMatchingCallCount == 2 { + setResult() + return errSecSuccess + } else { + return errSecItemNotFound + } +#else setResult() return errSecSuccess +#endif case .v2Found: if itemMatchingCallCount == 2 { setResult() diff --git a/Sources/SyncDataProviders/Credentials/CredentialsProvider.swift b/Sources/SyncDataProviders/Credentials/CredentialsProvider.swift index 1110adfe2..10ad4386b 100644 --- a/Sources/SyncDataProviders/Credentials/CredentialsProvider.swift +++ b/Sources/SyncDataProviders/Credentials/CredentialsProvider.swift @@ -25,19 +25,30 @@ import GRDB import SecureStorage import os.log +public struct CredentialsInput { + public var modifiedAccounts: [SecureVaultModels.WebsiteAccount] + public var deletedAccounts: [SecureVaultModels.WebsiteAccount] +} + public final class CredentialsProvider: DataProvider { + public private(set) var credentialsInput: CredentialsInput = .init(modifiedAccounts: [], deletedAccounts: []) + public init( secureVaultFactory: AutofillVaultFactory = AutofillSecureVaultFactory, secureVaultErrorReporter: SecureVaultReporting, metadataStore: SyncMetadataStore, metricsEvents: EventMapping? = nil, - syncDidUpdateData: @escaping () -> Void + syncDidUpdateData: @escaping () -> Void, + syncDidFinish: @escaping (CredentialsInput?) -> Void ) throws { self.secureVaultFactory = secureVaultFactory self.secureVaultErrorReporter = secureVaultErrorReporter self.metricsEvents = metricsEvents super.init(feature: .init(name: "credentials"), metadataStore: metadataStore, syncDidUpdateData: syncDidUpdateData) + self.syncDidFinish = { [weak self] in + syncDidFinish(self?.credentialsInput) + } } // MARK: - DataProviding @@ -166,6 +177,9 @@ public final class CredentialsProvider: DataProvider { ) try responseHandler.processReceivedCredentials() + + self.credentialsInput.modifiedAccounts = responseHandler.incomingModifiedAccounts + self.credentialsInput.deletedAccounts = responseHandler.incomingDeletedAccounts #if DEBUG try self.willSaveContextAfterApplyingSyncResponse() #endif diff --git a/Sources/SyncDataProviders/Credentials/internal/CredentialsResponseHandler.swift b/Sources/SyncDataProviders/Credentials/internal/CredentialsResponseHandler.swift index b3d00bcbb..80add013c 100644 --- a/Sources/SyncDataProviders/Credentials/internal/CredentialsResponseHandler.swift +++ b/Sources/SyncDataProviders/Credentials/internal/CredentialsResponseHandler.swift @@ -34,6 +34,9 @@ final class CredentialsResponseHandler { let allReceivedIDs: Set private var credentialsByUUID: [String: SecureVaultModels.SyncableCredentials] = [:] + var incomingModifiedAccounts = [SecureVaultModels.WebsiteAccount]() + var incomingDeletedAccounts = [SecureVaultModels.WebsiteAccount]() + private let decrypt: (String) throws -> String private let metricsEvents: EventMapping? @@ -117,6 +120,7 @@ final class CredentialsResponseHandler { if syncable.isDeleted { try secureVault.deleteSyncableCredentials(existingEntity, in: database) + trackCredentialChange(of: existingEntity, with: syncable) } else if isModifiedAfterSyncTimestamp { metricsEvents?.fire(.localTimestampResolutionTriggered(feature: feature)) } else { @@ -126,10 +130,10 @@ final class CredentialsResponseHandler { in: database, encryptedUsing: secureVaultEncryptionKey, hashedUsing: secureVaultHashingSalt) + trackCredentialChange(of: existingEntity, with: syncable) } } else if !syncable.isDeleted { - let newEntity = try SecureVaultModels.SyncableCredentials(syncable: syncable, decryptedUsing: decrypt) assert(newEntity.metadata.lastModified == nil, "lastModified should be nil for a new metadata entity") try secureVault.storeSyncableCredentials(newEntity, @@ -137,6 +141,7 @@ final class CredentialsResponseHandler { encryptedUsing: secureVaultEncryptionKey, hashedUsing: secureVaultHashingSalt) credentialsByUUID[syncableUUID] = newEntity + trackCredentialChange(of: newEntity, with: syncable) } } @@ -183,6 +188,18 @@ final class CredentialsResponseHandler { } return syncableCredentials.first(where: { $0.credentialsRecord?.password == nil }) } + + private func trackCredentialChange(of entity: SecureVaultModels.SyncableCredentials, with syncable: SyncableCredentialsAdapter) { + guard let account = entity.account else { + return + } + + if syncable.isDeleted { + incomingDeletedAccounts.append(account) + } else { + incomingModifiedAccounts.append(account) + } + } } extension SecureVaultModels.SyncableCredentials { diff --git a/Tests/BrowserServicesKitTests/SecureVault/AppGroupFileStorageManagerTests.swift b/Tests/BrowserServicesKitTests/SecureVault/AppGroupFileStorageManagerTests.swift new file mode 100644 index 000000000..ff9cda5e6 --- /dev/null +++ b/Tests/BrowserServicesKitTests/SecureVault/AppGroupFileStorageManagerTests.swift @@ -0,0 +1,116 @@ +// +// AppGroupFileStorageManagerTests.swift +// +// Copyright © 2024 DuckDuckGo. All rights reserved. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +// + +import XCTest +@testable import BrowserServicesKit + +final class AppGroupFileStorageManagerTests: XCTestCase { + + var mockFileManager: MockFileManager! + var fileStorageManager: AppGroupFileStorageManager! + + override func setUp() { + super.setUp() + mockFileManager = MockFileManager() + fileStorageManager = AppGroupFileStorageManager(fileManager: mockFileManager) + } + + func testWhenMigrateDatabaseToSharedStorageIfNeeded_ThenMigratesDatabaseSuccessfully() throws { + let originalURL = URL(fileURLWithPath: "/path/to/Vault.db") + let sharedDatabaseURL = URL(fileURLWithPath: "/shared/path/Vault.db") + + mockFileManager.files.insert(originalURL) + + let resultURL = try fileStorageManager.migrateDatabaseToSharedStorageIfNeeded(from: originalURL, to: sharedDatabaseURL) + + XCTAssertEqual(resultURL, sharedDatabaseURL, "The shared database URL should be returned after migration.") + XCTAssertTrue(mockFileManager.files.contains(sharedDatabaseURL), "Shared database should exist after migration.") + XCTAssertTrue(mockFileManager.files.contains(sharedDatabaseURL), "The file should have been copied to the shared location.") + XCTAssertFalse(mockFileManager.files.contains(originalURL), "The original file should have been deleted after a successful migration.") + } + + func testWhenMigrateDatabaseToSharedStorageIfNeeded_ThenDoesNotMigrateIfSharedDatabaseExists() throws { + let originalURL = URL(fileURLWithPath: "/path/to/Vault.db") + let sharedDatabaseURL = URL(fileURLWithPath: "/shared/path/Vault.db") + + mockFileManager.files.insert(originalURL) + mockFileManager.files.insert(sharedDatabaseURL) + + let resultURL = try fileStorageManager.migrateDatabaseToSharedStorageIfNeeded(from: originalURL, to: sharedDatabaseURL) + + XCTAssertEqual(resultURL, sharedDatabaseURL, "The shared database URL should be returned since it already exists.") + XCTAssertTrue(mockFileManager.files.contains(sharedDatabaseURL), "The shared database should still exist.") + XCTAssertTrue(mockFileManager.files.contains(originalURL), "The original file should not be deleted if the shared database already exists.") + } + + func testWhenMigrateDatabaseToSharedStorageIfNeeded_ThenRestoresOriginalIfCopyFails() throws { + let originalURL = URL(fileURLWithPath: "/path/to/Vault.db") + let sharedDatabaseURL = URL(fileURLWithPath: "/shared/path/Vault.db") + + mockFileManager.files.insert(originalURL) + // Simulate copy failure + mockFileManager.shouldFailOnCopy = true + + var returnedURL: URL? + do { + returnedURL = try fileStorageManager.migrateDatabaseToSharedStorageIfNeeded(from: originalURL, to: sharedDatabaseURL) + XCTFail("Expected failure when copying the file.") + } catch { + // Expected failure + } + + XCTAssertNil(returnedURL, "The migration should fail and no URL should be returned.") + XCTAssertTrue(mockFileManager.files.contains(originalURL), "The original file should still exist after a failed migration.") + } +} + +final class MockFileManager: FileManager { + var files: Set = [] + var createdDirectories: Set = [] + var copiedFiles: [(from: URL, to: URL)] = [] + var movedFiles: [(from: URL, to: URL)] = [] + var removedFiles: [URL] = [] + var shouldFailOnCopy = false + + override func fileExists(atPath path: String) -> Bool { + return files.contains(URL(fileURLWithPath: path)) + } + + override func copyItem(at srcURL: URL, to dstURL: URL) throws { + if shouldFailOnCopy { + throw NSError(domain: "MockFileManager", code: 1, userInfo: [NSLocalizedDescriptionKey: "Simulated copy failure"]) + } + if !files.contains(srcURL) { + throw NSError(domain: "MockFileManager", code: 1, userInfo: [NSLocalizedDescriptionKey: "File not found"]) + } + files.insert(dstURL) + copiedFiles.append((from: srcURL, to: dstURL)) + } + + override func createDirectory(at url: URL, withIntermediateDirectories createIntermediates: Bool, attributes: [FileAttributeKey: Any]? = nil) throws { + createdDirectories.insert(url) + } + + override func removeItem(at URL: URL) throws { + if !files.contains(URL) { + throw NSError(domain: "MockFileManager", code: 1, userInfo: [NSLocalizedDescriptionKey: "File not found"]) + } + files.remove(URL) + removedFiles.append(URL) + } +} diff --git a/Tests/BrowserServicesKitTests/SecureVault/AutofillCredentialIdentityStoreManagerTests.swift b/Tests/BrowserServicesKitTests/SecureVault/AutofillCredentialIdentityStoreManagerTests.swift new file mode 100644 index 000000000..0fed822e4 --- /dev/null +++ b/Tests/BrowserServicesKitTests/SecureVault/AutofillCredentialIdentityStoreManagerTests.swift @@ -0,0 +1,207 @@ +// +// AutofillCredentialIdentityStoreManagerTests.swift +// +// Copyright © 2024 DuckDuckGo. All rights reserved. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +// + +import XCTest +import AuthenticationServices +import Common +import SecureStorage +import SecureStorageTestsUtils +@testable import BrowserServicesKit + +final class AutofillCredentialIdentityStoreManagerTests: XCTestCase { + + var mockCryptoProvider = MockCryptoProvider() + var mockDatabaseProvider = (try! MockAutofillDatabaseProvider()) + var mockKeystoreProvider = MockKeystoreProvider() + var mockVault: (any AutofillSecureVault)! + var tld: TLD! + + var manager: AutofillCredentialIdentityStoreManaging! + var mockStore: MockASCredentialIdentityStore! + + override func setUp() { + super.setUp() + mockStore = MockASCredentialIdentityStore() + let providers = SecureStorageProviders(crypto: mockCryptoProvider, + database: mockDatabaseProvider, + keystore: mockKeystoreProvider) + + mockVault = DefaultAutofillSecureVault(providers: providers) + + tld = TLD() + manager = AutofillCredentialIdentityStoreManager(credentialStore: mockStore, vault: mockVault, tld: tld) + } + + override func tearDown() { + manager = nil + mockStore = nil + mockVault = nil + tld = nil + super.tearDown() + } + + func testCredentialStoreStateEnabled() async { + let isEnabled = await manager.credentialStoreStateEnabled() + XCTAssertTrue(isEnabled) + } + + func testPopulateCredentialStore() async throws { + let accounts = [ + createWebsiteAccount(id: "1", domain: "example.com", username: "user1", signature: "1234"), + createWebsiteAccount(id: "2", domain: "example.org", username: "user2", signature: "5678") + ] + + mockDatabaseProvider._accounts = accounts + await manager.populateCredentialStore() + + if #available(iOS 17.0, macOS 14.0, *) { + XCTAssertEqual(mockStore.savedCredentialIdentities.count, 2) + } else { + XCTAssertEqual(mockStore.savedPasswordCredentialIdentities.count, 2) + } + } + + func testPopulateCredentialStoreWithDuplicateAccounts() async throws { + let accounts = [ + createWebsiteAccount(id: "1", domain: "example.com", username: "user1", signature: "1234"), + createWebsiteAccount(id: "1", domain: "example.com", username: "user1", signature: "1234") + ] + + mockDatabaseProvider._accounts = accounts + await manager.populateCredentialStore() + + if #available(iOS 17.0, macOS 14.0, *) { + XCTAssertEqual(mockStore.savedCredentialIdentities.count, 1) + } else { + XCTAssertEqual(mockStore.savedPasswordCredentialIdentities.count, 1) + } + } + + func testReplaceCredentialStore() async throws { + let accounts = [ + createWebsiteAccount(id: "1", domain: "example.com", username: "user1", signature: "1234"), + createWebsiteAccount(id: "2", domain: "example.org", username: "user2", signature: "5678"), + createWebsiteAccount(id: "3", domain: "example.org", username: "newUser3", signature: "44") + ] + + mockDatabaseProvider._accounts = accounts + await manager.populateCredentialStore() + + let replacementAccounts = [ + createWebsiteAccount(id: "1", domain: "example.com", username: "newUser1", signature: "123"), + createWebsiteAccount(id: "2", domain: "example.org", username: "newUser2", signature: "567") + ] + mockDatabaseProvider._accounts = accounts + + await manager.replaceCredentialStore(with: replacementAccounts) + + if #available(iOS 17.0, macOS 14.0, *) { + XCTAssertEqual(mockStore.savedCredentialIdentities.count, 2) + // loop through the saved credential identities and check if the username is updated + for identity in mockStore.savedCredentialIdentities { + let replacedAccount = replacementAccounts.first { $0.id == identity.recordIdentifier } + XCTAssertEqual(identity.user, replacedAccount?.username) + } + + } else { + XCTAssertEqual(mockStore.savedPasswordCredentialIdentities.count, 2) + for identity in mockStore.savedPasswordCredentialIdentities { + let replacedAccount = replacementAccounts.first { $0.id == identity.recordIdentifier } + XCTAssertEqual(identity.user, replacedAccount?.username) + } + } + } + + func testUpdateCredentialStoreForDomain() async { + let accounts = [ + createWebsiteAccount(id: "1", domain: "example.com", username: "user1", signature: "1234"), + createWebsiteAccount(id: "2", domain: "example.com", username: "user2", signature: "5678"), + createWebsiteAccount(id: "3", domain: "example.com", username: "newUser3", signature: "44") + ] + + mockDatabaseProvider._accounts = accounts + await manager.populateCredentialStore() + + let updatedAccounts = [ + createWebsiteAccount(id: "1", domain: "example.com", username: "user1", signature: "1234", lastUsed: Date() - TimeInterval(60)), + createWebsiteAccount(id: "2", domain: "example.com", username: "user2", signature: "5678", lastUpdated: Date() - TimeInterval(60)), + createWebsiteAccount(id: "3", domain: "example.com", username: "newUser3", signature: "44", lastUsed: Date()) + ] + mockDatabaseProvider._accounts = updatedAccounts + + await manager.updateCredentialStore(for: "example.com") + + if #available(iOS 17.0, macOS 14.0, *) { + XCTAssertEqual(mockStore.savedCredentialIdentities.count, 3) + + let rankedCredentials = mockStore.savedCredentialIdentities.sorted { $0.rank < $1.rank } + XCTAssertEqual(rankedCredentials[0].recordIdentifier, "2") + XCTAssertEqual(rankedCredentials[1].recordIdentifier, "1") + XCTAssertEqual(rankedCredentials[2].recordIdentifier, "3") + } else { + XCTAssertEqual(mockStore.savedPasswordCredentialIdentities.count, 3) + + let rankedCredentials = mockStore.savedPasswordCredentialIdentities.sorted { $0.rank < $1.rank } + XCTAssertEqual(rankedCredentials[0].recordIdentifier, "2") + XCTAssertEqual(rankedCredentials[1].recordIdentifier, "1") + XCTAssertEqual(rankedCredentials[2].recordIdentifier, "3") + + } + + } + + func testUpdateCredentialStoreWithUpdatedAndDeletedAccounts() async { + let accounts = [ + createWebsiteAccount(id: "1", domain: "example.com", username: "user1", signature: "1234"), + createWebsiteAccount(id: "2", domain: "example.com", username: "user2", signature: "5678"), + createWebsiteAccount(id: "3", domain: "example.com", username: "newUser3", signature: "44"), + createWebsiteAccount(id: "4", domain: "example.com", username: "user4", signature: "4422") + ] + + mockDatabaseProvider._accounts = accounts + await manager.populateCredentialStore() + + let updatedAccounts = [ + createWebsiteAccount(id: "1", domain: "example.com", username: "user1a", signature: "1234", lastUsed: Date() - TimeInterval(60)), + createWebsiteAccount(id: "2", domain: "example.com", username: "user2b", signature: "5678"), + createWebsiteAccount(id: "5", domain: "example.com", username: "user5IsNew", signature: "1111") + ] + + let deletedAccounts = [ + createWebsiteAccount(id: "3", domain: "example.com", username: "newUser3", signature: "44") + ] + + await manager.updateCredentialStoreWith(updatedAccounts: updatedAccounts, deletedAccounts: deletedAccounts) + if #available(iOS 17.0, macOS 14.0, *) { + XCTAssertEqual(mockStore.savedCredentialIdentities.count, 4) + XCTAssertEqual(mockStore.savedCredentialIdentities.first { $0.recordIdentifier == "1" }?.user, "user1a") + XCTAssertEqual(mockStore.savedCredentialIdentities.first { $0.recordIdentifier == "2" }?.user, "user2b") + XCTAssertEqual(mockStore.savedCredentialIdentities.first { $0.recordIdentifier == "5" }?.user, "user5IsNew") + XCTAssertNil(mockStore.savedCredentialIdentities.first { $0.recordIdentifier == "3" }) + } else { + XCTAssertEqual(mockStore.savedPasswordCredentialIdentities.count, 4) + } + } + + // MARK: - Helper Methods + + private func createWebsiteAccount(id: String, domain: String, username: String, signature: String, created: Date = Date(), lastUpdated: Date = Date(), lastUsed: Date? = nil) -> SecureVaultModels.WebsiteAccount { + return SecureVaultModels.WebsiteAccount(id: id, username: username, domain: domain, signature: signature, created: created, lastUpdated: lastUpdated, lastUsed: lastUsed) + } + +} diff --git a/Tests/BrowserServicesKitTests/SecureVault/AutofillKeyStoreProviderTests.swift b/Tests/BrowserServicesKitTests/SecureVault/AutofillKeyStoreProviderTests.swift index 89bce30c3..0c53a0b59 100644 --- a/Tests/BrowserServicesKitTests/SecureVault/AutofillKeyStoreProviderTests.swift +++ b/Tests/BrowserServicesKitTests/SecureVault/AutofillKeyStoreProviderTests.swift @@ -23,6 +23,82 @@ import SecureStorageTestsUtils final class AutofillKeyStoreProviderTests: XCTestCase { +#if os(iOS) + let iOSPlatformProvider = iOSKeyStorePlatformProvider(appGroupName: "MockAppGroup") + + func testV1ToV4Migration() throws { + try AutofillKeyStoreProvider.EntryName.allCases.forEach { entry in + // Given + let keychainService = MockKeychainService() + keychainService.mode = .v1Found // Simulate a v1 keychain entry found + let sut = AutofillKeyStoreProvider(keychainService: keychainService, platformProvider: iOSPlatformProvider) + + // When + _ = try sut.readData(named: entry.keychainIdentifier(using: iOSPlatformProvider), serviceName: sut.keychainServiceName) + + // Then + XCTAssertEqual(keychainService.addCallCount, 1) // Migration should trigger a write + XCTAssertEqual(keychainService.latestAddQuery[kSecAttrService as String] as! String, AutofillKeyStoreProvider.Constants.v4ServiceName) // Ensure it's migrated to v4 + XCTAssertEqual(keychainService.latestAddQuery[kSecAttrAccessGroup as String] as! String, iOSPlatformProvider.keychainSecurityGroup as String) // Ensure correct accessibility + } + } + + func testV2ToV4Migration() throws { + try AutofillKeyStoreProvider.EntryName.allCases.forEach { entry in + // Given + let keychainService = MockKeychainService() + keychainService.mode = .v2Found // Simulate a v2 keychain entry found + let sut = AutofillKeyStoreProvider(keychainService: keychainService, platformProvider: iOSPlatformProvider) + + // When + _ = try sut.readData(named: entry.keychainIdentifier(using: iOSPlatformProvider), serviceName: sut.keychainServiceName) + + // Then + XCTAssertEqual(keychainService.addCallCount, 1) // Migration should trigger a write + XCTAssertEqual(keychainService.latestAddQuery[kSecAttrService as String] as! String, AutofillKeyStoreProvider.Constants.v4ServiceName) // Ensure it's migrated to v4 + XCTAssertEqual(keychainService.latestAddQuery[kSecAttrAccessGroup as String] as! String, iOSPlatformProvider.keychainSecurityGroup as String) // Ensure correct accessibility + } + } + + func testV3ToV4Migration() throws { + try AutofillKeyStoreProvider.EntryName.allCases.forEach { entry in + // Given + let keychainService = MockKeychainService() + keychainService.mode = .v3Found // Simulate a v3 keychain entry found + let sut = AutofillKeyStoreProvider(keychainService: keychainService, platformProvider: iOSPlatformProvider) + + // When + _ = try sut.readData(named: entry.keychainIdentifier(using: iOSPlatformProvider), serviceName: sut.keychainServiceName) + + // Then + XCTAssertEqual(keychainService.addCallCount, 1) // Migration should trigger a write + XCTAssertEqual(keychainService.latestAddQuery[kSecAttrService as String] as! String, AutofillKeyStoreProvider.Constants.v4ServiceName) // Ensure it's migrated to v4 + XCTAssertEqual(keychainService.latestAddQuery[kSecAttrAccessGroup as String] as! String, iOSPlatformProvider.keychainSecurityGroup as String) // Ensure correct accessibility + } + } + + func testWhenWriteData_v4KeychainUsed() throws { + try AutofillKeyStoreProvider.EntryName.allCases.forEach { entry in + // Given + let originalString = "Mock Keychain data for v4!" + let data = originalString.data(using: .utf8)! + let encodedString = data.base64EncodedString() + let mockData = encodedString.data(using: .utf8)! + let keychainService = MockKeychainService() + let sut = AutofillKeyStoreProvider(keychainService: keychainService, platformProvider: iOSPlatformProvider) + + // When + _ = try sut.writeData(mockData, named: entry.keychainIdentifier(using: iOSPlatformProvider), serviceName: AutofillKeyStoreProvider.Constants.v4ServiceName) + + // Then + XCTAssertEqual(keychainService.addCallCount, 1) + XCTAssertEqual(keychainService.latestAddQuery[kSecAttrService as String] as! String, AutofillKeyStoreProvider.Constants.v4ServiceName) // Ensure v4 is used for write + XCTAssertEqual(keychainService.latestAddQuery[kSecAttrAccessible as String] as! String, kSecAttrAccessibleWhenUnlocked as String) // Ensure correct accessibility + XCTAssertEqual(keychainService.latestAddQuery[kSecAttrAccessGroup as String] as! String, iOSPlatformProvider.keychainSecurityGroup as String) // Ensure correct accessibility + } + } + + #else func testWhenReadData_AndValueIsFound_NoFallbackSearchIsPerformed() throws { try AutofillKeyStoreProvider.EntryName.allCases.forEach { entry in @@ -32,7 +108,7 @@ final class AutofillKeyStoreProviderTests: XCTestCase { let sut = AutofillKeyStoreProvider(keychainService: keychainService) // When - let result = try sut.readData(named: entry.keychainIdentifier, serviceName: sut.keychainServiceName) + let result = try sut.readData(named: entry.keychainIdentifier(using: macOSKeyStorePlatformProvider()), serviceName: sut.keychainServiceName) // Then XCTAssertEqual(keychainService.itemMatchingCallCount, 1) @@ -48,7 +124,7 @@ final class AutofillKeyStoreProviderTests: XCTestCase { let sut = AutofillKeyStoreProvider(keychainService: keychainService) // When - _ = try sut.readData(named: entry.keychainIdentifier, serviceName: sut.keychainServiceName) + _ = try sut.readData(named: entry.keychainIdentifier(using: macOSKeyStorePlatformProvider()), serviceName: sut.keychainServiceName) // Then XCTAssertEqual(keychainService.itemMatchingCallCount, 3) @@ -64,7 +140,7 @@ final class AutofillKeyStoreProviderTests: XCTestCase { let sut = AutofillKeyStoreProvider(keychainService: keychainService) // When - let result = try sut.readData(named: entry.keychainIdentifier, serviceName: sut.keychainServiceName) + let result = try sut.readData(named: entry.keychainIdentifier(using: macOSKeyStorePlatformProvider()), serviceName: sut.keychainServiceName) // Then XCTAssertEqual(keychainService.itemMatchingCallCount, 2) @@ -83,7 +159,7 @@ final class AutofillKeyStoreProviderTests: XCTestCase { let sut = AutofillKeyStoreProvider(keychainService: keychainService) // When - _ = try sut.readData(named: entry.keychainIdentifier, serviceName: sut.keychainServiceName) + _ = try sut.readData(named: entry.keychainIdentifier(using: macOSKeyStorePlatformProvider()), serviceName: sut.keychainServiceName) // Then XCTAssertEqual(keychainService.itemMatchingCallCount, 3) @@ -101,11 +177,11 @@ final class AutofillKeyStoreProviderTests: XCTestCase { let sut = AutofillKeyStoreProvider(keychainService: keychainService) // When - let result = try sut.readData(named: entry.keychainIdentifier, serviceName: sut.keychainServiceName) + let result = try sut.readData(named: entry.keychainIdentifier(using: macOSKeyStorePlatformProvider()), serviceName: sut.keychainServiceName) // Then XCTAssertEqual(keychainService.addCallCount, 1) - XCTAssertEqual(keychainService.latestAddQuery[kSecAttrAccount as String] as! String, entry.keychainIdentifier) + XCTAssertEqual(keychainService.latestAddQuery[kSecAttrAccount as String] as! String, entry.keychainIdentifier(using: macOSKeyStorePlatformProvider())) XCTAssertEqual(keychainService.latestAddQuery[kSecAttrService as String] as! String, AutofillKeyStoreProvider.Constants.v3ServiceName) XCTAssertEqual(String(decoding: result!, as: UTF8.self), "Mock Keychain data!") } @@ -120,11 +196,11 @@ final class AutofillKeyStoreProviderTests: XCTestCase { let sut = AutofillKeyStoreProvider(keychainService: keychainService) // When - _ = try sut.readData(named: entry.keychainIdentifier, serviceName: sut.keychainServiceName) + _ = try sut.readData(named: entry.keychainIdentifier(using: macOSKeyStorePlatformProvider()), serviceName: sut.keychainServiceName) // Then XCTAssertEqual(keychainService.addCallCount, 1) - XCTAssertEqual(keychainService.latestAddQuery[kSecAttrAccount as String] as! String, entry.keychainIdentifier) + XCTAssertEqual(keychainService.latestAddQuery[kSecAttrAccount as String] as! String, entry.keychainIdentifier(using: macOSKeyStorePlatformProvider())) XCTAssertEqual(keychainService.latestAddQuery[kSecAttrService as String] as! String, AutofillKeyStoreProvider.Constants.v3ServiceName) } } @@ -140,11 +216,12 @@ final class AutofillKeyStoreProviderTests: XCTestCase { let sut = AutofillKeyStoreProvider(keychainService: keychainService) // When - _ = try sut.writeData(mockData, named: entry.keychainIdentifier, serviceName: sut.keychainServiceName) + _ = try sut.writeData(mockData, named: entry.keychainIdentifier(using: macOSKeyStorePlatformProvider()), serviceName: sut.keychainServiceName) // Then XCTAssertEqual(keychainService.addCallCount, 1) XCTAssertEqual(keychainService.latestAddQuery[kSecAttrAccessible as String] as! String, kSecAttrAccessibleWhenUnlocked as String) } } +#endif } diff --git a/Tests/BrowserServicesKitTests/SecureVault/DatabaseProviderTests.swift b/Tests/BrowserServicesKitTests/SecureVault/DatabaseProviderTests.swift index f791954ae..8ad2e578d 100644 --- a/Tests/BrowserServicesKitTests/SecureVault/DatabaseProviderTests.swift +++ b/Tests/BrowserServicesKitTests/SecureVault/DatabaseProviderTests.swift @@ -34,6 +34,13 @@ class DatabaseProviderTests: XCTestCase { try FileManager.default.removeItem(atPath: dbFileContainer.appendingPathComponent(file).path) } + #if os(iOS) + let sharedDbFileContainer = DefaultAutofillDatabaseProvider.defaultSharedDatabaseURL().deletingLastPathComponent() + for file in try FileManager.default.contentsOfDirectory(atPath: sharedDbFileContainer.path) { + guard ["db", "bak"].contains((file as NSString).pathExtension) else { continue } + try FileManager.default.removeItem(atPath: sharedDbFileContainer.appendingPathComponent(file).path) + } + #endif } catch let error as NSError { // File not found if error.domain != NSCocoaErrorDomain || error.code != 4 { diff --git a/Tests/BrowserServicesKitTests/SecureVault/MockASCredentialIdentityStore.swift b/Tests/BrowserServicesKitTests/SecureVault/MockASCredentialIdentityStore.swift new file mode 100644 index 000000000..6f00fab29 --- /dev/null +++ b/Tests/BrowserServicesKitTests/SecureVault/MockASCredentialIdentityStore.swift @@ -0,0 +1,125 @@ +// +// MockASCredentialIdentityStore.swift +// +// Copyright © 2024 DuckDuckGo. All rights reserved. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +// + +import Foundation +import AuthenticationServices +@testable import BrowserServicesKit + +final class MockASCredentialIdentityStore: ASCredentialIdentityStoring { + var isEnabled = true + var supportsIncrementalUpdates = true + var savedPasswordCredentialIdentities: [ASPasswordCredentialIdentity] = [] + var error: Error? + + // Using this computed property to handle iOS 17 availability for ASCredentialIdentity + private var _savedCredentialIdentities: [Any] = [] + + func state() async -> ASCredentialIdentityStoreState { + return MockASCredentialIdentityStoreState(isEnabled: isEnabled, supportsIncrementalUpdates: supportsIncrementalUpdates) + } + + func saveCredentialIdentities(_ credentials: [ASPasswordCredentialIdentity]) async throws { + if let error = error { + throw error + } + + for credential in credentials { + if let index = savedPasswordCredentialIdentities.firstIndex(where: { $0.recordIdentifier == credential.recordIdentifier }) { + savedPasswordCredentialIdentities[index] = credential + } else { + savedPasswordCredentialIdentities.append(credential) + } + } + } + + func removeCredentialIdentities(_ credentials: [ASPasswordCredentialIdentity]) async throws { + if let error = error { + throw error + } + let identifiersToRemove = Set(credentials.map { $0.recordIdentifier }) + savedPasswordCredentialIdentities.removeAll { identifiersToRemove.contains($0.recordIdentifier) } + } + + func replaceCredentialIdentities(with newCredentials: [ASPasswordCredentialIdentity]) async throws { + if let error = error { + throw error + } + savedPasswordCredentialIdentities = newCredentials + } + +} + +@available(iOS 17.0, macOS 14.0, *) +extension MockASCredentialIdentityStore { + + var savedCredentialIdentities: [ASCredentialIdentity] { + get { + return _savedCredentialIdentities as? [ASCredentialIdentity] ?? [] + } + set { + _savedCredentialIdentities = newValue + } + } + + func saveCredentialIdentities(_ credentials: [ASCredentialIdentity]) async throws { + if let error = error { + throw error + } + for credential in credentials { + if let index = savedCredentialIdentities.firstIndex(where: { $0.recordIdentifier == credential.recordIdentifier }) { + savedCredentialIdentities[index] = credential + } else { + savedCredentialIdentities.append(credential) + } + } + } + + func removeCredentialIdentities(_ credentials: [ASCredentialIdentity]) async throws { + if let error = error { + throw error + } + let identifiersToRemove = Set(credentials.map { $0.recordIdentifier }) + savedCredentialIdentities.removeAll { identifiersToRemove.contains($0.recordIdentifier) } + } + + func replaceCredentialIdentities(_ newCredentials: [ASCredentialIdentity]) async throws { + if let error = error { + throw error + } + savedCredentialIdentities = newCredentials + } +} + +private class MockASCredentialIdentityStoreState: ASCredentialIdentityStoreState { + private var _isEnabled: Bool + private var _supportsIncrementalUpdates: Bool + + override var isEnabled: Bool { + return _isEnabled + } + + override var supportsIncrementalUpdates: Bool { + return _supportsIncrementalUpdates + } + + init(isEnabled: Bool, supportsIncrementalUpdates: Bool) { + self._isEnabled = isEnabled + self._supportsIncrementalUpdates = supportsIncrementalUpdates + super.init() + } +} diff --git a/Tests/BrowserServicesKitTests/SecureVault/SecureVaultModelTests.swift b/Tests/BrowserServicesKitTests/SecureVault/SecureVaultModelTests.swift index 634e382bd..527e56c0a 100644 --- a/Tests/BrowserServicesKitTests/SecureVault/SecureVaultModelTests.swift +++ b/Tests/BrowserServicesKitTests/SecureVault/SecureVaultModelTests.swift @@ -527,6 +527,161 @@ class SecureVaultModelTests: XCTestCase { } } + func testSortedAndDeduplicatedForSameSignatureReturnsTLD() { + let controlAccounts = [ + testAccount("user1", "example.com", "sig1", 0), + testAccount("user1", "sub1.example.com", "sig1", 0, 1 * days), + testAccount("user1", "sub2.example.com", "sig1", 0), + ] + + let sortedAccounts = controlAccounts.sortedAndDeduplicated(tld: tld) + + XCTAssertEqual(sortedAccounts[0].domain, "example.com") + XCTAssertEqual(sortedAccounts.count, 1) + } + + func testSortedAndDeduplicatedForSameSignatureReturnsWww() { + let controlAccounts = [ + testAccount("user1", "sub.example.com", "sig1", 0, 1 * days), + testAccount("user1", "sub1.example.com", "sig1", 0), + testAccount("user1", "sub2.example.com", "sig1", 0), + testAccount("user1", "www.example.com", "sig1", 0), + ] + + let sortedAccounts = controlAccounts.sortedAndDeduplicated(tld: tld) + + XCTAssertEqual(sortedAccounts[0].domain, "www.example.com") + XCTAssertEqual(sortedAccounts.count, 1) + } + + func testSortedAndDeduplicatedForSameSignatureDifferentSubdomainsReturnsSortedLastUsed() { + let controlAccounts = [ + testAccount("user1", "sub.example.com", "sig1", 0), + testAccount("user1", "sub1.example.com", "sig1", 0), + testAccount("user1", "sub2.example.com", "sig1", 0, 1 * days), + testAccount("user1", "any.example.com", "sig1", 0), + ] + + let sortedAccounts = controlAccounts.sortedAndDeduplicated(tld: tld) + + XCTAssertEqual(sortedAccounts[0].domain, "sub2.example.com") + XCTAssertEqual(sortedAccounts.count, 1) + } + + func testSortedAndDeduplicatedForSameSignatureDifferentDomainsReturnsUniqueDomains() { + let controlAccounts = [ + testAccount("user1", "example.co.uk", "sig1", 0), + testAccount("user1", "sub.example.co.uk", "sig1", 0), + testAccount("user1", "domain.co.uk", "sig1", 0), + testAccount("user1", "www.domain.co.uk", "sig1", 0), + ] + + let sortedAccounts = controlAccounts.sortedAndDeduplicated(tld: tld) + + XCTAssertEqual(sortedAccounts[0].domain, "domain.co.uk") + XCTAssertEqual(sortedAccounts[1].domain, "example.co.uk") + XCTAssertEqual(sortedAccounts.count, 2) + } + + func testSortedAndDeduplicatedForNoSignatureReturnsAllAccounts() { + let controlAccounts = [ + SecureVaultModels.WebsiteAccount(id: "1234567890", + username: "username", + domain: "example.co.uk", + created: Date(), + lastUpdated: Date()), + SecureVaultModels.WebsiteAccount(id: "1234567890", + username: "username", + domain: "sub.example.co.uk", + created: Date(), + lastUpdated: Date()), + SecureVaultModels.WebsiteAccount(id: "1234567890", + username: "username", + domain: "domain.co.uk", + created: Date(), + lastUpdated: Date()), + SecureVaultModels.WebsiteAccount(id: "1234567890", + username: "username", + domain: "www.domain.co.uk", + created: Date(), + lastUpdated: Date()) + ] + + let sortedAccounts = controlAccounts.sortedAndDeduplicated(tld: tld) + + XCTAssertEqual(sortedAccounts.count, 4) + } + + func testSortedAndDeduplicatedWithComplexDomains() { + let accounts = [ + // Multiple subdomains + testAccount("user1", "deep.sub.example.com", "sig1", 0), + testAccount("user1", "other.sub.example.com", "sig1", 0), + + // Different ports + testAccount("user2", "example.com:8080", "sig2", 0), + testAccount("user2", "example.com:443", "sig2", 0), + + // Mix of www and non-www + testAccount("user3", "www.example.com", "sig3", 0), + testAccount("user3", "example.com", "sig3", 0), + + // Different TLDs + testAccount("user4", "example.com", "sig4", 0), + testAccount("user4", "example.net", "sig4", 0), + testAccount("user4", "example.org", "sig4", 0) + ] + + let sortedAccounts = accounts.sortedAndDeduplicated(tld: tld) + + // Verify subdomains are properly handled + let sig1Accounts = sortedAccounts.filter { $0.signature == "sig1" } + XCTAssertEqual(sig1Accounts[0].domain, "deep.sub.example.com") + XCTAssertEqual(sig1Accounts.count, 1) + + // Verify ports are considered in deduplication + let sig2Accounts = sortedAccounts.filter { $0.signature == "sig2" } + XCTAssertEqual(sig2Accounts[0].domain, "example.com:443") + XCTAssertEqual(sig2Accounts.count, 1) + + // Verify www and non-www are considered same domain + let sig3Accounts = sortedAccounts.filter { $0.signature == "sig3" } + XCTAssertEqual(sig3Accounts[0].domain, "example.com") + XCTAssertEqual(sig3Accounts.count, 1) + + // Verify different TLDs are preserved + let sig4Accounts = sortedAccounts.filter { $0.signature == "sig4" } + XCTAssertEqual(sig4Accounts.count, 3) + } + + func testSortedAndDeduplicatedWithLastUsedDates() { + let accounts = [ + // Same signature, different last used dates + testAccount("user1", "example.com", "sig1", 0, 3 * days), + testAccount("user1", "sub.example.com", "sig1", 0, 1 * days), + testAccount("user1", "other.example.com", "sig1", 0, 2 * days), + + // Different signatures, same domain, mixed dates + testAccount("user2", "example.com", "sig2", 0, 1 * days), + testAccount("user3", "example.com", "sig3", 0, 2 * days), + testAccount("user4", "example.com", "sig4", 0) // No last used date + ] + + let sortedAccounts = accounts.sortedAndDeduplicated(tld: tld) + + // Verify accounts are sorted by last used date + XCTAssertEqual(sortedAccounts[0].domain, "example.com") // 3 days ago + XCTAssertEqual(sortedAccounts[1].username, "user3") // 2 days ago + XCTAssertEqual(sortedAccounts[2].username, "user2") // 1 day ago + XCTAssertEqual(sortedAccounts[3].username, "user4") // No last used date + + // Verify deduplication still works with different dates + let sig1Accounts = sortedAccounts.filter { $0.signature == "sig1" } + XCTAssertEqual(sig1Accounts.count, 1) + // Verify the most recently used account is kept + XCTAssertEqual(sig1Accounts[0].lastUsed?.timeIntervalSince1970, 3 * days) + } + func testPatternMatchedTitle() { let domainTitles: [String] = [ diff --git a/Tests/BrowserServicesKitTests/SecureVault/SecureVaultSyncableCredentialsTests.swift b/Tests/BrowserServicesKitTests/SecureVault/SecureVaultSyncableCredentialsTests.swift index 26c0a649a..2270750e1 100644 --- a/Tests/BrowserServicesKitTests/SecureVault/SecureVaultSyncableCredentialsTests.swift +++ b/Tests/BrowserServicesKitTests/SecureVault/SecureVaultSyncableCredentialsTests.swift @@ -286,6 +286,13 @@ class SecureVaultSyncableCredentialsTests: XCTestCase { try FileManager.default.removeItem(atPath: dbFileContainer.appendingPathComponent(file).path) } +#if os(iOS) + let sharedDbFileContainer = DefaultAutofillDatabaseProvider.defaultSharedDatabaseURL().deletingLastPathComponent() + for file in try FileManager.default.contentsOfDirectory(atPath: sharedDbFileContainer.path) { + guard ["db", "bak"].contains((file as NSString).pathExtension) else { continue } + try FileManager.default.removeItem(atPath: sharedDbFileContainer.appendingPathComponent(file).path) + } +#endif } catch let error as NSError { // File not found if error.domain != NSCocoaErrorDomain || error.code != 4 { diff --git a/Tests/SyncDataProvidersTests/Credentials/helpers/CredentialsProviderTestsBase.swift b/Tests/SyncDataProvidersTests/Credentials/helpers/CredentialsProviderTestsBase.swift index 0de0a3f27..7ea3a282f 100644 --- a/Tests/SyncDataProvidersTests/Credentials/helpers/CredentialsProviderTestsBase.swift +++ b/Tests/SyncDataProvidersTests/Credentials/helpers/CredentialsProviderTestsBase.swift @@ -90,7 +90,8 @@ internal class CredentialsProviderTestsBase: XCTestCase { secureVaultFactory: secureVaultFactory, secureVaultErrorReporter: MockSecureVaultErrorReporter(), metadataStore: LocalSyncMetadataStore(database: metadataDatabase), - syncDidUpdateData: {} + syncDidUpdateData: {}, + syncDidFinish: { _ in } ) } From 55e7de13a99793329993367169c9b6bafbd07bac Mon Sep 17 00:00:00 2001 From: Shilpa Modi Date: Mon, 9 Dec 2024 12:31:49 -0800 Subject: [PATCH 08/12] Initial changes for compilation time tracking. (#1111) Please review the release process for BrowserServicesKit [here](https://app.asana.com/0/1200194497630846/1200837094583426). **Required**: Task/Issue URL: https://app.asana.com/0/1208613456171888/1208801514911205/f iOS PR: macOS PR: What kind of version bump will this require?: Major/Minor/Patch **Optional**: Tech Design URL: CC: **Description**: **Steps to test this PR**: 1. 1. **OS Testing**: * [ ] iOS 14 * [ ] iOS 15 * [ ] iOS 16 * [ ] macOS 10.15 * [ ] macOS 11 * [ ] macOS 12 --- ###### Internal references: [Software Engineering Expectations](https://app.asana.com/0/59792373528535/199064865822552) [Technical Design Template](https://app.asana.com/0/59792373528535/184709971311943) --------- Co-authored-by: Bartek Waresiak --- .../ContentBlockerDebugEvents.swift | 3 +- .../ContentBlockerRulesManager.swift | 28 +++---- .../ContentBlockingRulesCompilationTask.swift | 80 ++++++++++++++++++- .../ContentBlockingRulesLookupTask.swift | 12 +-- ...rRulesManagerInitialCompilationTests.swift | 14 ++-- .../ContentBlockerRulesManagerTests.swift | 26 +++--- 6 files changed, 114 insertions(+), 49 deletions(-) diff --git a/Sources/BrowserServicesKit/ContentBlocking/ContentBlockerDebugEvents.swift b/Sources/BrowserServicesKit/ContentBlocking/ContentBlockerDebugEvents.swift index 8e5e572ed..f148115a9 100644 --- a/Sources/BrowserServicesKit/ContentBlocking/ContentBlockerDebugEvents.swift +++ b/Sources/BrowserServicesKit/ContentBlocking/ContentBlockerDebugEvents.swift @@ -46,10 +46,9 @@ public enum ContentBlockerDebugEvents { case contentBlockingCompilationFailed(listName: String, component: Component) - case contentBlockingCompilationTime - case contentBlockingLookupRulesSucceeded case contentBlockingFetchLRCSucceeded case contentBlockingLRCMissing case contentBlockingNoMatchInLRC + case contentBlockingCompilationTaskPerformance(iterationCount: Int, timeBucketAggregation: Double) } diff --git a/Sources/BrowserServicesKit/ContentBlocking/ContentBlockerRulesManager.swift b/Sources/BrowserServicesKit/ContentBlocking/ContentBlockerRulesManager.swift index c7cb04bd9..5fc32c28b 100644 --- a/Sources/BrowserServicesKit/ContentBlocking/ContentBlockerRulesManager.swift +++ b/Sources/BrowserServicesKit/ContentBlocking/ContentBlockerRulesManager.swift @@ -79,7 +79,7 @@ public class ContentBlockerRulesManager: CompiledRuleListsSource { self.identifier = identifier } - internal init(compilationResult: (compiledRulesList: WKContentRuleList, model: ContentBlockerRulesSourceModel)) { + internal init(compilationResult: CompilationResult) { let surrogateTDS = ContentBlockerRulesManager.extractSurrogates(from: compilationResult.model.tds) let encodedData = try? JSONEncoder().encode(surrogateTDS) let encodedTrackerData = String(data: encodedData!, encoding: .utf8)! @@ -130,7 +130,6 @@ public class ContentBlockerRulesManager: CompiledRuleListsSource { public var sourceManagers = [String: ContentBlockerRulesSourceManager]() private var currentTasks = [CompilationTask]() - private var compilationStartTime: TimeInterval? private let workQueue = DispatchQueue(label: "ContentBlockerManagerQueue", qos: .userInitiated) @@ -229,7 +228,6 @@ public class ContentBlockerRulesManager: CompiledRuleListsSource { } state = .recompiling(currentTokens: [token]) - compilationStartTime = compilationStartTime ?? CACurrentMediaTime() lock.unlock() return true } @@ -243,10 +241,8 @@ public class ContentBlockerRulesManager: CompiledRuleListsSource { prepareSourceManagers() let initialCompilationTask = LookupRulesTask(sourceManagers: Array(sourceManagers.values)) - let result: [LookupRulesTask.LookupResult] - do { - result = try initialCompilationTask.lookupCachedRulesLists() + let result = try initialCompilationTask.lookupCachedRulesLists() let rules = result.map(Rules.init(compilationResult:)) Logger.contentBlocking.debug("🟩 Lookup Found \(rules.count, privacy: .public) rules") @@ -301,7 +297,6 @@ public class ContentBlockerRulesManager: CompiledRuleListsSource { } private func startCompilationProcess() { - Logger.contentBlocking.debug("Starting compilataion process") prepareSourceManagers() // Prepare compilation tasks based on the sources @@ -361,10 +356,11 @@ public class ContentBlockerRulesManager: CompiledRuleListsSource { Logger.contentBlocking.debug("Failed to complete task \(task.rulesList.name, privacy: .public)") return nil } + let rules = Rules(compilationResult: result) let diff: ContentBlockerRulesIdentifier.Difference - if let id = _currentRules.first(where: {$0.name == task.rulesList.name })?.identifier { + if let id = _currentRules.first(where: { $0.name == task.rulesList.name })?.identifier { diff = id.compare(with: result.model.rulesIdentifier) } else { diff = result.model.rulesIdentifier.compare(with: ContentBlockerRulesIdentifier(name: task.rulesList.name, @@ -374,6 +370,15 @@ public class ContentBlockerRulesManager: CompiledRuleListsSource { unprotectedSitesHash: nil)) } + if task.rulesList.name == DefaultContentBlockerRulesListsSource.Constants.trackerDataSetRulesListName && + result.resultType == .rulesCompilation { + if let perfInfo = result.performanceInfo { + self.errorReporting?.fire(.contentBlockingCompilationTaskPerformance(iterationCount: perfInfo.iterationCount, + timeBucketAggregation: perfInfo.compilationTime), + parameters: ["compilationTime": String(perfInfo.compilationTime)]) + } + } + changes[task.rulesList.name] = diff return rules } @@ -390,7 +395,6 @@ public class ContentBlockerRulesManager: CompiledRuleListsSource { _currentRules = rules let completionTokens: [CompletionToken] - let compilationTime = compilationStartTime.map { start in CACurrentMediaTime() - start } switch state { case .recompilingAndScheduled(let currentTokens, let pendingTokens): // New work has been scheduled - prepare for execution. @@ -400,12 +404,10 @@ public class ContentBlockerRulesManager: CompiledRuleListsSource { completionTokens = currentTokens state = .recompiling(currentTokens: pendingTokens) - compilationStartTime = CACurrentMediaTime() case .recompiling(let currentTokens): completionTokens = currentTokens state = .idle - compilationStartTime = nil case .idle: assertionFailure("Unexpected state") @@ -418,10 +420,6 @@ public class ContentBlockerRulesManager: CompiledRuleListsSource { updatesSubject.send(UpdateEvent(rules: rules, changes: changes, completionTokens: completionTokens)) DispatchQueue.main.async { - if let compilationTime = compilationTime { - self.errorReporting?.fire(.contentBlockingCompilationTime, parameters: ["compilationTime": String(compilationTime)]) - } - self.cleanup(currentIdentifiers: currentIdentifiers) } } diff --git a/Sources/BrowserServicesKit/ContentBlocking/ContentBlockingRulesCompilationTask.swift b/Sources/BrowserServicesKit/ContentBlocking/ContentBlockingRulesCompilationTask.swift index eee791ff5..357d36aed 100644 --- a/Sources/BrowserServicesKit/ContentBlocking/ContentBlockingRulesCompilationTask.swift +++ b/Sources/BrowserServicesKit/ContentBlocking/ContentBlockingRulesCompilationTask.swift @@ -24,17 +24,42 @@ import os.log extension ContentBlockerRulesManager { + internal struct CompilationResult { + let compiledRulesList: WKContentRuleList + let model: ContentBlockerRulesSourceModel + let resultType: ResultType + let performanceInfo: PerformanceInfo? + + struct PerformanceInfo { + let compilationTime: TimeInterval + let iterationCount: Int + + // default iterationCount = 1 for successful compilation on one try + init(compilationTime: TimeInterval, iterationCount: Int = 1) { + self.compilationTime = compilationTime + self.iterationCount = iterationCount + } + } + + enum ResultType { + case cacheLookup + case rulesCompilation + } + } + /** Encapsulates compilation steps for a single Task */ internal class CompilationTask { typealias Completion = (_ task: CompilationTask, _ success: Bool) -> Void + let workQueue: DispatchQueue let rulesList: ContentBlockerRulesList let sourceManager: ContentBlockerRulesSourceManager var isCompleted: Bool { result != nil || compilationImpossible } private(set) var compilationImpossible = false - private(set) var result: (compiledRulesList: WKContentRuleList, model: ContentBlockerRulesSourceModel)? + private(set) var result: CompilationResult? + private(set) var compilationStartTime: TimeInterval? init(workQueue: DispatchQueue, rulesList: ContentBlockerRulesList, @@ -53,6 +78,8 @@ extension ContentBlockerRulesManager { return } + self.compilationStartTime = CACurrentMediaTime() + guard !ignoreCache else { Logger.contentBlocking.log("❗️ ignoring cache") self.workQueue.async { @@ -65,10 +92,14 @@ extension ContentBlockerRulesManager { DispatchQueue.main.async { let identifier = model.rulesIdentifier.stringValue Logger.contentBlocking.debug("Lookup CBR with \(identifier, privacy: .public)") + WKContentRuleListStore.default()?.lookUpContentRuleList(forIdentifier: identifier) { ruleList, _ in if let ruleList = ruleList { Logger.contentBlocking.log("🟢 CBR loaded from cache: \(self.rulesList.name, privacy: .public)") - self.compilationSucceeded(with: ruleList, model: model, completionHandler: completionHandler) + self.compilationSucceeded(with: ruleList, + model: model, + resultType: .cacheLookup, + completionHandler: completionHandler) } else { self.workQueue.async { self.compile(model: model, completionHandler: completionHandler) @@ -81,9 +112,12 @@ extension ContentBlockerRulesManager { private func compilationSucceeded(with compiledRulesList: WKContentRuleList, model: ContentBlockerRulesSourceModel, + resultType: CompilationResult.ResultType, completionHandler: @escaping Completion) { workQueue.async { - self.result = (compiledRulesList, model) + self.result = self.getCompilationResult(ruleList: compiledRulesList, + model: model, + resultType: resultType) completionHandler(self, true) } } @@ -131,7 +165,10 @@ extension ContentBlockerRulesManager { if let ruleList = ruleList { Logger.contentBlocking.log("🟢 CBR compilation for \(self.rulesList.name, privacy: .public) succeeded") - self.compilationSucceeded(with: ruleList, model: model, completionHandler: completionHandler) + self.compilationSucceeded(with: ruleList, + model: model, + resultType: .rulesCompilation, + completionHandler: completionHandler) } else if let error = error { self.compilationFailed(for: model, with: error, completionHandler: completionHandler) } else { @@ -140,6 +177,41 @@ extension ContentBlockerRulesManager { } } } + + func getCompilationResult(ruleList: WKContentRuleList, + model: ContentBlockerRulesSourceModel, + resultType: CompilationResult.ResultType) -> CompilationResult { + let compilationTime = self.compilationStartTime.map { CACurrentMediaTime() - $0 } + + let perfInfo = compilationTime.map { + CompilationResult.PerformanceInfo(compilationTime: $0, + iterationCount: getCompilationIterationCount()) + } + + return CompilationResult(compiledRulesList: ruleList, + model: model, + resultType: resultType, + performanceInfo: perfInfo) + + } + + func getCompilationIterationCount() -> Int { + guard let brokenSources = sourceManager.brokenSources else { + // if none of the sources are broken, we do 1 successful iteration and do not do any retries + return 1 + } + + let identifiers = [ + brokenSources.allowListIdentifier, + brokenSources.tempListIdentifier, + brokenSources.unprotectedSitesIdentifier, + brokenSources.tdsIdentifier + ] + + // Add one to account for the first compilation aside from any retries + return (identifiers.compactMap { $0 }.count) + 1 + } + } } diff --git a/Sources/BrowserServicesKit/ContentBlocking/ContentBlockingRulesLookupTask.swift b/Sources/BrowserServicesKit/ContentBlocking/ContentBlockingRulesLookupTask.swift index fadfc6fbf..e0296d6bb 100644 --- a/Sources/BrowserServicesKit/ContentBlocking/ContentBlockingRulesLookupTask.swift +++ b/Sources/BrowserServicesKit/ContentBlocking/ContentBlockingRulesLookupTask.swift @@ -24,15 +24,13 @@ extension ContentBlockerRulesManager { final class LookupRulesTask { - typealias LookupResult = (compiledRulesList: WKContentRuleList, model: ContentBlockerRulesSourceModel) - private let sourceManagers: [ContentBlockerRulesSourceManager] init(sourceManagers: [ContentBlockerRulesSourceManager]) { self.sourceManagers = sourceManagers } - func lookupCachedRulesLists() throws -> [LookupResult] { + func lookupCachedRulesLists() throws -> [CompilationResult] { let models = sourceManagers.compactMap { $0.makeModel() } if models.count != sourceManagers.count { @@ -40,7 +38,7 @@ extension ContentBlockerRulesManager { throw WKError(.contentRuleListStoreLookUpFailed) } - var result = [LookupResult]() + var result = [CompilationResult]() let group = DispatchGroup() for model in models { @@ -54,10 +52,14 @@ extension ContentBlockerRulesManager { return } - result.append((ruleList, model)) + result.append(CompilationResult(compiledRulesList: ruleList, + model: model, + resultType: .cacheLookup, + performanceInfo: nil)) group.leave() } } + } let operationResult = group.wait(timeout: .now() + 6) diff --git a/Tests/BrowserServicesKitTests/ContentBlocker/ContentBlockerRulesManagerInitialCompilationTests.swift b/Tests/BrowserServicesKitTests/ContentBlocker/ContentBlockerRulesManagerInitialCompilationTests.swift index 455734d9b..772481b17 100644 --- a/Tests/BrowserServicesKitTests/ContentBlocker/ContentBlockerRulesManagerInitialCompilationTests.swift +++ b/Tests/BrowserServicesKitTests/ContentBlocker/ContentBlockerRulesManagerInitialCompilationTests.swift @@ -62,8 +62,8 @@ final class ContentBlockerRulesManagerInitialCompilationTests: XCTestCase { let errorHandler = EventMapping { event, _, params, _ in if case .contentBlockingLRCMissing = event { lookupAndFetchExp.fulfill() - } else if case .contentBlockingCompilationTime = event { - XCTAssertNotNil(params?["compilationTime"]) + } else if case .contentBlockingCompilationTaskPerformance(let iterationCount, _) = event { + XCTAssertEqual(iterationCount, 1) } else { XCTFail("Unexpected event: \(event)") } @@ -123,8 +123,6 @@ final class ContentBlockerRulesManagerInitialCompilationTests: XCTestCase { XCTFail("Should not fetch LRC") } else if case .contentBlockingLookupRulesSucceeded = event { lookupAndFetchExp.fulfill() - } else if case .contentBlockingCompilationTime = event { - XCTAssertNotNil(params?["compilationTime"]) } else { XCTFail("Unexpected event: \(event)") } @@ -211,10 +209,10 @@ final class ContentBlockerRulesManagerInitialCompilationTests: XCTestCase { let errorHandler = EventMapping { event, _, params, _ in if case .contentBlockingFetchLRCSucceeded = event { XCTFail("Should not fetch LRC") + } else if case .contentBlockingCompilationTaskPerformance(let iterationCount, _) = event { + XCTAssertEqual(iterationCount, 1) } else if case .contentBlockingNoMatchInLRC = event { lookupAndFetchExp.fulfill() - } else if case .contentBlockingCompilationTime = event { - XCTAssertNotNil(params?["compilationTime"]) } else { XCTFail("Unexpected event: \(event)") } @@ -271,8 +269,8 @@ final class ContentBlockerRulesManagerInitialCompilationTests: XCTestCase { let errorHandler = EventMapping { event, _, params, _ in if case .contentBlockingFetchLRCSucceeded = event { lookupAndFetchExp.fulfill() - } else if case .contentBlockingCompilationTime = event { - XCTAssertNotNil(params?["compilationTime"]) + } else if case .contentBlockingCompilationTaskPerformance(let iterationCount, _) = event { + XCTAssertEqual(iterationCount, 1) } else { XCTFail("Unexpected event: \(event)") } diff --git a/Tests/BrowserServicesKitTests/ContentBlocker/ContentBlockerRulesManagerTests.swift b/Tests/BrowserServicesKitTests/ContentBlocker/ContentBlockerRulesManagerTests.swift index 15321e1ba..a3424bb03 100644 --- a/Tests/BrowserServicesKitTests/ContentBlocker/ContentBlockerRulesManagerTests.swift +++ b/Tests/BrowserServicesKitTests/ContentBlocker/ContentBlockerRulesManagerTests.swift @@ -205,7 +205,6 @@ class ContentBlockerRulesManagerLoadingTests: ContentBlockerRulesManagerTests { errorExp.isInverted = true let lookupAndFetchExp = expectation(description: "Look and Fetch rules failed") - let compilationTimeExp = expectation(description: "Compilation Time reported") let errorHandler = EventMapping { event, _, params, _ in if case .contentBlockingCompilationFailed(let listName, let component) = event { XCTAssertEqual(listName, DefaultContentBlockerRulesListsSource.Constants.trackerDataSetRulesListName) @@ -216,11 +215,10 @@ class ContentBlockerRulesManagerLoadingTests: ContentBlockerRulesManagerTests { XCTFail("Unexpected component: \(component)") } - } else if case .contentBlockingCompilationTime = event { - XCTAssertNotNil(params?["compilationTime"]) - compilationTimeExp.fulfill() } else if case .contentBlockingLRCMissing = event { lookupAndFetchExp.fulfill() + } else if case .contentBlockingCompilationTaskPerformance(let iterationCount, _) = event { + XCTAssertEqual(iterationCount, 1) } else { XCTFail("Unexpected event: \(event)") } @@ -231,7 +229,7 @@ class ContentBlockerRulesManagerLoadingTests: ContentBlockerRulesManagerTests { updateListener: rulesUpdateListener, errorReporting: errorHandler) - wait(for: [exp, errorExp, compilationTimeExp, lookupAndFetchExp], timeout: 15.0) + wait(for: [exp, errorExp, lookupAndFetchExp], timeout: 15.0) XCTAssertNotNil(cbrm.currentRules) XCTAssertEqual(cbrm.currentRules.first?.etag, mockRulesSource.trackerData?.etag) @@ -270,8 +268,8 @@ class ContentBlockerRulesManagerLoadingTests: ContentBlockerRulesManagerTests { XCTFail("Unexpected component: \(component)") } - } else if case .contentBlockingCompilationTime = event { - XCTAssertNotNil(params?["compilationTime"]) + } else if case .contentBlockingCompilationTaskPerformance(let iterationCount, _) = event { + XCTAssertEqual(iterationCount, 2) } else if case .contentBlockingLRCMissing = event { lookupAndFetchExp.fulfill() } else { @@ -546,7 +544,7 @@ class ContentBlockerRulesManagerLoadingTests: ContentBlockerRulesManagerTests { } let errorExp = expectation(description: "Error reported") - errorExp.expectedFulfillmentCount = 5 + errorExp.expectedFulfillmentCount = 4 let lookupAndFetchExp = expectation(description: "Look and Fetch rules failed") @@ -562,11 +560,10 @@ class ContentBlockerRulesManagerLoadingTests: ContentBlockerRulesManagerTests { XCTFail("Unexpected component: \(component)") } - } else if case .contentBlockingCompilationTime = event { - XCTAssertNotNil(params?["compilationTime"]) - errorExp.fulfill() } else if case .contentBlockingLRCMissing = event { lookupAndFetchExp.fulfill() + } else if case .contentBlockingCompilationTaskPerformance(let iterationCount, _) = event { + XCTAssertEqual(iterationCount, 5) } else { XCTFail("Unexpected event: \(event)") } @@ -631,7 +628,7 @@ class ContentBlockerRulesManagerLoadingTests: ContentBlockerRulesManagerTests { } let errorExp = expectation(description: "Error reported") - errorExp.expectedFulfillmentCount = 4 + errorExp.expectedFulfillmentCount = 3 let lookupAndFetchExp = expectation(description: "Look and Fetch rules failed") @@ -647,9 +644,8 @@ class ContentBlockerRulesManagerLoadingTests: ContentBlockerRulesManagerTests { XCTFail("Unexpected component: \(component)") } - } else if case .contentBlockingCompilationTime = event { - XCTAssertNotNil(params?["compilationTime"]) - errorExp.fulfill() + } else if case .contentBlockingCompilationTaskPerformance(let iterationCount, _) = event { + XCTAssertEqual(iterationCount, 4) } else if case .contentBlockingLRCMissing = event { lookupAndFetchExp.fulfill() } else From 9195fd54a49468e78e233bc6babb1238046cee20 Mon Sep 17 00:00:00 2001 From: Dominik Kapusta Date: Mon, 9 Dec 2024 21:50:55 +0100 Subject: [PATCH 09/12] Add underlying error to PrivacyStatsError (#1130) Task/Issue URL: https://app.asana.com/0/72649045549333/1208936504720914/f Description: This change defines user info dictionary from CustomNSError for PrivacyStatsError. --- Sources/PrivacyStats/PrivacyStats.swift | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/Sources/PrivacyStats/PrivacyStats.swift b/Sources/PrivacyStats/PrivacyStats.swift index 168b04ff7..26875d66b 100644 --- a/Sources/PrivacyStats/PrivacyStats.swift +++ b/Sources/PrivacyStats/PrivacyStats.swift @@ -48,6 +48,10 @@ public enum PrivacyStatsError: CustomNSError { } } + public var errorUserInfo: [String: Any] { + [NSUnderlyingErrorKey: underlyingError] + } + public var underlyingError: Error { switch self { case .failedToFetchPrivacyStatsSummary(let error), From 9975e63265e617ce9c25ae1be6d531f6de5e6592 Mon Sep 17 00:00:00 2001 From: Alexey Martemyanov Date: Tue, 10 Dec 2024 16:10:36 +0600 Subject: [PATCH 10/12] Malware protection 6: Malware integration (#1099) Task/Issue URL: https://app.asana.com/0/481882893211075/1208033567421351/f iOS PR: https://github.com/duckduckgo/iOS/pull/3690 macOS PR: https://github.com/duckduckgo/macos-browser/pull/3604 --- Package.resolved | 8 +++--- Package.swift | 4 +-- .../DefaultConfigurationManager.swift | 3 ++- .../API/APIClient.swift | 8 +++++- .../API/APIRequest.swift | 6 ++--- .../Model/MaliciousSiteError.swift | 12 ++++----- .../Model/ThreatKind.swift | 2 +- .../Extensions/WKErrorExtension.swift | 4 +++ .../PrivacyDashboardUserScript.swift | 11 +++++--- .../SpecialErrorPages/SpecialErrorData.swift | 4 +-- .../SpecialErrorPageUserScript.swift | 25 ++++++++++--------- Sources/Suggestions/SuggestionLoading.swift | 10 ++++---- ...teProtectionEmbeddedDataProviderTest.swift | 4 +++ ...iousSiteProtectionUpdateManagerTests.swift | 2 +- ...usSiteProtectionEmbeddedDataProvider.swift | 1 - .../Resources/malwareFilterSet.json | 6 +++++ .../Resources/malwareHashPrefixes.json | 5 ++++ .../PrivacyDashboardControllerTests.swift | 14 ++++++----- 18 files changed, 80 insertions(+), 49 deletions(-) create mode 100644 Tests/MaliciousSiteProtectionTests/Resources/malwareFilterSet.json create mode 100644 Tests/MaliciousSiteProtectionTests/Resources/malwareHashPrefixes.json diff --git a/Package.resolved b/Package.resolved index dd982ae8a..dd10b441d 100644 --- a/Package.resolved +++ b/Package.resolved @@ -14,8 +14,8 @@ "kind" : "remoteSourceControl", "location" : "https://github.com/duckduckgo/content-scope-scripts", "state" : { - "revision" : "c4bb146afdf0c7a93fb9a7d95b1cb255708a470d", - "version" : "6.41.0" + "revision" : "93ea6c3e771bc0b743b38cefbff548c10add9898", + "version" : "6.42.0" } }, { @@ -50,8 +50,8 @@ "kind" : "remoteSourceControl", "location" : "https://github.com/duckduckgo/privacy-dashboard", "state" : { - "revision" : "49db79829dcb166b3524afdbc1c680890452ce1c", - "version" : "7.2.1" + "revision" : "022c845b06ace6a4aa712a4fa3e79da32193d5c6", + "version" : "7.4.0" } }, { diff --git a/Package.swift b/Package.swift index 7750c3930..21a162064 100644 --- a/Package.swift +++ b/Package.swift @@ -55,8 +55,8 @@ let package = Package( .package(url: "https://github.com/duckduckgo/TrackerRadarKit", exact: "3.0.0"), .package(url: "https://github.com/duckduckgo/sync_crypto", exact: "0.3.0"), .package(url: "https://github.com/gumob/PunycodeSwift.git", exact: "3.0.0"), - .package(url: "https://github.com/duckduckgo/content-scope-scripts", exact: "6.41.0"), - .package(url: "https://github.com/duckduckgo/privacy-dashboard", exact: "7.2.1"), + .package(url: "https://github.com/duckduckgo/content-scope-scripts", exact: "6.42.0"), + .package(url: "https://github.com/duckduckgo/privacy-dashboard", exact: "7.4.0"), .package(url: "https://github.com/httpswift/swifter.git", exact: "1.5.0"), .package(url: "https://github.com/duckduckgo/bloom_cpp.git", exact: "3.0.0"), .package(url: "https://github.com/1024jp/GzipSwift.git", exact: "6.0.1"), diff --git a/Sources/Configuration/DefaultConfigurationManager.swift b/Sources/Configuration/DefaultConfigurationManager.swift index 76996cd0b..b5e074e5d 100644 --- a/Sources/Configuration/DefaultConfigurationManager.swift +++ b/Sources/Configuration/DefaultConfigurationManager.swift @@ -96,7 +96,8 @@ open class DefaultConfigurationManager: NSObject { public func start() { Logger.config.debug("Starting configuration refresh timer") - refreshTask = Task.periodic(interval: Constants.refreshCheckIntervalSeconds) { [weak self] in + refreshTask = Task.periodic(delay: Constants.refreshCheckIntervalSeconds, // add initial delay as we‘re firing `refreshNow` + interval: Constants.refreshCheckIntervalSeconds) { [weak self] in Self.queue.async { [weak self] in self?.lastRefreshCheckTime = Date() self?.refreshIfNeeded() diff --git a/Sources/MaliciousSiteProtection/API/APIClient.swift b/Sources/MaliciousSiteProtection/API/APIClient.swift index d16669d4b..6bf0319f4 100644 --- a/Sources/MaliciousSiteProtection/API/APIClient.swift +++ b/Sources/MaliciousSiteProtection/API/APIClient.swift @@ -31,6 +31,11 @@ extension APIClient: APIClient.Mockable {} public protocol APIClientEnvironment { func headers(for requestType: APIRequestType) -> APIRequestV2.HeadersV2 func url(for requestType: APIRequestType) -> URL + func timeout(for requestType: APIRequestType) -> TimeInterval? +} + +public extension APIClientEnvironment { + func timeout(for requestType: APIRequestType) -> TimeInterval? { nil } } public extension MaliciousSiteDetector { @@ -100,8 +105,9 @@ struct APIClient { let requestType = requestConfig.requestType let headers = environment.headers(for: requestType) let url = environment.url(for: requestType) + let timeout = environment.timeout(for: requestType) ?? requestConfig.defaultTimeout ?? 60 - let apiRequest = APIRequestV2(url: url, method: .get, headers: headers, timeoutInterval: requestConfig.timeout ?? 60) + let apiRequest = APIRequestV2(url: url, method: .get, headers: headers, timeoutInterval: timeout) let response = try await service.fetch(request: apiRequest) let result: R.Response = try response.decodeBody() diff --git a/Sources/MaliciousSiteProtection/API/APIRequest.swift b/Sources/MaliciousSiteProtection/API/APIRequest.swift index 39fb623bd..16128ccb0 100644 --- a/Sources/MaliciousSiteProtection/API/APIRequest.swift +++ b/Sources/MaliciousSiteProtection/API/APIRequest.swift @@ -30,7 +30,7 @@ extension APIClient { protocol Request { associatedtype Response: Decodable // Strongly-typed response type var requestType: APIRequestType { get } // Enumerated type of request being made - var timeout: TimeInterval? { get } + var defaultTimeout: TimeInterval? { get } } // Protocol for requests that modify a set of malicious site detection data @@ -40,7 +40,7 @@ extension APIClient { } } extension APIClient.Request { - var timeout: TimeInterval? { nil } + var defaultTimeout: TimeInterval? { nil } } public extension APIRequestType { @@ -101,7 +101,7 @@ public extension APIRequestType { .matches(self) } - var timeout: TimeInterval? { 1 } + var defaultTimeout: TimeInterval? { 10 } } } /// extension to call generic `load(_: some Request)` method like this: `load(.matches(…))` diff --git a/Sources/MaliciousSiteProtection/Model/MaliciousSiteError.swift b/Sources/MaliciousSiteProtection/Model/MaliciousSiteError.swift index 8da2523f5..cfdfc97bd 100644 --- a/Sources/MaliciousSiteProtection/Model/MaliciousSiteError.swift +++ b/Sources/MaliciousSiteProtection/Model/MaliciousSiteError.swift @@ -22,7 +22,7 @@ public struct MaliciousSiteError: Error, Equatable { public enum Code: Int { case phishing = 1 - // case malware = 2 + case malware = 2 } public let code: Code public let failingUrl: URL @@ -37,8 +37,8 @@ public struct MaliciousSiteError: Error, Equatable { switch threat { case .phishing: code = .phishing - // case .malware: - // code = .malware + case .malware: + code = .malware } self.init(code: code, failingUrl: failingUrl) } @@ -46,7 +46,7 @@ public struct MaliciousSiteError: Error, Equatable { public var threatKind: ThreatKind { switch code { case .phishing: .phishing - // case .malware: .malware + case .malware: .malware } } @@ -70,8 +70,8 @@ extension MaliciousSiteError: LocalizedError { switch code { case .phishing: return "Phishing detected" - // case .malware: - // return "Malware detected" + case .malware: + return "Malware detected" } } diff --git a/Sources/MaliciousSiteProtection/Model/ThreatKind.swift b/Sources/MaliciousSiteProtection/Model/ThreatKind.swift index bec9e2996..ceb59219e 100644 --- a/Sources/MaliciousSiteProtection/Model/ThreatKind.swift +++ b/Sources/MaliciousSiteProtection/Model/ThreatKind.swift @@ -22,6 +22,6 @@ public enum ThreatKind: String, CaseIterable, Codable, CustomStringConvertible { public var description: String { rawValue } case phishing - // case malware + case malware } diff --git a/Sources/Navigation/Extensions/WKErrorExtension.swift b/Sources/Navigation/Extensions/WKErrorExtension.swift index de750e766..c6f4b43ed 100644 --- a/Sources/Navigation/Extensions/WKErrorExtension.swift +++ b/Sources/Navigation/Extensions/WKErrorExtension.swift @@ -36,6 +36,10 @@ extension WKError { public var isServerCertificateUntrusted: Bool { _nsError.isServerCertificateUntrusted } + + public var isWebContentProcessTerminated: Bool { + code == .webContentProcessTerminated && _nsError is WKError + } } extension NSError { public var isServerCertificateUntrusted: Bool { diff --git a/Sources/PrivacyDashboard/PrivacyDashboardUserScript.swift b/Sources/PrivacyDashboard/PrivacyDashboardUserScript.swift index 801cdd81c..effe229fa 100644 --- a/Sources/PrivacyDashboard/PrivacyDashboardUserScript.swift +++ b/Sources/PrivacyDashboard/PrivacyDashboardUserScript.swift @@ -427,12 +427,15 @@ final class PrivacyDashboardUserScript: NSObject, StaticUserScript { } func setMaliciousSiteDetectedThreatKind(_ detectedThreatKind: MaliciousSiteProtection.ThreatKind?, webView: WKWebView) { - let phishingStatus = ["phishingStatus": detectedThreatKind == .phishing] - guard let phishingStatusJson = try? JSONEncoder().encode(phishingStatus).utf8String() else { - assertionFailure("Can't encode phishingStatus into JSON") + let statusJson: String + do { + let obj = ["kind": detectedThreatKind?.rawValue ?? NSNull() as Any] + statusJson = try JSONSerialization.data(withJSONObject: obj).utf8String()! + } catch { + assertionFailure("Can't encode status: \(error)") return } - evaluate(js: "window.onChangePhishingStatus(\(phishingStatusJson))", in: webView) + evaluate(js: "window.onChangeMaliciousSiteStatus(\(statusJson))", in: webView) } func setIsPendingUpdates(_ isPendingUpdates: Bool, webView: WKWebView) { diff --git a/Sources/SpecialErrorPages/SpecialErrorData.swift b/Sources/SpecialErrorPages/SpecialErrorData.swift index 048077847..278ae6abc 100644 --- a/Sources/SpecialErrorPages/SpecialErrorData.swift +++ b/Sources/SpecialErrorPages/SpecialErrorData.swift @@ -22,7 +22,7 @@ import MaliciousSiteProtection public enum SpecialErrorKind: String, Encodable { case ssl case phishing - // case malware + case malware } public enum SpecialErrorData: Encodable, Equatable { @@ -70,7 +70,7 @@ public enum SpecialErrorData: Encodable, Equatable { public extension MaliciousSiteProtection.ThreatKind { var errorPageKind: SpecialErrorKind { switch self { - // case .malware: .malware + case .malware: .malware case .phishing: .phishing } } diff --git a/Sources/SpecialErrorPages/SpecialErrorPageUserScript.swift b/Sources/SpecialErrorPages/SpecialErrorPageUserScript.swift index f131358ef..b2f35f281 100644 --- a/Sources/SpecialErrorPages/SpecialErrorPageUserScript.swift +++ b/Sources/SpecialErrorPages/SpecialErrorPageUserScript.swift @@ -71,21 +71,22 @@ public final class SpecialErrorPageUserScript: NSObject, Subfeature { self.languageCode = languageCode } - @MainActor public func handler(forMethodNamed methodName: String) -> Subfeature.Handler? { - guard isEnabled, let messageName = MessageName(rawValue: methodName) else { return nil } - return methodHandlers[messageName] + guard isEnabled else { return nil } + + switch MessageName(rawValue: methodName) { + case .initialSetup: return initialSetup + case .reportPageException: return reportPageException + case .reportInitException: return reportInitException + case .leaveSite: return handleLeaveSiteAction + case .visitSite: return handleVisitSiteAction + case .advancedInfo: return handleAdvancedInfoPresented + default: + assertionFailure("SpecialErrorPageUserScript: Failed to parse User Script message: \(methodName)") + return nil + } } - private lazy var methodHandlers: [MessageName: Handler] = [ - .initialSetup: initialSetup, - .reportPageException: reportPageException, - .reportInitException: reportInitException, - .leaveSite: handleLeaveSiteAction, - .visitSite: handleVisitSiteAction, - .advancedInfo: handleAdvancedInfoPresented - ] - @MainActor private func initialSetup(params: Any, original: WKScriptMessage) async throws -> Encodable? { #if DEBUG diff --git a/Sources/Suggestions/SuggestionLoading.swift b/Sources/Suggestions/SuggestionLoading.swift index 690e9ffc5..704a1e487 100644 --- a/Sources/Suggestions/SuggestionLoading.swift +++ b/Sources/Suggestions/SuggestionLoading.swift @@ -90,11 +90,11 @@ public class SuggestionLoader: SuggestionLoading { guard let self = self else { return } let processor = SuggestionProcessing(platform: dataSource.platform, urlFactory: self.urlFactory) let result = processor.result(for: query, - from: history, - bookmarks: bookmarks, - internalPages: internalPages, - openTabs: openTabs, - apiResult: apiResult) + from: history, + bookmarks: bookmarks, + internalPages: internalPages, + openTabs: openTabs, + apiResult: apiResult) DispatchQueue.main.async { if let result = result { completion(result, apiError) diff --git a/Tests/MaliciousSiteProtectionTests/MaliciousSiteProtectionEmbeddedDataProviderTest.swift b/Tests/MaliciousSiteProtectionTests/MaliciousSiteProtectionEmbeddedDataProviderTest.swift index 1e3e0df40..850fed9cd 100644 --- a/Tests/MaliciousSiteProtectionTests/MaliciousSiteProtectionEmbeddedDataProviderTest.swift +++ b/Tests/MaliciousSiteProtectionTests/MaliciousSiteProtectionEmbeddedDataProviderTest.swift @@ -42,11 +42,15 @@ class MaliciousSiteProtectionEmbeddedDataProviderTest: XCTestCase { switch key.threatKind { case .phishing: "4fd2868a4f264501ec175ab866504a2a96c8d21a3b5195b405a4a83b51eae504" + case .malware: + "7f80bcae89250c4ecc4ed91b5a4c3a09fe6f098622369f2f46a8ab69023a7683" } case .hashPrefixSet(let key): switch key.threatKind { case .phishing: "21b047a9950fcaf86034a6b16181e18815cb8d276386d85c8977ca8c5f8aa05f" + case .malware: + "ef31819296d83136cdb131e877e08fd120571d4c82512ba8c3eb964885ec07bc" } } } diff --git a/Tests/MaliciousSiteProtectionTests/MaliciousSiteProtectionUpdateManagerTests.swift b/Tests/MaliciousSiteProtectionTests/MaliciousSiteProtectionUpdateManagerTests.swift index 8d46d5cf7..15718eb7b 100644 --- a/Tests/MaliciousSiteProtectionTests/MaliciousSiteProtectionUpdateManagerTests.swift +++ b/Tests/MaliciousSiteProtectionTests/MaliciousSiteProtectionUpdateManagerTests.swift @@ -335,7 +335,7 @@ class MaliciousSiteProtectionUpdateManagerTests: XCTestCase { XCTestExpectation(description: "Will Sleep 3"), ] self.willSleep = { _ in - sleepExpectations[sleepIndex].fulfill() + sleepExpectations[safe: sleepIndex]?.fulfill() sleepIndex += 1 } diff --git a/Tests/MaliciousSiteProtectionTests/Mocks/MockMaliciousSiteProtectionEmbeddedDataProvider.swift b/Tests/MaliciousSiteProtectionTests/Mocks/MockMaliciousSiteProtectionEmbeddedDataProvider.swift index 10a3e2643..329f92627 100644 --- a/Tests/MaliciousSiteProtectionTests/Mocks/MockMaliciousSiteProtectionEmbeddedDataProvider.swift +++ b/Tests/MaliciousSiteProtectionTests/Mocks/MockMaliciousSiteProtectionEmbeddedDataProvider.swift @@ -65,7 +65,6 @@ final class MockMaliciousSiteProtectionEmbeddedDataProvider: MaliciousSiteProtec } func data(withContentsOf url: URL) throws -> Data { - let data: Data switch url.absoluteString { case "filterSet": self.loadFilterSetCalled = true diff --git a/Tests/MaliciousSiteProtectionTests/Resources/malwareFilterSet.json b/Tests/MaliciousSiteProtectionTests/Resources/malwareFilterSet.json new file mode 100644 index 000000000..ad87c96d3 --- /dev/null +++ b/Tests/MaliciousSiteProtectionTests/Resources/malwareFilterSet.json @@ -0,0 +1,6 @@ +[ + { + "hash": "e4753ddad954dafd4ff4ef67f82b3c1a2db6ef4a51bda43513260170e558bd13", + "regex": "(?i)^https?\\:\\/\\/privacy-test-pages\\.site(?:\\:(?:80|443))?\\/security\\/badware\\/malware\\.html$" + } +] diff --git a/Tests/MaliciousSiteProtectionTests/Resources/malwareHashPrefixes.json b/Tests/MaliciousSiteProtectionTests/Resources/malwareHashPrefixes.json new file mode 100644 index 000000000..ad935a010 --- /dev/null +++ b/Tests/MaliciousSiteProtectionTests/Resources/malwareHashPrefixes.json @@ -0,0 +1,5 @@ +[ + "e4753dda", + "012db806", + "56fbfbf2" +] diff --git a/Tests/PrivacyDashboardTests/PrivacyDashboardControllerTests.swift b/Tests/PrivacyDashboardTests/PrivacyDashboardControllerTests.swift index 867e7b888..a3ee836fc 100644 --- a/Tests/PrivacyDashboardTests/PrivacyDashboardControllerTests.swift +++ b/Tests/PrivacyDashboardTests/PrivacyDashboardControllerTests.swift @@ -16,11 +16,13 @@ // limitations under the License. // -import XCTest import Common +import os import WebKit -@testable import PrivacyDashboard +import XCTest + @testable import BrowserServicesKit +@testable import PrivacyDashboard @MainActor final class PrivacyDashboardControllerTests: XCTestCase { @@ -268,8 +270,8 @@ final class PrivacyDashboardControllerTests: XCTestCase { privacyDashboardController.privacyInfo!.malicousSiteThreatKind = .phishing - wait(for: [expectation], timeout: 100) - XCTAssertEqual(mockWebView.capturedJavaScriptString, "window.onChangePhishingStatus({\"phishingStatus\":true})") + wait(for: [expectation], timeout: 5) + XCTAssertEqual(mockWebView.capturedJavaScriptString, "window.onChangeMaliciousSiteStatus({\"kind\":\"phishing\"})") } } @@ -287,8 +289,8 @@ class MockWebView: WKWebView { } override func evaluateJavaScript(_ javaScriptString: String) async throws -> Any { - print(javaScriptString) - if javaScriptString.contains("window.onChangePhishingStatus") { + Logger(subsystem: Bundle.main.bundleIdentifier!, category: "DDGTest").info("received javaScriptString \(javaScriptString, privacy: .public)") + if javaScriptString.contains("window.onChangeMaliciousSiteStatus") { capturedJavaScriptString = javaScriptString expectation.fulfill() } From b7f7ba99f7c77b576679eee426ddd80320dc74d8 Mon Sep 17 00:00:00 2001 From: Alexey Martemyanov Date: Tue, 10 Dec 2024 19:59:02 +0600 Subject: [PATCH 11/12] remove MaliciousSiteProtectionSubfeature (#1131) Task/Issue URL: https://app.asana.com/0/481882893211075/1208033567421351/f iOS PR: macOS PR: https://github.com/duckduckgo/macos-browser/pull/3604 --- .../PrivacyConfig/Features/PrivacyFeature.swift | 5 ----- 1 file changed, 5 deletions(-) diff --git a/Sources/BrowserServicesKit/PrivacyConfig/Features/PrivacyFeature.swift b/Sources/BrowserServicesKit/PrivacyConfig/Features/PrivacyFeature.swift index 56cdd44e6..dc95e2ad6 100644 --- a/Sources/BrowserServicesKit/PrivacyConfig/Features/PrivacyFeature.swift +++ b/Sources/BrowserServicesKit/PrivacyConfig/Features/PrivacyFeature.swift @@ -184,11 +184,6 @@ public enum DuckPlayerSubfeature: String, PrivacySubfeature { case enableDuckPlayer // iOS DuckPlayer rollout feature } -public enum MaliciousSiteProtectionSubfeature: String, PrivacySubfeature { - public var parent: PrivacyFeature { .maliciousSiteProtection } - case allowErrorPage -} - public enum SyncPromotionSubfeature: String, PrivacySubfeature { public var parent: PrivacyFeature { .syncPromotion } case bookmarks From e8654e1a51bd20fa8fa234a4155a9aec37411ed1 Mon Sep 17 00:00:00 2001 From: Pete Smith <5278441+aataraxiaa@users.noreply.github.com> Date: Wed, 11 Dec 2024 15:32:40 +0000 Subject: [PATCH 12/12] Privacy Pro Free Trials - Models and API (#1120) Task/Issue URL: https://app.asana.com/0/1208114992212396/1208796999534221/f iOS PR: https://github.com/duckduckgo/iOS/pull/3691 macOS PR: https://github.com/duckduckgo/macos-browser/pull/3641 What kind of version bump will this require?: Minor **Optional**: Tech Design URL: https://app.asana.com/0/481882893211075/1208773437150501/f **Description**: This PR includes the following: * Adds a new model type for Subscription Offers * Adds a new API to `StorePurchaseManager` to retrieve Free Trial Subscriptions * Updates existing `subscriptionOptions` API to NOT return Free Trial Subscriptions * Adds abstractions on StoreKit types to enable testing * Adds tests (**Note: Make sure to expand the `StorePurchaseManagerTests` file when reviewing**) --- .../Flows/Models/SubscriptionOptions.swift | 21 + .../ProductFetching.swift | 45 ++ .../StorePurchaseManager.swift | 112 ++-- .../SubscriptionProduct.swift | 101 ++++ ...SubscriptionProductIntroductoryOffer.swift | 67 +++ .../StoreSubscriptionConfiguration.swift | 4 +- .../Managers/StorePurchaseManagerMock.swift | 6 + .../SubscriptionFeatureMappingCacheMock.swift | 5 + .../Models/SubscriptionOptionsTests.swift | 24 +- .../Managers/StorePurchaseManagerTests.swift | 542 ++++++++++++++++++ 10 files changed, 885 insertions(+), 42 deletions(-) create mode 100644 Sources/Subscription/Managers/StorePurchaseManager/ProductFetching.swift rename Sources/Subscription/Managers/{ => StorePurchaseManager}/StorePurchaseManager.swift (84%) create mode 100644 Sources/Subscription/Managers/StorePurchaseManager/SubscriptionProduct.swift create mode 100644 Sources/Subscription/Managers/StorePurchaseManager/SubscriptionProductIntroductoryOffer.swift create mode 100644 Tests/SubscriptionTests/Managers/StorePurchaseManagerTests.swift diff --git a/Sources/Subscription/Flows/Models/SubscriptionOptions.swift b/Sources/Subscription/Flows/Models/SubscriptionOptions.swift index 5544cf680..2c69e31f6 100644 --- a/Sources/Subscription/Flows/Models/SubscriptionOptions.swift +++ b/Sources/Subscription/Flows/Models/SubscriptionOptions.swift @@ -50,6 +50,13 @@ public enum SubscriptionPlatformName: String, Encodable { public struct SubscriptionOption: Encodable, Equatable { let id: String let cost: SubscriptionOptionCost + let offer: SubscriptionOptionOffer? + + init(id: String, cost: SubscriptionOptionCost, offer: SubscriptionOptionOffer? = nil) { + self.id = id + self.cost = cost + self.offer = offer + } } struct SubscriptionOptionCost: Encodable, Equatable { @@ -60,3 +67,17 @@ struct SubscriptionOptionCost: Encodable, Equatable { public struct SubscriptionFeature: Encodable, Equatable { let name: Entitlement.ProductName } + +/// A `SubscriptionOptionOffer` represents an offer (e.g Free Trials) associated with a Subscription +public struct SubscriptionOptionOffer: Encodable, Equatable { + + public enum OfferType: String, Codable, CaseIterable { + case freeTrial + } + + let type: OfferType + let id: String + let displayPrice: String + let durationInDays: Int + let isUserEligible: Bool +} diff --git a/Sources/Subscription/Managers/StorePurchaseManager/ProductFetching.swift b/Sources/Subscription/Managers/StorePurchaseManager/ProductFetching.swift new file mode 100644 index 000000000..485bd8a6a --- /dev/null +++ b/Sources/Subscription/Managers/StorePurchaseManager/ProductFetching.swift @@ -0,0 +1,45 @@ +// +// ProductFetching.swift +// +// Copyright © 2024 DuckDuckGo. All rights reserved. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +// + +import Foundation +import StoreKit + +/// A protocol for types that can fetch subscription products. +@available(macOS 12.0, iOS 15.0, *) +public protocol ProductFetching { + /// Fetches products for the specified identifiers. + /// - Parameter identifiers: An array of product identifiers to fetch. + /// - Returns: An array of subscription products. + /// - Throws: An error if the fetch operation fails. + func products(for identifiers: [String]) async throws -> [any SubscriptionProduct] +} + +/// A default implementation of ProductFetching that uses StoreKit's standard product fetching. +@available(macOS 12.0, iOS 15.0, *) +public final class DefaultProductFetcher: ProductFetching { + /// Initializes a new DefaultProductFetcher instance. + public init() {} + + /// Fetches products using StoreKit's Product.products API. + /// - Parameter identifiers: An array of product identifiers to fetch. + /// - Returns: An array of subscription products. + /// - Throws: An error if the fetch operation fails. + public func products(for identifiers: [String]) async throws -> [any SubscriptionProduct] { + return try await Product.products(for: identifiers) + } +} diff --git a/Sources/Subscription/Managers/StorePurchaseManager.swift b/Sources/Subscription/Managers/StorePurchaseManager/StorePurchaseManager.swift similarity index 84% rename from Sources/Subscription/Managers/StorePurchaseManager.swift rename to Sources/Subscription/Managers/StorePurchaseManager/StorePurchaseManager.swift index 47791eb22..b23f24548 100644 --- a/Sources/Subscription/Managers/StorePurchaseManager.swift +++ b/Sources/Subscription/Managers/StorePurchaseManager/StorePurchaseManager.swift @@ -37,7 +37,16 @@ public enum StorePurchaseManagerError: Error { public protocol StorePurchaseManager { typealias TransactionJWS = String + /// Returns the available subscription options that DON'T include Free Trial periods. + /// - Returns: A `SubscriptionOptions` object containing the available subscription plans and pricing, + /// or `nil` if no options are available or cannot be fetched. func subscriptionOptions() async -> SubscriptionOptions? + + /// Returns the subscription options that include Free Trial periods. + /// - Returns: A `SubscriptionOptions` object containing subscription plans with free trial offers, + /// or `nil` if no free trial options are available or the user is not eligible. + func freeTrialSubscriptionOptions() async -> SubscriptionOptions? + var purchasedProductIDs: [String] { get } var purchaseQueue: [String] { get } var areProductsAvailable: Bool { get } @@ -61,7 +70,7 @@ public final class DefaultStorePurchaseManager: ObservableObject, StorePurchaseM private let subscriptionFeatureMappingCache: SubscriptionFeatureMappingCache private let subscriptionFeatureFlagger: FeatureFlaggerMapping? - @Published public private(set) var availableProducts: [Product] = [] + @Published public private(set) var availableProducts: [any SubscriptionProduct] = [] @Published public private(set) var purchasedProductIDs: [String] = [] @Published public private(set) var purchaseQueue: [String] = [] @@ -70,11 +79,15 @@ public final class DefaultStorePurchaseManager: ObservableObject, StorePurchaseM private var transactionUpdates: Task? private var storefrontChanges: Task? + private var productFetcher: ProductFetching + public init(subscriptionFeatureMappingCache: SubscriptionFeatureMappingCache, - subscriptionFeatureFlagger: FeatureFlaggerMapping? = nil) { + subscriptionFeatureFlagger: FeatureFlaggerMapping? = nil, + productFetcher: ProductFetching = DefaultProductFetcher()) { self.storeSubscriptionConfiguration = DefaultStoreSubscriptionConfiguration() self.subscriptionFeatureMappingCache = subscriptionFeatureMappingCache self.subscriptionFeatureFlagger = subscriptionFeatureFlagger + self.productFetcher = productFetcher transactionUpdates = observeTransactionUpdates() storefrontChanges = observeStorefrontChanges() } @@ -104,40 +117,13 @@ public final class DefaultStorePurchaseManager: ObservableObject, StorePurchaseM } public func subscriptionOptions() async -> SubscriptionOptions? { - Logger.subscription.info("[AppStorePurchaseFlow] subscriptionOptions") - let products = availableProducts - let monthly = products.first(where: { $0.subscription?.subscriptionPeriod.unit == .month && $0.subscription?.subscriptionPeriod.value == 1 }) - let yearly = products.first(where: { $0.subscription?.subscriptionPeriod.unit == .year && $0.subscription?.subscriptionPeriod.value == 1 }) - guard let monthly, let yearly else { - Logger.subscription.error("[AppStorePurchaseFlow] No products found") - return nil - } - - let platform: SubscriptionPlatformName = { -#if os(iOS) - .ios -#else - .macos -#endif - }() - - let options = [SubscriptionOption(id: monthly.id, - cost: .init(displayPrice: monthly.displayPrice, recurrence: "monthly")), - SubscriptionOption(id: yearly.id, - cost: .init(displayPrice: yearly.displayPrice, recurrence: "yearly"))] - - let features: [SubscriptionFeature] - - if let featureFlagger = subscriptionFeatureFlagger, featureFlagger.isFeatureOn(.isLaunchedROW) || featureFlagger.isFeatureOn(.isLaunchedROWOverride) { - features = await subscriptionFeatureMappingCache.subscriptionFeatures(for: monthly.id).compactMap { SubscriptionFeature(name: $0) } - } else { - let allFeatures: [Entitlement.ProductName] = [.networkProtection, .dataBrokerProtection, .identityTheftRestoration] - features = allFeatures.compactMap { SubscriptionFeature(name: $0) } - } + let nonFreeTrialProducts = availableProducts.filter { !$0.hasFreeTrialOffer } + return await subscriptionOptions(for: nonFreeTrialProducts) + } - return SubscriptionOptions(platform: platform, - options: options, - features: features) + public func freeTrialSubscriptionOptions() async -> SubscriptionOptions? { + let freeTrialProducts = availableProducts.filter { $0.hasFreeTrialOffer } + return await subscriptionOptions(for: freeTrialProducts) } @MainActor @@ -165,10 +151,10 @@ public final class DefaultStorePurchaseManager: ObservableObject, StorePurchaseM self.currentStorefrontRegion = storefrontRegion let applicableProductIdentifiers = storeSubscriptionConfiguration.subscriptionIdentifiers(for: storefrontRegion) - let availableProducts = try await Product.products(for: applicableProductIdentifiers) + let availableProducts = try await productFetcher.products(for: applicableProductIdentifiers) Logger.subscription.info("[StorePurchaseManager] updateAvailableProducts fetched \(availableProducts.count) products for \(storefrontCountryCode ?? "", privacy: .public)") - if self.availableProducts != availableProducts { + if Set(availableProducts.map { $0.id }) != Set(self.availableProducts.map { $0.id }) { self.availableProducts = availableProducts // Update cached subscription features mapping @@ -298,6 +284,40 @@ public final class DefaultStorePurchaseManager: ObservableObject, StorePurchaseM } } + private func subscriptionOptions(for products: [any SubscriptionProduct]) async -> SubscriptionOptions? { + Logger.subscription.info("[AppStorePurchaseFlow] subscriptionOptions") + let monthly = products.first(where: { $0.isMonthly }) + let yearly = products.first(where: { $0.isYearly }) + guard let monthly, let yearly else { + Logger.subscription.error("[AppStorePurchaseFlow] No products found") + return nil + } + + let platform: SubscriptionPlatformName = { +#if os(iOS) + .ios +#else + .macos +#endif + }() + + let options: [SubscriptionOption] = await [.init(from: monthly, withRecurrence: "monthly"), + .init(from: yearly, withRecurrence: "yearly")] + + let features: [SubscriptionFeature] + + if let featureFlagger = subscriptionFeatureFlagger, featureFlagger.isFeatureOn(.isLaunchedROW) || featureFlagger.isFeatureOn(.isLaunchedROWOverride) { + features = await subscriptionFeatureMappingCache.subscriptionFeatures(for: monthly.id).compactMap { SubscriptionFeature(name: $0) } + } else { + let allFeatures: [Entitlement.ProductName] = [.networkProtection, .dataBrokerProtection, .identityTheftRestoration] + features = allFeatures.compactMap { SubscriptionFeature(name: $0) } + } + + return SubscriptionOptions(platform: platform, + options: options, + features: features) + } + private func checkVerified(_ result: VerificationResult) throws -> T { // Check whether the JWS passes StoreKit verification. switch result { @@ -337,6 +357,24 @@ public final class DefaultStorePurchaseManager: ObservableObject, StorePurchaseM } } +@available(macOS 12.0, iOS 15.0, *) +private extension SubscriptionOption { + + init(from product: any SubscriptionProduct, withRecurrence recurrence: String) async { + var offer: SubscriptionOptionOffer? + + if let introOffer = product.introductoryOffer, introOffer.isFreeTrial { + + let durationInDays = introOffer.periodInDays + let isUserEligible = await product.isEligibleForIntroOffer + + offer = .init(type: .freeTrial, id: introOffer.id ?? "", displayPrice: introOffer.displayPrice, durationInDays: durationInDays, isUserEligible: isUserEligible) + } + + self.init(id: product.id, cost: .init(displayPrice: product.displayPrice, recurrence: recurrence), offer: offer) + } +} + public extension UserDefaults { enum Constants { diff --git a/Sources/Subscription/Managers/StorePurchaseManager/SubscriptionProduct.swift b/Sources/Subscription/Managers/StorePurchaseManager/SubscriptionProduct.swift new file mode 100644 index 000000000..4c0e64faa --- /dev/null +++ b/Sources/Subscription/Managers/StorePurchaseManager/SubscriptionProduct.swift @@ -0,0 +1,101 @@ +// +// SubscriptionProduct.swift +// +// Copyright © 2024 DuckDuckGo. All rights reserved. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +// + +import Foundation +import StoreKit + +/// A protocol that defines the core functionality and properties of a subscription product. +/// Conforming types must provide information about pricing, description, and subscription terms. +@available(macOS 12.0, iOS 15.0, *) +public protocol SubscriptionProduct: Equatable { + /// The unique identifier of the product. + var id: String { get } + + /// The user-facing name of the product. + var displayName: String { get } + + /// The formatted price that should be displayed to users. + var displayPrice: String { get } + + /// A detailed description of the product. + var description: String { get } + + /// Indicates whether this is a monthly subscription. + var isMonthly: Bool { get } + + /// Indicates whether this is a yearly subscription. + var isYearly: Bool { get } + + /// The introductory offer associated with this subscription, if any. + var introductoryOffer: SubscriptionProductIntroductoryOffer? { get } + + /// Indicates whether this subscription has a Free Trial offer available. + var hasFreeTrialOffer: Bool { get } + + /// Asynchronously determines whether the user is eligible for an introductory offer. + var isEligibleForIntroOffer: Bool { get async } + + /// Initiates a purchase of the subscription with the specified options. + /// - Parameter options: A set of options to configure the purchase. + /// - Returns: The result of the purchase attempt. + /// - Throws: An error if the purchase fails. + func purchase(options: Set) async throws -> Product.PurchaseResult +} + +/// Extends StoreKit's Product to conform to SubscriptionProduct. +@available(macOS 12.0, iOS 15.0, *) +extension Product: SubscriptionProduct { + /// Determines if this is a monthly subscription by checking if the subscription period + /// is exactly one month. + public var isMonthly: Bool { + guard let subscription else { return false } + return subscription.subscriptionPeriod.unit == .month && + subscription.subscriptionPeriod.value == 1 + } + + /// Determines if this is a yearly subscription by checking if the subscription period + /// is exactly one year. + public var isYearly: Bool { + guard let subscription else { return false } + return subscription.subscriptionPeriod.unit == .year && + subscription.subscriptionPeriod.value == 1 + } + + /// Returns the introductory offer for this subscription if available. + public var introductoryOffer: (any SubscriptionProductIntroductoryOffer)? { + subscription?.introductoryOffer + } + + /// Indicates whether this subscription has a Free Trial offer. + public var hasFreeTrialOffer: Bool { + return subscription?.introductoryOffer?.isFreeTrial ?? false + } + + /// Asynchronously checks if the user is eligible for an introductory offer. + public var isEligibleForIntroOffer: Bool { + get async { + guard let subscription else { return false } + return await subscription.isEligibleForIntroOffer + } + } + + /// Implements Equatable by comparing product IDs. + public static func == (lhs: Product, rhs: Product) -> Bool { + return lhs.id == rhs.id + } +} diff --git a/Sources/Subscription/Managers/StorePurchaseManager/SubscriptionProductIntroductoryOffer.swift b/Sources/Subscription/Managers/StorePurchaseManager/SubscriptionProductIntroductoryOffer.swift new file mode 100644 index 000000000..63bbd7a92 --- /dev/null +++ b/Sources/Subscription/Managers/StorePurchaseManager/SubscriptionProductIntroductoryOffer.swift @@ -0,0 +1,67 @@ +// +// SubscriptionProductIntroductoryOffer.swift +// +// Copyright © 2024 DuckDuckGo. All rights reserved. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +// + +import Foundation +import StoreKit + +/// A protocol that defines the properties of an introductory offer for a subscription product. +/// Use this protocol to represent trial periods, introductory prices, or other special offers. +@available(macOS 12.0, iOS 15.0, *) +public protocol SubscriptionProductIntroductoryOffer { + /// The unique identifier of the introductory offer. + var id: String? { get } + + /// The formatted price of the offer that should be displayed to users. + var displayPrice: String { get } + + /// The duration of the offer in days. + var periodInDays: Int { get } + + /// Indicates whether this offer represents a free trial period. + var isFreeTrial: Bool { get } +} + +/// Extends StoreKit's Product.SubscriptionOffer to conform to SubscriptionProductIntroductoryOffer. +@available(macOS 12.0, iOS 15.0, *) +extension Product.SubscriptionOffer: SubscriptionProductIntroductoryOffer { + /// Calculates the total number of days in the offer period by multiplying + /// the base period length by the period count. + public var periodInDays: Int { + period.periodInDays * periodCount + } + + /// Determines if this offer represents a free trial based on the payment mode. + public var isFreeTrial: Bool { + paymentMode == .freeTrial + } +} + +@available(macOS 12.0, iOS 15.0, *) +private extension Product.SubscriptionPeriod { + + var periodInDays: Int { + switch unit { + case .day: return value + case .week: return value * 7 + case .month: return value * 30 + case .year: return value * 365 + @unknown default: + return value + } + } +} diff --git a/Sources/Subscription/StoreSubscriptionConfiguration.swift b/Sources/Subscription/StoreSubscriptionConfiguration.swift index 8dacf7db4..be3347c62 100644 --- a/Sources/Subscription/StoreSubscriptionConfiguration.swift +++ b/Sources/Subscription/StoreSubscriptionConfiguration.swift @@ -44,7 +44,9 @@ final class DefaultStoreSubscriptionConfiguration: StoreSubscriptionConfiguratio appIdentifier: "com.duckduckgo.mobile.ios.alpha", environment: .staging, identifiersByRegion: [.usa: ["ios.subscription.1month", - "ios.subscription.1year"], + "ios.subscription.1year", + "ios.subscription.1month.freetrial.dev", + "ios.subscription.1year.freetrial.dev"], .restOfWorld: ["ios.subscription.1month.row", "ios.subscription.1year.row"]]), // macOS debug build diff --git a/Sources/SubscriptionTestingUtilities/Managers/StorePurchaseManagerMock.swift b/Sources/SubscriptionTestingUtilities/Managers/StorePurchaseManagerMock.swift index e326fd720..3d5c34f63 100644 --- a/Sources/SubscriptionTestingUtilities/Managers/StorePurchaseManagerMock.swift +++ b/Sources/SubscriptionTestingUtilities/Managers/StorePurchaseManagerMock.swift @@ -20,12 +20,14 @@ import Foundation import Subscription public final class StorePurchaseManagerMock: StorePurchaseManager { + public var purchasedProductIDs: [String] = [] public var purchaseQueue: [String] = [] public var areProductsAvailable: Bool = false public var currentStorefrontRegion: SubscriptionRegion = .usa public var subscriptionOptionsResult: SubscriptionOptions? + public var freeTrialSubscriptionOptionsResult: SubscriptionOptions? public var syncAppleIDAccountResultError: Error? public var mostRecentTransactionResult: String? @@ -44,6 +46,10 @@ public final class StorePurchaseManagerMock: StorePurchaseManager { subscriptionOptionsResult } + public func freeTrialSubscriptionOptions() async -> SubscriptionOptions? { + freeTrialSubscriptionOptionsResult + } + public func syncAppleIDAccount() async throws { if let syncAppleIDAccountResultError { throw syncAppleIDAccountResultError diff --git a/Sources/SubscriptionTestingUtilities/SubscriptionFeatureMappingCacheMock.swift b/Sources/SubscriptionTestingUtilities/SubscriptionFeatureMappingCacheMock.swift index c4612a9bd..ef39c4d04 100644 --- a/Sources/SubscriptionTestingUtilities/SubscriptionFeatureMappingCacheMock.swift +++ b/Sources/SubscriptionTestingUtilities/SubscriptionFeatureMappingCacheMock.swift @@ -21,11 +21,16 @@ import Subscription public final class SubscriptionFeatureMappingCacheMock: SubscriptionFeatureMappingCache { + public var didCallSubscriptionFeatures = false + public var lastCalledSubscriptionId: String? + public var mapping: [String: [Entitlement.ProductName]] = [:] public init() { } public func subscriptionFeatures(for subscriptionIdentifier: String) async -> [Entitlement.ProductName] { + didCallSubscriptionFeatures = true + lastCalledSubscriptionId = subscriptionIdentifier return mapping[subscriptionIdentifier] ?? [] } } diff --git a/Tests/SubscriptionTests/Flows/Models/SubscriptionOptionsTests.swift b/Tests/SubscriptionTests/Flows/Models/SubscriptionOptionsTests.swift index 3fb695733..8ea41fa66 100644 --- a/Tests/SubscriptionTests/Flows/Models/SubscriptionOptionsTests.swift +++ b/Tests/SubscriptionTests/Flows/Models/SubscriptionOptionsTests.swift @@ -23,12 +23,14 @@ import SubscriptionTestingUtilities final class SubscriptionOptionsTests: XCTestCase { func testEncoding() throws { + let monthlySubscriptionOffer = SubscriptionOptionOffer(type: .freeTrial, id: "1", displayPrice: "$0.00", durationInDays: 7, isUserEligible: true) + let yearlySubscriptionOffer = SubscriptionOptionOffer(type: .freeTrial, id: "2", displayPrice: "$0.00", durationInDays: 7, isUserEligible: true) let subscriptionOptions = SubscriptionOptions(platform: .macos, options: [ SubscriptionOption(id: "1", - cost: SubscriptionOptionCost(displayPrice: "9 USD", recurrence: "monthly")), + cost: SubscriptionOptionCost(displayPrice: "9 USD", recurrence: "monthly"), offer: monthlySubscriptionOffer), SubscriptionOption(id: "2", - cost: SubscriptionOptionCost(displayPrice: "99 USD", recurrence: "yearly")) + cost: SubscriptionOptionCost(displayPrice: "99 USD", recurrence: "yearly"), offer: yearlySubscriptionOffer) ], features: [ SubscriptionFeature(name: .networkProtection), @@ -60,14 +62,28 @@ final class SubscriptionOptionsTests: XCTestCase { "displayPrice" : "9 USD", "recurrence" : "monthly" }, - "id" : "1" + "id" : "1", + "offer" : { + "displayPrice" : "$0.00", + "durationInDays" : 7, + "id" : "1", + "isUserEligible" : true, + "type" : "freeTrial" + } }, { "cost" : { "displayPrice" : "99 USD", "recurrence" : "yearly" }, - "id" : "2" + "id" : "2", + "offer" : { + "displayPrice" : "$0.00", + "durationInDays" : 7, + "id" : "2", + "isUserEligible" : true, + "type" : "freeTrial" + } } ], "platform" : "macos" diff --git a/Tests/SubscriptionTests/Managers/StorePurchaseManagerTests.swift b/Tests/SubscriptionTests/Managers/StorePurchaseManagerTests.swift new file mode 100644 index 000000000..730134ec3 --- /dev/null +++ b/Tests/SubscriptionTests/Managers/StorePurchaseManagerTests.swift @@ -0,0 +1,542 @@ +// +// StorePurchaseManagerTests.swift +// +// Copyright © 2024 DuckDuckGo. All rights reserved. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +// + +import XCTest +@testable import Subscription +import SubscriptionTestingUtilities +import StoreKit + +final class StorePurchaseManagerTests: XCTestCase { + + private var sut: StorePurchaseManager! + private var mockCache: SubscriptionFeatureMappingCacheMock! + private var mockProductFetcher: MockProductFetcher! + private var mockFeatureFlagger: MockFeatureFlagger! + + override func setUpWithError() throws { + mockCache = SubscriptionFeatureMappingCacheMock() + mockProductFetcher = MockProductFetcher() + mockFeatureFlagger = MockFeatureFlagger() + sut = DefaultStorePurchaseManager(subscriptionFeatureMappingCache: mockCache, + subscriptionFeatureFlagger: mockFeatureFlagger, + productFetcher: mockProductFetcher) + } + + func testSubscriptionOptionsReturnsOnlyNonTrialProducts() async { + // Given + let monthlyProduct = MockSubscriptionProduct( + id: "com.test.monthly", + displayName: "Monthly Plan", + displayPrice: "$9.99", + isMonthly: true, + hasFreeTrialOffer: false + ) + + let yearlyProduct = MockSubscriptionProduct( + id: "com.test.yearly", + displayName: "Yearly Plan", + displayPrice: "$99.99", + isYearly: true, + hasFreeTrialOffer: false + ) + + let monthlyTrialProduct = MockSubscriptionProduct( + id: "com.test.monthly.trial", + displayName: "Monthly Plan with Trial", + displayPrice: "$9.99", + isMonthly: true, + hasFreeTrialOffer: true, + introOffer: MockIntroductoryOffer( + id: "trial1", + displayPrice: "Free", + periodInDays: 7, + isFreeTrial: true + ) + ) + + mockProductFetcher.mockProducts = [monthlyProduct, yearlyProduct, monthlyTrialProduct] + await sut.updateAvailableProducts() + + // When + let subscriptionOptions = await sut.subscriptionOptions() + + // Then + XCTAssertNotNil(subscriptionOptions) + XCTAssertEqual(subscriptionOptions?.options.count, 2) + + let productIds = subscriptionOptions?.options.map { $0.id } ?? [] + XCTAssertTrue(productIds.contains("com.test.monthly")) + XCTAssertTrue(productIds.contains("com.test.yearly")) + XCTAssertFalse(productIds.contains("com.test.monthly.trial")) + } + + func testFreeTrialSubscriptionOptionsReturnsOnlyTrialProducts() async { + // Given + let monthlyTrialProduct = MockSubscriptionProduct( + id: "com.test.monthly.trial", + displayName: "Monthly Plan with Trial", + displayPrice: "$9.99", + isMonthly: true, + hasFreeTrialOffer: true, + introOffer: MockIntroductoryOffer( + id: "trial1", + displayPrice: "Free", + periodInDays: 7, + isFreeTrial: true + ), + isEligibleForIntroOffer: true + ) + + let yearlyTrialProduct = MockSubscriptionProduct( + id: "com.test.yearly.trial", + displayName: "Yearly Plan with Trial", + displayPrice: "$99.99", + isYearly: true, + hasFreeTrialOffer: true, + introOffer: MockIntroductoryOffer( + id: "trial2", + displayPrice: "Free", + periodInDays: 7, + isFreeTrial: true + ), + isEligibleForIntroOffer: true + ) + + let regularProduct = MockSubscriptionProduct( + id: "com.test.regular", + displayName: "Regular Plan", + displayPrice: "$9.99", + isMonthly: true, + hasFreeTrialOffer: false + ) + + mockProductFetcher.mockProducts = [monthlyTrialProduct, yearlyTrialProduct, regularProduct] + await sut.updateAvailableProducts() + + // When + let subscriptionOptions = await sut.freeTrialSubscriptionOptions() + + // Then + XCTAssertNotNil(subscriptionOptions) + XCTAssertEqual(subscriptionOptions?.options.count, 2) + + let productIds = subscriptionOptions?.options.map { $0.id } ?? [] + XCTAssertTrue(productIds.contains("com.test.monthly.trial")) + XCTAssertTrue(productIds.contains("com.test.yearly.trial")) + XCTAssertFalse(productIds.contains("com.test.regular")) + } + + func testSubscriptionOptionsReturnsNilWhenNoValidProductPairExists() async { + // Given + let monthlyProduct = MockSubscriptionProduct( + id: "com.test.monthly", + displayName: "Monthly Plan", + displayPrice: "$9.99", + isMonthly: true, + hasFreeTrialOffer: false + ) + + mockProductFetcher.mockProducts = [monthlyProduct] + await sut.updateAvailableProducts() + + // When + let subscriptionOptions = await sut.subscriptionOptions() + + // Then + XCTAssertNil(subscriptionOptions) + } + + func testFreeTrialSubscriptionOptionsReturnsNilWhenNoValidProductPairExists() async { + // Given + let monthlyTrialProduct = MockSubscriptionProduct( + id: "com.test.monthly.trial", + displayName: "Monthly Plan with Trial", + displayPrice: "$9.99", + isMonthly: true, + hasFreeTrialOffer: true, + introOffer: MockIntroductoryOffer( + id: "trial1", + displayPrice: "Free", + periodInDays: 7, + isFreeTrial: true + ) + ) + + mockProductFetcher.mockProducts = [monthlyTrialProduct] + await sut.updateAvailableProducts() + + // When + let subscriptionOptions = await sut.freeTrialSubscriptionOptions() + + // Then + XCTAssertNil(subscriptionOptions) + } + + func testSubscriptionOptionsIncludesCorrectDetails() async { + // Given + let monthlyProduct = MockSubscriptionProduct( + id: "com.test.monthly", + displayName: "Monthly Plan", + displayPrice: "$9.99", + isMonthly: true, + hasFreeTrialOffer: false + ) + + let yearlyProduct = MockSubscriptionProduct( + id: "com.test.yearly", + displayName: "Yearly Plan", + displayPrice: "$99.99", + isYearly: true, + hasFreeTrialOffer: false + ) + + mockProductFetcher.mockProducts = [monthlyProduct, yearlyProduct] + await sut.updateAvailableProducts() + + // When + let subscriptionOptions = await sut.subscriptionOptions() + + // Then + XCTAssertNotNil(subscriptionOptions) + XCTAssertEqual(subscriptionOptions?.options.count, 2) + + let monthlyOption = subscriptionOptions?.options.first { $0.id == "com.test.monthly" } + XCTAssertNotNil(monthlyOption) + XCTAssertEqual(monthlyOption?.cost.displayPrice, "$9.99") + XCTAssertEqual(monthlyOption?.cost.recurrence, "monthly") + XCTAssertNil(monthlyOption?.offer) + + let yearlyOption = subscriptionOptions?.options.first { $0.id == "com.test.yearly" } + XCTAssertNotNil(yearlyOption) + XCTAssertEqual(yearlyOption?.cost.displayPrice, "$99.99") + XCTAssertEqual(yearlyOption?.cost.recurrence, "yearly") + XCTAssertNil(yearlyOption?.offer) + } + + func testFreeTrialSubscriptionOptionsIncludesCorrectTrialDetails() async { + // Given + let monthlyTrialProduct = MockSubscriptionProduct( + id: "com.test.monthly.trial", + displayName: "Monthly Plan with Trial", + displayPrice: "$9.99", + isMonthly: true, + hasFreeTrialOffer: true, + introOffer: MockIntroductoryOffer( + id: "trial1", + displayPrice: "$0.00", + periodInDays: 7, + isFreeTrial: true + ), + isEligibleForIntroOffer: true + ) + + let yearlyTrialProduct = MockSubscriptionProduct( + id: "com.test.yearly.trial", + displayName: "Yearly Plan with Trial", + displayPrice: "$99.99", + isYearly: true, + hasFreeTrialOffer: true, + introOffer: MockIntroductoryOffer( + id: "trial2", + displayPrice: "$0.00", + periodInDays: 14, + isFreeTrial: true + ), + isEligibleForIntroOffer: true + ) + + mockProductFetcher.mockProducts = [monthlyTrialProduct, yearlyTrialProduct] + await sut.updateAvailableProducts() + + // When + let subscriptionOptions = await sut.freeTrialSubscriptionOptions() + + // Then + XCTAssertNotNil(subscriptionOptions) + + let monthlyOption = subscriptionOptions?.options.first { $0.id == "com.test.monthly.trial" } + XCTAssertNotNil(monthlyOption) + XCTAssertNotNil(monthlyOption?.offer) + XCTAssertEqual(monthlyOption?.offer?.type, .freeTrial) + XCTAssertEqual(monthlyOption?.offer?.displayPrice, "$0.00") + XCTAssertEqual(monthlyOption?.offer?.durationInDays, 7) + XCTAssertTrue(monthlyOption?.offer?.isUserEligible ?? false) + + let yearlyOption = subscriptionOptions?.options.first { $0.id == "com.test.yearly.trial" } + XCTAssertNotNil(yearlyOption) + XCTAssertNotNil(yearlyOption?.offer) + XCTAssertEqual(yearlyOption?.offer?.type, .freeTrial) + XCTAssertEqual(yearlyOption?.offer?.displayPrice, "$0.00") + XCTAssertEqual(yearlyOption?.offer?.durationInDays, 14) + XCTAssertTrue(yearlyOption?.offer?.isUserEligible ?? false) + } + + func testUpdateAvailableProductsSuccessfully() async { + // Given + let monthlyProduct = createMonthlyProduct() + let yearlyProduct = createYearlyProduct() + mockProductFetcher.mockProducts = [monthlyProduct, yearlyProduct] + + // When + await sut.updateAvailableProducts() + + // Then + let products = (sut as? DefaultStorePurchaseManager)?.availableProducts ?? [] + XCTAssertEqual(products.count, 2) + XCTAssertTrue(products.contains(where: { $0.id == monthlyProduct.id })) + XCTAssertTrue(products.contains(where: { $0.id == yearlyProduct.id })) + } + + func testUpdateAvailableProductsWithError() async { + // Given + mockProductFetcher.fetchError = MockProductError.fetchFailed + + // When + await sut.updateAvailableProducts() + + // Then + let products = (sut as? DefaultStorePurchaseManager)?.availableProducts ?? [] + XCTAssertTrue(products.isEmpty) + } + + func testUpdateAvailableProductsWithDifferentRegions() async { + // Given + let usaMonthlyProduct = MockSubscriptionProduct( + id: "com.test.usa.monthly", + displayName: "USA Monthly Plan", + displayPrice: "$9.99", + isMonthly: true + ) + let usaYearlyProduct = MockSubscriptionProduct( + id: "com.test.usa.yearly", + displayName: "USA Yearly Plan", + displayPrice: "$99.99", + isYearly: true + ) + + let rowMonthlyProduct = MockSubscriptionProduct( + id: "com.test.row.monthly", + displayName: "ROW Monthly Plan", + displayPrice: "€8.99", + isMonthly: true + ) + let rowYearlyProduct = MockSubscriptionProduct( + id: "com.test.row.yearly", + displayName: "ROW Yearly Plan", + displayPrice: "€89.99", + isYearly: true + ) + + // Set USA products initially + mockProductFetcher.mockProducts = [usaMonthlyProduct, usaYearlyProduct] + mockFeatureFlagger.enabledFeatures = [] // No ROW features enabled - defaults to USA + + // When - Update for USA region + await sut.updateAvailableProducts() + + // Then - Verify USA products + let usaProducts = (sut as? DefaultStorePurchaseManager)?.availableProducts ?? [] + XCTAssertEqual(usaProducts.count, 2) + XCTAssertEqual((sut as? DefaultStorePurchaseManager)?.currentStorefrontRegion, .usa) + XCTAssertTrue(usaProducts.contains(where: { $0.id == "com.test.usa.monthly" })) + XCTAssertTrue(usaProducts.contains(where: { $0.id == "com.test.usa.yearly" })) + + // When - Switch to ROW region + mockProductFetcher.mockProducts = [rowMonthlyProduct, rowYearlyProduct] + mockFeatureFlagger.enabledFeatures = [.isLaunchedROW, .usePrivacyProROWRegionOverride] + await sut.updateAvailableProducts() + + // Then - Verify ROW products + let rowProducts = (sut as? DefaultStorePurchaseManager)?.availableProducts ?? [] + XCTAssertEqual(rowProducts.count, 2) + XCTAssertEqual((sut as? DefaultStorePurchaseManager)?.currentStorefrontRegion, .restOfWorld) + XCTAssertTrue(rowProducts.contains(where: { $0.id == "com.test.row.monthly" })) + XCTAssertTrue(rowProducts.contains(where: { $0.id == "com.test.row.yearly" })) + + // Verify pricing differences + let usaMonthlyPrice = usaProducts.first(where: { $0.isMonthly })?.displayPrice + let rowMonthlyPrice = rowProducts.first(where: { $0.isMonthly })?.displayPrice + XCTAssertEqual(usaMonthlyPrice, "$9.99") + XCTAssertEqual(rowMonthlyPrice, "€8.99") + } + + func testUpdateAvailableProductsUpdatesFeatureMapping() async { + // Given + let monthlyProduct = createMonthlyProduct() + let yearlyProduct = createYearlyProduct() + mockProductFetcher.mockProducts = [monthlyProduct, yearlyProduct] + + // When + await sut.updateAvailableProducts() + + // Then + XCTAssertTrue(mockCache.didCallSubscriptionFeatures) + XCTAssertEqual(mockCache.lastCalledSubscriptionId, yearlyProduct.id) + } +} + +private final class MockProductFetcher: ProductFetching { + var mockProducts: [any SubscriptionProduct] = [] + var fetchError: Error? + var fetchCount: Int = 0 + + public func products(for identifiers: [String]) async throws -> [any SubscriptionProduct] { + fetchCount += 1 + if let error = fetchError { + throw error + } + return mockProducts + } +} + +private enum MockProductError: Error { + case fetchFailed +} + +private extension StorePurchaseManagerTests { + func createMonthlyProduct(withTrial: Bool = false) -> MockSubscriptionProduct { + MockSubscriptionProduct( + id: "com.test.monthly\(withTrial ? ".trial" : "")", + displayName: "Monthly Plan\(withTrial ? " with Trial" : "")", + displayPrice: "$9.99", + isMonthly: true, + hasFreeTrialOffer: withTrial, + introOffer: withTrial ? MockIntroductoryOffer( + id: "trial1", + displayPrice: "Free", + periodInDays: 7, + isFreeTrial: true + ) : nil, + isEligibleForIntroOffer: withTrial + ) + } + + func createYearlyProduct(withTrial: Bool = false) -> MockSubscriptionProduct { + MockSubscriptionProduct( + id: "com.test.yearly\(withTrial ? ".trial" : "")", + displayName: "Yearly Plan\(withTrial ? " with Trial" : "")", + displayPrice: "$99.99", + isYearly: true, + hasFreeTrialOffer: withTrial, + introOffer: withTrial ? MockIntroductoryOffer( + id: "trial2", + displayPrice: "Free", + periodInDays: 14, + isFreeTrial: true + ) : nil, + isEligibleForIntroOffer: withTrial + ) + } +} + +private class MockSubscriptionProduct: SubscriptionProduct { + let id: String + let displayName: String + let displayPrice: String + let description: String + let isMonthly: Bool + let isYearly: Bool + let hasFreeTrialOffer: Bool + private let mockIntroOffer: MockIntroductoryOffer? + private let mockIsEligibleForIntroOffer: Bool + + init(id: String, + displayName: String = "Mock Product", + displayPrice: String = "$4.99", + description: String = "Mock Description", + isMonthly: Bool = false, + isYearly: Bool = false, + hasFreeTrialOffer: Bool = false, + introOffer: MockIntroductoryOffer? = nil, + isEligibleForIntroOffer: Bool = false) { + self.id = id + self.displayName = displayName + self.displayPrice = displayPrice + self.description = description + self.isMonthly = isMonthly + self.isYearly = isYearly + self.hasFreeTrialOffer = hasFreeTrialOffer + self.mockIntroOffer = introOffer + self.mockIsEligibleForIntroOffer = isEligibleForIntroOffer + } + + var introductoryOffer: SubscriptionProductIntroductoryOffer? { + return mockIntroOffer + } + + var isEligibleForIntroOffer: Bool { + get async { + return mockIsEligibleForIntroOffer + } + } + + func purchase(options: Set) async throws -> Product.PurchaseResult { + fatalError("Not implemented for tests") + } + + static func == (lhs: MockSubscriptionProduct, rhs: MockSubscriptionProduct) -> Bool { + return lhs.id == rhs.id + } +} + +private struct MockIntroductoryOffer: SubscriptionProductIntroductoryOffer { + var id: String? + var displayPrice: String + var periodInDays: Int + var isFreeTrial: Bool +} + +private class MockFeatureFlagger: FeatureFlaggerMapping { + var enabledFeatures: Set = [] + + init(enabledFeatures: Set = []) { + self.enabledFeatures = enabledFeatures + super.init(mapping: {_ in true}) + } + + override func isFeatureOn(_ feature: SubscriptionFeatureFlags) -> Bool { + return enabledFeatures.contains(feature) + } +} + +private class MockStoreSubscriptionConfiguration: StoreSubscriptionConfiguration { + let usaIdentifiers = ["com.test.usa.monthly", "com.test.usa.yearly"] + let rowIdentifiers = ["com.test.row.monthly", "com.test.row.yearly"] + + var allSubscriptionIdentifiers: [String] { + usaIdentifiers + rowIdentifiers + } + + func subscriptionIdentifiers(for region: SubscriptionRegion) -> [String] { + switch region { + case .usa: + return usaIdentifiers + case .restOfWorld: + return rowIdentifiers + } + } + + func subscriptionIdentifiers(for country: String) -> [String] { + switch country.uppercased() { + case "USA": + return usaIdentifiers + default: + return rowIdentifiers + } + } +}