Disables NetP when the waitlist is over #1588

diegoreymendez · 2023-09-04T16:24:30Z

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

BSK PR: duckduckgo/BrowserServicesKit#491
iOS PR: duckduckgo/iOS#1975

Description

Disables NetP when the waitlist is over.

Known limitation: we're not disabling NetP if the user interacts with the menu agent. This is something to discuss separately.

Testing:

The following flag is intentionally set to false, since this PR does not know which user signed up for the waitlist beta. It's important to keep this to false until we know with certainty, as setting this to true means we would not have any easter-egg users.

Test 1:

Keep the flag false
Run NetP and reset all state

Internal references:

Pull Request Review Checklist
Software Engineering Expectations
Technical Design Template
Pull Request Documentation

diegoreymendez · 2023-09-04T16:27:38Z

DuckDuckGo/Menus/MainMenu.storyboard

+                                                    <menuItem title="Reset All State Keeping Invite" id="Mqz-Le-LgQ">
+                                                        <modifierMask key="keyEquivalentModifierMask"/>
+                                                        <connections>
+                                                            <action selector="resetAllKeepingInvite:" target="Rah-lS-gno" id="ypd-Ao-CzY"/>
+                                                        </connections>
+                                                    </menuItem>
+                                                    <menuItem title="Reset All State" id="p4h-oC-tpw">


Since I fixed "Reset All State" to truly reset all state, it felt to me we may still want to reset all state while keeping the invite code / auth token.

👍 Agreed, this is very useful for debugging.

diegoreymendez · 2023-09-04T16:28:05Z

DuckDuckGo/NavigationBar/View/MoreOptionsMenu.swift

