VPN Domain exclusions (internal release) #3045

diegoreymendez · 2024-07-30T19:12:40Z

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

BSK PR: duckduckgo/BrowserServicesKit#918
iOS PR: duckduckgo/iOS#3164

Description

Implements domain exclusions for internal users only.

Testing

The tests were ordered in a way to try and make it easy to run them sequentially.

Make sure external users can't access domain exclusions

If you're logged in as an internal user go to Debug Menu > Remove Internal User State.
Open duckduckgo.com (or any other site)
Open the in-app VPN status view and ensure there's no domain exclusions section.
Connect the VPN.
Repeat the check in step 3.

Make sure internal users can access domain exclusions through the in-app status view

Login as an internal user at https://use-login.duckduckgo.com.
Open duckduckgo.com and search for "What is my ip". Ensure you see your public IP.
Make sure the VPN is stopped.
Open the in-app VPN status view and ensure there's no domain exclusions section.
Connect the VPN.
Refresh the page and ensure you see the VPN IP address.
Open the in-app VPN status view and ensure you see the domain exclusions section.
Exclude the site, the VPN will briefly reconnect.
Refresh the page and ensure you see the public IP address again.
Repeat the process and check the IP is always properly reflected.

Make sure internal users can access domain exclusions through settings

You can do similar steps in the previous tests, but checking also that Settings > VPN > Domain Exclusions properly shows and hides the domain exclusion section based on whether you're an internal user or not.
Also check that adding a domain or removing a domain properly updates the exclusion (check for your IP in the site).

Definition of Done:

Does this PR satisfy our Definition of Done?

Internal references:

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

quanganhdo · 2024-08-03T05:26:25Z

DuckDuckGo/NetworkProtection/ExcludedDomains/ExcludedDomainsModel.swift

