Skip to content

Commit 86775e3

Browse files
deansheatherdeansheather
committed
Revert "fix: opt-in tailscale vpn loop prevention (#148)"
This reverts commit 814b412.
1 parent 814b412 commit 86775e3

File tree

10 files changed

+34
-62
lines changed

10 files changed

+34
-62
lines changed

‎App/App.xaml.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -92,7 +92,6 @@ public App()
9292

9393
services.AddSingleton<IDispatcherQueueManager>(_ => this);
9494
services.AddSingleton<IDefaultNotificationHandler>(_ => this);
95-
services.AddSingleton<ISettingsManager<CoderConnectSettings>, SettingsManager<CoderConnectSettings>>();
9695
services.AddSingleton<ICredentialBackend>(_ =>
9796
new WindowsCredentialBackend(WindowsCredentialBackend.CoderCredentialsTargetName));
9897
services.AddSingleton<ICredentialManager, CredentialManager>();
@@ -121,6 +120,7 @@ public App()
121120
// FileSyncListMainPage is created by FileSyncListWindow.
122121
services.AddTransient<FileSyncListWindow>();
123122

123+
services.AddSingleton<ISettingsManager<CoderConnectSettings>, SettingsManager<CoderConnectSettings>>();
124124
services.AddSingleton<IStartupManager, StartupManager>();
125125
// SettingsWindow views and view models
126126
services.AddTransient<SettingsViewModel>();

‎App/Models/Settings.cs

Lines changed: 2 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -34,11 +34,6 @@ public class CoderConnectSettings : ISettings<CoderConnectSettings>
3434
/// </summary>
3535
public bool ConnectOnLaunch { get; set; }
3636

37-
/// <summary>
38-
/// When this is true Coder Connect will not attempt to protect against Tailscale loopback issues.
39-
/// </summary>
40-
public bool EnableCorporateVpnSupport { get; set; }
41-
4237
/// <summary>
4338
/// CoderConnect current settings version. Increment this when the settings schema changes.
4439
/// In future iterations we will be able to handle migrations when the user has
@@ -51,21 +46,17 @@ public CoderConnectSettings()
5146
Version = VERSION;
5247

5348
ConnectOnLaunch = false;
54-
55-
EnableCorporateVpnSupport = false;
5649
}
5750

58-
public CoderConnectSettings(int? version, bool connectOnLaunch, bool enableCorporateVpnSupport)
51+
public CoderConnectSettings(int? version, bool connectOnLaunch)
5952
{
6053
Version = version ?? VERSION;
6154

6255
ConnectOnLaunch = connectOnLaunch;
63-
64-
EnableCorporateVpnSupport = enableCorporateVpnSupport;
6556
}
6657

6758
public CoderConnectSettings Clone()
6859
{
69-
return new CoderConnectSettings(Version, ConnectOnLaunch, EnableCorporateVpnSupport);
60+
return new CoderConnectSettings(Version, ConnectOnLaunch);
7061
}
7162
}

‎App/Services/CredentialManager.cs

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -223,9 +223,8 @@ private async Task<CredentialModel> LoadCredentialsInner(CancellationToken ct)
223223
};
224224
}
225225

226-
// Grab the lock again so we can update the state. Don't use the CT
227-
// here as it may have already been canceled.
228-
using (await _opLock.LockAsync(TimeSpan.FromSeconds(5), CancellationToken.None))
226+
// Grab the lock again so we can update the state.
227+
using (await _opLock.LockAsync(ct))
229228
{
230229
// Prevent new LoadCredentials calls from returning this task.
231230
if (_loadCts != null)
@@ -243,8 +242,11 @@ private async Task<CredentialModel> LoadCredentialsInner(CancellationToken ct)
243242
if (latestCreds is not null) return latestCreds;
244243
}
245244

246-
UpdateState(model);
245+
// If there aren't any latest credentials after a cancellation, we
246+
// most likely timed out and should throw.
247247
ct.ThrowIfCancellationRequested();
248+
249+
UpdateState(model);
248250
return model;
249251
}
250252
}

