Switch vpn protocols during the runtime

[simonhong]

fix brave/brave-browser#35681

BraveVPNOSConnectionAPI becomes concrete class and it will have
proper ConnectionAPIImpl instance for selected vpn protocol.

Resolves

Submitter Checklist:

• I confirm that no security/privacy review is needed and no other type of reviews are needed, or that I have requested them
• There is a ticket for my issue
• Used Github auto-closing keywords in the PR description above
• Wrote a good PR/commit description
• Squashed any review feedback or "fixup" commits before merge, so that history is a record of what happened in the repo, not your PR
• Added appropriate labels (QA/Yes or QA/No; release-notes/include or release-notes/exclude; OS/...) to the associated issue
• Checked the PR locally:
  • npm run test -- brave_browser_tests, npm run test -- brave_unit_tests wiki
  • npm run lint, npm run presubmit wiki, npm run gn_check, npm run tslint
• Ran git rebase master (if needed)

Reviewer Checklist:

• A security review is not needed, or a link to one is included in the PR description
• New files have MPL-2.0 license header
• Adequate test coverage exists to prevent regressions
• Major classes, functions and non-trivial code blocks are well-commented
• Changes in component dependencies are properly reflected in gn
• Code follows the style guide
• Test plan is specified in PR before merging

After-merge Checklist:

• The associated issue milestone is set to the smallest version that the
  changes has landed on
• All relevant documentation has been updated, for instance:
  • https://github.com/brave/brave-browser/wiki/Deviations-from-Chromium-(features-we-disable-or-remove)
  • https://github.com/brave/brave-browser/wiki/Proxy-redirected-URLs
  • https://github.com/brave/brave-browser/wiki/Fingerprinting-Protections
  • https://github.com/brave/brave-browser/wiki/Brave%E2%80%99s-Use-of-Referral-Codes
  • https://github.com/brave/brave-browser/wiki/Web-Compatibility-Exceptions-in-Brave
  • https://github.com/brave/brave-browser/wiki/QA-Guide
  • https://github.com/brave/brave-browser/wiki/P3A

Test Plan:

Another test plan from issue.

1. Launch brave and set purchased state
2. Connect vpn and check it's connected with current settings option(wg or ikev2)
3. Disconnect
4. Toggle wireguard settings option
5. Connect vpn again and check it's connected with another option
6. Relaunch browser and connect with existing setting
7. Check connection is established with current setting
8. Disconnect and toggle wireguard settings and connect again
9. Check connection is made with toggled option

[brave-builds]

A Storybook has been deployed to preview UI for the latest push

[brave-builds]

A Storybook has been deployed to preview UI for the latest push

[brave-builds]

A Storybook has been deployed to preview UI for the latest push

[simonhong]

At first run, this dialog is launched after connected with wireguard.
and it's not shown anymore from not first run.
If wireguard is enabled initially before startup, this dialog is shown again.

Not sure this is expected behavior or not with current implementation.
checking... Maybe dns helper is not running at first run?

This dialog launching is handled by BraveVpnDnsObserverService.

I think this dialog doesn't have meaning when VPN is connected with wireguard.

Found the cause for this. When BraveVpnService is created another two vpn related services are also created. They are BraveVpnDnsObserverService and BraveVpnWireguardObserverService.
So far, we don't allow switching between ikev2 and wg during the runtime,
they could be instantiated properly at startup. but we changed to switch in runtime.
So, both service should be instantiated at startup and handle its logic for connection state change
by checking current active protocol.

  if (brave_vpn::IsBraveVPNWireguardEnabled(g_browser_process->local_state())) {
    auto* observer_service =
        brave_vpn::BraveVpnWireguardObserverFactory::GetInstance()
            ->GetServiceForContext(context);
    if (observer_service) {
      observer_service->Observe(vpn_service.get());
    }
  } else {
    auto* observer_service =
        brave_vpn::BraveVpnDnsObserverFactory::GetInstance()
            ->GetServiceForContext(context);
    if (observer_service) {
      observer_service->Observe(vpn_service.get());
    }
  }

[simonhong]