+import NetworkProtectionProxy
+
+protocol ExcludedDomainsViewModel {
+    var domains: [String] { get }


Maybe we can use a Set for this instead of an Array?

I considered that - the issue is that the underlying data in VPNSettings is an Array as we can't store a Set. I'm not sure this will work well if we change the structure.

Will explore in follow-up work if you don't mind since it's a detail.

quanganhdo · 2024-08-03T05:27:12Z

DuckDuckGo/NetworkProtection/ExcludedDomains/ExcludedDomainsViewController.swift

+        AddExcludedDomainView(title: "Add Website Exclusion", buttonsState: .compressed, cancelActionTitle: "Cancel", cancelAction: { dismiss in
+
+            dismiss()
+        }, defaultActionTitle: "Add Website") { [weak self] domain, dismiss in


Missing localizations

AFAIK we're currently not localizing the VPN. I'll leave as-is but if this is wrong, let me know and we can follow-up.

LocalPackages/NetworkProtectionMac/Sources/VPNAppLauncher/AppLauncher+VPNUIActionHandler.swift

quanganhdo

Testing steps work well, thanks for the detailed instruction. I'm still going through this, will add more comments later.

I think moving the refactoring into a separate PR would've made this more focused and easier to follow (e.g. the VPNUIActionHandler extraction, simplification of the popover initializer, update to the debug info view, etc)

quanganhdo · 2024-08-05T02:32:24Z

DuckDuckGo/NetworkProtection/ExcludedDomains/ExcludedDomainsModel.swift

Not in the scope of this PR, but we should probably adopt similar domain processing logic from FireproofDomains

DuckDuckGo/Preferences/Model/VPNPreferencesModel.swift

...lPackages/NetworkProtectionMac/Sources/NetworkProtectionProxy/TransparentProxyProvider.swift

quanganhdo

No blockers, mostly nits. Let's get this in internal users' hands 🙌

Task/Issue URL: https://app.asana.com/0/0/1207936340790549/f iOS PR: duckduckgo/iOS#3164 macOS PR: duckduckgo/macos-browser#3045 What kind of version bump will this require?: Patch ## Description Adds BSK support for implements domain exclusions in macOS for internal users only.

… iOS. (#3164) Task/Issue URL: https://app.asana.com/0/0/1207936340790549/f BSK PR: duckduckgo/BrowserServicesKit#918 macOS PR: duckduckgo/macos-browser#3045 ## Description Integrates latest macOS BSK changes for supporting domain exclusions.

Task/Issue URL: https://app.asana.com/0/0/1207936340790549/f iOS PR: duckduckgo/iOS#3164 macOS PR: duckduckgo/macos-browser#3045 What kind of version bump will this require?: Patch ## Description Adds BSK support for implements domain exclusions in macOS for internal users only.

… iOS. (#3164) Task/Issue URL: https://app.asana.com/0/0/1207936340790549/f BSK PR: duckduckgo/BrowserServicesKit#918 macOS PR: duckduckgo/macos-browser#3045 ## Description Integrates latest macOS BSK changes for supporting domain exclusions.

Task/Issue URL: https://app.asana.com/0/0/1207936340790549/f BSK PR: duckduckgo/BrowserServicesKit#918 iOS PR: duckduckgo/iOS#3164 ## Description Implements domain exclusions for internal users only.

diegoreymendez added 23 commits July 16, 2024 11:05

WIP

bcd7e47

Merge branch 'main' into diego/exclude-domains

9494bb4

WIP

de8cdd6

WIP

65016ab

Merges the latest from main

1e16f9d

WIP

d2d4107

Adds support for domain exclusions in VPN

db0301b

Disables VPN restart on launch to test exclusions

065e52d

WIP

89ea86b

Merges the latest from main

8629bf4

Fixes the button size and text in the add-domain dialog

af7808c

WIP

dcdcd61

Merge branch 'main' into diego/exclude-domains

1e4050c

WIP

ac4a7a8

Fixes a warning in the code

5e4bd3a

Drops the www prefix from VPN troubleshooting UI

2c3c94a

Some UI improvements to the domain exclusions UI

3f8d157

Addresses design feedback

5fb6862

Cleans up dead code

1796a25

Improves site troubleshooting UI

bf67171

Improvements to site troubleshooting

f589014

Merges the latest from main

05b0d90

Updates BSK

70e825b

diegoreymendez self-assigned this Jul 30, 2024

diegoreymendez mentioned this pull request Jul 30, 2024

VPN Domain exclusions (internal release) duckduckgo/BrowserServicesKit#918

Merged

diegoreymendez added 3 commits July 31, 2024 13:58

Refactored some of the status view code

114a9f9

Updates BSK

362e1eb

Updates BSK

fe04a2c

diegoreymendez mentioned this pull request Jul 31, 2024

Integrated macOS VPN Domain exclusion (internal release) changes into iOS. duckduckgo/iOS#3164

Merged

1 task

diegoreymendez marked this pull request as ready for review July 31, 2024 15:43

quanganhdo reviewed Aug 3, 2024

View reviewed changes

LocalPackages/NetworkProtectionMac/Sources/VPNAppLauncher/AppLauncher+VPNUIActionHandler.swift Outdated Show resolved Hide resolved

quanganhdo reviewed Aug 3, 2024

View reviewed changes

quanganhdo reviewed Aug 5, 2024

View reviewed changes

DuckDuckGo/Preferences/Model/VPNPreferencesModel.swift Outdated Show resolved Hide resolved

quanganhdo reviewed Aug 5, 2024

View reviewed changes

...lPackages/NetworkProtectionMac/Sources/NetworkProtectionProxy/TransparentProxyProvider.swift Outdated Show resolved Hide resolved

quanganhdo approved these changes Aug 5, 2024

View reviewed changes

diegoreymendez added 11 commits August 5, 2024 16:15

Fixes an error message

9bfcd22

Fills in a TBD

316d26e

Merges the latest from main

7e8947f

Updates BSK

a5f6d55

Fixes unit tests

1762b57

Fixes a compilation error

40f64ad

Fixes some compilation errors

61e1572

Fixes some code warnings

aed9c62

Fixes some conditional compilation errors

db9048e

Fixes a unit test

efaa013

Disabling a flaky test

4d71849

diegoreymendez added 2 commits August 5, 2024 22:03

Updates to BSK 180.0.1

649eadb

Disables testWhenPDFMainMenuPrintChosen_printDialogOpens

e972748

Disables a flaky test

1776333

diegoreymendez merged commit 879b969 into main Aug 5, 2024
18 checks passed

diegoreymendez deleted the diego/exclude-domains branch August 5, 2024 21:57

diegoreymendez mentioned this pull request Aug 7, 2024

Push domain exclusions to internal release #3076

Merged

1 task

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

VPN Domain exclusions (internal release) #3045

VPN Domain exclusions (internal release) #3045

diegoreymendez commented Jul 30, 2024 •

edited

Loading

quanganhdo Aug 3, 2024

diegoreymendez Aug 5, 2024

quanganhdo Aug 3, 2024

diegoreymendez Aug 5, 2024

quanganhdo left a comment

quanganhdo Aug 5, 2024

quanganhdo left a comment

VPN Domain exclusions (internal release) #3045

VPN Domain exclusions (internal release) #3045

Conversation

diegoreymendez commented Jul 30, 2024 • edited Loading

Description

Testing

Make sure external users can't access domain exclusions

Make sure internal users can access domain exclusions through the in-app status view

Make sure internal users can access domain exclusions through settings

Internal references:

quanganhdo Aug 3, 2024

Choose a reason for hiding this comment

diegoreymendez Aug 5, 2024

Choose a reason for hiding this comment

quanganhdo Aug 3, 2024

Choose a reason for hiding this comment

diegoreymendez Aug 5, 2024

Choose a reason for hiding this comment

quanganhdo left a comment

Choose a reason for hiding this comment

quanganhdo Aug 5, 2024

Choose a reason for hiding this comment

quanganhdo left a comment

Choose a reason for hiding this comment

diegoreymendez commented Jul 30, 2024 •

edited

Loading