‎App/Services/RpcController.cs

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -69,18 +69,16 @@ public interface IRpcController : IAsyncDisposable
6969
public class RpcController : IRpcController
7070
{
7171
private readonly ICredentialManager _credentialManager;
72-
private readonly ISettingsManager<CoderConnectSettings> _settingsManager;
7372

7473
private readonly RaiiSemaphoreSlim _operationLock = new(1, 1);
7574
private Speaker<ClientMessage, ServiceMessage>? _speaker;
7675

7776
private readonly RaiiSemaphoreSlim _stateLock = new(1, 1);
7877
private readonly RpcModel _state = new();
7978

80-
public RpcController(ICredentialManager credentialManager, ISettingsManager<CoderConnectSettings> settingsManager)
79+
public RpcController(ICredentialManager credentialManager)
8180
{
8281
_credentialManager = credentialManager;
83-
_settingsManager = settingsManager;
8482
}
8583

8684
public event EventHandler<RpcModel>? StateChanged;
@@ -158,7 +156,6 @@ public async Task StartVpn(CancellationToken ct = default)
158156
using var _ = await AcquireOperationLockNowAsync();
159157
AssertRpcConnected();
160158

161-
var coderConnectSettings = await _settingsManager.Read(ct);
162159
var credentials = _credentialManager.GetCachedCredentials();
163160
if (credentials.State != CredentialState.Valid)
164161
throw new RpcOperationException(
@@ -178,7 +175,6 @@ public async Task StartVpn(CancellationToken ct = default)
178175
{
179176
CoderUrl = credentials.CoderUrl?.ToString(),
180177
ApiToken = credentials.ApiToken,
181-
TunnelUseSoftNetIsolation = coderConnectSettings.EnableCorporateVpnSupport,
182178
},
183179
}, ct);
184180
}

‎App/ViewModels/SettingsViewModel.cs

Lines changed: 1 addition & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -13,9 +13,6 @@ public partial class SettingsViewModel : ObservableObject
1313
[ObservableProperty]
1414
public partial bool ConnectOnLaunch { get; set; }
1515

16-
[ObservableProperty]
17-
public partial bool DisableTailscaleLoopProtection { get; set; }
18-
1916
[ObservableProperty]
2017
public partial bool StartOnLoginDisabled { get; set; }
2118

@@ -34,7 +31,6 @@ public SettingsViewModel(ILogger<SettingsViewModel> logger, ISettingsManager<Cod
3431
_connectSettings = settingsManager.Read().GetAwaiter().GetResult();
3532
StartOnLogin = startupManager.IsEnabled();
3633
ConnectOnLaunch = _connectSettings.ConnectOnLaunch;
37-
DisableTailscaleLoopProtection = _connectSettings.EnableCorporateVpnSupport;
3834

3935
// Various policies can disable the "Start on login" option.
4036
// We disable the option in the UI if the policy is set.
@@ -47,21 +43,6 @@ public SettingsViewModel(ILogger<SettingsViewModel> logger, ISettingsManager<Cod
4743
}
4844
}
4945

50-
partial void OnDisableTailscaleLoopProtectionChanged(bool oldValue, bool newValue)
51-
{
52-
if (oldValue == newValue)
53-
return;
54-
try
55-
{
56-
_connectSettings.EnableCorporateVpnSupport = DisableTailscaleLoopProtection;
57-
_connectSettingsManager.Write(_connectSettings);
58-
}
59-
catch (Exception ex)
60-
{
61-
_logger.LogError($"Error saving Coder Connect {nameof(DisableTailscaleLoopProtection)} settings: {ex.Message}");
62-
}
63-
}
64-
6546
partial void OnConnectOnLaunchChanged(bool oldValue, bool newValue)
6647
{
6748
if (oldValue == newValue)
@@ -73,7 +54,7 @@ partial void OnConnectOnLaunchChanged(bool oldValue, bool newValue)
7354
}
7455
catch (Exception ex)
7556
{
76-
_logger.LogError($"Error saving Coder Connect {nameof(ConnectOnLaunch)} settings: {ex.Message}");
57+
_logger.LogError($"Error saving Coder Connect settings: {ex.Message}");
7758
}
7859
}
7960