How about this text change? @bsclifton @rebron
When clicks Change the settings, dialog is dismissed and wg settings is off.
and user will use vpn with ikev2.

[brave-builds]

A Storybook has been deployed to preview UI for the latest push

[bsclifton]

@simonhong text LGTM!

cc: @mattmcalister

[bsclifton]

Really nice work here! 😄

Thanks for the renaming - things are much easier to understand now. I tested this out and the changes work great. You can switch protocol without any issues as many times as you'd like... without having to restart.

[github-actions]

[puLL-Merge] - brave/brave-core@21934

Description

This PR refactors the Brave VPN connection management code to introduce a more generalized BraveVPNConnectionManager class to handle VPN connections, instead of the platform-specific BraveVPNOSConnectionAPI class. The motivation behind this change is to provide a unified interface for VPN connection management across different platforms and protocols (IKEv2, WireGuard).

Changes

Changes

• Browser/Connection:
  • Renamed and refactored BraveVPNOSConnectionAPI to BraveVPNConnectionManager to generalize VPN connection management.
  • Added new file brave_vpn_connection_manager.h/cc for the Brave VPN connection manager implementation.
  • Refactored IKEv2 and WireGuard specific connection management code into separate implementation files under respective directories.
• Browser/UI:
  • Updated user interface code to use the new BraveVPNConnectionManager class.
• Components/Resources:
  • Updated VPN related strings and resources to reflect changes in connection management logic.
• Test Code:
  • Refactored tests to use the new BraveVPNConnectionManager.

Security Hotspots

• VPN Connection Management (Low Risk): The refactoring introduces changes in how VPN connections are managed and established. It is crucial to ensure that the new BraveVPNConnectionManager securely handles connection credentials, establishes secure connections, and correctly handles connection errors and states.
• Code Execution Paths (Medium Risk): Changes in VPN connection logic could potentially introduce new code execution paths that may not be properly authenticated or validated, leading to security vulnerabilities.
• Data Handling and Storage (Medium Risk): The new connection manager should securely handle sensitive information such as VPN configurations, credentials, and user settings to prevent unauthorized access or leaks.

Please review the changes with a focus on security implications, ensuring that the new VPN connection management approach maintains or improves the security posture of the Brave VPN feature.

[brave-builds]

A Storybook has been deployed to preview UI for the latest push

[bsclifton]

string reviewers ++

[MadhaviSeelam]

Verification PASSED using

Brave | 1.65.42 Chromium: 122.0.6261.94 (Official Build) nightly (64-bit)
-- | --
Revision | 169e4fc2ee1896cd009c59e7b537220ba880a882
OS | Windows 11 Version 23H2 (Build 22631.3155)

1. Installed 1.65.42
2. launched Brave
3. opened brave://settings/system and verified Use Wireguard protocol in Brave VPN toggleOFF
4. visited account.bravesoftware.com in a new tab
5. entered basic-auth credentials
6. entered [email protected], clicked Get login link
7. clicked the link in the email
8. clicked Browse plans
9. scrolled down and clicked on Start free trial for Brave VPN Subscription
10. completed the Stripe purchase flow
11. confirmed credentials are loaded and VPN is disconnected state
12. connected to VPN via VPN panel
13. opened brave://settings/system and confirmed the settingUse Wireguard protocol in Brave VPN toggle ON
14. disconnected VPN via VPN panel
15. opened brave://settings/system and toggled Off Use Wireguard protocol in Brave VPN setting
16. connected vpn again and confirmed fallback to Ikev2 VPN and is connected and green checkmark on the wifi symbol in the taskbar
17. opened Windows OS settings and confirmed BraveVPNNightly connected state
18. closed and relaunched Brave and confirmed VPN is still connected
19. disconnected Brave VPN via hamburger menu
20. opened brave://settings/system and toggled On Use Wireguard protocol in Brave VPN setting
21. connected VPN via wireguard VPN icon in the taskbar
22. confirmed the wireguard VPN connection is enabled
23. confirmed BraveVPNNightlyWIreguardTunnelService is running in the Taskmananger/Services

step 3	step 11	step 12	step 13	step 14	step 15	step 16a	step 16b	step 17

step 18	step 19	step 21	step 22	step 23