Skip to content

Commit 5302087

Browse files
ethanndicksonethanndickson
committed
fix: manually upgrade network extension
1 parent 314edbe commit 5302087

File tree

5 files changed

+94
-42
lines changed

5 files changed

+94
-42
lines changed

‎Coder-Desktop/Coder-Desktop/VPN/NetworkExtension.swift‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -58,8 +58,9 @@ extension CoderVPNService {
5858
try await tm.saveToPreferences()
5959
neState = .disabled
6060
} catch {
61+
// This typically fails when the user declines the permission dialog
6162
logger.error("save tunnel failed: \(error)")
62-
neState = .failed(error.localizedDescription)
63+
neState = .failed("Failed to save tunnel: \(error.localizedDescription). Try logging in and out again.")
6364
}
6465
}
6566

‎Coder-Desktop/Coder-Desktop/VPN/VPNService.swift‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -81,12 +81,13 @@ final class CoderVPNService: NSObject, VPNService {
8181
// systemExtnDelegate holds a reference to the SystemExtensionDelegate so that it doesn't get
8282
// garbage collected while the OSSystemExtensionRequest is in flight, since the OS framework
8383
// only stores a weak reference to the delegate.
84-
var systemExtnDelegate: SystemExtensionDelegate<CoderVPNService>?
84+
var systemExtnDelegate: SystemExtensionDelegate<CoderVPNService>!
8585

8686
var serverAddress: String?
8787

8888
override init() {
8989
super.init()
90+
systemExtnDelegate = SystemExtensionDelegate(asyncDelegate: self)
9091
}
9192

9293
func start() async {

‎Coder-Desktop/Coder-Desktop/VPN/VPNSystemExtension.swift‎

Lines changed: 70 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,35 @@ enum SystemExtensionState: Equatable, Sendable {
2222
}
2323
}
2424

25+
let extensionBundle: Bundle = {
26+
let extensionsDirectoryURL = URL(
27+
fileURLWithPath: "Contents/Library/SystemExtensions",
28+
relativeTo: Bundle.main.bundleURL
29+
)
30+
let extensionURLs: [URL]
31+
do {
32+
extensionURLs = try FileManager.default.contentsOfDirectory(at: extensionsDirectoryURL,
33+
includingPropertiesForKeys: nil,
34+
options: .skipsHiddenFiles)
35+
} catch {
36+
fatalError("Failed to get the contents of " +
37+
"\(extensionsDirectoryURL.absoluteString): \(error.localizedDescription)")
38+
}
39+
40+
// here we're just going to assume that there is only ever going to be one SystemExtension
41+
// packaged up in the application bundle. If we ever need to ship multiple versions or have
42+
// multiple extensions, we'll need to revisit this assumption.
43+
guard let extensionURL = extensionURLs.first else {
44+
fatalError("Failed to find any system extensions")
45+
}
46+
47+
guard let extensionBundle = Bundle(url: extensionURL) else {
48+
fatalError("Failed to create a bundle with URL \(extensionURL.absoluteString)")
49+
}
50+
51+
return extensionBundle
52+
}()
53+
2554
protocol SystemExtensionAsyncRecorder: Sendable {
2655
func recordSystemExtensionState(_ state: SystemExtensionState) async
2756
}
@@ -36,35 +65,6 @@ extension CoderVPNService: SystemExtensionAsyncRecorder {
3665
}
3766
}
3867

39-
var extensionBundle: Bundle {
40-
let extensionsDirectoryURL = URL(
41-
fileURLWithPath: "Contents/Library/SystemExtensions",
42-
relativeTo: Bundle.main.bundleURL
43-
)
44-
let extensionURLs: [URL]
45-
do {
46-
extensionURLs = try FileManager.default.contentsOfDirectory(at: extensionsDirectoryURL,
47-
includingPropertiesForKeys: nil,
48-
options: .skipsHiddenFiles)
49-
} catch {
50-
fatalError("Failed to get the contents of " +
51-
"\(extensionsDirectoryURL.absoluteString): \(error.localizedDescription)")
52-
}
53-
54-
// here we're just going to assume that there is only ever going to be one SystemExtension
55-
// packaged up in the application bundle. If we ever need to ship multiple versions or have
56-
// multiple extensions, we'll need to revisit this assumption.
57-
guard let extensionURL = extensionURLs.first else {
58-
fatalError("Failed to find any system extensions")
59-
}
60-
61-
guard let extensionBundle = Bundle(url: extensionURL) else {
62-
fatalError("Failed to create a bundle with URL \(extensionURL.absoluteString)")
63-
}
64-
65-
return extensionBundle
66-
}
67-
6868
func installSystemExtension() {
6969
logger.info("activating SystemExtension")
7070
guard let bundleID = extensionBundle.bundleIdentifier else {
@@ -75,9 +75,7 @@ extension CoderVPNService: SystemExtensionAsyncRecorder {
7575
forExtensionWithIdentifier: bundleID,
7676
queue: .main
7777
)
78-
let delegate = SystemExtensionDelegate(asyncDelegate: self)
79-
systemExtnDelegate = delegate
80-
request.delegate = delegate
78+
request.delegate = systemExtnDelegate
8179
OSSystemExtensionManager.shared.submitRequest(request)
8280
logger.info("submitted SystemExtension request with bundleID: \(bundleID)")
8381
}
@@ -90,6 +88,10 @@ class SystemExtensionDelegate<AsyncDelegate: SystemExtensionAsyncRecorder>:
9088
{
9189
private var logger = Logger(subsystem: Bundle.main.bundleIdentifier!, category: "vpn-installer")
9290
private var asyncDelegate: AsyncDelegate
91+
// The `didFinishWithResult` function is called for both activation and
92+
// deactivation requests and the API provides no way to differentiate them.
93+
// https://developer.apple.com/forums/thread/684021
94+
private var isReinstalling = false
9395

9496
init(asyncDelegate: AsyncDelegate) {
9597
self.asyncDelegate = asyncDelegate
@@ -109,9 +111,23 @@ class SystemExtensionDelegate<AsyncDelegate: SystemExtensionAsyncRecorder>:
109111
}
110112
return
111113
}
112-
logger.info("SystemExtension activated")
113-
Task { [asyncDelegate] in
114-
await asyncDelegate.recordSystemExtensionState(SystemExtensionState.installed)
114+
if isReinstalling {
115+
logger.info("SystemExtension deleted")
116+
Task { [asyncDelegate] in
117+
await asyncDelegate.recordSystemExtensionState(SystemExtensionState.uninstalled)
118+
}
119+
let request = OSSystemExtensionRequest.activationRequest(
120+
forExtensionWithIdentifier: extensionBundle.bundleIdentifier!,
121+
queue: .main
122+
)
123+
request.delegate = self
124+
isReinstalling = false
125+
OSSystemExtensionManager.shared.submitRequest(request)
126+
} else {
127+
logger.info("SystemExtension installed")
128+
Task { [asyncDelegate] in
129+
await asyncDelegate.recordSystemExtensionState(SystemExtensionState.installed)
130+
}
115131
}
116132
}
117133

@@ -131,12 +147,27 @@ class SystemExtensionDelegate<AsyncDelegate: SystemExtensionAsyncRecorder>:
131147
}
132148