‎App/Views/Pages/SettingsMainPage.xaml

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -44,12 +44,6 @@
4444
>
4545
<ToggleSwitch IsOn="{x:Bind ViewModel.ConnectOnLaunch, Mode=TwoWay}" />
4646
</controls:SettingsCard>
47-
<controls:SettingsCard Description="This setting loosens some VPN loop protection checks in Coder Connect, allowing traffic to flow to a Coder deployment behind a corporate VPN. We only recommend enabling this option if Coder Connect doesn't work with your Coder deployment behind a corporate VPN."
48-
Header="Enable support for corporate VPNs"
49-
HeaderIcon="{ui:FontIcon Glyph=&#xE705;}"
50-
>
51-
<ToggleSwitch IsOn="{x:Bind ViewModel.DisableTailscaleLoopProtection, Mode=TwoWay}" />
52-
</controls:SettingsCard>
5347
</StackPanel>
5448
</Grid>
5549
</ScrollViewer>

‎App/Views/SettingsWindow.xaml

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -9,8 +9,8 @@
99
xmlns:winuiex="using:WinUIEx"
1010
mc:Ignorable="d"
1111
Title="Coder Settings"
12-
Width="600" Height="500"
13-
MinWidth="600" MinHeight="500">
12+
Width="600" Height="350"
13+
MinWidth="600" MinHeight="350">
1414

1515
<Window.SystemBackdrop>
1616
<MicaBackdrop/>

‎App/Views/TrayWindow.xaml.cs

Lines changed: 18 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -5,9 +5,11 @@
55
using Coder.Desktop.App.Views.Pages;
66
using CommunityToolkit.Mvvm.Input;
77
using Microsoft.UI;
8+
using Microsoft.UI.Input;
89
using Microsoft.UI.Windowing;
910
using Microsoft.UI.Xaml;
1011
using Microsoft.UI.Xaml.Controls;
12+
using Microsoft.UI.Xaml.Documents;
1113
using Microsoft.UI.Xaml.Media.Animation;
1214
using System;
1315
using System.Collections.Generic;
@@ -17,7 +19,6 @@
1719
using Windows.Graphics;
1820
using Windows.System;
1921
using Windows.UI.Core;
20-
using Microsoft.UI.Input;
2122
using WinRT.Interop;
2223
using WindowActivatedEventArgs = Microsoft.UI.Xaml.WindowActivatedEventArgs;
2324

@@ -40,6 +41,7 @@ public sealed partial class TrayWindow : Window
4041

4142
private readonly IRpcController _rpcController;
4243
private readonly ICredentialManager _credentialManager;
44+
private readonly ISyncSessionController _syncSessionController;
4345
private readonly IUpdateController _updateController;
4446
private readonly IUserNotifier _userNotifier;
4547
private readonly TrayWindowLoadingPage _loadingPage;
@@ -49,13 +51,15 @@ public sealed partial class TrayWindow : Window
4951