@@ -131,6 +131,8 @@ final class MoreOptionsMenu: NSMenu {
            addItem(withTitle: UserText.networkProtection, action: #selector(showNetworkProtectionStatus(_:)), keyEquivalent: "")
                .targetting(self)
                .withImage(.image(for: .vpnIcon))
+        } else {
+            networkProtectionFeatureVisibility.disableForWaitlistUsers()


Disabling NetP at different points in the UI, and I'm making it a bit more explicit than before.

Sounds good, I would like to also add some hooks to check feature flags when the privacy config changes, and when the app becomes active, just to make sure we're catching it as often as possible.

diegoreymendez · 2023-09-04T16:28:22Z

DuckDuckGo/NavigationBar/View/NavigationBarPopovers.swift

+            let featureVisibility = DefaultNetworkProtectionVisibility()
+
+            if featureVisibility.isNetworkProtectionVisible() {
+                showNetworkProtectionPopover(usingView: view, withDelegate: delegate)
+            } else {
+                featureVisibility.disableForWaitlistUsers()
+            }


More waitlist verification walls.

diegoreymendez · 2023-09-04T16:28:46Z

DuckDuckGo/NetworkProtection/AppTargets/BothAppTargets/NetworkProtectionDebugMenu.swift

+            guard case .alertFirstButtonReturn = await NSAlert.resetNetworkProtectionAlert().runModal() else { return }
+
+            do {
+                try await debugUtilities.resetAllState(keepAuthToken: true)


This is the gist of the new menu option.

diegoreymendez · 2023-09-04T16:29:08Z

DuckDuckGo/NetworkProtection/AppTargets/BothAppTargets/NetworkProtectionDebugUtilities.swift

-    @UserDefaultsWrapper(key: .networkProtectionOnboardingStatusRawValue, defaultValue: OnboardingStatus.default.rawValue, defaults: .shared)
-    private(set) var onboardingStatusRawValue: OnboardingStatus.RawValue


Moving this to a specific class designed to disable NetP that we can use from the debug menu and from here.

diegoreymendez · 2023-09-04T16:29:38Z

DuckDuckGo/NetworkProtection/AppTargets/BothAppTargets/NetworkProtectionDebugUtilities.swift

-        try await removeSystemExtensionAndAgents()
-        onboardingStatusRawValue = OnboardingStatus.default.rawValue
+    func resetAllState(keepAuthToken: Bool) async throws {
+        networkProtectionFeatureDisabler.disable(keepAuthToken: keepAuthToken, uninstallSystemExtension: true)


For the debug options we do want to reset the system extension.

diegoreymendez · 2023-09-04T16:30:14Z

DuckDuckGo/NetworkProtection/AppTargets/BothAppTargets/NetworkProtectionDebugUtilities.swift

+        networkProtectionFeatureDisabler.disableLoginItems()
+        try await networkProtectionFeatureDisabler.disableSystemExtension()


Adjusting this to do these things all through the disabler.

DuckDuckGo/Waitlist/NetworkProtectionFeatureVisibility.swift

diegoreymendez · 2023-09-04T16:32:08Z

DuckDuckGo/Waitlist/NetworkProtectionFeatureVisibility.swift

        guard isWaitlistUser else {
            return
        }

-        print("NetP Debug: waitlist user - disabling NetP")
-#endif
+        featureDisabler.disable(keepAuthToken: false, uninstallSystemExtension: false)


It's wired, so we need to keep the isWaitlistUser to false for now, because that interferes with easter egg users.

UnitTests/Menus/MoreOptionsMenuTests.swift

samsymons

LGTM!

…a small bug with a callback (#491) Task/Issue URL: https://app.asana.com/0/0/1205416442924462/f iOS PR: duckduckgo/iOS#1975 macOS PR: duckduckgo/macos-browser#1588 What kind of version bump will this require?: Patch ## Description When the extension receives a `resetAllState` message, it's now properly cleaning the stored auth token. Executes a callback that was missing.

Task/Issue URL: https://app.asana.com/0/0/1205416442924463/f BSK PR: duckduckgo/BrowserServicesKit#491 macOS PR: duckduckgo/macos-browser#1588 ## Description Integrates some changes from BSK that make NetP now clear the auth token in the extension when `resetAllState` is called. Also fixes a small bugfix with a callback not being called when it should.

# By Fernando Bunn (3) and others # Via GitHub * develop: Updates sorting for Autofill UI Logins (#1596) Update Scheduler (#1599) PacketTunnelProvider crash buttons (#1597) swizzle WKNavigationAction.request, return empty request if nil (#1595) Better error messages in DBP result screen (#1590) Disables NetP when the waitlist is over (#1588) Settings Sync Provider with support for syncing Email Protection (#1565) Tie Find In Page to Bookmarks Bar (#1591) Move LoginItem to its own package (#1577) Update post merge failed task description (#1589) # Conflicts: # DuckDuckGo.xcodeproj/project.pbxproj # DuckDuckGo/NavigationBar/View/MoreOptionsMenu.swift # DuckDuckGo/NetworkProtection/AppTargets/BothAppTargets/NetworkProtectionDebugUtilities.swift # DuckDuckGo/Waitlist/NetworkProtectionFeatureVisibility.swift

Task/Issue URL: https://app.asana.com/0/0/1205416442924461/f BSK PR: duckduckgo/BrowserServicesKit#491 iOS PR: duckduckgo/iOS#1975 ## Description Disables NetP when the waitlist is over. Known limitation: we're not disabling NetP if the user interacts with the menu agent. This is something to discuss separately. # Conflicts: # DuckDuckGo.xcodeproj/project.pbxproj # DuckDuckGo.xcodeproj/project.xcworkspace/xcshareddata/swiftpm/Package.resolved # LocalPackages/DataBrokerProtection/Package.swift # LocalPackages/NetworkProtectionUI/Package.swift

diegoreymendez added 3 commits September 4, 2023 18:04

Disables NetP when the waitlist is over

32e96d0

Improves the debug menu for NetP

59ea518

Rolls back some unintentional changes

12feb44

diegoreymendez commented Sep 4, 2023

View reviewed changes

DuckDuckGo/Waitlist/NetworkProtectionFeatureVisibility.swift Show resolved Hide resolved

diegoreymendez commented Sep 4, 2023

View reviewed changes

DuckDuckGo/Waitlist/NetworkProtectionFeatureVisibility.swift Show resolved Hide resolved

diegoreymendez commented Sep 4, 2023

View reviewed changes

diegoreymendez requested a review from samsymons September 4, 2023 16:32

diegoreymendez marked this pull request as ready for review September 4, 2023 16:32

diegoreymendez mentioned this pull request Sep 4, 2023

NetP now clears the auth token in the extension + small bug fix. duckduckgo/iOS#1975

Merged

samsymons reviewed Sep 5, 2023

View reviewed changes

UnitTests/Menus/MoreOptionsMenuTests.swift Show resolved Hide resolved

diegoreymendez mentioned this pull request Sep 5, 2023

Removes the auth token when a resetAllState message is received duckduckgo/BrowserServicesKit#491

Merged

samsymons approved these changes Sep 5, 2023

View reviewed changes

Merges the latest from develop

7c068a4

Merges BSK 77.0.1

16fd90b

Updates BSK in some local packages

ed9141e

diegoreymendez merged commit 627946b into develop Sep 5, 2023

diegoreymendez deleted the diego/disable-netp-when-waitlist-is-over branch September 5, 2023 21:03

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Disables NetP when the waitlist is over #1588

Disables NetP when the waitlist is over #1588

diegoreymendez commented Sep 4, 2023 •

edited

Loading

diegoreymendez Sep 4, 2023

samsymons Sep 5, 2023

diegoreymendez Sep 4, 2023

samsymons Sep 5, 2023

diegoreymendez Sep 4, 2023

diegoreymendez Sep 4, 2023

diegoreymendez Sep 4, 2023

diegoreymendez Sep 4, 2023

diegoreymendez Sep 4, 2023

diegoreymendez Sep 4, 2023

samsymons left a comment

		@UserDefaultsWrapper(key: .networkProtectionOnboardingStatusRawValue, defaultValue: OnboardingStatus.default.rawValue, defaults: .shared)
		private(set) var onboardingStatusRawValue: OnboardingStatus.RawValue

		networkProtectionFeatureDisabler.disableLoginItems()
		try await networkProtectionFeatureDisabler.disableSystemExtension()

Disables NetP when the waitlist is over #1588

Disables NetP when the waitlist is over #1588

Conversation

diegoreymendez commented Sep 4, 2023 • edited Loading

Description

Testing:

Test 1:

Internal references:

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

samsymons left a comment

Choose a reason for hiding this comment

diegoreymendez commented Sep 4, 2023 •

edited

Loading