133149
func request(
134-
_ request: OSSystemExtensionRequest,
150+
_: OSSystemExtensionRequest,
135151
actionForReplacingExtension existing: OSSystemExtensionProperties,
136152
withExtension extension: OSSystemExtensionProperties
137153
) -> OSSystemExtensionRequest.ReplacementAction {
138-
// swiftlint:disable:next line_length
139-
logger.info("Replacing \(request.identifier) v\(existing.bundleShortVersion) with v\(`extension`.bundleShortVersion)")
140-
return .replace
154+
// This is counterintuitive, but this function is only called if the versions are the same in a dev environment.
155+
// In a release build, this only gets called when the version string is different.
156+
// We don't want to manually reinstall the extension in a dev environment, because the bug doesn't happen.
157+
if existing.bundleVersion == `extension`.bundleVersion {
158+
return .replace
159+
}
160+
// To work around the bug described in https://github.com/coder/coder-desktop-macos/issues/121,
161+
// we're not going to return .replace, instead we're going to manually reinstall the extension.
162+
defer {
163+
let request = OSSystemExtensionRequest.deactivationRequest(
164+
forExtensionWithIdentifier: extensionBundle.bundleIdentifier!,
165+
queue: .main
166+
)
167+
request.delegate = self
168+
isReinstalling = true
169+
OSSystemExtensionManager.shared.submitRequest(request)
170+
}
171+
return .cancel
141172
}
142173
}

‎Coder-Desktop/Coder-Desktop/Views/VPN/VPNMenu.swift‎

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -81,6 +81,21 @@ struct VPNMenu<VPN: VPNService, FS: FileSyncDaemon>: View {
8181
}.buttonStyle(.plain)
8282
TrayDivider()
8383
}
84+
// This shows when
85+
// 1. The user is logged in
86+
// 2. The network extension is installed
87+
// 3. The VPN is unconfigured
88+
// It's accompanied by a message in the VPNState view
89+
// that the user needs to reconfigure.
90+
if state.hasSession, vpn.state == .failed(.networkExtensionError(.unconfigured)) {
91+
Button {
92+
state.reconfigure()
93+
} label: {
94+
ButtonRowView {
95+
Text("Reconfigure VPN")
96+
}
97+
}.buttonStyle(.plain)
98+
}
8499
if vpn.state == .failed(.systemExtensionError(.needsUserApproval)) {
85100
Button {
86101
openSystemExtensionSettings()

‎Coder-Desktop/Coder-Desktop/Views/VPN/VPNState.swift‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,10 @@ struct VPNState<VPN: VPNService>: View {
1717
Text("Sign in to use Coder Desktop")
1818
.font(.body)
1919
.foregroundColor(.secondary)
20+
case (.failed(.networkExtensionError(.unconfigured)), _):
21+
Text("The system VPN requires reconfiguration.")
22+
.font(.body)
23+
.foregroundStyle(.secondary)
2024
case (.disabled, _):
2125
Text("Enable Coder Connect to see workspaces")
2226
.font(.body)
@@ -38,7 +42,7 @@ struct VPNState<VPN: VPNService>: View {
3842
.padding(.horizontal, Theme.Size.trayInset)
3943
.padding(.vertical, Theme.Size.trayPadding)
4044
.frame(maxWidth: .infinity)
41-
default:
45+
case (.connected, true):
4246
EmptyView()
4347
}
4448
}

0 commit comments

Comments
 (0)