Ship review improvements

DuckDuckGo/MainWindow/MainViewController.swift

@@ -71,8 +71,8 @@ final class MainViewController: NSViewController {
                 return NetPPopoverManagerMock()
             }
 #endif
-            let vpnBundleID = Bundle.main.vpnMenuAgentBundleId
-            let ipcClient = TunnelControllerIPCClient(machServiceName: vpnBundleID)
+
+            let ipcClient = TunnelControllerIPCClient()
             ipcClient.register()
 
             return NetworkProtectionNavBarPopoverManager(ipcClient: ipcClient, networkProtectionFeatureDisabler: NetworkProtectionFeatureDisabler())

DuckDuckGo/NetworkProtection/AppTargets/BothAppTargets/NetworkProtection+ConvenienceInitializers.swift

@@ -20,6 +20,7 @@
 
 import Foundation
 import NetworkProtection
+import NetworkProtectionIPC
 import Common
 
 #if SUBSCRIPTION
@@ -53,7 +54,7 @@ extension NetworkProtectionCodeRedemptionCoordinator {
 
 extension NetworkProtectionKeychainTokenStore {
     convenience init() {
-        self.init(isSubscriptionEnabled: NSApp.delegateTyped.subscriptionFeatureAvailability.isFeatureAvailable)
+        self.init(isSubscriptionEnabled: DefaultSubscriptionFeatureAvailability().isFeatureAvailable)
     }
 
     convenience init(isSubscriptionEnabled: Bool) {
@@ -83,9 +84,16 @@ extension NetworkProtectionLocationListCompositeRepository {
             environment: settings.selectedEnvironment,
             tokenStore: NetworkProtectionKeychainTokenStore(),
             errorEvents: .networkProtectionAppDebugEvents,
-            isSubscriptionEnabled: NSApp.delegateTyped.subscriptionFeatureAvailability.isFeatureAvailable
+            isSubscriptionEnabled: DefaultSubscriptionFeatureAvailability().isFeatureAvailable
         )
     }
 }
 
+extension TunnelControllerIPCClient {
+
+    convenience init() {
+        self.init(machServiceName: Bundle.main.vpnMenuAgentBundleId)
+    }
+}
+
 #endif

DuckDuckGo/NetworkProtection/AppTargets/BothAppTargets/NetworkProtectionAppEvents.swift

@@ -24,6 +24,7 @@ import NetworkProtection
 import NetworkProtectionUI
 import NetworkProtectionIPC
 import NetworkExtension
+import Subscription
 
 /// Implements the sequence of steps that the VPN needs to execute when the App starts up.
 ///
@@ -81,6 +82,10 @@ final class NetworkProtectionAppEvents {
     func applicationDidBecomeActive() {
         Task { @MainActor in
             await featureVisibility.disableIfUserHasNoAccess()
+
+#if SUBSCRIPTION
+            _ = await AccountManager().hasEntitlement(for: .networkProtection, cachePolicy: .reloadIgnoringLocalCacheData)
+#endif
         }
     }
 

DuckDuckGo/NetworkProtection/AppTargets/BothAppTargets/NetworkProtectionDebugUtilities.swift

@@ -48,7 +48,7 @@ final class NetworkProtectionDebugUtilities {
         self.loginItemsManager = loginItemsManager
         self.settings = settings
 
-        let ipcClient = TunnelControllerIPCClient(machServiceName: Bundle.main.vpnMenuAgentBundleId)
+        let ipcClient = TunnelControllerIPCClient()
 
         self.ipcClient = ipcClient
         self.networkProtectionFeatureDisabler = NetworkProtectionFeatureDisabler(ipcClient: ipcClient)

DuckDuckGo/NetworkProtection/AppTargets/DeveloperIDTarget/NetworkProtectionIPCTunnelController.swift

@@ -18,34 +18,52 @@
 
 #if NETWORK_PROTECTION
 
+import Common
 import Foundation
 import NetworkProtection
 import NetworkProtectionIPC
 
 final class NetworkProtectionIPCTunnelController: TunnelController {
 
+    private let featureVisibility: NetworkProtectionFeatureVisibility
     private let loginItemsManager: LoginItemsManager
     private let ipcClient: NetworkProtectionIPCClient
 
-    init(loginItemsManager: LoginItemsManager = LoginItemsManager(),
+    init(featureVisibility: NetworkProtectionFeatureVisibility = DefaultNetworkProtectionVisibility(),
+         loginItemsManager: LoginItemsManager = LoginItemsManager(),
          ipcClient: NetworkProtectionIPCClient) {
 
+        self.featureVisibility = featureVisibility
         self.loginItemsManager = loginItemsManager
         self.ipcClient = ipcClient
     }
 
     @MainActor
     func start() async {
-        enableLoginItems()
+        do {
+            guard try await enableLoginItems() else {
+                os_log("🔴 IPC Controller refusing to start the VPN menu app.  Not authorized.", log: .networkProtection)
+                return
+            }
 
-        ipcClient.start()
+            ipcClient.start()
+        } catch {
+            os_log("🔴 IPC Controller found en error when starting the VPN: \(error)", log: .networkProtection)
+        }
     }
 
     @MainActor
     func stop() async {
-        enableLoginItems()
+        do {
+            guard try await enableLoginItems() else {
+                os_log("🔴 IPC Controller refusing to start the VPN.  Not authorized.", log: .networkProtection)
+                return
+            }
 
-        ipcClient.stop()
+            ipcClient.stop()
+        } catch {
+            os_log("🔴 IPC Controller found en error when starting the VPN: \(error)", log: .networkProtection)
+        }
     }
 
     /// Queries VPN to know if it's connected.
@@ -64,8 +82,14 @@ final class NetworkProtectionIPCTunnelController: TunnelController {
 
     // MARK: - Login Items Manager
 
-    private func enableLoginItems() {
+    private func enableLoginItems() async throws -> Bool {
+        guard try await featureVisibility.isFeatureEnabled() else {
+            // We shouldn't enable the menu app is the VPN feature is disabled.
+            return false
+        }
+
         loginItemsManager.enableLoginItems(LoginItemsManager.networkProtectionLoginItems, log: .networkProtection)
+        return true
     }
 }
 

DuckDuckGo/NetworkProtection/AppTargets/DeveloperIDTarget/NetworkProtectionSubscriptionEventHandler.swift

@@ -18,6 +18,7 @@
 
 #if NETWORK_PROTECTION && SUBSCRIPTION
 
+import Combine
 import Foundation
 import Subscription
 import NetworkProtection
@@ -30,6 +31,7 @@
     private let networkProtectionTokenStorage: NetworkProtectionTokenStore
     private let networkProtectionFeatureDisabler: NetworkProtectionFeatureDisabling
     private let userDefaults: UserDefaults
+    private var cancellables = Set<AnyCancellable>()
 
     init(accountManager: AccountManaging = AccountManager(),
          networkProtectionRedemptionCoordinator: NetworkProtectionCodeRedeeming = NetworkProtectionCodeRedemptionCoordinator(),
@@ -41,34 +43,57 @@
         self.networkProtectionTokenStorage = networkProtectionTokenStorage
         self.networkProtectionFeatureDisabler = networkProtectionFeatureDisabler
         self.userDefaults = userDefaults
+
+        subscribeToEntitlementChanges()
     }
 
-    private lazy var entitlementMonitor = NetworkProtectionEntitlementMonitor()
+    private func subscribeToEntitlementChanges() {
+        Task {
+            switch await AccountManager().hasEntitlement(for: .networkProtection) {
+            case .success(let hasEntitlements):
+                handleEntitlementsChange(hasEntitlements: hasEntitlements)
+            case .failure(let error):
+                break
+            }
+
+            NotificationCenter.default
+                .publisher(for: .entitlementsDidChange)
+                .receive(on: DispatchQueue.main)
+                .sink { [weak self] notification in
+                    guard let self else {
+                        return
+                    }
 
-    private func setUpEntitlementMonitoring() {
-        guard AccountManager().isUserAuthenticated else { return }
-        let entitlementsCheck = {
-            await AccountManager().hasEntitlement(for: .networkProtection, cachePolicy: .reloadIgnoringLocalCacheData)
-        }
+                    guard let entitlements = notification.userInfo?[UserDefaultsCacheKey.subscriptionEntitlements] as? [Entitlement] else {
 
-        Task {
-            await entitlementMonitor.start(entitlementCheck: entitlementsCheck) { result in
-                switch result {
-                case .validEntitlement:
-                    UserDefaults.netP.networkProtectionEntitlementsExpired = false
-                case .invalidEntitlement:
-                    UserDefaults.netP.networkProtectionEntitlementsExpired = true
-                case .error:
-                    break
+                        assertionFailure("Missing entitlements are truly unexpected")
+                        return
+                    }
+
+                    let hasEntitlements = entitlements.contains { entitlement in
+                        entitlement.product == .networkProtection
+                    }
+
+                    handleEntitlementsChange(hasEntitlements: hasEntitlements)
                 }
+                .store(in: &cancellables)
+        }
+    }
+
+    private func handleEntitlementsChange(hasEntitlements: Bool) {
+        if hasEntitlements {
+            UserDefaults.netP.networkProtectionEntitlementsExpired = false
+        } else {
+            Task {
+                await self.networkProtectionFeatureDisabler.disable(keepAuthToken: false, uninstallSystemExtension: false)
+                UserDefaults.netP.networkProtectionEntitlementsExpired = true
             }
         }
     }
 
     func registerForSubscriptionAccountManagerEvents() {
         NotificationCenter.default.addObserver(self, selector: #selector(handleAccountDidSignIn), name: .accountDidSignIn, object: nil)
         NotificationCenter.default.addObserver(self, selector: #selector(handleAccountDidSignOut), name: .accountDidSignOut, object: nil)
-        setUpEntitlementMonitoring()
     }
 
     @objc private func handleAccountDidSignIn() {
@@ -77,12 +102,10 @@
             return
         }
         userDefaults.networkProtectionEntitlementsExpired = false
-        setUpEntitlementMonitoring()
     }
 
     @objc private func handleAccountDidSignOut() {
         print("[NetP Subscription] Deleted NetP auth token after signing out from Privacy Pro")
-        userDefaults.networkProtectionEntitlementsExpired = true
 
         Task {
             await networkProtectionFeatureDisabler.disable(keepAuthToken: false, uninstallSystemExtension: false)

DuckDuckGo/Preferences/Model/PreferencesSidebarModel.swift

@@ -83,10 +83,8 @@ final class PreferencesSidebarModel: ObservableObject {
         userDefaults: UserDefaults = .netP
     ) {
         let loadSections = {
-#if SUBSCRIPTION
-            let includingVPN = !userDefaults.networkProtectionEntitlementsExpired && DefaultNetworkProtectionVisibility().isOnboarded
-#elseif NETWORK_PROTECTION
-            let includingVPN = DefaultNetworkProtectionVisibility().isOnboarded
+#if NETWORK_PROTECTION
+            let includingVPN = DefaultNetworkProtectionVisibility().isInstalled
 #else
             let includingVPN = false
 #endif

DuckDuckGo/Tab/Model/Tab.swift

@@ -540,8 +540,7 @@ protocol NewWindowPolicyDecisionMaker {
             .sink { [weak self] onboardingStatus in
                 guard onboardingStatus == .completed else { return }
 
-                let machServiceName = Bundle.main.vpnMenuAgentBundleId
-                let ipcClient = TunnelControllerIPCClient(machServiceName: machServiceName)
+                let ipcClient = TunnelControllerIPCClient()
                 ipcClient.register()
 
                 self?.tunnelController = NetworkProtectionIPCTunnelController(ipcClient: ipcClient)

DuckDuckGo/VPNFeedbackForm/VPNMetadataCollector.swift

@@ -115,8 +115,7 @@ final class DefaultVPNMetadataCollector: VPNMetadataCollector {
     private let statusReporter: NetworkProtectionStatusReporter
 
     init() {
-        let machServiceName = Bundle.main.vpnMenuAgentBundleId
-        let ipcClient = TunnelControllerIPCClient(machServiceName: machServiceName)
+        let ipcClient = TunnelControllerIPCClient()
         ipcClient.register()
 
         self.statusReporter = DefaultNetworkProtectionStatusReporter(

DuckDuckGo/Waitlist/NetworkProtectionFeatureDisabler.swift

@@ -45,7 +45,7 @@ final class NetworkProtectionFeatureDisabler: NetworkProtectionFeatureDisabling
          pinningManager: LocalPinningManager = .shared,
          userDefaults: UserDefaults = .netP,
          settings: VPNSettings = .init(defaults: .netP),
-         ipcClient: TunnelControllerIPCClient = TunnelControllerIPCClient(machServiceName: Bundle.main.vpnMenuAgentBundleId),
+         ipcClient: TunnelControllerIPCClient = TunnelControllerIPCClient(),
          log: OSLog = .networkProtection) {
 
         self.log = log
@@ -123,6 +123,7 @@ final class NetworkProtectionFeatureDisabler: NetworkProtectionFeatureDisabling
 
     private func resetUserDefaults() {
         settings.resetToDefaults()
+        userDefaults.networkProtectionOnboardingStatus = .default
     }
 
     private func notifyVPNUninstalled() {

DuckDuckGo/Waitlist/NetworkProtectionFeatureVisibility.swift

@@ -34,6 +34,7 @@ import Subscription
 protocol NetworkProtectionFeatureVisibility {
     var isEligibleForThankYouMessage: Bool { get }
 
+    func isFeatureEnabled() async throws -> Bool
     func isNetworkProtectionVisible() -> Bool
     func shouldUninstallAutomatically() -> Bool
     func disableForAllUsers() async
@@ -80,6 +81,25 @@ struct DefaultNetworkProtectionVisibility: NetworkProtectionFeatureVisibility {
         return isEasterEggUser || waitlistIsOngoing
     }
 
+    var isInstalled: Bool {
+        LoginItem.vpnMenu.status.isInstalled && isOnboarded
+    }
+
+    /// Replaces `isNetworkProtectionVisible` to add subscriptions support
+    ///
+    func isFeatureEnabled() async throws -> Bool {
+        guard subscriptionFeatureAvailability.isFeatureAvailable else {
+            return isNetworkProtectionVisible()
+        }
+
+        switch await AccountManager().hasEntitlement(for: .networkProtection) {
+        case .success(let hasEntitlement):
+            return hasEntitlement
+        case .failure(let error):
+            throw error
+        }
+    }
+
     /// We've had to add this method because accessing the singleton in app delegate is crashing the integration tests.
     ///
     var subscriptionFeatureAvailability: DefaultSubscriptionFeatureAvailability {

DuckDuckGoVPN/NetworkProtectionBouncer.swift

@@ -41,7 +41,9 @@ final class NetworkProtectionBouncer {
         case .success(true):
             return
         case .failure:
-            break
+            guard accountManager.accessToken == nil else {
+                return
+            }
         case .success(false):
             os_log(.error, log: .networkProtection, "🔴 Stopping: DuckDuckGo VPN not authorized. Missing entitlement.")
             await controller.stop()

UnitTests/Menus/MoreOptionsMenuTests.swift

@@ -190,6 +190,10 @@ final class NetworkProtectionVisibilityMock: NetworkProtectionFeatureVisibility
         return visible
     }
 
+    func isFeatureEnabled() async throws -> Bool {
+        return false
+    }
+
     func disableForAllUsers() async {
         // intentional no-op
     }