From a33ad825c59fdb35ba183905f44811191093bac9 Mon Sep 17 00:00:00 2001 From: Fernando Bunn Date: Tue, 21 May 2024 14:58:04 +0100 Subject: [PATCH 01/16] WIP: checking for entitlement https://app.asana.com/0/1201011656765697/1206366654222841/f --- ...erProtectionSubscriptionEventHandler.swift | 5 ++ ...rokerProtectionEntitlementMonitoring.swift | 71 +++++++++++++++++ .../DataBrokerProtectionAgentManager.swift | 38 ++++++---- .../DataBrokerProtectionAgentKiller.swift | 76 +++++++++++++++++++ .../DataBrokerProtection/Utils/Logging.swift | 8 ++ 5 files changed, 183 insertions(+), 15 deletions(-) create mode 100644 LocalPackages/DataBrokerProtection/Sources/DataBrokerProtection/Authentication/DataBrokerProtectionEntitlementMonitoring.swift create mode 100644 LocalPackages/DataBrokerProtection/Sources/DataBrokerProtection/Utils/DataBrokerProtectionAgentKiller.swift diff --git a/DuckDuckGo/DBP/DataBrokerProtectionSubscriptionEventHandler.swift b/DuckDuckGo/DBP/DataBrokerProtectionSubscriptionEventHandler.swift index 50f8a36d4b..52686cc0ad 100644 --- a/DuckDuckGo/DBP/DataBrokerProtectionSubscriptionEventHandler.swift +++ b/DuckDuckGo/DBP/DataBrokerProtectionSubscriptionEventHandler.swift @@ -31,11 +31,16 @@ final class DataBrokerProtectionSubscriptionEventHandler { func registerForSubscriptionAccountManagerEvents() { NotificationCenter.default.addObserver(self, selector: #selector(handleAccountDidSignOut), name: .accountDidSignOut, object: nil) + NotificationCenter.default.addObserver(self, selector: #selector(handleAccountDidSignOut), name: .entitlementsDidChange, object: nil) } @objc private func handleAccountDidSignOut() { featureDisabler.disableAndDelete() } + + @objc private func entitlementsDidChange() { + #warning("Validate if valid and delete if necessary after sending pixels") + } } #endif diff --git a/LocalPackages/DataBrokerProtection/Sources/DataBrokerProtection/Authentication/DataBrokerProtectionEntitlementMonitoring.swift b/LocalPackages/DataBrokerProtection/Sources/DataBrokerProtection/Authentication/DataBrokerProtectionEntitlementMonitoring.swift new file mode 100644 index 0000000000..6c4a5a7a0c --- /dev/null +++ b/LocalPackages/DataBrokerProtection/Sources/DataBrokerProtection/Authentication/DataBrokerProtectionEntitlementMonitoring.swift @@ -0,0 +1,71 @@ +// +// DataBrokerProtectionEntitlementMonitoring.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 + +protocol DataBrokerProtectionEntitlementMonitoring { + func start(checkEntitlementFunction: @escaping () async throws -> Bool, intervalInMinutes: Int, callback: @escaping (DataBrokerProtectionEntitlementMonitorResult) -> Void) + func stop() +} + +extension DataBrokerProtectionEntitlementMonitoring { + func start(checkEntitlementFunction: @escaping () async throws -> Bool, callback: @escaping (DataBrokerProtectionEntitlementMonitorResult) -> Void) { + start(checkEntitlementFunction: checkEntitlementFunction, + intervalInMinutes: DataBrokerProtectionEntitlementMonitor.monitoringInterval, + callback: callback) + } +} + +public enum DataBrokerProtectionEntitlementMonitorResult { + case enabled + case disabled + case error +} + +final class DataBrokerProtectionEntitlementMonitor: DataBrokerProtectionEntitlementMonitoring { + private var timer: Timer? + static let monitoringInterval = 20 + + func start(checkEntitlementFunction: @escaping () async throws -> Bool, intervalInMinutes: Int, callback: @escaping (DataBrokerProtectionEntitlementMonitorResult) -> Void) { + timer = Timer.scheduledTimer(withTimeInterval: TimeInterval(intervalInMinutes * 60), repeats: true) { _ in + Task { + do { + switch try await checkEntitlementFunction() { + case true: + callback(.enabled) + case false: + callback(.disabled) + } + } catch { + callback(.error) + } + } + } + } + + func stop() { + timer?.invalidate() + timer = nil + } +} + +final class DataBrokerProtectionAgentBouncer { + func validatePreRequisitesAndKillIfNecessary() { + + } +} diff --git a/LocalPackages/DataBrokerProtection/Sources/DataBrokerProtection/Scheduler/DataBrokerProtectionAgentManager.swift b/LocalPackages/DataBrokerProtection/Sources/DataBrokerProtection/Scheduler/DataBrokerProtectionAgentManager.swift index fae04e7e17..dd6f620651 100644 --- a/LocalPackages/DataBrokerProtection/Sources/DataBrokerProtection/Scheduler/DataBrokerProtectionAgentManager.swift +++ b/LocalPackages/DataBrokerProtection/Sources/DataBrokerProtection/Scheduler/DataBrokerProtectionAgentManager.swift @@ -71,6 +71,11 @@ public class DataBrokerProtectionAgentManagerProvider { contentScopeProperties: contentScopeProperties, emailService: emailService, captchaService: captchaService) + + let agentKiller = DefaultDataBrokerProtectionAgentKiller(dataManager: dataManager, + entitlementMonitor: DataBrokerProtectionEntitlementMonitor(), + authenticationManager: authenticationManager) + let operationDependencies = DefaultDataBrokerOperationDependencies( database: dataManager.database, config: executionConfig, @@ -86,7 +91,8 @@ public class DataBrokerProtectionAgentManagerProvider { queueManager: queueManager, dataManager: dataManager, operationDependencies: operationDependencies, - pixelHandler: pixelHandler) + pixelHandler: pixelHandler, + agentKiller: agentKiller) } } @@ -99,6 +105,7 @@ public final class DataBrokerProtectionAgentManager { private let dataManager: DataBrokerProtectionDataManaging private let operationDependencies: DataBrokerOperationDependencies private let pixelHandler: EventMapping + private let agentKiller: DataBrokerProtectionAgentKiller // Used for debug functions only, so not injected private lazy var browserWindowManager = BrowserWindowManager() @@ -111,7 +118,9 @@ public final class DataBrokerProtectionAgentManager { queueManager: DataBrokerProtectionQueueManager, dataManager: DataBrokerProtectionDataManaging, operationDependencies: DataBrokerOperationDependencies, - pixelHandler: EventMapping) { + pixelHandler: EventMapping, + agentKiller: DataBrokerProtectionAgentKiller + ) { self.userNotificationService = userNotificationService self.activityScheduler = activityScheduler self.ipcServer = ipcServer @@ -119,6 +128,7 @@ public final class DataBrokerProtectionAgentManager { self.dataManager = dataManager self.operationDependencies = operationDependencies self.pixelHandler = pixelHandler + self.agentKiller = agentKiller self.activityScheduler.delegate = self self.ipcServer.serverDelegate = self @@ -127,20 +137,17 @@ public final class DataBrokerProtectionAgentManager { public func agentFinishedLaunching() { - do { - // If there's no saved profile we don't need to start the scheduler - // Theoretically this should never happen, if there's no data, the agent shouldn't be running - guard (try dataManager.fetchProfile()) != nil else { - return - } - } catch { - os_log("Error during AgentManager.agentFinishedLaunching when trying to fetchProfile, error: %{public}@", log: .dataBrokerProtection, error.localizedDescription) - return - } + Task { @MainActor in + // The browser shouldn't start the agent if these prerequisites aren't met + // But since the agent can auto-start after a reboot without the browser, we need to validate it again + await agentKiller.validatePreRequisitesAndKillAgentIfNecessary() + + activityScheduler.startScheduler() + didStartActivityScheduler = true + queueManager.startScheduledOperationsIfPermitted(showWebView: false, operationDependencies: operationDependencies, completion: nil) - activityScheduler.startScheduler() - didStartActivityScheduler = true - queueManager.startScheduledOperationsIfPermitted(showWebView: false, operationDependencies: operationDependencies, completion: nil) + agentKiller.monitorEntitlementAndKillAgentIfNecessary() + } } } @@ -283,3 +290,4 @@ extension DataBrokerProtectionAgentManager: DataBrokerProtectionAgentDebugComman extension DataBrokerProtectionAgentManager: DataBrokerProtectionAppToAgentInterface { } + diff --git a/LocalPackages/DataBrokerProtection/Sources/DataBrokerProtection/Utils/DataBrokerProtectionAgentKiller.swift b/LocalPackages/DataBrokerProtection/Sources/DataBrokerProtection/Utils/DataBrokerProtectionAgentKiller.swift new file mode 100644 index 0000000000..cb86ea63f8 --- /dev/null +++ b/LocalPackages/DataBrokerProtection/Sources/DataBrokerProtection/Utils/DataBrokerProtectionAgentKiller.swift @@ -0,0 +1,76 @@ +// +// DataBrokerProtectionAgentKiller.swift +// +// Copyright © 2023 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 Common + +protocol DataBrokerProtectionAgentKiller { + func validatePreRequisitesAndKillAgentIfNecessary() async + func monitorEntitlementAndKillAgentIfNecessary() + func killAgent() +} + +struct DefaultDataBrokerProtectionAgentKiller: DataBrokerProtectionAgentKiller { + private let dataManager: DataBrokerProtectionDataManaging + private let entitlementMonitor: DataBrokerProtectionEntitlementMonitoring + private let authenticationManager: DataBrokerProtectionAuthenticationManaging + + init(dataManager: DataBrokerProtectionDataManaging, + entitlementMonitor: DataBrokerProtectionEntitlementMonitoring, + authenticationManager: DataBrokerProtectionAuthenticationManaging) { + self.dataManager = dataManager + self.entitlementMonitor = entitlementMonitor + self.authenticationManager = authenticationManager + } + + public func validatePreRequisitesAndKillAgentIfNecessary() async { + do { + guard try dataManager.fetchProfile() != nil, + authenticationManager.isUserAuthenticated, + try await authenticationManager.hasValidEntitlement() else { + + os_log("Prerequisites are invalid", log: .dataBrokerProtection) + killAgent() + return + } + os_log("Prerequisites are valid", log: .dataBrokerProtection) + } catch { + os_log("Error validating prerequisites, error: %{public}@", log: .dataBrokerProtection, error.localizedDescription) + killAgent() + } + } + + public func monitorEntitlementAndKillAgentIfNecessary() { + entitlementMonitor.start(checkEntitlementFunction: authenticationManager.hasValidEntitlement) { result in + switch result { + case .enabled: +#warning("Send pixel enabled") + killAgent() + case .disabled: +#warning("Send pixel disabled") + case .error: +#warning("Send pixel error") + } + } + } + + func killAgent() { + os_log("Killing DataBrokerProtection Agent", log: .dataBrokerProtection) + exit(EXIT_SUCCESS) + } +} diff --git a/LocalPackages/DataBrokerProtection/Sources/DataBrokerProtection/Utils/Logging.swift b/LocalPackages/DataBrokerProtection/Sources/DataBrokerProtection/Utils/Logging.swift index 2b8016c99a..c24c0023e3 100644 --- a/LocalPackages/DataBrokerProtection/Sources/DataBrokerProtection/Utils/Logging.swift +++ b/LocalPackages/DataBrokerProtection/Sources/DataBrokerProtection/Utils/Logging.swift @@ -43,6 +43,10 @@ struct Logging { fileprivate static let backgroundAgentPixelLoggingEnabled = true fileprivate static let backgroundAgentPixel: OSLog = OSLog(subsystem: subsystem, category: "Data Broker Protection Background Agent Pixel") + + fileprivate static let dataBrokerProtectionMonitoringEnabled = true + fileprivate static let dataBrokerProtectionMonitoring: OSLog = OSLog(subsystem: subsystem, category: "Data Broker Protection Monitoring") + } extension OSLog { @@ -74,4 +78,8 @@ extension OSLog { public static var dbpBackgroundAgentPixel: OSLog { Logging.backgroundAgentPixelLoggingEnabled ? Logging.backgroundAgentPixel : .disabled } + + public static var dataBrokerProtectionMonitoring: OSLog { + Logging.dataBrokerProtectionMonitoringEnabled ? Logging.dataBrokerProtectionMonitoring : .disabled + } } From d0a43b1b15279f12d4ae36082c735cdb9bccd91f Mon Sep 17 00:00:00 2001 From: Fernando Bunn Date: Tue, 21 May 2024 15:12:45 +0100 Subject: [PATCH 02/16] Renames --- ...rokerProtectionEntitlementMonitoring.swift | 6 ----- .../DataBrokerProtectionAgentManager.swift | 22 ++++++++--------- .../DataBrokerProtectionAgentKiller.swift | 24 +++++++++---------- 3 files changed, 23 insertions(+), 29 deletions(-) diff --git a/LocalPackages/DataBrokerProtection/Sources/DataBrokerProtection/Authentication/DataBrokerProtectionEntitlementMonitoring.swift b/LocalPackages/DataBrokerProtection/Sources/DataBrokerProtection/Authentication/DataBrokerProtectionEntitlementMonitoring.swift index 6c4a5a7a0c..f0b0ce2467 100644 --- a/LocalPackages/DataBrokerProtection/Sources/DataBrokerProtection/Authentication/DataBrokerProtectionEntitlementMonitoring.swift +++ b/LocalPackages/DataBrokerProtection/Sources/DataBrokerProtection/Authentication/DataBrokerProtectionEntitlementMonitoring.swift @@ -63,9 +63,3 @@ final class DataBrokerProtectionEntitlementMonitor: DataBrokerProtectionEntitlem timer = nil } } - -final class DataBrokerProtectionAgentBouncer { - func validatePreRequisitesAndKillIfNecessary() { - - } -} diff --git a/LocalPackages/DataBrokerProtection/Sources/DataBrokerProtection/Scheduler/DataBrokerProtectionAgentManager.swift b/LocalPackages/DataBrokerProtection/Sources/DataBrokerProtection/Scheduler/DataBrokerProtectionAgentManager.swift index dd6f620651..b787bcb9b8 100644 --- a/LocalPackages/DataBrokerProtection/Sources/DataBrokerProtection/Scheduler/DataBrokerProtectionAgentManager.swift +++ b/LocalPackages/DataBrokerProtection/Sources/DataBrokerProtection/Scheduler/DataBrokerProtectionAgentManager.swift @@ -71,12 +71,12 @@ public class DataBrokerProtectionAgentManagerProvider { contentScopeProperties: contentScopeProperties, emailService: emailService, captchaService: captchaService) - - let agentKiller = DefaultDataBrokerProtectionAgentKiller(dataManager: dataManager, - entitlementMonitor: DataBrokerProtectionEntitlementMonitor(), - authenticationManager: authenticationManager) - let operationDependencies = DefaultDataBrokerOperationDependencies( + let agentstopper = DefaultDataBrokerProtectionAgentStopper(dataManager: dataManager, + entitlementMonitor: DataBrokerProtectionEntitlementMonitor(), + authenticationManager: authenticationManager) + + let operationDependencies = DefaultDataBrokerOperationDependencies( database: dataManager.database, config: executionConfig, runnerProvider: runnerProvider, @@ -92,7 +92,7 @@ public class DataBrokerProtectionAgentManagerProvider { dataManager: dataManager, operationDependencies: operationDependencies, pixelHandler: pixelHandler, - agentKiller: agentKiller) + agentStopper: agentstopper) } } @@ -105,7 +105,7 @@ public final class DataBrokerProtectionAgentManager { private let dataManager: DataBrokerProtectionDataManaging private let operationDependencies: DataBrokerOperationDependencies private let pixelHandler: EventMapping - private let agentKiller: DataBrokerProtectionAgentKiller + private let agentStopper: DataBrokerProtectionAgentStopper // Used for debug functions only, so not injected private lazy var browserWindowManager = BrowserWindowManager() @@ -119,7 +119,7 @@ public final class DataBrokerProtectionAgentManager { dataManager: DataBrokerProtectionDataManaging, operationDependencies: DataBrokerOperationDependencies, pixelHandler: EventMapping, - agentKiller: DataBrokerProtectionAgentKiller + agentStopper: DataBrokerProtectionAgentStopper ) { self.userNotificationService = userNotificationService self.activityScheduler = activityScheduler @@ -128,7 +128,7 @@ public final class DataBrokerProtectionAgentManager { self.dataManager = dataManager self.operationDependencies = operationDependencies self.pixelHandler = pixelHandler - self.agentKiller = agentKiller + self.agentStopper = agentStopper self.activityScheduler.delegate = self self.ipcServer.serverDelegate = self @@ -140,13 +140,13 @@ public final class DataBrokerProtectionAgentManager { Task { @MainActor in // The browser shouldn't start the agent if these prerequisites aren't met // But since the agent can auto-start after a reboot without the browser, we need to validate it again - await agentKiller.validatePreRequisitesAndKillAgentIfNecessary() + await agentStopper.validateRunPreRequisitesAndStopAgentIfNecessary() activityScheduler.startScheduler() didStartActivityScheduler = true queueManager.startScheduledOperationsIfPermitted(showWebView: false, operationDependencies: operationDependencies, completion: nil) - agentKiller.monitorEntitlementAndKillAgentIfNecessary() + agentStopper.monitorEntitlementAndStopAgentIfNecessary() } } } diff --git a/LocalPackages/DataBrokerProtection/Sources/DataBrokerProtection/Utils/DataBrokerProtectionAgentKiller.swift b/LocalPackages/DataBrokerProtection/Sources/DataBrokerProtection/Utils/DataBrokerProtectionAgentKiller.swift index cb86ea63f8..505cb7c4f4 100644 --- a/LocalPackages/DataBrokerProtection/Sources/DataBrokerProtection/Utils/DataBrokerProtectionAgentKiller.swift +++ b/LocalPackages/DataBrokerProtection/Sources/DataBrokerProtection/Utils/DataBrokerProtectionAgentKiller.swift @@ -19,13 +19,13 @@ import Foundation import Common -protocol DataBrokerProtectionAgentKiller { - func validatePreRequisitesAndKillAgentIfNecessary() async - func monitorEntitlementAndKillAgentIfNecessary() - func killAgent() +protocol DataBrokerProtectionAgentStopper { + func validateRunPreRequisitesAndStopAgentIfNecessary() async + func monitorEntitlementAndStopAgentIfNecessary() + func stopAgent() } -struct DefaultDataBrokerProtectionAgentKiller: DataBrokerProtectionAgentKiller { +struct DefaultDataBrokerProtectionAgentStopper: DataBrokerProtectionAgentStopper { private let dataManager: DataBrokerProtectionDataManaging private let entitlementMonitor: DataBrokerProtectionEntitlementMonitoring private let authenticationManager: DataBrokerProtectionAuthenticationManaging @@ -38,29 +38,29 @@ struct DefaultDataBrokerProtectionAgentKiller: DataBrokerProtectionAgentKiller { self.authenticationManager = authenticationManager } - public func validatePreRequisitesAndKillAgentIfNecessary() async { + public func validateRunPreRequisitesAndStopAgentIfNecessary() async { do { guard try dataManager.fetchProfile() != nil, authenticationManager.isUserAuthenticated, try await authenticationManager.hasValidEntitlement() else { os_log("Prerequisites are invalid", log: .dataBrokerProtection) - killAgent() + stopAgent() return } os_log("Prerequisites are valid", log: .dataBrokerProtection) } catch { os_log("Error validating prerequisites, error: %{public}@", log: .dataBrokerProtection, error.localizedDescription) - killAgent() + stopAgent() } } - public func monitorEntitlementAndKillAgentIfNecessary() { + public func monitorEntitlementAndStopAgentIfNecessary() { entitlementMonitor.start(checkEntitlementFunction: authenticationManager.hasValidEntitlement) { result in switch result { case .enabled: #warning("Send pixel enabled") - killAgent() + stopAgent() case .disabled: #warning("Send pixel disabled") case .error: @@ -69,8 +69,8 @@ struct DefaultDataBrokerProtectionAgentKiller: DataBrokerProtectionAgentKiller { } } - func killAgent() { - os_log("Killing DataBrokerProtection Agent", log: .dataBrokerProtection) + func stopAgent() { + os_log("Stopping DataBrokerProtection Agent", log: .dataBrokerProtection) exit(EXIT_SUCCESS) } } From 815202742d582f4648199fbdc7df869c71278379 Mon Sep 17 00:00:00 2001 From: Fernando Bunn Date: Tue, 21 May 2024 15:14:05 +0100 Subject: [PATCH 03/16] Rename file --- ...AgentKiller.swift => DataBrokerProtectionAgentStopper.swift} | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) rename LocalPackages/DataBrokerProtection/Sources/DataBrokerProtection/Utils/{DataBrokerProtectionAgentKiller.swift => DataBrokerProtectionAgentStopper.swift} (98%) diff --git a/LocalPackages/DataBrokerProtection/Sources/DataBrokerProtection/Utils/DataBrokerProtectionAgentKiller.swift b/LocalPackages/DataBrokerProtection/Sources/DataBrokerProtection/Utils/DataBrokerProtectionAgentStopper.swift similarity index 98% rename from LocalPackages/DataBrokerProtection/Sources/DataBrokerProtection/Utils/DataBrokerProtectionAgentKiller.swift rename to LocalPackages/DataBrokerProtection/Sources/DataBrokerProtection/Utils/DataBrokerProtectionAgentStopper.swift index 505cb7c4f4..388295e58f 100644 --- a/LocalPackages/DataBrokerProtection/Sources/DataBrokerProtection/Utils/DataBrokerProtectionAgentKiller.swift +++ b/LocalPackages/DataBrokerProtection/Sources/DataBrokerProtection/Utils/DataBrokerProtectionAgentStopper.swift @@ -1,5 +1,5 @@ // -// DataBrokerProtectionAgentKiller.swift +// DataBrokerProtectionAgentStopper.swift // // Copyright © 2023 DuckDuckGo. All rights reserved. // From cf789fef3dca249c5e090c607fde61a4c20ea123 Mon Sep 17 00:00:00 2001 From: Fernando Bunn Date: Tue, 21 May 2024 17:43:48 +0100 Subject: [PATCH 04/16] PR feedback --- .../DataBrokerProtectionEntitlementMonitoring.swift | 5 +++-- .../Scheduler/DataBrokerProtectionAgentManager.swift | 5 +++-- 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/LocalPackages/DataBrokerProtection/Sources/DataBrokerProtection/Authentication/DataBrokerProtectionEntitlementMonitoring.swift b/LocalPackages/DataBrokerProtection/Sources/DataBrokerProtection/Authentication/DataBrokerProtectionEntitlementMonitoring.swift index f0b0ce2467..e65fb4872c 100644 --- a/LocalPackages/DataBrokerProtection/Sources/DataBrokerProtection/Authentication/DataBrokerProtectionEntitlementMonitoring.swift +++ b/LocalPackages/DataBrokerProtection/Sources/DataBrokerProtection/Authentication/DataBrokerProtectionEntitlementMonitoring.swift @@ -25,8 +25,10 @@ protocol DataBrokerProtectionEntitlementMonitoring { extension DataBrokerProtectionEntitlementMonitoring { func start(checkEntitlementFunction: @escaping () async throws -> Bool, callback: @escaping (DataBrokerProtectionEntitlementMonitorResult) -> Void) { + let defaultMonitoringInterval = 20 + start(checkEntitlementFunction: checkEntitlementFunction, - intervalInMinutes: DataBrokerProtectionEntitlementMonitor.monitoringInterval, + intervalInMinutes: defaultMonitoringInterval, callback: callback) } } @@ -39,7 +41,6 @@ public enum DataBrokerProtectionEntitlementMonitorResult { final class DataBrokerProtectionEntitlementMonitor: DataBrokerProtectionEntitlementMonitoring { private var timer: Timer? - static let monitoringInterval = 20 func start(checkEntitlementFunction: @escaping () async throws -> Bool, intervalInMinutes: Int, callback: @escaping (DataBrokerProtectionEntitlementMonitorResult) -> Void) { timer = Timer.scheduledTimer(withTimeInterval: TimeInterval(intervalInMinutes * 60), repeats: true) { _ in diff --git a/LocalPackages/DataBrokerProtection/Sources/DataBrokerProtection/Scheduler/DataBrokerProtectionAgentManager.swift b/LocalPackages/DataBrokerProtection/Sources/DataBrokerProtection/Scheduler/DataBrokerProtectionAgentManager.swift index b787bcb9b8..02e1b1c1ae 100644 --- a/LocalPackages/DataBrokerProtection/Sources/DataBrokerProtection/Scheduler/DataBrokerProtectionAgentManager.swift +++ b/LocalPackages/DataBrokerProtection/Sources/DataBrokerProtection/Scheduler/DataBrokerProtectionAgentManager.swift @@ -138,8 +138,9 @@ public final class DataBrokerProtectionAgentManager { public func agentFinishedLaunching() { Task { @MainActor in - // The browser shouldn't start the agent if these prerequisites aren't met - // But since the agent can auto-start after a reboot without the browser, we need to validate it again + // The browser shouldn't start the agent if these prerequisites aren't met. + // However, since the agent can auto-start after a reboot without the browser, we need to validate it again. + // If the agent needs to be stopped, this function will stop it, so the subsequent calls after it will not be made. await agentStopper.validateRunPreRequisitesAndStopAgentIfNecessary() activityScheduler.startScheduler() From a1a17a940e01440c80bb84baeeb01c3cfb4d9dc1 Mon Sep 17 00:00:00 2001 From: Fernando Bunn Date: Tue, 21 May 2024 17:52:52 +0100 Subject: [PATCH 05/16] add comments --- .../Utils/DataBrokerProtectionAgentStopper.swift | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/LocalPackages/DataBrokerProtection/Sources/DataBrokerProtection/Utils/DataBrokerProtectionAgentStopper.swift b/LocalPackages/DataBrokerProtection/Sources/DataBrokerProtection/Utils/DataBrokerProtectionAgentStopper.swift index 388295e58f..6cdd6aa7d9 100644 --- a/LocalPackages/DataBrokerProtection/Sources/DataBrokerProtection/Utils/DataBrokerProtectionAgentStopper.swift +++ b/LocalPackages/DataBrokerProtection/Sources/DataBrokerProtection/Utils/DataBrokerProtectionAgentStopper.swift @@ -20,8 +20,13 @@ import Foundation import Common protocol DataBrokerProtectionAgentStopper { + /// Validates if the user has profile data, is authenticated, and has valid entitlement. If any of these conditions are not met, the agent will be stopped. func validateRunPreRequisitesAndStopAgentIfNecessary() async + + /// Monitors the entitlement package. If the entitlement check returns false, the agent will be stopped. func monitorEntitlementAndStopAgentIfNecessary() + + /// Stops the agent func stopAgent() } From 821112eed870c48a61a5a1c4f262dce9827a32fb Mon Sep 17 00:00:00 2001 From: Fernando Bunn Date: Tue, 21 May 2024 18:04:46 +0100 Subject: [PATCH 06/16] Update DuckDuckGo/DBP/DataBrokerProtectionSubscriptionEventHandler.swift Co-authored-by: Pete Smith --- .../DBP/DataBrokerProtectionSubscriptionEventHandler.swift | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/DuckDuckGo/DBP/DataBrokerProtectionSubscriptionEventHandler.swift b/DuckDuckGo/DBP/DataBrokerProtectionSubscriptionEventHandler.swift index 52686cc0ad..596f78d577 100644 --- a/DuckDuckGo/DBP/DataBrokerProtectionSubscriptionEventHandler.swift +++ b/DuckDuckGo/DBP/DataBrokerProtectionSubscriptionEventHandler.swift @@ -31,7 +31,7 @@ final class DataBrokerProtectionSubscriptionEventHandler { func registerForSubscriptionAccountManagerEvents() { NotificationCenter.default.addObserver(self, selector: #selector(handleAccountDidSignOut), name: .accountDidSignOut, object: nil) - NotificationCenter.default.addObserver(self, selector: #selector(handleAccountDidSignOut), name: .entitlementsDidChange, object: nil) + NotificationCenter.default.addObserver(self, selector: #selector(entitlementsDidChange), name: .entitlementsDidChange, object: nil) } @objc private func handleAccountDidSignOut() { From 37a4515f23e564705b15899be0406ba772d7d1d8 Mon Sep 17 00:00:00 2001 From: Fernando Bunn Date: Tue, 21 May 2024 18:57:49 +0100 Subject: [PATCH 07/16] PR feedback --- .../DataBrokerProtectionEntitlementMonitoring.swift | 10 ---------- .../DataBrokerProtectionAgentManager.swift | 4 ++-- .../Utils/DataBrokerProtectionAgentStopper.swift | 13 +++++++------ .../DataBrokerProtection/Utils/Logging.swift | 8 -------- 4 files changed, 9 insertions(+), 26 deletions(-) diff --git a/LocalPackages/DataBrokerProtection/Sources/DataBrokerProtection/Authentication/DataBrokerProtectionEntitlementMonitoring.swift b/LocalPackages/DataBrokerProtection/Sources/DataBrokerProtection/Authentication/DataBrokerProtectionEntitlementMonitoring.swift index e65fb4872c..5ab13f019e 100644 --- a/LocalPackages/DataBrokerProtection/Sources/DataBrokerProtection/Authentication/DataBrokerProtectionEntitlementMonitoring.swift +++ b/LocalPackages/DataBrokerProtection/Sources/DataBrokerProtection/Authentication/DataBrokerProtectionEntitlementMonitoring.swift @@ -23,16 +23,6 @@ protocol DataBrokerProtectionEntitlementMonitoring { func stop() } -extension DataBrokerProtectionEntitlementMonitoring { - func start(checkEntitlementFunction: @escaping () async throws -> Bool, callback: @escaping (DataBrokerProtectionEntitlementMonitorResult) -> Void) { - let defaultMonitoringInterval = 20 - - start(checkEntitlementFunction: checkEntitlementFunction, - intervalInMinutes: defaultMonitoringInterval, - callback: callback) - } -} - public enum DataBrokerProtectionEntitlementMonitorResult { case enabled case disabled diff --git a/LocalPackages/DataBrokerProtection/Sources/DataBrokerProtection/Scheduler/DataBrokerProtectionAgentManager.swift b/LocalPackages/DataBrokerProtection/Sources/DataBrokerProtection/Scheduler/DataBrokerProtectionAgentManager.swift index 02e1b1c1ae..0a5880e28c 100644 --- a/LocalPackages/DataBrokerProtection/Sources/DataBrokerProtection/Scheduler/DataBrokerProtectionAgentManager.swift +++ b/LocalPackages/DataBrokerProtection/Sources/DataBrokerProtection/Scheduler/DataBrokerProtectionAgentManager.swift @@ -141,13 +141,13 @@ public final class DataBrokerProtectionAgentManager { // The browser shouldn't start the agent if these prerequisites aren't met. // However, since the agent can auto-start after a reboot without the browser, we need to validate it again. // If the agent needs to be stopped, this function will stop it, so the subsequent calls after it will not be made. - await agentStopper.validateRunPreRequisitesAndStopAgentIfNecessary() + await agentStopper.validateRunPrerequisitesAndStopAgentIfNecessary() activityScheduler.startScheduler() didStartActivityScheduler = true queueManager.startScheduledOperationsIfPermitted(showWebView: false, operationDependencies: operationDependencies, completion: nil) - agentStopper.monitorEntitlementAndStopAgentIfNecessary() + agentStopper.monitorEntitlementAndStopAgentIfEntitlementIsInvalid() } } } diff --git a/LocalPackages/DataBrokerProtection/Sources/DataBrokerProtection/Utils/DataBrokerProtectionAgentStopper.swift b/LocalPackages/DataBrokerProtection/Sources/DataBrokerProtection/Utils/DataBrokerProtectionAgentStopper.swift index 6cdd6aa7d9..1c0ad2d01e 100644 --- a/LocalPackages/DataBrokerProtection/Sources/DataBrokerProtection/Utils/DataBrokerProtectionAgentStopper.swift +++ b/LocalPackages/DataBrokerProtection/Sources/DataBrokerProtection/Utils/DataBrokerProtectionAgentStopper.swift @@ -21,11 +21,12 @@ import Common protocol DataBrokerProtectionAgentStopper { /// Validates if the user has profile data, is authenticated, and has valid entitlement. If any of these conditions are not met, the agent will be stopped. - func validateRunPreRequisitesAndStopAgentIfNecessary() async + func validateRunPrerequisitesAndStopAgentIfNecessary() async /// Monitors the entitlement package. If the entitlement check returns false, the agent will be stopped. - func monitorEntitlementAndStopAgentIfNecessary() - + /// This function ensures that the agent is stopped if the user's subscription has expired, even if the browser is not active. Regularly checking for entitlement is required since notifications are not posted to agents. + func monitorEntitlementAndStopAgentIfEntitlementIsInvalid() + /// Stops the agent func stopAgent() } @@ -43,7 +44,7 @@ struct DefaultDataBrokerProtectionAgentStopper: DataBrokerProtectionAgentStopper self.authenticationManager = authenticationManager } - public func validateRunPreRequisitesAndStopAgentIfNecessary() async { + public func validateRunPrerequisitesAndStopAgentIfNecessary() async { do { guard try dataManager.fetchProfile() != nil, authenticationManager.isUserAuthenticated, @@ -60,8 +61,8 @@ struct DefaultDataBrokerProtectionAgentStopper: DataBrokerProtectionAgentStopper } } - public func monitorEntitlementAndStopAgentIfNecessary() { - entitlementMonitor.start(checkEntitlementFunction: authenticationManager.hasValidEntitlement) { result in + public func monitorEntitlementAndStopAgentIfEntitlementIsInvalid() { + entitlementMonitor.start(checkEntitlementFunction: authenticationManager.hasValidEntitlement, intervalInMinutes: 60) { result in switch result { case .enabled: #warning("Send pixel enabled") diff --git a/LocalPackages/DataBrokerProtection/Sources/DataBrokerProtection/Utils/Logging.swift b/LocalPackages/DataBrokerProtection/Sources/DataBrokerProtection/Utils/Logging.swift index c24c0023e3..2b8016c99a 100644 --- a/LocalPackages/DataBrokerProtection/Sources/DataBrokerProtection/Utils/Logging.swift +++ b/LocalPackages/DataBrokerProtection/Sources/DataBrokerProtection/Utils/Logging.swift @@ -43,10 +43,6 @@ struct Logging { fileprivate static let backgroundAgentPixelLoggingEnabled = true fileprivate static let backgroundAgentPixel: OSLog = OSLog(subsystem: subsystem, category: "Data Broker Protection Background Agent Pixel") - - fileprivate static let dataBrokerProtectionMonitoringEnabled = true - fileprivate static let dataBrokerProtectionMonitoring: OSLog = OSLog(subsystem: subsystem, category: "Data Broker Protection Monitoring") - } extension OSLog { @@ -78,8 +74,4 @@ extension OSLog { public static var dbpBackgroundAgentPixel: OSLog { Logging.backgroundAgentPixelLoggingEnabled ? Logging.backgroundAgentPixel : .disabled } - - public static var dataBrokerProtectionMonitoring: OSLog { - Logging.dataBrokerProtectionMonitoringEnabled ? Logging.dataBrokerProtectionMonitoring : .disabled - } } From e3b5d008b80949234b59770fccb655de310e1a33 Mon Sep 17 00:00:00 2001 From: Fernando Bunn Date: Wed, 22 May 2024 11:27:21 +0100 Subject: [PATCH 08/16] Add pixels for subscriptionEventHandler --- DuckDuckGo/Application/AppDelegate.swift | 7 +++- .../DataBrokerProtectionPixelsHandler.swift | 6 +++- ...erProtectionSubscriptionEventHandler.swift | 36 ++++++++++++++++--- .../Pixels/DataBrokerProtectionPixels.swift | 19 +++++++++- 4 files changed, 61 insertions(+), 7 deletions(-) diff --git a/DuckDuckGo/Application/AppDelegate.swift b/DuckDuckGo/Application/AppDelegate.swift index afbeaf6d23..46896100cd 100644 --- a/DuckDuckGo/Application/AppDelegate.swift +++ b/DuckDuckGo/Application/AppDelegate.swift @@ -99,7 +99,12 @@ final class AppDelegate: NSObject, NSApplicationDelegate { }() #if DBP - private let dataBrokerProtectionSubscriptionEventHandler = DataBrokerProtectionSubscriptionEventHandler() + private lazy var dataBrokerProtectionSubscriptionEventHandler: DataBrokerProtectionSubscriptionEventHandler = { + let authManager = DataBrokerAuthenticationManagerBuilder.buildAuthenticationManager() + return DataBrokerProtectionSubscriptionEventHandler(featureDisabler: DataBrokerProtectionFeatureDisabler(), + authenticationManager: authManager, + pixelHandler: DataBrokerProtectionPixelsHandler()) + }() #endif private var didFinishLaunching = false diff --git a/DuckDuckGo/DBP/DataBrokerProtectionPixelsHandler.swift b/DuckDuckGo/DBP/DataBrokerProtectionPixelsHandler.swift index 494d141be5..3632dc549b 100644 --- a/DuckDuckGo/DBP/DataBrokerProtectionPixelsHandler.swift +++ b/DuckDuckGo/DBP/DataBrokerProtectionPixelsHandler.swift @@ -98,7 +98,11 @@ public class DataBrokerProtectionPixelsHandler: EventMapping - init(featureDisabler: DataBrokerProtectionFeatureDisabling = DataBrokerProtectionFeatureDisabler()) { + init(featureDisabler: DataBrokerProtectionFeatureDisabling, + authenticationManager: DataBrokerProtectionAuthenticationManaging, + pixelHandler: EventMapping) { self.featureDisabler = featureDisabler + self.authenticationManager = authenticationManager + self.pixelHandler = pixelHandler } func registerForSubscriptionAccountManagerEvents() { - NotificationCenter.default.addObserver(self, selector: #selector(handleAccountDidSignOut), name: .accountDidSignOut, object: nil) - NotificationCenter.default.addObserver(self, selector: #selector(entitlementsDidChange), name: .entitlementsDidChange, object: nil) + NotificationCenter.default.addObserver(self, + selector: #selector(handleAccountDidSignOut), + name: .accountDidSignOut, + object: nil) + + NotificationCenter.default.addObserver(self, + selector: #selector(entitlementsDidChange), + name: .entitlementsDidChange, + object: nil) } @objc private func handleAccountDidSignOut() { @@ -39,7 +53,21 @@ final class DataBrokerProtectionSubscriptionEventHandler { } @objc private func entitlementsDidChange() { - #warning("Validate if valid and delete if necessary after sending pixels") + Task { @MainActor in + do { + if try await authenticationManager.hasValidEntitlement() { + pixelHandler.fire(.entitlementCheckValid) + } else { + pixelHandler.fire(.entitlementCheckInvalid) + featureDisabler.disableAndDelete() + } + } catch { + /// We don't want to disable the agent in case of an error while checking for entitlements. + /// Since this is a destructive action, the only situation that should cause the data to be deleted and the agent to be removed is .success(false) + pixelHandler.fire(.entitlementCheckError) + assertionFailure("Error validating entitlement \(error)") + } + } } } diff --git a/LocalPackages/DataBrokerProtection/Sources/DataBrokerProtection/Pixels/DataBrokerProtectionPixels.swift b/LocalPackages/DataBrokerProtection/Sources/DataBrokerProtection/Pixels/DataBrokerProtectionPixels.swift index f83d3c6aeb..3e5b87a0f4 100644 --- a/LocalPackages/DataBrokerProtection/Sources/DataBrokerProtection/Pixels/DataBrokerProtectionPixels.swift +++ b/LocalPackages/DataBrokerProtection/Sources/DataBrokerProtection/Pixels/DataBrokerProtectionPixels.swift @@ -164,6 +164,11 @@ public enum DataBrokerProtectionPixels { case initialScanSiteLoadDuration(duration: Double, hasError: Bool, brokerURL: String, sleepDuration: Double) case initialScanPostLoadingDuration(duration: Double, hasError: Bool, brokerURL: String, sleepDuration: Double) case initialScanPreStartDuration(duration: Double) + + // Entitlements + case entitlementCheckValid + case entitlementCheckInvalid + case entitlementCheckError } extension DataBrokerProtectionPixels: PixelKitEvent { @@ -267,6 +272,11 @@ extension DataBrokerProtectionPixels: PixelKitEvent { case .initialScanSiteLoadDuration: return "m_mac_dbp_scan_broker_site_loaded" case .initialScanPostLoadingDuration: return "m_mac_dbp_initial_scan_broker_post_loading" case .initialScanPreStartDuration: return "m_mac_dbp_initial_scan_pre_start_duration" + + // Entitlements + case .entitlementCheckValid: return "m_mac_dbp_macos_entitlement_valid" + case .entitlementCheckInvalid: return "m_mac_dbp_macos_entitlement_invalid" + case .entitlementCheckError: return "m_mac_dbp_macos_entitlement_error" } } @@ -362,6 +372,10 @@ extension DataBrokerProtectionPixels: PixelKitEvent { .homeViewCTAMoveApplicationClicked, .homeViewCTAGrantPermissionClicked, + .entitlementCheckValid, + .entitlementCheckInvalid, + .entitlementCheckError, + .secureVaultInitError, .secureVaultError: return [:] @@ -486,7 +500,10 @@ public class DataBrokerProtectionPixelsHandler: EventMapping Date: Wed, 22 May 2024 14:43:05 +0100 Subject: [PATCH 09/16] Handle entitlement check in the background agent --- .../DataBrokerProtectionAgentManager.swift | 3 +- .../DataBrokerProtectionAgentStopper.swift | 51 +++++++++++++------ 2 files changed, 38 insertions(+), 16 deletions(-) diff --git a/LocalPackages/DataBrokerProtection/Sources/DataBrokerProtection/Scheduler/DataBrokerProtectionAgentManager.swift b/LocalPackages/DataBrokerProtection/Sources/DataBrokerProtection/Scheduler/DataBrokerProtectionAgentManager.swift index 0a5880e28c..4b25a19412 100644 --- a/LocalPackages/DataBrokerProtection/Sources/DataBrokerProtection/Scheduler/DataBrokerProtectionAgentManager.swift +++ b/LocalPackages/DataBrokerProtection/Sources/DataBrokerProtection/Scheduler/DataBrokerProtectionAgentManager.swift @@ -74,7 +74,8 @@ public class DataBrokerProtectionAgentManagerProvider { let agentstopper = DefaultDataBrokerProtectionAgentStopper(dataManager: dataManager, entitlementMonitor: DataBrokerProtectionEntitlementMonitor(), - authenticationManager: authenticationManager) + authenticationManager: authenticationManager, + pixelHandler: pixelHandler) let operationDependencies = DefaultDataBrokerOperationDependencies( database: dataManager.database, diff --git a/LocalPackages/DataBrokerProtection/Sources/DataBrokerProtection/Utils/DataBrokerProtectionAgentStopper.swift b/LocalPackages/DataBrokerProtection/Sources/DataBrokerProtection/Utils/DataBrokerProtectionAgentStopper.swift index 1c0ad2d01e..445ce41fff 100644 --- a/LocalPackages/DataBrokerProtection/Sources/DataBrokerProtection/Utils/DataBrokerProtectionAgentStopper.swift +++ b/LocalPackages/DataBrokerProtection/Sources/DataBrokerProtection/Utils/DataBrokerProtectionAgentStopper.swift @@ -26,7 +26,7 @@ protocol DataBrokerProtectionAgentStopper { /// Monitors the entitlement package. If the entitlement check returns false, the agent will be stopped. /// This function ensures that the agent is stopped if the user's subscription has expired, even if the browser is not active. Regularly checking for entitlement is required since notifications are not posted to agents. func monitorEntitlementAndStopAgentIfEntitlementIsInvalid() - + /// Stops the agent func stopAgent() } @@ -35,21 +35,22 @@ struct DefaultDataBrokerProtectionAgentStopper: DataBrokerProtectionAgentStopper private let dataManager: DataBrokerProtectionDataManaging private let entitlementMonitor: DataBrokerProtectionEntitlementMonitoring private let authenticationManager: DataBrokerProtectionAuthenticationManaging + private let pixelHandler: EventMapping init(dataManager: DataBrokerProtectionDataManaging, entitlementMonitor: DataBrokerProtectionEntitlementMonitoring, - authenticationManager: DataBrokerProtectionAuthenticationManaging) { + authenticationManager: DataBrokerProtectionAuthenticationManaging, + pixelHandler: EventMapping) { self.dataManager = dataManager self.entitlementMonitor = entitlementMonitor self.authenticationManager = authenticationManager + self.pixelHandler = pixelHandler } public func validateRunPrerequisitesAndStopAgentIfNecessary() async { do { guard try dataManager.fetchProfile() != nil, - authenticationManager.isUserAuthenticated, - try await authenticationManager.hasValidEntitlement() else { - + authenticationManager.isUserAuthenticated else { os_log("Prerequisites are invalid", log: .dataBrokerProtection) stopAgent() return @@ -59,19 +60,21 @@ struct DefaultDataBrokerProtectionAgentStopper: DataBrokerProtectionAgentStopper os_log("Error validating prerequisites, error: %{public}@", log: .dataBrokerProtection, error.localizedDescription) stopAgent() } + + do { + let result = try await authenticationManager.hasValidEntitlement() + stopAgentBasedOnEntitlementCheckResult(result ? .enabled : .disabled) + } catch { + stopAgentBasedOnEntitlementCheckResult(.error) + } } + /// Monitors entitlement changes every 60 minutes to optimize system performance and resource utilization by avoiding unnecessary operations when entitlement is invalid. + /// While keeping the agent active with invalid entitlement has no significant risk, setting the monitoring interval at 60 minutes is a good balance to minimize backend checks. public func monitorEntitlementAndStopAgentIfEntitlementIsInvalid() { - entitlementMonitor.start(checkEntitlementFunction: authenticationManager.hasValidEntitlement, intervalInMinutes: 60) { result in - switch result { - case .enabled: -#warning("Send pixel enabled") - stopAgent() - case .disabled: -#warning("Send pixel disabled") - case .error: -#warning("Send pixel error") - } + entitlementMonitor.start(checkEntitlementFunction: authenticationManager.hasValidEntitlement, + intervalInMinutes: 60) { result in + stopAgentBasedOnEntitlementCheckResult(result) } } @@ -79,4 +82,22 @@ struct DefaultDataBrokerProtectionAgentStopper: DataBrokerProtectionAgentStopper os_log("Stopping DataBrokerProtection Agent", log: .dataBrokerProtection) exit(EXIT_SUCCESS) } + + private func stopAgentBasedOnEntitlementCheckResult(_ result: DataBrokerProtectionEntitlementMonitorResult) { + switch result { + case .enabled: + os_log("Valid entitlement", log: .dataBrokerProtection) + pixelHandler.fire(.entitlementCheckValid) + case .disabled: + os_log("Invalid entitlement", log: .dataBrokerProtection) + pixelHandler.fire(.entitlementCheckInvalid) + pixelHandler.fire(.entitlementCheckInvalid) + stopAgent() + case .error: + os_log("Error when checking entitlement", log: .dataBrokerProtection) + /// We don't want to disable the agent in case of an error while checking for entitlements. + /// Since this is a destructive action, the only situation that should cause the data to be deleted and the agent to be removed is .success(false) + pixelHandler.fire(.entitlementCheckError) + } + } } From 7a950d49b24bb06670efe5c1170bba9157277c8f Mon Sep 17 00:00:00 2001 From: Fernando Bunn Date: Wed, 22 May 2024 16:50:08 +0100 Subject: [PATCH 10/16] Add tests --- ...rokerProtectionEntitlementMonitoring.swift | 6 +- .../DataBrokerProtectionAgentManager.swift | 4 +- .../DataBrokerProtectionAgentStopper.swift | 28 ++- ...ataBrokerProtectionAgentManagerTests.swift | 32 ++- ...ataBrokerProtectionAgentStopperTests.swift | 198 ++++++++++++++++++ .../DataBrokerProtectionTests/Mocks.swift | 41 +++- 6 files changed, 285 insertions(+), 24 deletions(-) create mode 100644 LocalPackages/DataBrokerProtection/Tests/DataBrokerProtectionTests/DataBrokerProtectionAgentStopperTests.swift diff --git a/LocalPackages/DataBrokerProtection/Sources/DataBrokerProtection/Authentication/DataBrokerProtectionEntitlementMonitoring.swift b/LocalPackages/DataBrokerProtection/Sources/DataBrokerProtection/Authentication/DataBrokerProtectionEntitlementMonitoring.swift index 5ab13f019e..77b6d883b2 100644 --- a/LocalPackages/DataBrokerProtection/Sources/DataBrokerProtection/Authentication/DataBrokerProtectionEntitlementMonitoring.swift +++ b/LocalPackages/DataBrokerProtection/Sources/DataBrokerProtection/Authentication/DataBrokerProtectionEntitlementMonitoring.swift @@ -19,7 +19,7 @@ import Foundation protocol DataBrokerProtectionEntitlementMonitoring { - func start(checkEntitlementFunction: @escaping () async throws -> Bool, intervalInMinutes: Int, callback: @escaping (DataBrokerProtectionEntitlementMonitorResult) -> Void) + func start(checkEntitlementFunction: @escaping () async throws -> Bool, interval: TimeInterval, callback: @escaping (DataBrokerProtectionEntitlementMonitorResult) -> Void) func stop() } @@ -32,8 +32,8 @@ public enum DataBrokerProtectionEntitlementMonitorResult { final class DataBrokerProtectionEntitlementMonitor: DataBrokerProtectionEntitlementMonitoring { private var timer: Timer? - func start(checkEntitlementFunction: @escaping () async throws -> Bool, intervalInMinutes: Int, callback: @escaping (DataBrokerProtectionEntitlementMonitorResult) -> Void) { - timer = Timer.scheduledTimer(withTimeInterval: TimeInterval(intervalInMinutes * 60), repeats: true) { _ in + func start(checkEntitlementFunction: @escaping () async throws -> Bool, interval: TimeInterval, callback: @escaping (DataBrokerProtectionEntitlementMonitorResult) -> Void) { + timer = Timer.scheduledTimer(withTimeInterval: interval, repeats: true) { _ in Task { do { switch try await checkEntitlementFunction() { diff --git a/LocalPackages/DataBrokerProtection/Sources/DataBrokerProtection/Scheduler/DataBrokerProtectionAgentManager.swift b/LocalPackages/DataBrokerProtection/Sources/DataBrokerProtection/Scheduler/DataBrokerProtectionAgentManager.swift index 4b25a19412..8adeb6f767 100644 --- a/LocalPackages/DataBrokerProtection/Sources/DataBrokerProtection/Scheduler/DataBrokerProtectionAgentManager.swift +++ b/LocalPackages/DataBrokerProtection/Sources/DataBrokerProtection/Scheduler/DataBrokerProtectionAgentManager.swift @@ -148,7 +148,9 @@ public final class DataBrokerProtectionAgentManager { didStartActivityScheduler = true queueManager.startScheduledOperationsIfPermitted(showWebView: false, operationDependencies: operationDependencies, completion: nil) - agentStopper.monitorEntitlementAndStopAgentIfEntitlementIsInvalid() + /// Monitors entitlement changes every 60 minutes to optimize system performance and resource utilization by avoiding unnecessary operations when entitlement is invalid. + /// While keeping the agent active with invalid entitlement has no significant risk, setting the monitoring interval at 60 minutes is a good balance to minimize backend checks. + agentStopper.monitorEntitlementAndStopAgentIfEntitlementIsInvalid(interval: .minutes(60)) } } } diff --git a/LocalPackages/DataBrokerProtection/Sources/DataBrokerProtection/Utils/DataBrokerProtectionAgentStopper.swift b/LocalPackages/DataBrokerProtection/Sources/DataBrokerProtection/Utils/DataBrokerProtectionAgentStopper.swift index 445ce41fff..2854b39d9f 100644 --- a/LocalPackages/DataBrokerProtection/Sources/DataBrokerProtection/Utils/DataBrokerProtectionAgentStopper.swift +++ b/LocalPackages/DataBrokerProtection/Sources/DataBrokerProtection/Utils/DataBrokerProtectionAgentStopper.swift @@ -25,7 +25,7 @@ protocol DataBrokerProtectionAgentStopper { /// Monitors the entitlement package. If the entitlement check returns false, the agent will be stopped. /// This function ensures that the agent is stopped if the user's subscription has expired, even if the browser is not active. Regularly checking for entitlement is required since notifications are not posted to agents. - func monitorEntitlementAndStopAgentIfEntitlementIsInvalid() + func monitorEntitlementAndStopAgentIfEntitlementIsInvalid(interval: TimeInterval) /// Stops the agent func stopAgent() @@ -36,15 +36,18 @@ struct DefaultDataBrokerProtectionAgentStopper: DataBrokerProtectionAgentStopper private let entitlementMonitor: DataBrokerProtectionEntitlementMonitoring private let authenticationManager: DataBrokerProtectionAuthenticationManaging private let pixelHandler: EventMapping + private let stopperUseCase: DataBrokerProtectionStopperUseCase init(dataManager: DataBrokerProtectionDataManaging, entitlementMonitor: DataBrokerProtectionEntitlementMonitoring, authenticationManager: DataBrokerProtectionAuthenticationManaging, - pixelHandler: EventMapping) { + pixelHandler: EventMapping, + stopperUseCase: DataBrokerProtectionStopperUseCase = AgentStopperUseCase()) { self.dataManager = dataManager self.entitlementMonitor = entitlementMonitor self.authenticationManager = authenticationManager self.pixelHandler = pixelHandler + self.stopperUseCase = stopperUseCase } public func validateRunPrerequisitesAndStopAgentIfNecessary() async { @@ -69,18 +72,15 @@ struct DefaultDataBrokerProtectionAgentStopper: DataBrokerProtectionAgentStopper } } - /// Monitors entitlement changes every 60 minutes to optimize system performance and resource utilization by avoiding unnecessary operations when entitlement is invalid. - /// While keeping the agent active with invalid entitlement has no significant risk, setting the monitoring interval at 60 minutes is a good balance to minimize backend checks. - public func monitorEntitlementAndStopAgentIfEntitlementIsInvalid() { + public func monitorEntitlementAndStopAgentIfEntitlementIsInvalid(interval: TimeInterval) { entitlementMonitor.start(checkEntitlementFunction: authenticationManager.hasValidEntitlement, - intervalInMinutes: 60) { result in + interval: interval) { result in stopAgentBasedOnEntitlementCheckResult(result) } } func stopAgent() { - os_log("Stopping DataBrokerProtection Agent", log: .dataBrokerProtection) - exit(EXIT_SUCCESS) + stopperUseCase.stopAgent() } private func stopAgentBasedOnEntitlementCheckResult(_ result: DataBrokerProtectionEntitlementMonitorResult) { @@ -91,7 +91,6 @@ struct DefaultDataBrokerProtectionAgentStopper: DataBrokerProtectionAgentStopper case .disabled: os_log("Invalid entitlement", log: .dataBrokerProtection) pixelHandler.fire(.entitlementCheckInvalid) - pixelHandler.fire(.entitlementCheckInvalid) stopAgent() case .error: os_log("Error when checking entitlement", log: .dataBrokerProtection) @@ -101,3 +100,14 @@ struct DefaultDataBrokerProtectionAgentStopper: DataBrokerProtectionAgentStopper } } } + +protocol DataBrokerProtectionStopperUseCase { + func stopAgent() +} + +struct AgentStopperUseCase: DataBrokerProtectionStopperUseCase { + func stopAgent() { + os_log("Stopping DataBrokerProtection Agent", log: .dataBrokerProtection) + exit(EXIT_SUCCESS) + } +} diff --git a/LocalPackages/DataBrokerProtection/Tests/DataBrokerProtectionTests/DataBrokerProtectionAgentManagerTests.swift b/LocalPackages/DataBrokerProtection/Tests/DataBrokerProtectionTests/DataBrokerProtectionAgentManagerTests.swift index f4d00a5d5c..3661387be7 100644 --- a/LocalPackages/DataBrokerProtection/Tests/DataBrokerProtectionTests/DataBrokerProtectionAgentManagerTests.swift +++ b/LocalPackages/DataBrokerProtection/Tests/DataBrokerProtectionTests/DataBrokerProtectionAgentManagerTests.swift @@ -31,12 +31,14 @@ final class DataBrokerProtectionAgentManagerTests: XCTestCase { private var mockPixelHandler: MockPixelHandler! private var mockDependencies: DefaultDataBrokerOperationDependencies! private var mockProfile: DataBrokerProtectionProfile! + private var mockAgentStopper: MockAgentStopper! override func setUpWithError() throws { mockPixelHandler = MockPixelHandler() mockActivityScheduler = MockDataBrokerProtectionBackgroundActivityScheduler() mockNotificationService = MockUserNotificationService() + mockAgentStopper = MockAgentStopper() let mockDatabase = MockDatabase() let mockMismatchCalculator = MockMismatchCalculator(database: mockDatabase, pixelHandler: mockPixelHandler) @@ -75,7 +77,8 @@ final class DataBrokerProtectionAgentManagerTests: XCTestCase { queueManager: mockQueueManager, dataManager: mockDataManager, operationDependencies: mockDependencies, - pixelHandler: mockPixelHandler) + pixelHandler: mockPixelHandler, + agentStopper: mockAgentStopper) mockDataManager.profileToReturn = mockProfile @@ -106,7 +109,8 @@ final class DataBrokerProtectionAgentManagerTests: XCTestCase { queueManager: mockQueueManager, dataManager: mockDataManager, operationDependencies: mockDependencies, - pixelHandler: mockPixelHandler) + pixelHandler: mockPixelHandler, + agentStopper: mockAgentStopper) mockDataManager.profileToReturn = nil @@ -137,7 +141,8 @@ final class DataBrokerProtectionAgentManagerTests: XCTestCase { queueManager: mockQueueManager, dataManager: mockDataManager, operationDependencies: mockDependencies, - pixelHandler: mockPixelHandler) + pixelHandler: mockPixelHandler, + agentStopper: mockAgentStopper) mockDataManager.profileToReturn = mockProfile @@ -162,7 +167,8 @@ final class DataBrokerProtectionAgentManagerTests: XCTestCase { queueManager: mockQueueManager, dataManager: mockDataManager, operationDependencies: mockDependencies, - pixelHandler: mockPixelHandler) + pixelHandler: mockPixelHandler, + agentStopper: mockAgentStopper) mockDataManager.profileToReturn = mockProfile @@ -187,7 +193,8 @@ final class DataBrokerProtectionAgentManagerTests: XCTestCase { queueManager: mockQueueManager, dataManager: mockDataManager, operationDependencies: mockDependencies, - pixelHandler: mockPixelHandler) + pixelHandler: mockPixelHandler, + agentStopper: mockAgentStopper) mockNotificationService.reset() @@ -207,7 +214,8 @@ final class DataBrokerProtectionAgentManagerTests: XCTestCase { queueManager: mockQueueManager, dataManager: mockDataManager, operationDependencies: mockDependencies, - pixelHandler: mockPixelHandler) + pixelHandler: mockPixelHandler, + agentStopper: mockAgentStopper) mockNotificationService.reset() @@ -227,7 +235,8 @@ final class DataBrokerProtectionAgentManagerTests: XCTestCase { queueManager: mockQueueManager, dataManager: mockDataManager, operationDependencies: mockDependencies, - pixelHandler: mockPixelHandler) + pixelHandler: mockPixelHandler, + agentStopper: mockAgentStopper) mockNotificationService.reset() mockQueueManager.startImmediateOperationsIfPermittedCompletionError = DataBrokerProtectionAgentErrorCollection(oneTimeError: NSError(domain: "test", code: 10)) @@ -248,7 +257,8 @@ final class DataBrokerProtectionAgentManagerTests: XCTestCase { queueManager: mockQueueManager, dataManager: mockDataManager, operationDependencies: mockDependencies, - pixelHandler: mockPixelHandler) + pixelHandler: mockPixelHandler, + agentStopper: mockAgentStopper) mockNotificationService.reset() mockDataManager.shouldReturnHasMatches = true @@ -269,7 +279,8 @@ final class DataBrokerProtectionAgentManagerTests: XCTestCase { queueManager: mockQueueManager, dataManager: mockDataManager, operationDependencies: mockDependencies, - pixelHandler: mockPixelHandler) + pixelHandler: mockPixelHandler, + agentStopper: mockAgentStopper) mockNotificationService.reset() mockDataManager.shouldReturnHasMatches = false @@ -290,7 +301,8 @@ final class DataBrokerProtectionAgentManagerTests: XCTestCase { queueManager: mockQueueManager, dataManager: mockDataManager, operationDependencies: mockDependencies, - pixelHandler: mockPixelHandler) + pixelHandler: mockPixelHandler, + agentStopper: mockAgentStopper) var startScheduledScansCalled = false mockQueueManager.startScheduledOperationsIfPermittedCalledCompletion = { _ in diff --git a/LocalPackages/DataBrokerProtection/Tests/DataBrokerProtectionTests/DataBrokerProtectionAgentStopperTests.swift b/LocalPackages/DataBrokerProtection/Tests/DataBrokerProtectionTests/DataBrokerProtectionAgentStopperTests.swift new file mode 100644 index 0000000000..cad9eeac3f --- /dev/null +++ b/LocalPackages/DataBrokerProtection/Tests/DataBrokerProtectionTests/DataBrokerProtectionAgentStopperTests.swift @@ -0,0 +1,198 @@ +// +// DataBrokerProtectionAgentStopperTests.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 XCTest +import Common + +@testable import DataBrokerProtection + +final class DataBrokerProtectionAgentStopperTests: XCTestCase { + var mockPixelHandler: EventMapping! + var mockAuthenticationManager: MockAuthenticationManager! + var mockEntitlementMonitor: DataBrokerProtectionEntitlementMonitor! + var mockDataManager: MockDataBrokerProtectionDataManager! + var mockStopperUseCase: MockStopperUseCase! + + private var fakeProfile: DataBrokerProtectionProfile { + let name = DataBrokerProtectionProfile.Name(firstName: "John", lastName: "Doe") + let address = DataBrokerProtectionProfile.Address(city: "City", state: "State") + + return DataBrokerProtectionProfile(names: [name], addresses: [address], phones: [String](), birthYear: 1900) + } + + override func setUp() { + mockPixelHandler = MockDataBrokerProtectionPixelsHandler() + mockAuthenticationManager = MockAuthenticationManager() + mockPixelHandler = MockPixelHandler() + mockEntitlementMonitor = DataBrokerProtectionEntitlementMonitor() + mockDataManager = MockDataBrokerProtectionDataManager(pixelHandler: mockPixelHandler, + fakeBrokerFlag: DataBrokerDebugFlagFakeBroker()) + mockStopperUseCase = MockStopperUseCase() + } + + override func tearDown() { + mockPixelHandler = nil + mockAuthenticationManager = nil + mockPixelHandler = nil + mockEntitlementMonitor = nil + mockDataManager = nil + mockStopperUseCase = nil + } + + func testNoProfile_thenStopAgentIsCalled() async { + mockAuthenticationManager.isUserAuthenticatedValue = true + mockAuthenticationManager.hasValidEntitlementValue = true + mockDataManager.profileToReturn = nil + + let stopper = DefaultDataBrokerProtectionAgentStopper(dataManager: mockDataManager, + entitlementMonitor: mockEntitlementMonitor, + authenticationManager: mockAuthenticationManager, + pixelHandler: mockPixelHandler, + stopperUseCase: mockStopperUseCase) + await stopper.validateRunPrerequisitesAndStopAgentIfNecessary() + + XCTAssertTrue(mockStopperUseCase.wasStopCalled) + } + + func testInvalidEntitlement_thenStopAgentIsCalled() async { + mockAuthenticationManager.isUserAuthenticatedValue = true + mockAuthenticationManager.hasValidEntitlementValue = false + mockDataManager.profileToReturn = fakeProfile + + let stopper = DefaultDataBrokerProtectionAgentStopper(dataManager: mockDataManager, + entitlementMonitor: mockEntitlementMonitor, + authenticationManager: mockAuthenticationManager, + pixelHandler: mockPixelHandler, + stopperUseCase: mockStopperUseCase) + await stopper.validateRunPrerequisitesAndStopAgentIfNecessary() + + XCTAssertTrue(mockStopperUseCase.wasStopCalled) + } + + func testUserNotAuthenticated_thenStopAgentIsCalled() async { + mockAuthenticationManager.isUserAuthenticatedValue = false + mockAuthenticationManager.hasValidEntitlementValue = true + mockDataManager.profileToReturn = fakeProfile + + let stopper = DefaultDataBrokerProtectionAgentStopper(dataManager: mockDataManager, + entitlementMonitor: mockEntitlementMonitor, + authenticationManager: mockAuthenticationManager, + pixelHandler: mockPixelHandler, + stopperUseCase: mockStopperUseCase) + await stopper.validateRunPrerequisitesAndStopAgentIfNecessary() + + XCTAssertTrue(mockStopperUseCase.wasStopCalled) + } + + func testErrorEntitlement_thenStopAgentIsNotCalled() async { + mockAuthenticationManager.isUserAuthenticatedValue = true + mockAuthenticationManager.shouldThrowEntitlementError = true + mockDataManager.profileToReturn = fakeProfile + + let stopper = DefaultDataBrokerProtectionAgentStopper(dataManager: mockDataManager, + entitlementMonitor: mockEntitlementMonitor, + authenticationManager: mockAuthenticationManager, + pixelHandler: mockPixelHandler, + stopperUseCase: mockStopperUseCase) + await stopper.validateRunPrerequisitesAndStopAgentIfNecessary() + + XCTAssertFalse(mockStopperUseCase.wasStopCalled) + } + + func testValidEntitlement_thenStopAgentIsNotCalled() async { + mockAuthenticationManager.isUserAuthenticatedValue = true + mockAuthenticationManager.hasValidEntitlementValue = true + mockDataManager.profileToReturn = fakeProfile + + let stopper = DefaultDataBrokerProtectionAgentStopper(dataManager: mockDataManager, + entitlementMonitor: mockEntitlementMonitor, + authenticationManager: mockAuthenticationManager, + pixelHandler: mockPixelHandler, + stopperUseCase: mockStopperUseCase) + await stopper.validateRunPrerequisitesAndStopAgentIfNecessary() + + XCTAssertFalse(mockStopperUseCase.wasStopCalled) + } + + func testEntitlementMonitorWithValidResult_thenStopAgentIsNotCalled() { + mockAuthenticationManager.isUserAuthenticatedValue = true + mockAuthenticationManager.hasValidEntitlementValue = true + mockDataManager.profileToReturn = fakeProfile + + let stopper = DefaultDataBrokerProtectionAgentStopper(dataManager: mockDataManager, + entitlementMonitor: mockEntitlementMonitor, + authenticationManager: mockAuthenticationManager, + pixelHandler: mockPixelHandler, + stopperUseCase: mockStopperUseCase) + + let expectation = XCTestExpectation(description: "Wait for monitor") + stopper.monitorEntitlementAndStopAgentIfEntitlementIsInvalid(interval: 0.1) + + DispatchQueue.main.asyncAfter(deadline: .now() + 0.2) { [self] in + XCTAssertFalse(mockStopperUseCase.wasStopCalled) + expectation.fulfill() + } + + wait(for: [expectation], timeout: 1) + } + + func testEntitlementMonitorWithInValidResult_thenStopAgentIsCalled() { + mockAuthenticationManager.isUserAuthenticatedValue = true + mockAuthenticationManager.hasValidEntitlementValue = false + mockDataManager.profileToReturn = fakeProfile + + let stopper = DefaultDataBrokerProtectionAgentStopper(dataManager: mockDataManager, + entitlementMonitor: mockEntitlementMonitor, + authenticationManager: mockAuthenticationManager, + pixelHandler: mockPixelHandler, + stopperUseCase: mockStopperUseCase) + + let expectation = XCTestExpectation(description: "Wait for monitor") + stopper.monitorEntitlementAndStopAgentIfEntitlementIsInvalid(interval: 0.1) + + DispatchQueue.main.asyncAfter(deadline: .now() + 0.2) { [self] in + XCTAssertTrue(mockStopperUseCase.wasStopCalled) + expectation.fulfill() + } + + wait(for: [expectation], timeout: 1) + } + + func testEntitlementMonitorWithErrorResult_thenStopAgentIsNotCalled() { + mockAuthenticationManager.isUserAuthenticatedValue = true + mockAuthenticationManager.shouldThrowEntitlementError = true + mockDataManager.profileToReturn = fakeProfile + + let stopper = DefaultDataBrokerProtectionAgentStopper(dataManager: mockDataManager, + entitlementMonitor: mockEntitlementMonitor, + authenticationManager: mockAuthenticationManager, + pixelHandler: mockPixelHandler, + stopperUseCase: mockStopperUseCase) + + let expectation = XCTestExpectation(description: "Wait for monitor") + stopper.monitorEntitlementAndStopAgentIfEntitlementIsInvalid(interval: 0.1) + + DispatchQueue.main.asyncAfter(deadline: .now() + 0.2) { [self] in + XCTAssertFalse(mockStopperUseCase.wasStopCalled) + expectation.fulfill() + } + + wait(for: [expectation], timeout: 1) + } +} diff --git a/LocalPackages/DataBrokerProtection/Tests/DataBrokerProtectionTests/Mocks.swift b/LocalPackages/DataBrokerProtection/Tests/DataBrokerProtectionTests/Mocks.swift index c4b2804e3f..887fdb31fe 100644 --- a/LocalPackages/DataBrokerProtection/Tests/DataBrokerProtectionTests/Mocks.swift +++ b/LocalPackages/DataBrokerProtection/Tests/DataBrokerProtectionTests/Mocks.swift @@ -1374,13 +1374,18 @@ final class MockAuthenticationManager: DataBrokerProtectionAuthenticationManagin var shouldAskForInviteCodeValue = false var redeemCodeCalled = false var authHeaderValue: String? = "fake auth header" + var hasValidEntitlementValue = false + var shouldThrowEntitlementError = false var isUserAuthenticated: Bool { isUserAuthenticatedValue } var accessToken: String? { accessTokenValue } func hasValidEntitlement() async throws -> Bool { - return true + if shouldThrowEntitlementError { + throw NSError(domain: "duck.com", code: 0, userInfo: [NSLocalizedDescriptionKey: "Error"]) + } + return hasValidEntitlementValue } func shouldAskForInviteCode() -> Bool { shouldAskForInviteCodeValue } @@ -1397,5 +1402,39 @@ final class MockAuthenticationManager: DataBrokerProtectionAuthenticationManagin shouldAskForInviteCodeValue = false redeemCodeCalled = false authHeaderValue = "fake auth header" + hasValidEntitlementValue = false + shouldThrowEntitlementError = false + } +} + +final class MockAgentStopper: DataBrokerProtectionAgentStopper { + var stopAgentWasCalled = false + + func validateRunPrerequisitesAndStopAgentIfNecessary() async { + + } + + func monitorEntitlementAndStopAgentIfEntitlementIsInvalid(interval: TimeInterval) { + + } + + func stopAgent() { + stopAgentWasCalled = true + } + + func reset() { + stopAgentWasCalled = false + } +} + +final class MockStopperUseCase: DataBrokerProtectionStopperUseCase { + var wasStopCalled = false + + func stopAgent() { + wasStopCalled = true + } + + func reset() { + wasStopCalled = false } } From 53d81064e664c28d8de287e69ce3b89a674dffb1 Mon Sep 17 00:00:00 2001 From: Fernando Bunn Date: Wed, 22 May 2024 17:05:01 +0100 Subject: [PATCH 11/16] Linter --- .../Pixels/DataBrokerProtectionPixels.swift | 2 -- .../Scheduler/DataBrokerProtectionAgentManager.swift | 1 - 2 files changed, 3 deletions(-) diff --git a/LocalPackages/DataBrokerProtection/Sources/DataBrokerProtection/Pixels/DataBrokerProtectionPixels.swift b/LocalPackages/DataBrokerProtection/Sources/DataBrokerProtection/Pixels/DataBrokerProtectionPixels.swift index 3e5b87a0f4..45f954a4d4 100644 --- a/LocalPackages/DataBrokerProtection/Sources/DataBrokerProtection/Pixels/DataBrokerProtectionPixels.swift +++ b/LocalPackages/DataBrokerProtection/Sources/DataBrokerProtection/Pixels/DataBrokerProtectionPixels.swift @@ -371,11 +371,9 @@ extension DataBrokerProtectionPixels: PixelKitEvent { .homeViewShowBadPathError, .homeViewCTAMoveApplicationClicked, .homeViewCTAGrantPermissionClicked, - .entitlementCheckValid, .entitlementCheckInvalid, .entitlementCheckError, - .secureVaultInitError, .secureVaultError: return [:] diff --git a/LocalPackages/DataBrokerProtection/Sources/DataBrokerProtection/Scheduler/DataBrokerProtectionAgentManager.swift b/LocalPackages/DataBrokerProtection/Sources/DataBrokerProtection/Scheduler/DataBrokerProtectionAgentManager.swift index 8adeb6f767..aada01d0c3 100644 --- a/LocalPackages/DataBrokerProtection/Sources/DataBrokerProtection/Scheduler/DataBrokerProtectionAgentManager.swift +++ b/LocalPackages/DataBrokerProtection/Sources/DataBrokerProtection/Scheduler/DataBrokerProtectionAgentManager.swift @@ -294,4 +294,3 @@ extension DataBrokerProtectionAgentManager: DataBrokerProtectionAgentDebugComman extension DataBrokerProtectionAgentManager: DataBrokerProtectionAppToAgentInterface { } - From 3367dd361e136528b5df3d330bc3fb1661cbfcf3 Mon Sep 17 00:00:00 2001 From: Fernando Bunn Date: Wed, 22 May 2024 17:23:44 +0100 Subject: [PATCH 12/16] Fix tests testWhenAgentStart_andProfileDoesNotExist_thenActivityIsNotScheduled_andSheduledOpereationsNotRun is not required anymore since this flow is being tested on DataBrokerProtectionAgentStopperTests --- ...erProtectionSubscriptionEventHandler.swift | 2 +- ...ataBrokerProtectionAgentManagerTests.swift | 37 +++---------------- 2 files changed, 6 insertions(+), 33 deletions(-) diff --git a/DuckDuckGo/DBP/DataBrokerProtectionSubscriptionEventHandler.swift b/DuckDuckGo/DBP/DataBrokerProtectionSubscriptionEventHandler.swift index 0690bd4914..82650b68fb 100644 --- a/DuckDuckGo/DBP/DataBrokerProtectionSubscriptionEventHandler.swift +++ b/DuckDuckGo/DBP/DataBrokerProtectionSubscriptionEventHandler.swift @@ -37,7 +37,7 @@ final class DataBrokerProtectionSubscriptionEventHandler { } func registerForSubscriptionAccountManagerEvents() { - NotificationCenter.default.addObserver(self, + NotificationCenter.default.addObserver(self, selector: #selector(handleAccountDidSignOut), name: .accountDidSignOut, object: nil) diff --git a/LocalPackages/DataBrokerProtection/Tests/DataBrokerProtectionTests/DataBrokerProtectionAgentManagerTests.swift b/LocalPackages/DataBrokerProtection/Tests/DataBrokerProtectionTests/DataBrokerProtectionAgentManagerTests.swift index 3661387be7..e45a767d9d 100644 --- a/LocalPackages/DataBrokerProtection/Tests/DataBrokerProtectionTests/DataBrokerProtectionAgentManagerTests.swift +++ b/LocalPackages/DataBrokerProtection/Tests/DataBrokerProtectionTests/DataBrokerProtectionAgentManagerTests.swift @@ -82,56 +82,29 @@ final class DataBrokerProtectionAgentManagerTests: XCTestCase { mockDataManager.profileToReturn = mockProfile + let schedulerStartedExpectation = XCTestExpectation(description: "Scheduler started") var schedulerStarted = false mockActivityScheduler.startSchedulerCompletion = { schedulerStarted = true + schedulerStartedExpectation.fulfill() } + let scanCalledExpectation = XCTestExpectation(description: "Scan called") var startScheduledScansCalled = false mockQueueManager.startScheduledOperationsIfPermittedCalledCompletion = { _ in startScheduledScansCalled = true + scanCalledExpectation.fulfill() } // When sut.agentFinishedLaunching() // Then + await fulfillment(of: [scanCalledExpectation, schedulerStartedExpectation], timeout: 1.0) XCTAssertTrue(schedulerStarted) XCTAssertTrue(startScheduledScansCalled) } - func testWhenAgentStart_andProfileDoesNotExist_thenActivityIsNotScheduled_andSheduledOpereationsNotRun() async throws { - // Given - sut = DataBrokerProtectionAgentManager( - userNotificationService: mockNotificationService, - activityScheduler: mockActivityScheduler, - ipcServer: mockIPCServer, - queueManager: mockQueueManager, - dataManager: mockDataManager, - operationDependencies: mockDependencies, - pixelHandler: mockPixelHandler, - agentStopper: mockAgentStopper) - - mockDataManager.profileToReturn = nil - - var schedulerStarted = false - mockActivityScheduler.startSchedulerCompletion = { - schedulerStarted = true - } - - var startScheduledScansCalled = false - mockQueueManager.startScheduledOperationsIfPermittedCalledCompletion = { _ in - startScheduledScansCalled = true - } - - // When - sut.agentFinishedLaunching() - - // Then - XCTAssertFalse(schedulerStarted) - XCTAssertFalse(startScheduledScansCalled) - } - func testWhenActivitySchedulerTriggers_thenSheduledOpereationsRun() async throws { // Given sut = DataBrokerProtectionAgentManager( From 5b973bf0dce23db2db634f427020f9205ac4362c Mon Sep 17 00:00:00 2001 From: Fernando Bunn Date: Wed, 22 May 2024 19:07:39 +0100 Subject: [PATCH 13/16] PR feedback, stop action --- .../DataBrokerProtectionAgentStopper.swift | 17 ++++----- ...ataBrokerProtectionAgentStopperTests.swift | 38 +++++++++---------- .../DataBrokerProtectionTests/Mocks.swift | 12 +----- 3 files changed, 27 insertions(+), 40 deletions(-) diff --git a/LocalPackages/DataBrokerProtection/Sources/DataBrokerProtection/Utils/DataBrokerProtectionAgentStopper.swift b/LocalPackages/DataBrokerProtection/Sources/DataBrokerProtection/Utils/DataBrokerProtectionAgentStopper.swift index 2854b39d9f..fe6c7734be 100644 --- a/LocalPackages/DataBrokerProtection/Sources/DataBrokerProtection/Utils/DataBrokerProtectionAgentStopper.swift +++ b/LocalPackages/DataBrokerProtection/Sources/DataBrokerProtection/Utils/DataBrokerProtectionAgentStopper.swift @@ -26,9 +26,6 @@ protocol DataBrokerProtectionAgentStopper { /// Monitors the entitlement package. If the entitlement check returns false, the agent will be stopped. /// This function ensures that the agent is stopped if the user's subscription has expired, even if the browser is not active. Regularly checking for entitlement is required since notifications are not posted to agents. func monitorEntitlementAndStopAgentIfEntitlementIsInvalid(interval: TimeInterval) - - /// Stops the agent - func stopAgent() } struct DefaultDataBrokerProtectionAgentStopper: DataBrokerProtectionAgentStopper { @@ -36,18 +33,18 @@ struct DefaultDataBrokerProtectionAgentStopper: DataBrokerProtectionAgentStopper private let entitlementMonitor: DataBrokerProtectionEntitlementMonitoring private let authenticationManager: DataBrokerProtectionAuthenticationManaging private let pixelHandler: EventMapping - private let stopperUseCase: DataBrokerProtectionStopperUseCase + private let stopAction: DataProtectionStopAction init(dataManager: DataBrokerProtectionDataManaging, entitlementMonitor: DataBrokerProtectionEntitlementMonitoring, authenticationManager: DataBrokerProtectionAuthenticationManaging, pixelHandler: EventMapping, - stopperUseCase: DataBrokerProtectionStopperUseCase = AgentStopperUseCase()) { + stopAction: DataProtectionStopAction = DefaultDataProtectionStopAction()) { self.dataManager = dataManager self.entitlementMonitor = entitlementMonitor self.authenticationManager = authenticationManager self.pixelHandler = pixelHandler - self.stopperUseCase = stopperUseCase + self.stopAction = stopAction } public func validateRunPrerequisitesAndStopAgentIfNecessary() async { @@ -79,8 +76,8 @@ struct DefaultDataBrokerProtectionAgentStopper: DataBrokerProtectionAgentStopper } } - func stopAgent() { - stopperUseCase.stopAgent() + private func stopAgent() { + stopAction.stopAgent() } private func stopAgentBasedOnEntitlementCheckResult(_ result: DataBrokerProtectionEntitlementMonitorResult) { @@ -101,11 +98,11 @@ struct DefaultDataBrokerProtectionAgentStopper: DataBrokerProtectionAgentStopper } } -protocol DataBrokerProtectionStopperUseCase { +protocol DataProtectionStopAction { func stopAgent() } -struct AgentStopperUseCase: DataBrokerProtectionStopperUseCase { +struct DefaultDataProtectionStopAction: DataProtectionStopAction { func stopAgent() { os_log("Stopping DataBrokerProtection Agent", log: .dataBrokerProtection) exit(EXIT_SUCCESS) diff --git a/LocalPackages/DataBrokerProtection/Tests/DataBrokerProtectionTests/DataBrokerProtectionAgentStopperTests.swift b/LocalPackages/DataBrokerProtection/Tests/DataBrokerProtectionTests/DataBrokerProtectionAgentStopperTests.swift index cad9eeac3f..fb17547849 100644 --- a/LocalPackages/DataBrokerProtection/Tests/DataBrokerProtectionTests/DataBrokerProtectionAgentStopperTests.swift +++ b/LocalPackages/DataBrokerProtection/Tests/DataBrokerProtectionTests/DataBrokerProtectionAgentStopperTests.swift @@ -27,7 +27,7 @@ final class DataBrokerProtectionAgentStopperTests: XCTestCase { var mockAuthenticationManager: MockAuthenticationManager! var mockEntitlementMonitor: DataBrokerProtectionEntitlementMonitor! var mockDataManager: MockDataBrokerProtectionDataManager! - var mockStopperUseCase: MockStopperUseCase! + var mockStopAction: MockDataProtectionStopAction! private var fakeProfile: DataBrokerProtectionProfile { let name = DataBrokerProtectionProfile.Name(firstName: "John", lastName: "Doe") @@ -43,7 +43,7 @@ final class DataBrokerProtectionAgentStopperTests: XCTestCase { mockEntitlementMonitor = DataBrokerProtectionEntitlementMonitor() mockDataManager = MockDataBrokerProtectionDataManager(pixelHandler: mockPixelHandler, fakeBrokerFlag: DataBrokerDebugFlagFakeBroker()) - mockStopperUseCase = MockStopperUseCase() + mockStopAction = MockDataProtectionStopAction() } override func tearDown() { @@ -52,7 +52,7 @@ final class DataBrokerProtectionAgentStopperTests: XCTestCase { mockPixelHandler = nil mockEntitlementMonitor = nil mockDataManager = nil - mockStopperUseCase = nil + mockStopAction = nil } func testNoProfile_thenStopAgentIsCalled() async { @@ -64,10 +64,10 @@ final class DataBrokerProtectionAgentStopperTests: XCTestCase { entitlementMonitor: mockEntitlementMonitor, authenticationManager: mockAuthenticationManager, pixelHandler: mockPixelHandler, - stopperUseCase: mockStopperUseCase) + stopperUseCase: mockStopAction) await stopper.validateRunPrerequisitesAndStopAgentIfNecessary() - XCTAssertTrue(mockStopperUseCase.wasStopCalled) + XCTAssertTrue(mockStopAction.wasStopCalled) } func testInvalidEntitlement_thenStopAgentIsCalled() async { @@ -79,10 +79,10 @@ final class DataBrokerProtectionAgentStopperTests: XCTestCase { entitlementMonitor: mockEntitlementMonitor, authenticationManager: mockAuthenticationManager, pixelHandler: mockPixelHandler, - stopperUseCase: mockStopperUseCase) + stopperUseCase: mockStopAction) await stopper.validateRunPrerequisitesAndStopAgentIfNecessary() - XCTAssertTrue(mockStopperUseCase.wasStopCalled) + XCTAssertTrue(mockStopAction.wasStopCalled) } func testUserNotAuthenticated_thenStopAgentIsCalled() async { @@ -94,10 +94,10 @@ final class DataBrokerProtectionAgentStopperTests: XCTestCase { entitlementMonitor: mockEntitlementMonitor, authenticationManager: mockAuthenticationManager, pixelHandler: mockPixelHandler, - stopperUseCase: mockStopperUseCase) + stopperUseCase: mockStopAction) await stopper.validateRunPrerequisitesAndStopAgentIfNecessary() - XCTAssertTrue(mockStopperUseCase.wasStopCalled) + XCTAssertTrue(mockStopAction.wasStopCalled) } func testErrorEntitlement_thenStopAgentIsNotCalled() async { @@ -109,10 +109,10 @@ final class DataBrokerProtectionAgentStopperTests: XCTestCase { entitlementMonitor: mockEntitlementMonitor, authenticationManager: mockAuthenticationManager, pixelHandler: mockPixelHandler, - stopperUseCase: mockStopperUseCase) + stopperUseCase: mockStopAction) await stopper.validateRunPrerequisitesAndStopAgentIfNecessary() - XCTAssertFalse(mockStopperUseCase.wasStopCalled) + XCTAssertFalse(mockStopAction.wasStopCalled) } func testValidEntitlement_thenStopAgentIsNotCalled() async { @@ -124,10 +124,10 @@ final class DataBrokerProtectionAgentStopperTests: XCTestCase { entitlementMonitor: mockEntitlementMonitor, authenticationManager: mockAuthenticationManager, pixelHandler: mockPixelHandler, - stopperUseCase: mockStopperUseCase) + stopperUseCase: mockStopAction) await stopper.validateRunPrerequisitesAndStopAgentIfNecessary() - XCTAssertFalse(mockStopperUseCase.wasStopCalled) + XCTAssertFalse(mockStopAction.wasStopCalled) } func testEntitlementMonitorWithValidResult_thenStopAgentIsNotCalled() { @@ -139,13 +139,13 @@ final class DataBrokerProtectionAgentStopperTests: XCTestCase { entitlementMonitor: mockEntitlementMonitor, authenticationManager: mockAuthenticationManager, pixelHandler: mockPixelHandler, - stopperUseCase: mockStopperUseCase) + stopperUseCase: mockStopAction) let expectation = XCTestExpectation(description: "Wait for monitor") stopper.monitorEntitlementAndStopAgentIfEntitlementIsInvalid(interval: 0.1) DispatchQueue.main.asyncAfter(deadline: .now() + 0.2) { [self] in - XCTAssertFalse(mockStopperUseCase.wasStopCalled) + XCTAssertFalse(mockStopAction.wasStopCalled) expectation.fulfill() } @@ -161,13 +161,13 @@ final class DataBrokerProtectionAgentStopperTests: XCTestCase { entitlementMonitor: mockEntitlementMonitor, authenticationManager: mockAuthenticationManager, pixelHandler: mockPixelHandler, - stopperUseCase: mockStopperUseCase) + stopperUseCase: mockStopAction) let expectation = XCTestExpectation(description: "Wait for monitor") stopper.monitorEntitlementAndStopAgentIfEntitlementIsInvalid(interval: 0.1) DispatchQueue.main.asyncAfter(deadline: .now() + 0.2) { [self] in - XCTAssertTrue(mockStopperUseCase.wasStopCalled) + XCTAssertTrue(mockStopAction.wasStopCalled) expectation.fulfill() } @@ -183,13 +183,13 @@ final class DataBrokerProtectionAgentStopperTests: XCTestCase { entitlementMonitor: mockEntitlementMonitor, authenticationManager: mockAuthenticationManager, pixelHandler: mockPixelHandler, - stopperUseCase: mockStopperUseCase) + stopperUseCase: mockStopAction) let expectation = XCTestExpectation(description: "Wait for monitor") stopper.monitorEntitlementAndStopAgentIfEntitlementIsInvalid(interval: 0.1) DispatchQueue.main.asyncAfter(deadline: .now() + 0.2) { [self] in - XCTAssertFalse(mockStopperUseCase.wasStopCalled) + XCTAssertFalse(mockStopAction.wasStopCalled) expectation.fulfill() } diff --git a/LocalPackages/DataBrokerProtection/Tests/DataBrokerProtectionTests/Mocks.swift b/LocalPackages/DataBrokerProtection/Tests/DataBrokerProtectionTests/Mocks.swift index 887fdb31fe..dfb0f55486 100644 --- a/LocalPackages/DataBrokerProtection/Tests/DataBrokerProtectionTests/Mocks.swift +++ b/LocalPackages/DataBrokerProtection/Tests/DataBrokerProtectionTests/Mocks.swift @@ -1408,8 +1408,6 @@ final class MockAuthenticationManager: DataBrokerProtectionAuthenticationManagin } final class MockAgentStopper: DataBrokerProtectionAgentStopper { - var stopAgentWasCalled = false - func validateRunPrerequisitesAndStopAgentIfNecessary() async { } @@ -1417,17 +1415,9 @@ final class MockAgentStopper: DataBrokerProtectionAgentStopper { func monitorEntitlementAndStopAgentIfEntitlementIsInvalid(interval: TimeInterval) { } - - func stopAgent() { - stopAgentWasCalled = true - } - - func reset() { - stopAgentWasCalled = false - } } -final class MockStopperUseCase: DataBrokerProtectionStopperUseCase { +final class MockDataProtectionStopAction: DataProtectionStopAction { var wasStopCalled = false func stopAgent() { From 29eea9196f107872edde8695721e900f6f055e69 Mon Sep 17 00:00:00 2001 From: Fernando Bunn Date: Wed, 22 May 2024 19:08:39 +0100 Subject: [PATCH 14/16] PR feedback, final properties --- .../DataBrokerProtectionAgentStopperTests.swift | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/LocalPackages/DataBrokerProtection/Tests/DataBrokerProtectionTests/DataBrokerProtectionAgentStopperTests.swift b/LocalPackages/DataBrokerProtection/Tests/DataBrokerProtectionTests/DataBrokerProtectionAgentStopperTests.swift index fb17547849..c7abe832d7 100644 --- a/LocalPackages/DataBrokerProtection/Tests/DataBrokerProtectionTests/DataBrokerProtectionAgentStopperTests.swift +++ b/LocalPackages/DataBrokerProtection/Tests/DataBrokerProtectionTests/DataBrokerProtectionAgentStopperTests.swift @@ -23,11 +23,11 @@ import Common @testable import DataBrokerProtection final class DataBrokerProtectionAgentStopperTests: XCTestCase { - var mockPixelHandler: EventMapping! - var mockAuthenticationManager: MockAuthenticationManager! - var mockEntitlementMonitor: DataBrokerProtectionEntitlementMonitor! - var mockDataManager: MockDataBrokerProtectionDataManager! - var mockStopAction: MockDataProtectionStopAction! + private var mockPixelHandler: EventMapping! + private var mockAuthenticationManager: MockAuthenticationManager! + private var mockEntitlementMonitor: DataBrokerProtectionEntitlementMonitor! + private var mockDataManager: MockDataBrokerProtectionDataManager! + private var mockStopAction: MockDataProtectionStopAction! private var fakeProfile: DataBrokerProtectionProfile { let name = DataBrokerProtectionProfile.Name(firstName: "John", lastName: "Doe") From 3d67be5f0606f1bd670f7d0da12389445834905a Mon Sep 17 00:00:00 2001 From: Fernando Bunn Date: Wed, 22 May 2024 19:25:38 +0100 Subject: [PATCH 15/16] Add completion handler for agentFinishedLaunching --- .../DataBrokerProtectionAgentManager.swift | 3 +- ...ataBrokerProtectionAgentManagerTests.swift | 36 +++++++++++++++++++ ...ataBrokerProtectionAgentStopperTests.swift | 16 ++++----- 3 files changed, 46 insertions(+), 9 deletions(-) diff --git a/LocalPackages/DataBrokerProtection/Sources/DataBrokerProtection/Scheduler/DataBrokerProtectionAgentManager.swift b/LocalPackages/DataBrokerProtection/Sources/DataBrokerProtection/Scheduler/DataBrokerProtectionAgentManager.swift index aada01d0c3..f4e7cdd4e1 100644 --- a/LocalPackages/DataBrokerProtection/Sources/DataBrokerProtection/Scheduler/DataBrokerProtectionAgentManager.swift +++ b/LocalPackages/DataBrokerProtection/Sources/DataBrokerProtection/Scheduler/DataBrokerProtectionAgentManager.swift @@ -136,7 +136,7 @@ public final class DataBrokerProtectionAgentManager { self.ipcServer.activate() } - public func agentFinishedLaunching() { + public func agentFinishedLaunching(completion: (() -> Void)? = nil) { Task { @MainActor in // The browser shouldn't start the agent if these prerequisites aren't met. @@ -151,6 +151,7 @@ public final class DataBrokerProtectionAgentManager { /// Monitors entitlement changes every 60 minutes to optimize system performance and resource utilization by avoiding unnecessary operations when entitlement is invalid. /// While keeping the agent active with invalid entitlement has no significant risk, setting the monitoring interval at 60 minutes is a good balance to minimize backend checks. agentStopper.monitorEntitlementAndStopAgentIfEntitlementIsInvalid(interval: .minutes(60)) + completion?() } } } diff --git a/LocalPackages/DataBrokerProtection/Tests/DataBrokerProtectionTests/DataBrokerProtectionAgentManagerTests.swift b/LocalPackages/DataBrokerProtection/Tests/DataBrokerProtectionTests/DataBrokerProtectionAgentManagerTests.swift index e45a767d9d..0c56ab7a79 100644 --- a/LocalPackages/DataBrokerProtection/Tests/DataBrokerProtectionTests/DataBrokerProtectionAgentManagerTests.swift +++ b/LocalPackages/DataBrokerProtection/Tests/DataBrokerProtectionTests/DataBrokerProtectionAgentManagerTests.swift @@ -105,6 +105,42 @@ final class DataBrokerProtectionAgentManagerTests: XCTestCase { XCTAssertTrue(startScheduledScansCalled) } + func testWhenAgentStart_andProfileDoesNotExist_thenActivityIsNotScheduled_andSheduledOpereationsNotRun() async throws { + // Given + sut = DataBrokerProtectionAgentManager( + userNotificationService: mockNotificationService, + activityScheduler: mockActivityScheduler, + ipcServer: mockIPCServer, + queueManager: mockQueueManager, + dataManager: mockDataManager, + operationDependencies: mockDependencies, + pixelHandler: mockPixelHandler, + agentStopper: mockAgentStopper) + + mockDataManager.profileToReturn = nil + + var schedulerStarted = false + mockActivityScheduler.startSchedulerCompletion = { + schedulerStarted = true + } + + var startScheduledScansCalled = false + mockQueueManager.startScheduledOperationsIfPermittedCalledCompletion = { _ in + startScheduledScansCalled = true + } + + let agentFinishedLaunchingExpectation = XCTestExpectation(description: "Finished expectation") + + // When + sut.agentFinishedLaunching(completion: { + agentFinishedLaunchingExpectation.fulfill() + }) + + // Then + XCTAssertFalse(schedulerStarted) + XCTAssertFalse(startScheduledScansCalled) + } + func testWhenActivitySchedulerTriggers_thenSheduledOpereationsRun() async throws { // Given sut = DataBrokerProtectionAgentManager( diff --git a/LocalPackages/DataBrokerProtection/Tests/DataBrokerProtectionTests/DataBrokerProtectionAgentStopperTests.swift b/LocalPackages/DataBrokerProtection/Tests/DataBrokerProtectionTests/DataBrokerProtectionAgentStopperTests.swift index c7abe832d7..5d8c25ebee 100644 --- a/LocalPackages/DataBrokerProtection/Tests/DataBrokerProtectionTests/DataBrokerProtectionAgentStopperTests.swift +++ b/LocalPackages/DataBrokerProtection/Tests/DataBrokerProtectionTests/DataBrokerProtectionAgentStopperTests.swift @@ -64,7 +64,7 @@ final class DataBrokerProtectionAgentStopperTests: XCTestCase { entitlementMonitor: mockEntitlementMonitor, authenticationManager: mockAuthenticationManager, pixelHandler: mockPixelHandler, - stopperUseCase: mockStopAction) + stopAction: mockStopAction) await stopper.validateRunPrerequisitesAndStopAgentIfNecessary() XCTAssertTrue(mockStopAction.wasStopCalled) @@ -79,7 +79,7 @@ final class DataBrokerProtectionAgentStopperTests: XCTestCase { entitlementMonitor: mockEntitlementMonitor, authenticationManager: mockAuthenticationManager, pixelHandler: mockPixelHandler, - stopperUseCase: mockStopAction) + stopAction: mockStopAction) await stopper.validateRunPrerequisitesAndStopAgentIfNecessary() XCTAssertTrue(mockStopAction.wasStopCalled) @@ -94,7 +94,7 @@ final class DataBrokerProtectionAgentStopperTests: XCTestCase { entitlementMonitor: mockEntitlementMonitor, authenticationManager: mockAuthenticationManager, pixelHandler: mockPixelHandler, - stopperUseCase: mockStopAction) + stopAction: mockStopAction) await stopper.validateRunPrerequisitesAndStopAgentIfNecessary() XCTAssertTrue(mockStopAction.wasStopCalled) @@ -109,7 +109,7 @@ final class DataBrokerProtectionAgentStopperTests: XCTestCase { entitlementMonitor: mockEntitlementMonitor, authenticationManager: mockAuthenticationManager, pixelHandler: mockPixelHandler, - stopperUseCase: mockStopAction) + stopAction: mockStopAction) await stopper.validateRunPrerequisitesAndStopAgentIfNecessary() XCTAssertFalse(mockStopAction.wasStopCalled) @@ -124,7 +124,7 @@ final class DataBrokerProtectionAgentStopperTests: XCTestCase { entitlementMonitor: mockEntitlementMonitor, authenticationManager: mockAuthenticationManager, pixelHandler: mockPixelHandler, - stopperUseCase: mockStopAction) + stopAction: mockStopAction) await stopper.validateRunPrerequisitesAndStopAgentIfNecessary() XCTAssertFalse(mockStopAction.wasStopCalled) @@ -139,7 +139,7 @@ final class DataBrokerProtectionAgentStopperTests: XCTestCase { entitlementMonitor: mockEntitlementMonitor, authenticationManager: mockAuthenticationManager, pixelHandler: mockPixelHandler, - stopperUseCase: mockStopAction) + stopAction: mockStopAction) let expectation = XCTestExpectation(description: "Wait for monitor") stopper.monitorEntitlementAndStopAgentIfEntitlementIsInvalid(interval: 0.1) @@ -161,7 +161,7 @@ final class DataBrokerProtectionAgentStopperTests: XCTestCase { entitlementMonitor: mockEntitlementMonitor, authenticationManager: mockAuthenticationManager, pixelHandler: mockPixelHandler, - stopperUseCase: mockStopAction) + stopAction: mockStopAction) let expectation = XCTestExpectation(description: "Wait for monitor") stopper.monitorEntitlementAndStopAgentIfEntitlementIsInvalid(interval: 0.1) @@ -183,7 +183,7 @@ final class DataBrokerProtectionAgentStopperTests: XCTestCase { entitlementMonitor: mockEntitlementMonitor, authenticationManager: mockAuthenticationManager, pixelHandler: mockPixelHandler, - stopperUseCase: mockStopAction) + stopAction: mockStopAction) let expectation = XCTestExpectation(description: "Wait for monitor") stopper.monitorEntitlementAndStopAgentIfEntitlementIsInvalid(interval: 0.1) From 42264a41504969a78f3a29913cb5aefd5904ee4a Mon Sep 17 00:00:00 2001 From: Fernando Bunn Date: Thu, 23 May 2024 11:04:01 +0100 Subject: [PATCH 16/16] Improve tests --- .../DataBrokerProtectionAgentManager.swift | 3 +- ...ataBrokerProtectionAgentManagerTests.swift | 67 +++++++++++++++---- .../DataBrokerProtectionTests/Mocks.swift | 9 ++- 3 files changed, 61 insertions(+), 18 deletions(-) diff --git a/LocalPackages/DataBrokerProtection/Sources/DataBrokerProtection/Scheduler/DataBrokerProtectionAgentManager.swift b/LocalPackages/DataBrokerProtection/Sources/DataBrokerProtection/Scheduler/DataBrokerProtectionAgentManager.swift index f4e7cdd4e1..aada01d0c3 100644 --- a/LocalPackages/DataBrokerProtection/Sources/DataBrokerProtection/Scheduler/DataBrokerProtectionAgentManager.swift +++ b/LocalPackages/DataBrokerProtection/Sources/DataBrokerProtection/Scheduler/DataBrokerProtectionAgentManager.swift @@ -136,7 +136,7 @@ public final class DataBrokerProtectionAgentManager { self.ipcServer.activate() } - public func agentFinishedLaunching(completion: (() -> Void)? = nil) { + public func agentFinishedLaunching() { Task { @MainActor in // The browser shouldn't start the agent if these prerequisites aren't met. @@ -151,7 +151,6 @@ public final class DataBrokerProtectionAgentManager { /// Monitors entitlement changes every 60 minutes to optimize system performance and resource utilization by avoiding unnecessary operations when entitlement is invalid. /// While keeping the agent active with invalid entitlement has no significant risk, setting the monitoring interval at 60 minutes is a good balance to minimize backend checks. agentStopper.monitorEntitlementAndStopAgentIfEntitlementIsInvalid(interval: .minutes(60)) - completion?() } } } diff --git a/LocalPackages/DataBrokerProtection/Tests/DataBrokerProtectionTests/DataBrokerProtectionAgentManagerTests.swift b/LocalPackages/DataBrokerProtection/Tests/DataBrokerProtectionTests/DataBrokerProtectionAgentManagerTests.swift index 0c56ab7a79..185e21f2f8 100644 --- a/LocalPackages/DataBrokerProtection/Tests/DataBrokerProtectionTests/DataBrokerProtectionAgentManagerTests.swift +++ b/LocalPackages/DataBrokerProtection/Tests/DataBrokerProtectionTests/DataBrokerProtectionAgentManagerTests.swift @@ -105,8 +105,14 @@ final class DataBrokerProtectionAgentManagerTests: XCTestCase { XCTAssertTrue(startScheduledScansCalled) } - func testWhenAgentStart_andProfileDoesNotExist_thenActivityIsNotScheduled_andSheduledOpereationsNotRun() async throws { + func testWhenAgentStart_andProfileDoesNotExist_thenActivityIsNotScheduled_andStopAgentIsCalled() async throws { // Given + let mockStopAction = MockDataProtectionStopAction() + let agentStopper = DefaultDataBrokerProtectionAgentStopper(dataManager: mockDataManager, + entitlementMonitor: DataBrokerProtectionEntitlementMonitor(), + authenticationManager: MockAuthenticationManager(), + pixelHandler: mockPixelHandler, + stopAction: mockStopAction) sut = DataBrokerProtectionAgentManager( userNotificationService: mockNotificationService, activityScheduler: mockActivityScheduler, @@ -115,30 +121,63 @@ final class DataBrokerProtectionAgentManagerTests: XCTestCase { dataManager: mockDataManager, operationDependencies: mockDependencies, pixelHandler: mockPixelHandler, - agentStopper: mockAgentStopper) + agentStopper: agentStopper) mockDataManager.profileToReturn = nil - var schedulerStarted = false - mockActivityScheduler.startSchedulerCompletion = { - schedulerStarted = true + let stopAgentExpectation = XCTestExpectation(description: "Stop agent expectation") + + var stopAgentWasCalled = false + mockStopAction.stopAgentCompletion = { + stopAgentWasCalled = true + stopAgentExpectation.fulfill() } - var startScheduledScansCalled = false - mockQueueManager.startScheduledOperationsIfPermittedCalledCompletion = { _ in - startScheduledScansCalled = true + // When + sut.agentFinishedLaunching() + await fulfillment(of: [stopAgentExpectation], timeout: 1.0) + + // Then + XCTAssertTrue(stopAgentWasCalled) + } + + func testWhenAgentStart_thenPrerequisitesAreValidated_andEntitlementsAreMonitored() async { + // Given + let mockAgentStopper = MockAgentStopper() + + sut = DataBrokerProtectionAgentManager( + userNotificationService: mockNotificationService, + activityScheduler: mockActivityScheduler, + ipcServer: mockIPCServer, + queueManager: mockQueueManager, + dataManager: mockDataManager, + operationDependencies: mockDependencies, + pixelHandler: mockPixelHandler, + agentStopper: mockAgentStopper) + + mockDataManager.profileToReturn = nil + + let preRequisitesExpectation = XCTestExpectation(description: "preRequisitesExpectation expectation") + var runPrerequisitesWasCalled = false + mockAgentStopper.validateRunPrerequisitesCompletion = { + runPrerequisitesWasCalled = true + preRequisitesExpectation.fulfill() } - let agentFinishedLaunchingExpectation = XCTestExpectation(description: "Finished expectation") + let monitorEntitlementExpectation = XCTestExpectation(description: "monitorEntitlement expectation") + var monitorEntitlementWasCalled = false + mockAgentStopper.monitorEntitlementCompletion = { + monitorEntitlementWasCalled = true + monitorEntitlementExpectation.fulfill() + } // When - sut.agentFinishedLaunching(completion: { - agentFinishedLaunchingExpectation.fulfill() - }) + sut.agentFinishedLaunching() + await fulfillment(of: [preRequisitesExpectation, monitorEntitlementExpectation], timeout: 1.0) // Then - XCTAssertFalse(schedulerStarted) - XCTAssertFalse(startScheduledScansCalled) + XCTAssertTrue(runPrerequisitesWasCalled) + XCTAssertTrue(monitorEntitlementWasCalled) } func testWhenActivitySchedulerTriggers_thenSheduledOpereationsRun() async throws { diff --git a/LocalPackages/DataBrokerProtection/Tests/DataBrokerProtectionTests/Mocks.swift b/LocalPackages/DataBrokerProtection/Tests/DataBrokerProtectionTests/Mocks.swift index dfb0f55486..d6bb74149d 100644 --- a/LocalPackages/DataBrokerProtection/Tests/DataBrokerProtectionTests/Mocks.swift +++ b/LocalPackages/DataBrokerProtection/Tests/DataBrokerProtectionTests/Mocks.swift @@ -1408,20 +1408,25 @@ final class MockAuthenticationManager: DataBrokerProtectionAuthenticationManagin } final class MockAgentStopper: DataBrokerProtectionAgentStopper { - func validateRunPrerequisitesAndStopAgentIfNecessary() async { + var validateRunPrerequisitesCompletion: (() -> Void)? + var monitorEntitlementCompletion: (() -> Void)? + func validateRunPrerequisitesAndStopAgentIfNecessary() async { + validateRunPrerequisitesCompletion?() } func monitorEntitlementAndStopAgentIfEntitlementIsInvalid(interval: TimeInterval) { - + monitorEntitlementCompletion?() } } final class MockDataProtectionStopAction: DataProtectionStopAction { var wasStopCalled = false + var stopAgentCompletion: (() -> Void)? func stopAgent() { wasStopCalled = true + stopAgentCompletion?() } func reset() {