From 918c7ef70f70ca2c62ef781dd6aae8d53cf15db4 Mon Sep 17 00:00:00 2001 From: Anh Do <18567+quanganhdo@users.noreply.github.com> Date: Mon, 25 Mar 2024 00:39:47 -0400 Subject: [PATCH 1/4] Collect extra metadata (#737) * Use anonymousDescription for lastPathChange --- ...etworkProtectionTunnelFailureMonitor.swift | 25 ++++++++++++++++++- 1 file changed, 24 insertions(+), 1 deletion(-) diff --git a/Sources/NetworkProtection/Diagnostics/NetworkProtectionTunnelFailureMonitor.swift b/Sources/NetworkProtection/Diagnostics/NetworkProtectionTunnelFailureMonitor.swift index 654269fbd..12d92a7ff 100644 --- a/Sources/NetworkProtection/Diagnostics/NetworkProtectionTunnelFailureMonitor.swift +++ b/Sources/NetworkProtection/Diagnostics/NetworkProtectionTunnelFailureMonitor.swift @@ -84,7 +84,7 @@ public actor NetworkProtectionTunnelFailureMonitor { firstCheckSkipped = false networkMonitor.pathUpdateHandler = { path in - callback(.networkPathChanged(path.debugDescription)) + callback(.networkPathChanged(path.anonymousDescription)) } task = Task.periodic(interval: Self.monitoringInterval) { [weak self] in @@ -142,3 +142,26 @@ public actor NetworkProtectionTunnelFailureMonitor { return [.wifi, .eth, .cellular].contains(connectionType) && path.status == .satisfied } } + +extension Network.NWPath { + /// A description that's safe from a privacy standpoint. + /// + /// Ref: https://app.asana.com/0/0/1206712493935053/1206712516729780/f + /// + public var anonymousDescription: String { + var description = "NWPath(" + + description += "status: \(status), " + + if #available(iOS 14.2, *), case .unsatisfied = status { + description += "unsatisfiedReason: \(unsatisfiedReason), " + } + + description += "availableInterfaces: \(availableInterfaces), " + description += "isConstrained: \(isConstrained ? "true" : "false"), " + description += "isExpensive: \(isExpensive ? "true" : "false")" + description += ")" + + return description + } +} From 5fb0b91e53ca33d20cfc8d8f9b4bf71b74eff30a Mon Sep 17 00:00:00 2001 From: Michal Smaga Date: Mon, 25 Mar 2024 12:49:41 +0100 Subject: [PATCH 2/4] Send correct platform value for App Store purchase options (#747) * For App Store purchase flow use proper platform when sendind purchase options * Also send correct plaform value for empty options object --- .../Flows/AppStore/AppStorePurchaseFlow.swift | 10 +++++++++- Sources/Subscription/Flows/PurchaseFlow.swift | 9 ++++++++- 2 files changed, 17 insertions(+), 2 deletions(-) diff --git a/Sources/Subscription/Flows/AppStore/AppStorePurchaseFlow.swift b/Sources/Subscription/Flows/AppStore/AppStorePurchaseFlow.swift index 95a202194..16df59be3 100644 --- a/Sources/Subscription/Flows/AppStore/AppStorePurchaseFlow.swift +++ b/Sources/Subscription/Flows/AppStore/AppStorePurchaseFlow.swift @@ -51,7 +51,15 @@ public final class AppStorePurchaseFlow { let features = SubscriptionFeatureName.allCases.map { SubscriptionFeature(name: $0.rawValue) } - return .success(SubscriptionOptions(platform: SubscriptionPlatformName.macos.rawValue, + let platform: SubscriptionPlatformName + +#if os(iOS) + platform = .ios +#else + platform = .macos +#endif + + return .success(SubscriptionOptions(platform: platform.rawValue, options: options, features: features)) } diff --git a/Sources/Subscription/Flows/PurchaseFlow.swift b/Sources/Subscription/Flows/PurchaseFlow.swift index 58c244b69..e9ad1bffa 100644 --- a/Sources/Subscription/Flows/PurchaseFlow.swift +++ b/Sources/Subscription/Flows/PurchaseFlow.swift @@ -24,7 +24,13 @@ public struct SubscriptionOptions: Encodable { let features: [SubscriptionFeature] public static var empty: SubscriptionOptions { let features = SubscriptionFeatureName.allCases.map { SubscriptionFeature(name: $0.rawValue) } - return SubscriptionOptions(platform: "macos", options: [], features: features) + let platform: SubscriptionPlatformName +#if os(iOS) + platform = .ios +#else + platform = .macos +#endif + return SubscriptionOptions(platform: platform.rawValue, options: [], features: features) } } @@ -55,6 +61,7 @@ public enum SubscriptionFeatureName: String, CaseIterable { } public enum SubscriptionPlatformName: String { + case ios case macos case stripe } From c34a4d0298a33c4c7f3b8c0ac4ca249b51e350db Mon Sep 17 00:00:00 2001 From: Michal Smaga Date: Mon, 25 Mar 2024 12:49:56 +0100 Subject: [PATCH 3/4] Update subscription and accounts cache after the confirmation of purchase completes (#746) * Update subscription and accounts cache after the confirmation of purchase completes * Reuse same logic for updating caches * Add Subscription typealias to avoid clash with Subscrpiton from Combine * Post subscrption did change when the cache is being updated --- Sources/Subscription/AccountManager.swift | 26 +++++++++++-------- .../Flows/AppStore/AppStorePurchaseFlow.swift | 7 +++-- .../Services/Model/Subscription.swift | 4 ++- .../Services/SubscriptionService.swift | 16 +++++++++--- 4 files changed, 36 insertions(+), 17 deletions(-) diff --git a/Sources/Subscription/AccountManager.swift b/Sources/Subscription/AccountManager.swift index 9e2500d25..a5e04c6fe 100644 --- a/Sources/Subscription/AccountManager.swift +++ b/Sources/Subscription/AccountManager.swift @@ -23,6 +23,7 @@ public extension Notification.Name { static let accountDidSignIn = Notification.Name("com.duckduckgo.subscription.AccountDidSignIn") static let accountDidSignOut = Notification.Name("com.duckduckgo.subscription.AccountDidSignOut") static let entitlementsDidChange = Notification.Name("com.duckduckgo.subscription.EntitlementsDidChange") + static let subscriptionDidChange = Notification.Name("com.duckduckgo.subscription.SubscriptionDidChange") } public protocol AccountManagerKeychainAccessDelegate: AnyObject { @@ -245,20 +246,10 @@ public class AccountManager: AccountManaging { return .failure(EntitlementsError.noAccessToken) } - let cachedEntitlements: [Entitlement] = entitlementsCache.get() ?? [] - switch await AuthService.validateToken(accessToken: accessToken) { case .success(let response): let entitlements = response.account.entitlements - - if entitlements != cachedEntitlements { - if entitlements.isEmpty { - entitlementsCache.reset() - } else { - entitlementsCache.set(entitlements) - } - NotificationCenter.default.post(name: .entitlementsDidChange, object: self, userInfo: [UserDefaultsCacheKey.subscriptionEntitlements: entitlements]) - } + updateCache(with: entitlements) return .success(entitlements) case .failure(let error): @@ -267,6 +258,19 @@ public class AccountManager: AccountManaging { } } + public func updateCache(with entitlements: [Entitlement]) { + let cachedEntitlements: [Entitlement] = entitlementsCache.get() ?? [] + + if entitlements != cachedEntitlements { + if entitlements.isEmpty { + entitlementsCache.reset() + } else { + entitlementsCache.set(entitlements) + } + NotificationCenter.default.post(name: .entitlementsDidChange, object: self, userInfo: [UserDefaultsCacheKey.subscriptionEntitlements: entitlements]) + } + } + public func fetchEntitlements(cachePolicy: CachePolicy = .returnCacheDataElseLoad) async -> Result<[Entitlement], Error> { switch cachePolicy { diff --git a/Sources/Subscription/Flows/AppStore/AppStorePurchaseFlow.swift b/Sources/Subscription/Flows/AppStore/AppStorePurchaseFlow.swift index 16df59be3..b0bb18ed3 100644 --- a/Sources/Subscription/Flows/AppStore/AppStorePurchaseFlow.swift +++ b/Sources/Subscription/Flows/AppStore/AppStorePurchaseFlow.swift @@ -130,12 +130,15 @@ public final class AppStorePurchaseFlow { SubscriptionService.signOut() os_log(.info, log: .subscription, "[AppStorePurchaseFlow] completeSubscriptionPurchase") + let accountManager = AccountManager(subscriptionAppGroup: subscriptionAppGroup) - guard let accessToken = AccountManager(subscriptionAppGroup: subscriptionAppGroup).accessToken else { return .failure(.missingEntitlements) } + guard let accessToken = accountManager.accessToken else { return .failure(.missingEntitlements) } let result = await callWithRetries(retry: 5, wait: 2.0) { switch await SubscriptionService.confirmPurchase(accessToken: accessToken, signature: transactionJWS) { - case .success: + case .success(let confirmation): + SubscriptionService.updateCache(with: confirmation.subscription) + accountManager.updateCache(with: confirmation.entitlements) return true case .failure: return false diff --git a/Sources/Subscription/Services/Model/Subscription.swift b/Sources/Subscription/Services/Model/Subscription.swift index 391adaca5..703b1617a 100644 --- a/Sources/Subscription/Services/Model/Subscription.swift +++ b/Sources/Subscription/Services/Model/Subscription.swift @@ -18,7 +18,9 @@ import Foundation -public struct Subscription: Codable { +public typealias DDGSubscription = Subscription // to avoid conflicts when Combine is imported + +public struct Subscription: Codable, Equatable { public let productId: String public let name: String public let billingPeriod: BillingPeriod diff --git a/Sources/Subscription/Services/SubscriptionService.swift b/Sources/Subscription/Services/SubscriptionService.swift index e29e2cb86..6c7c9be17 100644 --- a/Sources/Subscription/Services/SubscriptionService.swift +++ b/Sources/Subscription/Services/SubscriptionService.swift @@ -56,15 +56,25 @@ public final class SubscriptionService: APIService { switch result { case .success(let subscriptionResponse): - let defaultExpiryDate = Date().addingTimeInterval(subscriptionCache.settings.defaultExpirationInterval) - let expiryDate = min(defaultExpiryDate, subscriptionResponse.expiresOrRenewsAt) - subscriptionCache.set(subscriptionResponse, expires: expiryDate) + updateCache(with: subscriptionResponse) return .success(subscriptionResponse) case .failure(let error): return .failure(.apiError(error)) } } + public static func updateCache(with subscription: Subscription) { + let cachedSubscription: Subscription? = subscriptionCache.get() + + if subscription != cachedSubscription { + let defaultExpiryDate = Date().addingTimeInterval(subscriptionCache.settings.defaultExpirationInterval) + let expiryDate = min(defaultExpiryDate, subscription.expiresOrRenewsAt) + + subscriptionCache.set(subscription, expires: expiryDate) + NotificationCenter.default.post(name: .subscriptionDidChange, object: self, userInfo: [UserDefaultsCacheKey.subscription: subscription]) + } + } + public static func getSubscription(accessToken: String, cachePolicy: CachePolicy = .returnCacheDataElseLoad) async -> Result { switch cachePolicy { From 0509e0cf15f1ff8b0f32ac39c262da131fba7f3e Mon Sep 17 00:00:00 2001 From: amddg44 Date: Mon, 25 Mar 2024 14:18:18 +0100 Subject: [PATCH 4/4] Autofill Script performance improvements (#740) Task/Issue URL: https://app.asana.com/0/414709148257752/1206900948756760/f iOS PR: duckduckgo/iOS#2625 macOS PR: duckduckgo/macos-browser#2481 What kind of version bump will this require?: Patch Description: Greatly reduces the amount of data passed into the autofill script to improve performance and resolve occasional out of memory crashes --- .../AutofillUserScript+SourceProvider.swift | 49 +++++++++- ...utofillUserScriptSourceProviderTests.swift | 93 ++++++++++++++++++- 2 files changed, 137 insertions(+), 5 deletions(-) diff --git a/Sources/BrowserServicesKit/Autofill/AutofillUserScript+SourceProvider.swift b/Sources/BrowserServicesKit/Autofill/AutofillUserScript+SourceProvider.swift index 1c91dd82f..0f9a267ef 100644 --- a/Sources/BrowserServicesKit/Autofill/AutofillUserScript+SourceProvider.swift +++ b/Sources/BrowserServicesKit/Autofill/AutofillUserScript+SourceProvider.swift @@ -18,6 +18,7 @@ import Foundation import Autofill +import Common public protocol AutofillUserScriptSourceProvider { var source: String { get } @@ -101,15 +102,59 @@ public class DefaultAutofillSourceProvider: AutofillUserScriptSourceProvider { } private func buildReplacementsData() -> ProviderData? { - guard let userUnprotectedDomains = try? JSONEncoder().encode(privacyConfigurationManager.privacyConfig.userUnprotectedDomains), + guard let filteredPrivacyConfigData = filteredDataFrom(configData: privacyConfigurationManager.currentConfig, + keepingTopLevelKeys: ["features", "unprotectedTemporary"], + andSubKey: "autofill", + inTopLevelKey: "features"), + let userUnprotectedDomains = try? JSONEncoder().encode(privacyConfigurationManager.privacyConfig.userUnprotectedDomains), let jsonProperties = try? JSONEncoder().encode(properties) else { return nil } - return ProviderData(privacyConfig: privacyConfigurationManager.currentConfig, + + return ProviderData(privacyConfig: filteredPrivacyConfigData, userUnprotectedDomains: userUnprotectedDomains, userPreferences: jsonProperties) } + /// `contentScope` only needs these properties from the privacy config, so creating a filtered version to improve performance + /// { + /// features: { + /// autofill: { + /// state: 'enabled', + /// exceptions: [] + /// } + /// }, + /// unprotectedTemporary: [] + /// } + private func filteredDataFrom(configData: Data, keepingTopLevelKeys topLevelKeys: [String], andSubKey subKey: String, inTopLevelKey topLevelKey: String) -> Data? { + do { + if let jsonDict = try JSONSerialization.jsonObject(with: configData, options: []) as? [String: Any] { + var filteredDict = [String: Any]() + + // Keep the specified top-level keys + for key in topLevelKeys { + if let value = jsonDict[key] { + filteredDict[key] = value + } + } + + // Handle the nested dictionary for a specific top-level key to keep only the sub-key + if let nestedDict = jsonDict[topLevelKey] as? [String: Any], + let valueToKeep = nestedDict[subKey] { + filteredDict[topLevelKey] = [subKey: valueToKeep] + } + + // Convert filtered dictionary back to Data + let filteredData = try JSONSerialization.data(withJSONObject: filteredDict, options: []) + return filteredData + } + } catch { + os_log(.debug, "Error during JSON serialization of privacy config: \(error.localizedDescription)") + } + + return nil + } + public class Builder { private var privacyConfigurationManager: PrivacyConfigurationManaging private var properties: ContentScopeProperties diff --git a/Tests/BrowserServicesKitTests/Autofill/AutofillUserScriptSourceProviderTests.swift b/Tests/BrowserServicesKitTests/Autofill/AutofillUserScriptSourceProviderTests.swift index 0668c9442..64b2c3504 100644 --- a/Tests/BrowserServicesKitTests/Autofill/AutofillUserScriptSourceProviderTests.swift +++ b/Tests/BrowserServicesKitTests/Autofill/AutofillUserScriptSourceProviderTests.swift @@ -26,11 +26,65 @@ final class AutofillUserScriptSourceProviderTests: XCTestCase { { "features": { "autofill": { - "status": "enabled", - "exceptions": [] + "state": "enabled", + "features": { + "credentialsSaving": { + "state": "enabled", + "minSupportedVersion": "7.74.0" + }, + "credentialsAutofill": { + "state": "enabled", + "minSupportedVersion": "7.74.0" + }, + "inlineIconCredentials": { + "state": "enabled", + "minSupportedVersion": "7.74.0" + }, + "accessCredentialManagement": { + "state": "enabled", + "minSupportedVersion": "7.74.0" + }, + "autofillPasswordGeneration": { + "state": "enabled", + "minSupportedVersion": "7.75.0" + }, + "onByDefault": { + "state": "enabled", + "minSupportedVersion": "7.93.0", + "rollout": { + "steps": [ + { + "percent": 1 + }, + { + "percent": 10 + }, + { + "percent": 100 + } + ] + } + } + }, + "hash": "ffaa2e81fb2bf264cb5ce2dadac549e1" + }, + "contentBlocking": { + "state": "enabled", + "exceptions": [ + { + "domain": "test-domain.com" + } + ], + "hash": "910e25ffe4d683b3c708a1578d097a16" + }, + "voiceSearch": { + "exceptions": [], + "state": "disabled", + "hash": "728493ef7a1488e4781656d3f9db84aa" } }, - "unprotectedTemporary": [] + "unprotectedTemporary": [], + "unprotectedOtherKey": [] } """.data(using: .utf8)! lazy var privacyConfig = AutofillTestHelper.preparePrivacyConfig(embeddedConfig: embeddedConfig) @@ -53,4 +107,37 @@ final class AutofillUserScriptSourceProviderTests: XCTestCase { XCTAssertNotNil(runtimeConfiguration) XCTAssertFalse(runtimeConfiguration!.isEmpty) } + + func testWhenBuildRuntimeConfigurationThenContentScopeContainsRequiredAutofillKeys() throws { + let runtimeConfiguration = DefaultAutofillSourceProvider.Builder(privacyConfigurationManager: privacyConfig, + properties: properties) + .build() + .buildRuntimeConfigResponse() + + let jsonData = runtimeConfiguration!.data(using: .utf8)! + let json = try JSONSerialization.jsonObject(with: jsonData, options: []) as? [String: Any] + let success = json?["success"] as? [String: Any] + let contentScope = success?["contentScope"] as? [String: Any] + let features = contentScope?["features"] as? [String: Any] + XCTAssertNotNil(features?["autofill"] as? [String: Any]) + XCTAssertNotNil(contentScope?["unprotectedTemporary"] as? [Any]) + XCTAssertNil(features?["contentBlocking"]) + } + + func testWhenBuildRuntimeConfigurationThenContentScopeDoesNotContainUnnecessaryKeys() throws { + let runtimeConfiguration = DefaultAutofillSourceProvider.Builder(privacyConfigurationManager: privacyConfig, + properties: properties) + .build() + .buildRuntimeConfigResponse() + + let jsonData = runtimeConfiguration!.data(using: .utf8)! + let json = try JSONSerialization.jsonObject(with: jsonData, options: []) as? [String: Any] + let success = json?["success"] as? [String: Any] + let contentScope = success?["contentScope"] as? [String: Any] + XCTAssertNil(contentScope?["unprotectedOtherKey"]) + + let features = contentScope?["features"] as? [String: Any] + XCTAssertNil(features?["contentBlocking"]) + XCTAssertNil(features?["voiceSearch"]) + } }