Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

PIA-1291: Make wireguard the default option #28

Merged

Conversation

kp-juan-docal
Copy link
Contributor

@kp-juan-docal kp-juan-docal commented Feb 7, 2024

Summary

It introduces the changes to make wireguard the default protocol on new installations and the logic to allow us to migrate to wireguard those on ikev2 with default settings.

Sanity Testers

Tests were performed with the application changes introduced here

  • Clean install. Open app. Confirm wireguard is the selected protocol.
  • Install current production. Without changing any settings. Upgrade to this version. Confirm wireguard is the selected protocol.
  • Install current production. Change any ikev2 setting. Upgrade to this version. Confirm ikev2 is the selected protocol.
  • After the above. Change back to ikev2 defaults. Kill process. Open app. Confirm ikev2 is the selected protocol.
  • After the above. Reset settings. Confirm wireguard is the selected protocol.

Copy link
Collaborator

@kp-laura-sempere kp-laura-sempere left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @kp-juan-docal , it looks good to me. I just added some comments about how we can only set this up for iOS

Sources/PIALibrary/Client+Preferences.swift Show resolved Hide resolved
@@ -225,7 +225,7 @@ extension Client {
enablesServerPings = false
minPingInterval = 120000

availableVPNProfiles = [IKEv2Profile()]
availableVPNProfiles = []
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we apply this update only when the platform is not tvOS?

@@ -481,7 +495,7 @@ extension Client.Preferences {
useWiFiProtection = true
trustCellularData = false
nmtMigrationSuccess = false
vpnType = IKEv2Profile.vpnType
vpnType = PIAWGTunnelProfile.vpnType
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same as the other comments: for tvOS we still want IKEv2 to be the default

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We need to also have all references to PIAWGTunnelProfile inside the iOS condition since this class is not available on tvOS otherwise it won't compile on tvOS

@kp-juan-docal kp-juan-docal force-pushed the juan.docal/PIA-1291-make-wireguard-the-default-option branch from ca45534 to 18e340f Compare February 7, 2024 10:40
Copy link
Collaborator

@kp-said-rehouni kp-said-rehouni left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great job @kp-juan-docal! Just one comment to address in order to have it compile for tvOS as well

@kp-juan-docal kp-juan-docal force-pushed the juan.docal/PIA-1291-make-wireguard-the-default-option branch from 18e340f to b751e20 Compare February 7, 2024 17:58
Copy link
Collaborator

@kp-laura-sempere kp-laura-sempere left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @kp-juan-docal , LGTM ✅

@kp-juan-docal kp-juan-docal merged commit b0e0707 into master Feb 8, 2024
1 check passed
@kp-juan-docal kp-juan-docal deleted the juan.docal/PIA-1291-make-wireguard-the-default-option branch February 8, 2024 09:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants