From 6b3851d9ddc9f81a1befa465442c77fb2db29a90 Mon Sep 17 00:00:00 2001 From: Spike Curtis Date: Tue, 13 May 2025 09:44:32 +0400 Subject: [PATCH] feat: add check for coder:// URI authority section (#97) Fixes #52 Checks for the authority string, i.e. `coder.example.com` in `coder://coder.example.com/v0/open/...` links matches the HTTP(S) URL we are signed into. This ensures that the names we use are properly scoped and links generated on one Coder deployment won't accidentally open workspaces on another. --- App/Services/UriHandler.cs | 37 +++++++++- Tests.App/Services/UriHandlerTest.cs | 100 ++++++++++++++++++++++++--- 2 files changed, 124 insertions(+), 13 deletions(-) diff --git a/App/Services/UriHandler.cs b/App/Services/UriHandler.cs index b0b0a9a..16a2543 100644 --- a/App/Services/UriHandler.cs +++ b/App/Services/UriHandler.cs @@ -20,7 +20,8 @@ public class UriHandler( ILogger logger, IRpcController rpcController, IUserNotifier userNotifier, - IRdpConnector rdpConnector) : IUriHandler + IRdpConnector rdpConnector, + ICredentialManager credentialManager) : IUriHandler { private const string OpenWorkspacePrefix = "/v0/open/ws/"; @@ -64,11 +65,13 @@ private async Task HandleUriThrowingErrors(Uri uri, CancellationToken ct = defau public async Task HandleOpenWorkspaceApp(Uri uri, CancellationToken ct = default) { const string errTitle = "Open Workspace Application Error"; + CheckAuthority(uri, errTitle); + var subpath = uri.AbsolutePath[OpenWorkspacePrefix.Length..]; var components = subpath.Split("/"); if (components.Length != 4 || components[1] != "agent") { - logger.LogWarning("unsupported open workspace app format in URI {path}", uri.AbsolutePath); + logger.LogWarning("unsupported open workspace app format in URI '{path}'", uri.AbsolutePath); throw new UriException(errTitle, $"Failed to open '{uri.AbsolutePath}' because the format is unsupported."); } @@ -120,6 +123,36 @@ public async Task HandleOpenWorkspaceApp(Uri uri, CancellationToken ct = default await OpenRDP(agent.Fqdn.First(), uri.Query, ct); } + private void CheckAuthority(Uri uri, string errTitle) + { + if (string.IsNullOrEmpty(uri.Authority)) + { + logger.LogWarning("cannot open workspace app without a URI authority on path '{path}'", uri.AbsolutePath); + throw new UriException(errTitle, + $"Failed to open '{uri.AbsolutePath}' because no Coder server was given in the URI"); + } + + var credentialModel = credentialManager.GetCachedCredentials(); + if (credentialModel.State != CredentialState.Valid) + { + logger.LogWarning("cannot open workspace app because credentials are '{state}'", credentialModel.State); + throw new UriException(errTitle, + $"Failed to open '{uri.AbsolutePath}' because you are not signed in."); + } + + // here we assume that the URL is non-null since the credentials are marked valid. If not it's an internal error + // and the App will handle catching the exception and logging it. + var coderUri = credentialModel.CoderUrl!; + if (uri.Authority != coderUri.Authority) + { + logger.LogWarning( + "cannot open workspace app because it was for '{uri_authority}', be we are signed into '{signed_in_authority}'", + uri.Authority, coderUri.Authority); + throw new UriException(errTitle, + $"Failed to open workspace app because it was for '{uri.Authority}', be we are signed into '{coderUri.Authority}'"); + } + } + public async Task OpenRDP(string domainName, string queryString, CancellationToken ct = default) { const string errTitle = "Workspace Remote Desktop Error"; diff --git a/Tests.App/Services/UriHandlerTest.cs b/Tests.App/Services/UriHandlerTest.cs index 65c886c..069d8b8 100644 --- a/Tests.App/Services/UriHandlerTest.cs +++ b/Tests.App/Services/UriHandlerTest.cs @@ -23,17 +23,23 @@ public void SetupMocksAndUriHandler() _mUserNotifier = new Mock(MockBehavior.Strict); _mRdpConnector = new Mock(MockBehavior.Strict); _mRpcController = new Mock(MockBehavior.Strict); + _mCredentialManager = new Mock(MockBehavior.Strict); - uriHandler = new UriHandler(logger, _mRpcController.Object, _mUserNotifier.Object, _mRdpConnector.Object); + uriHandler = new UriHandler(logger, + _mRpcController.Object, + _mUserNotifier.Object, + _mRdpConnector.Object, + _mCredentialManager.Object); } private Mock _mUserNotifier; private Mock _mRdpConnector; private Mock _mRpcController; + private Mock _mCredentialManager; private UriHandler uriHandler; // Unit under test. [SetUp] - public void AgentAndWorkspaceFixtures() + public void AgentWorkspaceAndCredentialFixtures() { agent11 = new Agent(); agent11.Fqdn.Add("workspace1.coder"); @@ -54,72 +60,91 @@ public void AgentAndWorkspaceFixtures() Workspaces = [workspace1], Agents = [agent11], }; + + credentialModel1 = new CredentialModel + { + State = CredentialState.Valid, + CoderUrl = new Uri("https://coder.test/"), + }; } private Agent agent11; private Workspace workspace1; private RpcModel modelWithWorkspace1; + private CredentialModel credentialModel1; [Test(Description = "Open RDP with username & password")] [CancelAfter(30_000)] public async Task Mainline(CancellationToken ct) { - var input = new Uri("coder:/v0/open/ws/workspace1/agent/agent11/rdp?username=testy&password=sesame"); + var input = new Uri( + "coder://coder.test/v0/open/ws/workspace1/agent/agent11/rdp?username=testy&password=sesame"); + _mCredentialManager.Setup(m => m.GetCachedCredentials()).Returns(credentialModel1); _mRpcController.Setup(m => m.GetState()).Returns(modelWithWorkspace1); var expectedCred = new RdpCredentials("testy", "sesame"); _ = _mRdpConnector.Setup(m => m.WriteCredentials(agent11.Fqdn[0], expectedCred)); _ = _mRdpConnector.Setup(m => m.Connect(agent11.Fqdn[0], IRdpConnector.DefaultPort, ct)) .Returns(Task.CompletedTask); await uriHandler.HandleUri(input, ct); + _mRdpConnector.Verify(m => m.WriteCredentials(It.IsAny(), It.IsAny())); + _mRdpConnector.Verify(m => m.Connect(It.IsAny(), It.IsAny(), ct), Times.Once); } [Test(Description = "Open RDP with no credentials")] [CancelAfter(30_000)] public async Task NoCredentials(CancellationToken ct) { - var input = new Uri("coder:/v0/open/ws/workspace1/agent/agent11/rdp"); + var input = new Uri("coder://coder.test/v0/open/ws/workspace1/agent/agent11/rdp"); + _mCredentialManager.Setup(m => m.GetCachedCredentials()).Returns(credentialModel1); _mRpcController.Setup(m => m.GetState()).Returns(modelWithWorkspace1); _ = _mRdpConnector.Setup(m => m.Connect(agent11.Fqdn[0], IRdpConnector.DefaultPort, ct)) .Returns(Task.CompletedTask); await uriHandler.HandleUri(input, ct); + _mRdpConnector.Verify(m => m.Connect(It.IsAny(), It.IsAny(), ct), Times.Once); } [Test(Description = "Unknown app slug")] [CancelAfter(30_000)] public async Task UnknownApp(CancellationToken ct) { - var input = new Uri("coder:/v0/open/ws/workspace1/agent/agent11/someapp"); + var input = new Uri("coder://coder.test/v0/open/ws/workspace1/agent/agent11/someapp"); + _mCredentialManager.Setup(m => m.GetCachedCredentials()).Returns(credentialModel1); _mRpcController.Setup(m => m.GetState()).Returns(modelWithWorkspace1); _mUserNotifier.Setup(m => m.ShowErrorNotification(It.IsAny(), It.IsRegex("someapp"), ct)) .Returns(Task.CompletedTask); await uriHandler.HandleUri(input, ct); + _mUserNotifier.Verify(m => m.ShowErrorNotification(It.IsAny(), It.IsAny(), ct), Times.Once()); } [Test(Description = "Unknown agent name")] [CancelAfter(30_000)] public async Task UnknownAgent(CancellationToken ct) { - var input = new Uri("coder:/v0/open/ws/workspace1/agent/wrongagent/rdp"); + var input = new Uri("coder://coder.test/v0/open/ws/workspace1/agent/wrongagent/rdp"); + _mCredentialManager.Setup(m => m.GetCachedCredentials()).Returns(credentialModel1); _mRpcController.Setup(m => m.GetState()).Returns(modelWithWorkspace1); _mUserNotifier.Setup(m => m.ShowErrorNotification(It.IsAny(), It.IsRegex("wrongagent"), ct)) .Returns(Task.CompletedTask); await uriHandler.HandleUri(input, ct); + _mUserNotifier.Verify(m => m.ShowErrorNotification(It.IsAny(), It.IsAny(), ct), Times.Once()); } [Test(Description = "Unknown workspace name")] [CancelAfter(30_000)] public async Task UnknownWorkspace(CancellationToken ct) { - var input = new Uri("coder:/v0/open/ws/wrongworkspace/agent/agent11/rdp"); + var input = new Uri("coder://coder.test/v0/open/ws/wrongworkspace/agent/agent11/rdp"); + _mCredentialManager.Setup(m => m.GetCachedCredentials()).Returns(credentialModel1); _mRpcController.Setup(m => m.GetState()).Returns(modelWithWorkspace1); _mUserNotifier.Setup(m => m.ShowErrorNotification(It.IsAny(), It.IsRegex("wrongworkspace"), ct)) .Returns(Task.CompletedTask); await uriHandler.HandleUri(input, ct); + _mUserNotifier.Verify(m => m.ShowErrorNotification(It.IsAny(), It.IsAny(), ct), Times.Once()); } [Test(Description = "Malformed Query String")] @@ -127,21 +152,24 @@ public async Task UnknownWorkspace(CancellationToken ct) public async Task MalformedQuery(CancellationToken ct) { // there might be some query string that gets the parser to throw an exception, but I could not find one. - var input = new Uri("coder:/v0/open/ws/workspace1/agent/agent11/rdp?%&##"); + var input = new Uri("coder://coder.test/v0/open/ws/workspace1/agent/agent11/rdp?%&##"); + _mCredentialManager.Setup(m => m.GetCachedCredentials()).Returns(credentialModel1); _mRpcController.Setup(m => m.GetState()).Returns(modelWithWorkspace1); // treated the same as if we just didn't include credentials _ = _mRdpConnector.Setup(m => m.Connect(agent11.Fqdn[0], IRdpConnector.DefaultPort, ct)) .Returns(Task.CompletedTask); await uriHandler.HandleUri(input, ct); + _mRdpConnector.Verify(m => m.Connect(It.IsAny(), It.IsAny(), ct), Times.Once); } [Test(Description = "VPN not started")] [CancelAfter(30_000)] public async Task VPNNotStarted(CancellationToken ct) { - var input = new Uri("coder:/v0/open/ws/wrongworkspace/agent/agent11/rdp"); + var input = new Uri("coder://coder.test/v0/open/ws/wrongworkspace/agent/agent11/rdp"); + _mCredentialManager.Setup(m => m.GetCachedCredentials()).Returns(credentialModel1); _mRpcController.Setup(m => m.GetState()).Returns(new RpcModel { VpnLifecycle = VpnLifecycle.Starting, @@ -150,29 +178,79 @@ public async Task VPNNotStarted(CancellationToken ct) _mUserNotifier.Setup(m => m.ShowErrorNotification(It.IsAny(), It.IsRegex("Coder Connect"), ct)) .Returns(Task.CompletedTask); await uriHandler.HandleUri(input, ct); + _mUserNotifier.Verify(m => m.ShowErrorNotification(It.IsAny(), It.IsAny(), ct), Times.Once()); } [Test(Description = "Wrong number of components")] [CancelAfter(30_000)] public async Task UnknownNumComponents(CancellationToken ct) { - var input = new Uri("coder:/v0/open/ws/wrongworkspace/agent11/rdp"); + var input = new Uri("coder://coder.test/v0/open/ws/wrongworkspace/agent11/rdp"); + _mCredentialManager.Setup(m => m.GetCachedCredentials()).Returns(credentialModel1); _mRpcController.Setup(m => m.GetState()).Returns(modelWithWorkspace1); _mUserNotifier.Setup(m => m.ShowErrorNotification(It.IsAny(), It.IsAny(), ct)) .Returns(Task.CompletedTask); await uriHandler.HandleUri(input, ct); + _mUserNotifier.Verify(m => m.ShowErrorNotification(It.IsAny(), It.IsAny(), ct), Times.Once()); } [Test(Description = "Unknown prefix")] [CancelAfter(30_000)] public async Task UnknownPrefix(CancellationToken ct) { - var input = new Uri("coder:/v300/open/ws/workspace1/agent/agent11/rdp"); + var input = new Uri("coder://coder.test/v300/open/ws/workspace1/agent/agent11/rdp"); + _mCredentialManager.Setup(m => m.GetCachedCredentials()).Returns(credentialModel1); _mRpcController.Setup(m => m.GetState()).Returns(modelWithWorkspace1); _mUserNotifier.Setup(m => m.ShowErrorNotification(It.IsAny(), It.IsAny(), ct)) .Returns(Task.CompletedTask); await uriHandler.HandleUri(input, ct); + _mUserNotifier.Verify(m => m.ShowErrorNotification(It.IsAny(), It.IsAny(), ct), Times.Once()); + } + + [Test(Description = "Unknown authority")] + [CancelAfter(30_000)] + public async Task UnknownAuthority(CancellationToken ct) + { + var input = new Uri("coder://unknown.test/v0/open/ws/workspace1/agent/agent11/rdp"); + + _mCredentialManager.Setup(m => m.GetCachedCredentials()).Returns(credentialModel1); + _mRpcController.Setup(m => m.GetState()).Returns(modelWithWorkspace1); + _mUserNotifier.Setup(m => m.ShowErrorNotification(It.IsAny(), It.IsRegex(@"unknown\.test"), ct)) + .Returns(Task.CompletedTask); + await uriHandler.HandleUri(input, ct); + _mUserNotifier.Verify(m => m.ShowErrorNotification(It.IsAny(), It.IsAny(), ct), Times.Once()); + } + + [Test(Description = "Missing authority")] + [CancelAfter(30_000)] + public async Task MissingAuthority(CancellationToken ct) + { + var input = new Uri("coder:/v0/open/ws/workspace1/agent/agent11/rdp"); + + _mCredentialManager.Setup(m => m.GetCachedCredentials()).Returns(credentialModel1); + _mRpcController.Setup(m => m.GetState()).Returns(modelWithWorkspace1); + _mUserNotifier.Setup(m => m.ShowErrorNotification(It.IsAny(), It.IsRegex("Coder server"), ct)) + .Returns(Task.CompletedTask); + await uriHandler.HandleUri(input, ct); + _mUserNotifier.Verify(m => m.ShowErrorNotification(It.IsAny(), It.IsAny(), ct), Times.Once()); + } + + [Test(Description = "Not signed in")] + [CancelAfter(30_000)] + public async Task NotSignedIn(CancellationToken ct) + { + var input = new Uri("coder://coder.test/v0/open/ws/workspace1/agent/agent11/rdp"); + + _mCredentialManager.Setup(m => m.GetCachedCredentials()).Returns(new CredentialModel() + { + State = CredentialState.Invalid, + }); + _mRpcController.Setup(m => m.GetState()).Returns(modelWithWorkspace1); + _mUserNotifier.Setup(m => m.ShowErrorNotification(It.IsAny(), It.IsRegex("signed in"), ct)) + .Returns(Task.CompletedTask); + await uriHandler.HandleUri(input, ct); + _mUserNotifier.Verify(m => m.ShowErrorNotification(It.IsAny(), It.IsAny(), ct), Times.Once()); } }