5052
public TrayWindow(
5153
IRpcController rpcController, ICredentialManager credentialManager,
52-
IUpdateController updateController, IUserNotifier userNotifier,
54+
ISyncSessionController syncSessionController, IUpdateController updateController,
55+
IUserNotifier userNotifier,
5356
TrayWindowLoadingPage loadingPage,
5457
TrayWindowDisconnectedPage disconnectedPage, TrayWindowLoginRequiredPage loginRequiredPage,
5558
TrayWindowMainPage mainPage)
5659
{
5760
_rpcController = rpcController;
5861
_credentialManager = credentialManager;
62+
_syncSessionController = syncSessionController;
5963
_updateController = updateController;
6064
_userNotifier = userNotifier;
6165
_loadingPage = loadingPage;
@@ -70,7 +74,9 @@ public TrayWindow(
7074

7175
_rpcController.StateChanged += RpcController_StateChanged;
7276
_credentialManager.CredentialsChanged += CredentialManager_CredentialsChanged;
73-
SetPageByState(_rpcController.GetState(), _credentialManager.GetCachedCredentials());
77+
_syncSessionController.StateChanged += SyncSessionController_StateChanged;
78+
SetPageByState(_rpcController.GetState(), _credentialManager.GetCachedCredentials(),
79+
_syncSessionController.GetState());
7480

7581
// Setting these directly in the .xaml doesn't seem to work for whatever reason.
7682
TrayIcon.OpenCommand = Tray_OpenCommand;
@@ -121,7 +127,8 @@ public TrayWindow(
121127
};
122128
}
123129

124-
private void SetPageByState(RpcModel rpcModel, CredentialModel credentialModel)
130+
private void SetPageByState(RpcModel rpcModel, CredentialModel credentialModel,
131+
SyncSessionControllerStateModel syncSessionModel)
125132
{
126133
if (credentialModel.State == CredentialState.Unknown)
127134
{
@@ -194,13 +201,18 @@ private void MaybeNotifyUser(RpcModel rpcModel)
194201

195202
private void RpcController_StateChanged(object? _, RpcModel model)
196203
{
197-
SetPageByState(model, _credentialManager.GetCachedCredentials());
204+
SetPageByState(model, _credentialManager.GetCachedCredentials(), _syncSessionController.GetState());
198205
MaybeNotifyUser(model);
199206
}
200207

201208
private void CredentialManager_CredentialsChanged(object? _, CredentialModel model)
202209
{
203-
SetPageByState(_rpcController.GetState(), model);
210+
SetPageByState(_rpcController.GetState(), model, _syncSessionController.GetState());
211+
}
212+
213+
private void SyncSessionController_StateChanged(object? _, SyncSessionControllerStateModel model)
214+
{
215+
SetPageByState(_rpcController.GetState(), _credentialManager.GetCachedCredentials(), model);
204216
}
205217

206218
// Sadly this is necessary because Window.Content.SizeChanged doesn't

‎Tests.App/Services/CredentialManagerTest.cs

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -317,10 +317,7 @@ public async Task SetDuringLoad(CancellationToken ct)
317317
var loadTask = manager.LoadCredentials(ct);
318318
// Then fully perform a set.
319319
await manager.SetCredentials(TestServerUrl, TestApiToken, ct).WaitAsync(ct);
320-
321-
// The load should complete with the new valid credentials
322-
var result = await loadTask;
323-
Assert.That(result.State, Is.EqualTo(CredentialState.Valid));
324-
Assert.That(result.CoderUrl?.ToString(), Is.EqualTo(TestServerUrl));
320+
// The load should have been cancelled.
321+
Assert.ThrowsAsync<TaskCanceledException>(() => loadTask);
325322
}
326323
}

‎Vpn.Proto/vpn.proto

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -62,7 +62,7 @@ message ServiceMessage {
6262
StartResponse start = 2;
6363
StopResponse stop = 3;
6464
Status status = 4; // either in reply to a StatusRequest or broadcasted
65-
StartProgress start_progress = 5; // broadcasted during startup (used exclusively by Windows)
65+
StartProgress start_progress = 5; // broadcasted during startup
6666
}
6767
}
6868

@@ -214,7 +214,6 @@ message NetworkSettingsResponse {
214214
// StartResponse.
215215
message StartRequest {
216216
int32 tunnel_file_descriptor = 1;
217-
bool tunnel_use_soft_net_isolation = 8;
218217
string coder_url = 2;
219218
string api_token = 3;
220219
// Additional HTTP headers added to all requests

0 commit comments

Comments
 (0)