From d0ee9c36c5ff8e1099345e28e4717286dfd37c4b Mon Sep 17 00:00:00 2001 From: Sam Symons Date: Mon, 25 Mar 2024 08:09:27 -0700 Subject: [PATCH] VPN ship review changes (#2630) Task/Issue URL: https://app.asana.com/0/414235014887631/1206914003023279/f Tech Design URL: CC: Description: This PR fixes a number of ship review issues: The option to show the VPN screen after purchase was pushing the VC multiple times - this is now fixed The VPN is now fully removed after entitlements expire The VPN disconnected notification is now only displayed if the VPN is installed This PR also pulls in a crash fix from @miasma13 It also cleans up some waitlist code that is no longer really desired in the app. --- DuckDuckGo.xcodeproj/project.pbxproj | 2 +- .../xcshareddata/swiftpm/Package.resolved | 4 +-- DuckDuckGo/AppDelegate+Waitlists.swift | 32 ++----------------- DuckDuckGo/AppDelegate.swift | 17 ++++++---- DuckDuckGo/MainViewController.swift | 8 +++-- .../NetworkProtectionAccessController.swift | 1 - .../NetworkProtectionFeatureVisibility.swift | 13 ++++++-- .../NetworkProtectionTunnelController.swift | 7 ++++ DuckDuckGo/SettingsView.swift | 9 ++++-- ...workProtectionFeatureVisibilityTests.swift | 10 +++--- 10 files changed, 50 insertions(+), 53 deletions(-) diff --git a/DuckDuckGo.xcodeproj/project.pbxproj b/DuckDuckGo.xcodeproj/project.pbxproj index f7a7364ded..2e5f393308 100644 --- a/DuckDuckGo.xcodeproj/project.pbxproj +++ b/DuckDuckGo.xcodeproj/project.pbxproj @@ -10033,7 +10033,7 @@ repositoryURL = "https://github.com/DuckDuckGo/BrowserServicesKit"; requirement = { kind = exactVersion; - version = 129.1.8; + version = 129.2.0; }; }; B6F997C22B8F374300476735 /* XCRemoteSwiftPackageReference "apple-toolbox" */ = { diff --git a/DuckDuckGo.xcodeproj/project.xcworkspace/xcshareddata/swiftpm/Package.resolved b/DuckDuckGo.xcodeproj/project.xcworkspace/xcshareddata/swiftpm/Package.resolved index 9f67334479..15e467a17c 100644 --- a/DuckDuckGo.xcodeproj/project.xcworkspace/xcshareddata/swiftpm/Package.resolved +++ b/DuckDuckGo.xcodeproj/project.xcworkspace/xcshareddata/swiftpm/Package.resolved @@ -32,8 +32,8 @@ "kind" : "remoteSourceControl", "location" : "https://github.com/DuckDuckGo/BrowserServicesKit", "state" : { - "revision" : "0509e0cf15f1ff8b0f32ac39c262da131fba7f3e", - "version" : "129.1.8" + "revision" : "284c328a097132a12e8abcf94d8f4d369063dcb4", + "version" : "129.2.0" } }, { diff --git a/DuckDuckGo/AppDelegate+Waitlists.swift b/DuckDuckGo/AppDelegate+Waitlists.swift index 5d58408534..4f46ee1f3c 100644 --- a/DuckDuckGo/AppDelegate+Waitlists.swift +++ b/DuckDuckGo/AppDelegate+Waitlists.swift @@ -37,7 +37,7 @@ extension AppDelegate { func checkWaitlists() { #if NETWORK_PROTECTION - if vpnFeatureVisibilty.shouldKeepVPNAccessViaWaitlist() { + if vpnFeatureVisibility.shouldKeepVPNAccessViaWaitlist() { checkNetworkProtectionWaitlist() } #endif @@ -54,35 +54,7 @@ extension AppDelegate { VPNWaitlist.shared.fetchInviteCodeIfAvailable { [weak self] error in guard error == nil else { - - if error == .alreadyHasInviteCode, UIApplication.shared.applicationState == .active { - // If the user already has an invite code but their auth token has gone missing, attempt to redeem it again. - let tokenStore = NetworkProtectionKeychainTokenStore() - let waitlistStorage = VPNWaitlist.shared.waitlistStorage - let configManager = ContentBlocking.shared.privacyConfigurationManager - let waitlistBetaActive = configManager.privacyConfig.isSubfeatureEnabled(NetworkProtectionSubfeature.waitlistBetaActive) - - if let inviteCode = waitlistStorage.getWaitlistInviteCode(), - !tokenStore.isFeatureActivated, - waitlistBetaActive { - let pixel: Pixel.Event = .networkProtectionWaitlistRetriedInviteCodeRedemption - - do { - if let token = try tokenStore.fetchToken() { - DailyPixel.fireDailyAndCount(pixel: pixel, withAdditionalParameters: [ "tokenState": "found" ]) - } else { - DailyPixel.fireDailyAndCount(pixel: pixel, withAdditionalParameters: [ "tokenState": "nil" ]) - } - } catch { - DailyPixel.fireDailyAndCount(pixel: pixel, error: error, withAdditionalParameters: [ "tokenState": "error" ]) - } - - self?.fetchVPNWaitlistAuthToken(inviteCode: inviteCode) - } - } - return - } guard let inviteCode = VPNWaitlist.shared.waitlistStorage.getWaitlistInviteCode() else { @@ -95,7 +67,7 @@ extension AppDelegate { #endif private func checkWaitlistBackgroundTasks() { - guard vpnFeatureVisibilty.shouldKeepVPNAccessViaWaitlist() else { return } + guard vpnFeatureVisibility.shouldKeepVPNAccessViaWaitlist() else { return } BGTaskScheduler.shared.getPendingTaskRequests { tasks in diff --git a/DuckDuckGo/AppDelegate.swift b/DuckDuckGo/AppDelegate.swift index 1c141e9ae1..320ecc1636 100644 --- a/DuckDuckGo/AppDelegate.swift +++ b/DuckDuckGo/AppDelegate.swift @@ -75,7 +75,7 @@ class AppDelegate: UIResponder, UIApplicationDelegate { #if NETWORK_PROTECTION private let widgetRefreshModel = NetworkProtectionWidgetRefreshModel() private let tunnelDefaults = UserDefaults.networkProtectionGroupDefaults - lazy var vpnFeatureVisibilty = DefaultNetworkProtectionVisibility() + lazy var vpnFeatureVisibility = DefaultNetworkProtectionVisibility() #endif private var autoClear: AutoClear? @@ -304,7 +304,7 @@ class AppDelegate: UIResponder, UIApplicationDelegate { AppConfigurationFetch.registerBackgroundRefreshTaskHandler() #if NETWORK_PROTECTION - if vpnFeatureVisibilty.shouldKeepVPNAccessViaWaitlist() { + if vpnFeatureVisibility.shouldKeepVPNAccessViaWaitlist() { VPNWaitlist.shared.registerBackgroundRefreshTaskHandler() } #endif @@ -335,7 +335,7 @@ class AppDelegate: UIResponder, UIApplicationDelegate { setupSubscriptionsEnvironment() #endif - if vpnFeatureVisibilty.shouldKeepVPNAccessViaWaitlist() { + if vpnFeatureVisibility.shouldKeepVPNAccessViaWaitlist() { clearDebugWaitlistState() } @@ -490,7 +490,7 @@ class AppDelegate: UIResponder, UIApplicationDelegate { #if NETWORK_PROTECTION widgetRefreshModel.refreshVPNWidget() - if vpnFeatureVisibilty.shouldShowThankYouMessaging() && !tunnelDefaults.vpnEarlyAccessOverAlertAlreadyShown { + if vpnFeatureVisibility.shouldShowThankYouMessaging() && !tunnelDefaults.vpnEarlyAccessOverAlertAlreadyShown { presentVPNEarlyAccessOverAlert() } @@ -817,7 +817,10 @@ class AppDelegate: UIResponder, UIApplicationDelegate { #if NETWORK_PROTECTION if shortcutItem.type == ShortcutKey.openVPNSettings { - presentNetworkProtectionStatusSettingsModal() + let visibility = DefaultNetworkProtectionVisibility() + if visibility.shouldShowVPNShortcut() { + presentNetworkProtectionStatusSettingsModal() + } } #endif @@ -852,7 +855,7 @@ class AppDelegate: UIResponder, UIApplicationDelegate { func refreshShortcuts() { #if NETWORK_PROTECTION - guard vpnFeatureVisibilty.shouldShowVPNShortcut() else { + guard vpnFeatureVisibility.shouldShowVPNShortcut() else { UIApplication.shared.shortcutItems = nil return } @@ -929,7 +932,7 @@ extension AppDelegate: UNUserNotificationCenterDelegate { presentNetworkProtectionStatusSettingsModal() } - if vpnFeatureVisibilty.shouldKeepVPNAccessViaWaitlist(), identifier == VPNWaitlist.notificationIdentifier { + if vpnFeatureVisibility.shouldKeepVPNAccessViaWaitlist(), identifier == VPNWaitlist.notificationIdentifier { presentNetworkProtectionWaitlistModal() DailyPixel.fire(pixel: .networkProtectionWaitlistNotificationLaunched) } diff --git a/DuckDuckGo/MainViewController.swift b/DuckDuckGo/MainViewController.swift index 3544f62240..688a45e365 100644 --- a/DuckDuckGo/MainViewController.swift +++ b/DuckDuckGo/MainViewController.swift @@ -1427,10 +1427,14 @@ class MainViewController: UIViewController { Task { guard case .success(false) = await AccountManager().hasEntitlement(for: .networkProtection) else { return } - tunnelDefaults.enableEntitlementMessaging() - let controller = NetworkProtectionTunnelController() + + if await controller.isInstalled { + tunnelDefaults.enableEntitlementMessaging() + } + await controller.stop() + await controller.removeVPN() } } diff --git a/DuckDuckGo/NetworkProtectionAccessController.swift b/DuckDuckGo/NetworkProtectionAccessController.swift index 289addbfa4..a1acf99f11 100644 --- a/DuckDuckGo/NetworkProtectionAccessController.swift +++ b/DuckDuckGo/NetworkProtectionAccessController.swift @@ -143,7 +143,6 @@ struct NetworkProtectionAccessController: NetworkProtectionAccess { } func revokeNetworkProtectionAccess() { - networkProtectionWaitlistStorage.deleteWaitlistState() try? NetworkProtectionKeychainTokenStore().deleteToken() Task { diff --git a/DuckDuckGo/NetworkProtectionFeatureVisibility.swift b/DuckDuckGo/NetworkProtectionFeatureVisibility.swift index a698deb730..45f47e0153 100644 --- a/DuckDuckGo/NetworkProtectionFeatureVisibility.swift +++ b/DuckDuckGo/NetworkProtectionFeatureVisibility.swift @@ -18,6 +18,7 @@ // import Foundation +import Subscription public protocol NetworkProtectionFeatureVisibility { func isWaitlistBetaActive() -> Bool @@ -49,8 +50,16 @@ public extension NetworkProtectionFeatureVisibility { !isPrivacyProLaunched() && isWaitlistBetaActive() && isWaitlistUser() } - // todo - https://app.asana.com/0/0/1206827703748771/f func shouldShowVPNShortcut() -> Bool { - isPrivacyProLaunched() || shouldKeepVPNAccessViaWaitlist() + if isPrivacyProLaunched() { +#if SUBSCRIPTION + let accountManager = AccountManager() + return accountManager.isUserAuthenticated +#else + return false +#endif + } else { + return shouldKeepVPNAccessViaWaitlist() + } } } diff --git a/DuckDuckGo/NetworkProtectionTunnelController.swift b/DuckDuckGo/NetworkProtectionTunnelController.swift index 1d130bf0cc..aaba16d63b 100644 --- a/DuckDuckGo/NetworkProtectionTunnelController.swift +++ b/DuckDuckGo/NetworkProtectionTunnelController.swift @@ -85,6 +85,13 @@ final class NetworkProtectionTunnelController: TunnelController { // MARK: - Connection Status Querying + var isInstalled: Bool { + get async { + let tunnelManager = await loadTunnelManager() + return tunnelManager != nil + } + } + /// Queries Network Protection to know if its VPN is connected. /// /// - Returns: `true` if the VPN is connected, connecting or reasserting, and `false` otherwise. diff --git a/DuckDuckGo/SettingsView.swift b/DuckDuckGo/SettingsView.swift index c15d879d28..d3f1a90a99 100644 --- a/DuckDuckGo/SettingsView.swift +++ b/DuckDuckGo/SettingsView.swift @@ -97,10 +97,13 @@ struct SettingsView: View { } }) - .onReceive(viewModel.$deepLinkTarget, perform: { link in - guard let link else { return } + .onReceive(viewModel.$deepLinkTarget.removeDuplicates(), perform: { link in + guard let link, link != self.deepLinkTarget else { + return + } + self.deepLinkTarget = link - + switch link.type { case .sheet: DispatchQueue.main.async { diff --git a/DuckDuckGoTests/NetworkProtectionFeatureVisibilityTests.swift b/DuckDuckGoTests/NetworkProtectionFeatureVisibilityTests.swift index 24ce4c9297..579ed042c3 100644 --- a/DuckDuckGoTests/NetworkProtectionFeatureVisibilityTests.swift +++ b/DuckDuckGoTests/NetworkProtectionFeatureVisibilityTests.swift @@ -60,28 +60,28 @@ final class NetworkProtectionFeatureVisibilityTests: XCTestCase { XCTAssertTrue(mockWithVPNAccess.shouldMonitorEntitlement()) XCTAssertFalse(mockWithVPNAccess.shouldKeepVPNAccessViaWaitlist()) XCTAssertTrue(mockWithVPNAccess.shouldShowThankYouMessaging()) - XCTAssertTrue(mockWithVPNAccess.shouldShowVPNShortcut()) + XCTAssertFalse(mockWithVPNAccess.shouldShowVPNShortcut()) // Not current waitlist user -> Enforce entitlement check, no more VPN use, no thank-you let mockWithBetaActive = baseMock.adding([.isWaitlistBetaActive]) XCTAssertTrue(mockWithBetaActive.shouldMonitorEntitlement()) XCTAssertFalse(mockWithBetaActive.shouldKeepVPNAccessViaWaitlist()) XCTAssertFalse(mockWithBetaActive.shouldShowThankYouMessaging()) - XCTAssertTrue(mockWithBetaActive.shouldShowVPNShortcut()) + XCTAssertFalse(mockWithBetaActive.shouldShowVPNShortcut()) // Waitlist beta OFF, current waitlist user -> Show thank-you, enforce entitlement check, no more VPN use let mockWithBetaInactive = baseMock.adding([.isWaitlistUser]) XCTAssertTrue(mockWithBetaInactive.shouldMonitorEntitlement()) XCTAssertFalse(mockWithBetaInactive.shouldKeepVPNAccessViaWaitlist()) XCTAssertTrue(mockWithBetaInactive.shouldShowThankYouMessaging()) - XCTAssertTrue(mockWithBetaInactive.shouldShowVPNShortcut()) + XCTAssertFalse(mockWithBetaInactive.shouldShowVPNShortcut()) - // Waitlist beta OFF, not current wailist user -> Enforce entitlement check, nothing else + // Waitlist beta OFF, not current waitlist user -> Enforce entitlement check, nothing else let mockWithNothingElse = baseMock XCTAssertTrue(mockWithNothingElse.shouldMonitorEntitlement()) XCTAssertFalse(mockWithNothingElse.shouldKeepVPNAccessViaWaitlist()) XCTAssertFalse(mockWithNothingElse.shouldShowThankYouMessaging()) - XCTAssertTrue(mockWithNothingElse.shouldShowVPNShortcut()) + XCTAssertFalse(mockWithNothingElse.shouldShowVPNShortcut()) } }