fix(apple): Force enable VPN configuration on autoStart#8814
Merged
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
Contributor
There was a problem hiding this comment.
Pull Request Overview
This PR ensures that the VPN configuration is re-enabled during autoStart on Apple OSes, preventing deactivation if another VPN is activated on the system.
- In Store.swift, the autoStart flow now explicitly calls enableConfiguration.
- In VPNConfigurationManager.swift, duplicated configuration updates are consolidated into the enableConfiguration() helper.
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| swift/apple/FirezoneKit/Sources/FirezoneKit/Stores/Store.swift | Added call to enable VPN configuration during autoStart |
| swift/apple/FirezoneKit/Sources/FirezoneKit/Managers/VPNConfigurationManager.swift | Consolidated configuration update logic into enableConfiguration() and removed redundant inline updates |
Comment on lines
102
to
103
| try await vpnConfigurationManager?.enableConfiguration() | ||
| try ipcClient().start() |
There was a problem hiding this comment.
Since enabling the VPN configuration is critical during autoStart, using optional chaining might silently fail if vpnConfigurationManager is nil. Consider explicitly handling the nil case (for example, by logging an error or throwing an exception) to ensure the configuration is properly enabled.
Suggested change
| try await vpnConfigurationManager?.enableConfiguration() | |
| try ipcClient().start() | |
| guard let manager = vpnConfigurationManager else { | |
| Log.error("VPNConfigurationManager is nil. Unable to enable configuration during autoStart.") | |
| throw NSError(domain: "StoreError", code: 1, userInfo: [NSLocalizedDescriptionKey: "VPNConfigurationManager is not initialized."]) | |
| } | |
| try await manager.enableConfiguration() |
Member
Author
|
Tested and works on live device. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
If another VPN has been activated on the system while Firezone is active, Apple OSes will deactivate our configuration, and never reactivate it.
We knew this already, and always activated the configuration when starting during the sign in flow, but failed to also do this when autoStarting on launch.
This PR updates ensures that during autoStart, we re-enable the configuration as well.
Fixes #8813