VPN visibility fixes (#2507)

Task/Issue URL: https://app.asana.com/0/0/1206886385075835/f

Description

Changes:

VPN shortcut is still available when the VPN is visible.
VPN settings now visible when the VPN is installed.
VPN can be uninstalled when the login item is enabled.
VPN has a flag to prevent multiple uninstallations of the VPN.

Configuration/Tests/UnitTestsAppStore.xcconfig

@@ -16,7 +16,7 @@
 #include "UnitTests.xcconfig"
 #include "../AppStore.xcconfig"
 
-FEATURE_FLAGS = FEEDBACK NETWORK_PROTECTION DBP
+FEATURE_FLAGS = FEEDBACK NETWORK_PROTECTION DBP SUBSCRIPTION
 
 PRODUCT_BUNDLE_IDENTIFIER = com.duckduckgo.mobile.ios.DuckDuckGoTests
 

DuckDuckGo/Menus/MainMenu.swift

@@ -544,18 +544,20 @@ import SubscriptionUI
     }
 
     private func updateShortcutMenuItems() {
-        toggleAutofillShortcutMenuItem.title = LocalPinningManager.shared.shortcutTitle(for: .autofill)
-        toggleBookmarksShortcutMenuItem.title = LocalPinningManager.shared.shortcutTitle(for: .bookmarks)
-        toggleDownloadsShortcutMenuItem.title = LocalPinningManager.shared.shortcutTitle(for: .downloads)
+        Task { @MainActor in
+            toggleAutofillShortcutMenuItem.title = LocalPinningManager.shared.shortcutTitle(for: .autofill)
+            toggleBookmarksShortcutMenuItem.title = LocalPinningManager.shared.shortcutTitle(for: .bookmarks)
+            toggleDownloadsShortcutMenuItem.title = LocalPinningManager.shared.shortcutTitle(for: .downloads)
 
 #if NETWORK_PROTECTION
-        if NetworkProtectionKeychainTokenStore().isFeatureActivated {
-            toggleNetworkProtectionShortcutMenuItem.isHidden = false
-            toggleNetworkProtectionShortcutMenuItem.title = LocalPinningManager.shared.shortcutTitle(for: .networkProtection)
-        } else {
-            toggleNetworkProtectionShortcutMenuItem.isHidden = true
-        }
+            if await DefaultNetworkProtectionVisibility().isVPNVisible() {
+                toggleNetworkProtectionShortcutMenuItem.isHidden = false
+                toggleNetworkProtectionShortcutMenuItem.title = LocalPinningManager.shared.shortcutTitle(for: .networkProtection)
+            } else {
+                toggleNetworkProtectionShortcutMenuItem.isHidden = true
+            }
 #endif
+        }
     }
 
     // MARK: - Debug

DuckDuckGo/NavigationBar/View/MoreOptionsMenu.swift

@@ -341,7 +341,7 @@ final class MoreOptionsMenu: NSMenu {
 #endif
 
 #if NETWORK_PROTECTION
-        if networkProtectionFeatureVisibility.isNetworkProtectionVisible() {
+        if networkProtectionFeatureVisibility.isNetworkProtectionBetaVisible() {
             let networkProtectionItem: NSMenuItem
 
             networkProtectionItem = makeNetworkProtectionItem()

DuckDuckGo/NavigationBar/View/NavigationBarViewController.swift

@@ -320,7 +320,7 @@ final class NavigationBarViewController: NSViewController {
 
     private func toggleNetworkProtectionPopover() {
         let featureVisibility = DefaultNetworkProtectionVisibility()
-        guard featureVisibility.isNetworkProtectionVisible() else {
+        guard featureVisibility.isNetworkProtectionBetaVisible() else {
             featureVisibility.disableForWaitlistUsers()
             LocalPinningManager.shared.unpin(.networkProtection)
             return
@@ -405,7 +405,7 @@ final class NavigationBarViewController: NSViewController {
                     self.updateHomeButton()
 #if NETWORK_PROTECTION
                 case .networkProtection:
-                    networkProtectionButtonModel.updateVisibility()
+                    self.networkProtectionButtonModel.updateVisibility()
 #endif
                 }
             } else {
@@ -945,7 +945,7 @@ extension NavigationBarViewController: NSMenuDelegate {
 #if NETWORK_PROTECTION
         let isPopUpWindow = view.window?.isPopUpWindow ?? false
 
-        if !isPopUpWindow && networkProtectionFeatureActivation.isFeatureActivated {
+        if !isPopUpWindow && DefaultNetworkProtectionVisibility().isVPNVisible() {
             let networkProtectionTitle = LocalPinningManager.shared.shortcutTitle(for: .networkProtection)
             menu.addItem(withTitle: networkProtectionTitle, action: #selector(toggleNetworkProtectionPanelPinning), keyEquivalent: "N")
         }
@@ -980,7 +980,7 @@ extension NavigationBarViewController: NSMenuDelegate {
     func showNetworkProtectionStatus() {
         let featureVisibility = DefaultNetworkProtectionVisibility()
 
-        if featureVisibility.isNetworkProtectionVisible() {
+        if featureVisibility.isNetworkProtectionBetaVisible() {
             popovers.showNetworkProtectionPopover(positionedBelow: networkProtectionButton,
                                                   withDelegate: networkProtectionButtonModel)
         } else {

DuckDuckGo/NetworkProtection/AppTargets/BothAppTargets/NetworkProtectionNavBarButtonModel.swift

@@ -178,7 +178,7 @@ final class NetworkProtectionNavBarButtonModel: NSObject, ObservableObject {
     func updateVisibility() {
         // The button is visible in the case where NetP has not been activated, but the user has been invited and they haven't accepted T&Cs.
         let networkProtectionVisibility = DefaultNetworkProtectionVisibility()
-        if networkProtectionVisibility.isNetworkProtectionVisible() {
+        if networkProtectionVisibility.isNetworkProtectionBetaVisible() {
             if NetworkProtectionWaitlist().readyToAcceptTermsAndConditions {
                 DailyPixel.fire(pixel: .networkProtectionWaitlistEntryPointToolbarButtonDisplayed,
                                 frequency: .dailyOnly,

DuckDuckGo/NetworkProtection/AppTargets/BothAppTargets/NetworkProtectionNavBarPopoverManager.swift

@@ -144,7 +144,7 @@ final class NetworkProtectionNavBarPopoverManager: NetPPopoverManager {
         } else {
             let featureVisibility = DefaultNetworkProtectionVisibility()
 
-            if featureVisibility.isNetworkProtectionVisible() {
+            if featureVisibility.isNetworkProtectionBetaVisible() {
                 show(positionedBelow: view, withDelegate: delegate)
             } else {
                 featureVisibility.disableForWaitlistUsers()

DuckDuckGo/NetworkProtection/AppTargets/DeveloperIDTarget/NetworkProtectionIPCTunnelController.swift

@@ -83,7 +83,7 @@ final class NetworkProtectionIPCTunnelController: TunnelController {
     // MARK: - Login Items Manager
 
     private func enableLoginItems() async throws -> Bool {
-        guard try await featureVisibility.isFeatureEnabled() else {
+        guard try await featureVisibility.canStartVPN() else {
             // We shouldn't enable the menu app is the VPN feature is disabled.
             return false
         }

DuckDuckGo/NetworkProtection/AppTargets/DeveloperIDTarget/NetworkProtectionRemoteMessaging/NetworkProtectionRemoteMessaging.swift

@@ -133,7 +133,7 @@ final class DefaultNetworkProtectionRemoteMessaging: NetworkProtectionRemoteMess
             }
 
             // Next, check if the message requires access to NetP but it's not visible:
-            if message.requiresNetworkProtectionAccess, !networkProtectionVisibility.isNetworkProtectionVisible() {
+            if message.requiresNetworkProtectionAccess, !networkProtectionVisibility.isNetworkProtectionBetaVisible() {
                 return false
             }
 

DuckDuckGo/Preferences/Model/PreferencesSidebarModel.swift

@@ -108,26 +108,12 @@ final class PreferencesSidebarModel: ObservableObject {
     private func setupVPNPaneVisibility() {
         DefaultNetworkProtectionVisibility().onboardStatusPublisher
             .receive(on: DispatchQueue.main)
-            .sink { [weak self] onboardingStatus in
+            .sink { [weak self] _ in
                 guard let self else { return }
 
-                if onboardingStatus != .completed && self.selectedPane == .vpn {
-                    self.selectedPane = .general
-                }
-
                 self.refreshSections()
             }
             .store(in: &cancellables)
-
-        UserDefaults.netP.publisher(for: \.networkProtectionEntitlementsExpired)
-            .receive(on: DispatchQueue.main)
-            .sink { [weak self] entitlementsExpired in
-                guard let self else { return }
-                if !entitlementsExpired && self.selectedPane == .vpn {
-                    self.selectedPane = .general
-                }
-                self.refreshSections()
-        }.store(in: &cancellables)
     }
 #endif
 

DuckDuckGo/Preferences/Model/VPNPreferencesModel.swift

@@ -61,7 +61,7 @@ final class VPNPreferencesModel: ObservableObject {
 
     private var onboardingStatus: OnboardingStatus {
         didSet {
-            showUninstallVPN = onboardingStatus != .default
+            showUninstallVPN = DefaultNetworkProtectionVisibility().isInstalled
         }
     }
 

DuckDuckGo/Waitlist/NetworkProtectionFeatureDisabler.swift

@@ -62,24 +62,9 @@ final class NetworkProtectionFeatureDisabler: NetworkProtectionFeatureDisabling
         self.ipcClient = ipcClient
     }
 
-    private var isSystemExtensionInstalled: Bool {
-#if NETP_SYSTEM_EXTENSION
-        userDefaults.networkProtectionOnboardingStatus != .isOnboarding(step: .userNeedsToAllowExtension)
-#else
-        return false
-#endif
-    }
-
-    private var isVPNConfigurationInstalled: Bool {
-        userDefaults.networkProtectionOnboardingStatus == .completed
-    }
-
     @MainActor
     private func canUninstall(includingSystemExtension: Bool) -> Bool {
-        !isDisabling
-        && LoginItem.vpnMenu.status.isInstalled
-        && ((includingSystemExtension && isSystemExtensionInstalled)
-            || isVPNConfigurationInstalled)
+        !isDisabling && LoginItem.vpnMenu.status.isInstalled
     }
 
     /// This method disables the VPN and clear all of its state.
@@ -93,17 +78,17 @@ final class NetworkProtectionFeatureDisabler: NetworkProtectionFeatureDisabling
     func disable(keepAuthToken: Bool, uninstallSystemExtension: Bool) async -> Bool {
         // To disable NetP we need the login item to be running
         // This should be fine though as we'll disable them further down below
+        guard canUninstall(includingSystemExtension: uninstallSystemExtension) else {
+            return true
+        }
+
+        isDisabling = true
 
         defer {
             unpinNetworkProtection()
             resetUserDefaults(uninstallSystemExtension: uninstallSystemExtension)
         }
 
-        guard canUninstall(includingSystemExtension: uninstallSystemExtension) else {
-            return true
-        }
-
-        isDisabling = true
         enableLoginItems()
 
         // Allow some time for the login items to fully launch

DuckDuckGo/Waitlist/NetworkProtectionFeatureVisibility.swift

@@ -33,9 +33,11 @@ import Subscription
 
 protocol NetworkProtectionFeatureVisibility {
     var isEligibleForThankYouMessage: Bool { get }
+    var isInstalled: Bool { get }
 
-    func isFeatureEnabled() async throws -> Bool
-    func isNetworkProtectionVisible() -> Bool
+    func canStartVPN() async throws -> Bool
+    func isVPNVisible() -> Bool
+    func isNetworkProtectionBetaVisible() -> Bool
     func shouldUninstallAutomatically() -> Bool
     func disableForAllUsers() async
     func disableForWaitlistUsers()
@@ -77,19 +79,22 @@ struct DefaultNetworkProtectionVisibility: NetworkProtectionFeatureVisibility {
     /// 2. If no auth token is found, the feature is visible if the waitlist feature flag is enabled
     ///
     /// Once the waitlist beta has ended, we can trigger a remote change that removes the user's auth token and turn off the waitlist flag, hiding the VPN from the user.
-    func isNetworkProtectionVisible() -> Bool {
+    func isNetworkProtectionBetaVisible() -> Bool {
         return isEasterEggUser || waitlistIsOngoing
     }
 
     var isInstalled: Bool {
-        LoginItem.vpnMenu.status.isInstalled && isOnboarded
+        LoginItem.vpnMenu.status.isInstalled
     }
 
-    /// Replaces `isNetworkProtectionVisible` to add subscriptions support
+    /// Whether the user can start the VPN.
     ///
-    func isFeatureEnabled() async throws -> Bool {
+    /// For beta users this means they have an auth token.
+    /// For subscription users this means they have entitlements.
+    ///
+    func canStartVPN() async throws -> Bool {
         guard subscriptionFeatureAvailability.isFeatureAvailable else {
-            return isNetworkProtectionVisible()
+            return isNetworkProtectionBetaVisible()
         }
 
         switch await AccountManager().hasEntitlement(for: .networkProtection) {
@@ -100,6 +105,19 @@ struct DefaultNetworkProtectionVisibility: NetworkProtectionFeatureVisibility {
         }
     }
 
+    /// Whether the user can see the VPN entry points in the UI.
+    ///
+    /// For beta users this means they have an auth token.
+    /// For subscription users this means they are authenticated.
+    ///
+    func isVPNVisible() -> Bool {
+        guard subscriptionFeatureAvailability.isFeatureAvailable else {
+            return isNetworkProtectionBetaVisible()
+        }
+
+        return AccountManager().isUserAuthenticated
+    }
+
     /// 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

@@ -36,23 +36,9 @@ final class NetworkProtectionBouncer {
     func requireAuthTokenOrKillApp(controller: TunnelController) async {
 #if SUBSCRIPTION
         let accountManager = AccountManager(subscriptionAppGroup: Bundle.main.appGroup(bundle: .subs))
-        let result = await accountManager.hasEntitlement(for: .networkProtection, cachePolicy: .reloadIgnoringLocalCacheData)
-        switch result {
-        case .success(true):
-            return
-        case .failure:
-            guard accountManager.accessToken == nil else {
-                return
-            }
-        case .success(false):
-            os_log(.error, log: .networkProtection, "🔴 Stopping: DuckDuckGo VPN not authorized. Missing entitlement.")
-            await controller.stop()
 
-            // EXIT_SUCCESS ensures the login item won't relaunch
-            // Ref: https://developer.apple.com/documentation/servicemanagement/smappservice/register()
-            // See where it mentions:
-            //      "If the helper crashes or exits with a non-zero status, the system relaunches it"
-            exit(EXIT_SUCCESS)
+        guard !accountManager.isUserAuthenticated else {
+            return
         }
 #endif
         let keychainStore = NetworkProtectionKeychainTokenStore(keychainType: .default,