From b9bb8ddd5e7e2a6bfdbe0e4f5ae7687b6b48fa29 Mon Sep 17 00:00:00 2001 From: Steven Masley Date: Wed, 22 Mar 2023 16:49:15 -0500 Subject: [PATCH 1/7] feat: Add feature to allow config-ssh value overrides --- src/sshConfig.test.ts | 38 ++++++++++++++++++++++++++++++ src/sshConfig.ts | 55 ++++++++++++++++++++++++++++++++++++++++--- 2 files changed, 90 insertions(+), 3 deletions(-) diff --git a/src/sshConfig.test.ts b/src/sshConfig.test.ts index c44c5e46..afe41bce 100644 --- a/src/sshConfig.test.ts +++ b/src/sshConfig.test.ts @@ -167,3 +167,41 @@ Host coder-vscode--* mode: 384, }) }) + +it("override values", async () => { + mockFileSystem.readFile.mockRejectedValueOnce("No file found") + const sshConfig = new SSHConfig(sshFilePath, mockFileSystem, { + ssh_config_options: { + loglevel: "DEBUG", // This tests case insensitive + ConnectTimeout: "500", + ExtraKey: "ExtraValue", + Foo: "bar", + Buzz: "baz", + }, + hostname_prefix: "", + }) + await sshConfig.load() + await sshConfig.update({ + Host: "coder-vscode--*", + ProxyCommand: "some-command-here", + ConnectTimeout: "0", + StrictHostKeyChecking: "no", + UserKnownHostsFile: "/dev/null", + LogLevel: "ERROR", + }) + + const expectedOutput = `# --- START CODER VSCODE --- +Host coder-vscode--* + ProxyCommand some-command-here + ConnectTimeout 500 + StrictHostKeyChecking no + UserKnownHostsFile /dev/null + LogLevel DEBUG + Buzz baz + ExtraKey ExtraValue + Foo bar +# --- END CODER VSCODE ---` + + expect(mockFileSystem.readFile).toBeCalledWith(sshFilePath, expect.anything()) + expect(mockFileSystem.writeFile).toBeCalledWith(sshFilePath, expectedOutput, expect.anything()) +}) \ No newline at end of file diff --git a/src/sshConfig.ts b/src/sshConfig.ts index 9fdf2bea..5950275b 100644 --- a/src/sshConfig.ts +++ b/src/sshConfig.ts @@ -1,3 +1,4 @@ +import { SSHConfigResponse } from "coder/site/src/api/typesGenerated" import { writeFile, readFile } from "fs/promises" import { ensureDir } from "fs-extra" import path from "path" @@ -30,16 +31,27 @@ const defaultFileSystem: FileSystem = { writeFile, } +const defaultSSHConfigResponse: SSHConfigResponse = { + ssh_config_options: {}, + hostname_prefix: "coder.", +} + export class SSHConfig { private filePath: string private fileSystem: FileSystem + private deploymentConfig: SSHConfigResponse private raw: string | undefined private startBlockComment = "# --- START CODER VSCODE ---" private endBlockComment = "# --- END CODER VSCODE ---" - constructor(filePath: string, fileSystem: FileSystem = defaultFileSystem) { + constructor( + filePath: string, + fileSystem: FileSystem = defaultFileSystem, + sshConfig: SSHConfigResponse = defaultSSHConfigResponse, + ) { this.filePath = filePath this.fileSystem = fileSystem + this.deploymentConfig = sshConfig } async load() { @@ -59,7 +71,7 @@ export class SSHConfig { if (block) { this.eraseBlock(block) } - this.appendBlock(values) + this.appendBlock(values, this.deploymentConfig.ssh_config_options) await this.save() } @@ -102,12 +114,49 @@ export class SSHConfig { this.raw = this.getRaw().replace(block.raw, "") } - private appendBlock({ Host, ...otherValues }: SSHValues) { + /** + * + * appendBlock builds the ssh config block. The order of the keys is determinstic based on the input. + * Expected values are always in a consistent order followed by any additional overrides in sorted order. + * + * @param param0 - SSHValues are the expected SSH values for using ssh with coder. + * @param overrides - Overrides typically come from the deployment api and are used to override the default values. + * The overrides are given as key:value pairs where the key is the ssh config file key. + * If the key matches an expected value, the expected value is overridden. If it does not + * match an expected value, it is appended to the end of the block. + */ + private appendBlock({ Host, ...otherValues }: SSHValues, overrides: Record) { const lines = [this.startBlockComment, `Host ${Host}`] + // We need to do a case insensitive match for the overrides as ssh config keys are case insensitive. + // To get the correct key:value, use: + // key = caseInsensitiveOverrides[key.toLowerCase()] + // value = overrides[key] + const caseInsensitiveOverrides: Record = {} + Object.keys(overrides).forEach((key) => { + caseInsensitiveOverrides[key.toLowerCase()] = key + }) + const keys = Object.keys(otherValues) as Array keys.forEach((key) => { + const lower = key.toLowerCase() + if (caseInsensitiveOverrides[lower]) { + const correctCaseKey = caseInsensitiveOverrides[lower] + // If the key is in overrides, use the override value. + // Doing it this way maintains the default order of the keys. + lines.push(this.withIndentation(`${key} ${overrides[correctCaseKey]}`)) + // Remove the key from the overrides so we don't write it again. + delete caseInsensitiveOverrides[lower] + return + } lines.push(this.withIndentation(`${key} ${otherValues[key]}`)) }) + // Write remaining overrides that have not been written yet. Sort to maintain deterministic order. + const remainingKeys = (Object.keys(caseInsensitiveOverrides) as Array).sort() + remainingKeys.forEach((key) => { + const correctKey = caseInsensitiveOverrides[key] + lines.push(this.withIndentation(`${key} ${overrides[correctKey]}`)) + }) + lines.push(this.endBlockComment) const raw = this.getRaw() From 6caaf9b9798355d64fed51a267407bef17f70e5f Mon Sep 17 00:00:00 2001 From: Steven Masley Date: Thu, 23 Mar 2023 13:35:43 -0500 Subject: [PATCH 2/7] Call api for ssh config overrides --- src/remote.ts | 38 +++++++++++++++++++++++++++++++++++--- src/sshConfig.ts | 15 +++++---------- yarn.lock | 2 +- 3 files changed, 41 insertions(+), 14 deletions(-) diff --git a/src/remote.ts b/src/remote.ts index cf47f965..a72b4298 100644 --- a/src/remote.ts +++ b/src/remote.ts @@ -6,8 +6,9 @@ import { getWorkspaceBuildLogs, getWorkspaceByOwnerAndName, startWorkspace, + getDeploymentSSHConfig, } from "coder/site/src/api/api" -import { ProvisionerJobLog, Workspace, WorkspaceAgent } from "coder/site/src/api/typesGenerated" +import { ProvisionerJobLog, SSHConfigResponse, Workspace, WorkspaceAgent } from "coder/site/src/api/typesGenerated" import EventSource from "eventsource" import find from "find-process" import * as fs from "fs/promises" @@ -18,7 +19,7 @@ import prettyBytes from "pretty-bytes" import * as semver from "semver" import * as vscode from "vscode" import * as ws from "ws" -import { SSHConfig } from "./sshConfig" +import { SSHConfig, defaultSSHConfigResponse } from "./sshConfig" import { Storage } from "./storage" export class Remote { @@ -440,6 +441,37 @@ export class Remote { // updateSSHConfig updates the SSH configuration with a wildcard that handles // all Coder entries. private async updateSSHConfig() { + let deploymentConfig: SSHConfigResponse = defaultSSHConfigResponse + try { + deploymentConfig = await getDeploymentSSHConfig() + } catch (error) { + if (!axios.isAxiosError(error)) { + throw error + } + switch (error.response?.status) { + case 404: { + // Deployment does not support overriding ssh config yet. Likely an + // older version, just use the default. + deploymentConfig = defaultSSHConfigResponse + break + } + case 401: { + const result = await this.vscodeProposed.window.showInformationMessage( + "Your session expired...", + { + useCustom: true, + modal: true, + detail: "You must login again to access your workspace.", + }, + "Login", + ) + throw error + } + default: + throw error + } + } + let sshConfigFile = vscode.workspace.getConfiguration().get("remote.SSH.configFile") if (!sshConfigFile) { sshConfigFile = path.join(os.homedir(), ".ssh", "config") @@ -480,7 +512,7 @@ export class Remote { SetEnv: "CODER_SSH_SESSION_TYPE=vscode", } - await sshConfig.update(sshValues) + await sshConfig.update(sshValues, deploymentConfig) } // showNetworkUpdates finds the SSH process ID that is being used by this diff --git a/src/sshConfig.ts b/src/sshConfig.ts index 5950275b..9c9a4bb7 100644 --- a/src/sshConfig.ts +++ b/src/sshConfig.ts @@ -31,27 +31,22 @@ const defaultFileSystem: FileSystem = { writeFile, } -const defaultSSHConfigResponse: SSHConfigResponse = { +export const defaultSSHConfigResponse: SSHConfigResponse = { ssh_config_options: {}, + // The prefix is not used by the vscode-extension hostname_prefix: "coder.", } export class SSHConfig { private filePath: string private fileSystem: FileSystem - private deploymentConfig: SSHConfigResponse private raw: string | undefined private startBlockComment = "# --- START CODER VSCODE ---" private endBlockComment = "# --- END CODER VSCODE ---" - constructor( - filePath: string, - fileSystem: FileSystem = defaultFileSystem, - sshConfig: SSHConfigResponse = defaultSSHConfigResponse, - ) { + constructor(filePath: string, fileSystem: FileSystem = defaultFileSystem) { this.filePath = filePath this.fileSystem = fileSystem - this.deploymentConfig = sshConfig } async load() { @@ -63,7 +58,7 @@ export class SSHConfig { } } - async update(values: SSHValues) { + async update(values: SSHValues, overrides: SSHConfigResponse = defaultSSHConfigResponse) { // We should remove this in March 2023 because there is not going to have // old configs this.cleanUpOldConfig() @@ -71,7 +66,7 @@ export class SSHConfig { if (block) { this.eraseBlock(block) } - this.appendBlock(values, this.deploymentConfig.ssh_config_options) + this.appendBlock(values, overrides.ssh_config_options) await this.save() } diff --git a/yarn.lock b/yarn.lock index f0a31e41..775f0269 100644 --- a/yarn.lock +++ b/yarn.lock @@ -1418,7 +1418,7 @@ co@3.1.0: "coder@https://github.com/coder/coder": version "0.0.0" - resolved "https://github.com/coder/coder#7a1731b6205d9c68f6308ee362ff2d62124b6950" + resolved "https://github.com/coder/coder#a6fa8cac582f2fc54eca0191bd54fd43d6d67ac2" collapse-white-space@^1.0.2: version "1.0.6" From 7eaa9844ea2d34c30706edb6c06d62866acf1430 Mon Sep 17 00:00:00 2001 From: Steven Masley Date: Thu, 23 Mar 2023 13:39:41 -0500 Subject: [PATCH 3/7] Fix unit test --- src/sshConfig.test.ts | 39 +++++++++++++++++++++------------------ 1 file changed, 21 insertions(+), 18 deletions(-) diff --git a/src/sshConfig.test.ts b/src/sshConfig.test.ts index afe41bce..7d693402 100644 --- a/src/sshConfig.test.ts +++ b/src/sshConfig.test.ts @@ -170,25 +170,28 @@ Host coder-vscode--* it("override values", async () => { mockFileSystem.readFile.mockRejectedValueOnce("No file found") - const sshConfig = new SSHConfig(sshFilePath, mockFileSystem, { - ssh_config_options: { - loglevel: "DEBUG", // This tests case insensitive - ConnectTimeout: "500", - ExtraKey: "ExtraValue", - Foo: "bar", - Buzz: "baz", - }, - hostname_prefix: "", - }) + const sshConfig = new SSHConfig(sshFilePath, mockFileSystem) await sshConfig.load() - await sshConfig.update({ - Host: "coder-vscode--*", - ProxyCommand: "some-command-here", - ConnectTimeout: "0", - StrictHostKeyChecking: "no", - UserKnownHostsFile: "/dev/null", - LogLevel: "ERROR", - }) + await sshConfig.update( + { + Host: "coder-vscode--*", + ProxyCommand: "some-command-here", + ConnectTimeout: "0", + StrictHostKeyChecking: "no", + UserKnownHostsFile: "/dev/null", + LogLevel: "ERROR", + }, + { + ssh_config_options: { + loglevel: "DEBUG", // This tests case insensitive + ConnectTimeout: "500", + ExtraKey: "ExtraValue", + Foo: "bar", + Buzz: "baz", + }, + hostname_prefix: "", + }, + ) const expectedOutput = `# --- START CODER VSCODE --- Host coder-vscode--* From 667ad9d03114267d0bcc8a408eabced65fb6c7ca Mon Sep 17 00:00:00 2001 From: Steven Masley Date: Thu, 23 Mar 2023 13:43:19 -0500 Subject: [PATCH 4/7] Fix getUser -> getAuthenticatedUser --- src/extension.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/extension.ts b/src/extension.ts index a6078ba6..e5e73cd7 100644 --- a/src/extension.ts +++ b/src/extension.ts @@ -1,6 +1,6 @@ "use strict" -import { getUser } from "coder/site/src/api/api" +import { getAuthenticatedUser } from "coder/site/src/api/api" import * as module from "module" import * as vscode from "vscode" import { Commands } from "./commands" @@ -12,7 +12,7 @@ export async function activate(ctx: vscode.ExtensionContext): Promise { const storage = new Storage(output, ctx.globalState, ctx.secrets, ctx.globalStorageUri, ctx.logUri) await storage.init() - getUser() + getAuthenticatedUser() .then(() => { vscode.commands.executeCommand("setContext", "coder.authenticated", true) }) From 64c4cf853377ff5b9fe5edeb02e8a7a1aabceb3b Mon Sep 17 00:00:00 2001 From: Steven Masley Date: Thu, 23 Mar 2023 13:55:26 -0500 Subject: [PATCH 5/7] Handle authenticated user --- src/commands.ts | 39 +++++++++++++++++++++++---------------- src/remote.ts | 10 +--------- src/sshConfig.test.ts | 2 +- 3 files changed, 25 insertions(+), 26 deletions(-) diff --git a/src/commands.ts b/src/commands.ts index 4db02219..ccc2c653 100644 --- a/src/commands.ts +++ b/src/commands.ts @@ -1,5 +1,5 @@ import axios from "axios" -import { getUser, getWorkspaces, updateWorkspaceVersion } from "coder/site/src/api/api" +import { getAuthenticatedUser, getWorkspaces, updateWorkspaceVersion } from "coder/site/src/api/api" import { Workspace, WorkspaceAgent } from "coder/site/src/api/typesGenerated" import * as vscode from "vscode" import { Remote } from "./remote" @@ -73,21 +73,28 @@ export class Commands { await this.storage.setURL(url) await this.storage.setSessionToken(token) - const user = await getUser() - await vscode.commands.executeCommand("setContext", "coder.authenticated", true) - vscode.window - .showInformationMessage( - `Welcome to Coder, ${user.username}!`, - { - detail: "You can now use the Coder extension to manage your Coder instance.", - }, - "Open Workspace", - ) - .then((action) => { - if (action === "Open Workspace") { - vscode.commands.executeCommand("coder.open") - } - }) + try { + const user = await getAuthenticatedUser() + if (!user) { + throw new Error("Failed to get authenticated user") + } + await vscode.commands.executeCommand("setContext", "coder.authenticated", true) + vscode.window + .showInformationMessage( + `Welcome to Coder, ${user.username}!`, + { + detail: "You can now use the Coder extension to manage your Coder instance.", + }, + "Open Workspace", + ) + .then((action) => { + if (action === "Open Workspace") { + vscode.commands.executeCommand("coder.open") + } + }) + } catch (error) { + vscode.window.showErrorMessage("Failed to authenticate with Coder: " + error) + } } public async logout(): Promise { diff --git a/src/remote.ts b/src/remote.ts index a72b4298..8c11c1aa 100644 --- a/src/remote.ts +++ b/src/remote.ts @@ -456,15 +456,7 @@ export class Remote { break } case 401: { - const result = await this.vscodeProposed.window.showInformationMessage( - "Your session expired...", - { - useCustom: true, - modal: true, - detail: "You must login again to access your workspace.", - }, - "Login", - ) + await this.vscodeProposed.window.showErrorMessage("Your session expired...") throw error } default: diff --git a/src/sshConfig.test.ts b/src/sshConfig.test.ts index 7d693402..5bd22ecc 100644 --- a/src/sshConfig.test.ts +++ b/src/sshConfig.test.ts @@ -207,4 +207,4 @@ Host coder-vscode--* expect(mockFileSystem.readFile).toBeCalledWith(sshFilePath, expect.anything()) expect(mockFileSystem.writeFile).toBeCalledWith(sshFilePath, expectedOutput, expect.anything()) -}) \ No newline at end of file +}) From 67d5d0581a5233e937ea2112ca3ac03a69d6fa8b Mon Sep 17 00:00:00 2001 From: Steven Masley Date: Thu, 23 Mar 2023 15:29:30 -0500 Subject: [PATCH 6/7] Fix key casing --- src/sshConfig.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/sshConfig.ts b/src/sshConfig.ts index 9c9a4bb7..56ef31a7 100644 --- a/src/sshConfig.ts +++ b/src/sshConfig.ts @@ -149,7 +149,7 @@ export class SSHConfig { const remainingKeys = (Object.keys(caseInsensitiveOverrides) as Array).sort() remainingKeys.forEach((key) => { const correctKey = caseInsensitiveOverrides[key] - lines.push(this.withIndentation(`${key} ${overrides[correctKey]}`)) + lines.push(this.withIndentation(`${correctKey} ${overrides[correctKey]}`)) }) lines.push(this.endBlockComment) From bb7c40da0667a67ec3b09334c2f9ffcc9e1e3d4c Mon Sep 17 00:00:00 2001 From: Steven Masley Date: Tue, 28 Mar 2023 09:14:05 -0500 Subject: [PATCH 7/7] Support removing keys --- src/sshConfig.test.ts | 4 +++- src/sshConfig.ts | 18 ++++++++++++++---- 2 files changed, 17 insertions(+), 5 deletions(-) diff --git a/src/sshConfig.test.ts b/src/sshConfig.test.ts index 5bd22ecc..2c20d520 100644 --- a/src/sshConfig.test.ts +++ b/src/sshConfig.test.ts @@ -188,6 +188,9 @@ it("override values", async () => { ExtraKey: "ExtraValue", Foo: "bar", Buzz: "baz", + // Remove this key + StrictHostKeyChecking: "", + ExtraRemove: "", }, hostname_prefix: "", }, @@ -197,7 +200,6 @@ it("override values", async () => { Host coder-vscode--* ProxyCommand some-command-here ConnectTimeout 500 - StrictHostKeyChecking no UserKnownHostsFile /dev/null LogLevel DEBUG Buzz baz diff --git a/src/sshConfig.ts b/src/sshConfig.ts index 56ef31a7..21c5a2e7 100644 --- a/src/sshConfig.ts +++ b/src/sshConfig.ts @@ -136,11 +136,17 @@ export class SSHConfig { const lower = key.toLowerCase() if (caseInsensitiveOverrides[lower]) { const correctCaseKey = caseInsensitiveOverrides[lower] - // If the key is in overrides, use the override value. - // Doing it this way maintains the default order of the keys. - lines.push(this.withIndentation(`${key} ${overrides[correctCaseKey]}`)) + const value = overrides[correctCaseKey] // Remove the key from the overrides so we don't write it again. delete caseInsensitiveOverrides[lower] + if (value === "") { + // If the value is empty, don't write it. Prevent writing the default + // value as well. + return + } + // If the key is in overrides, use the override value. + // Doing it this way maintains the default order of the keys. + lines.push(this.withIndentation(`${key} ${value}`)) return } lines.push(this.withIndentation(`${key} ${otherValues[key]}`)) @@ -149,7 +155,11 @@ export class SSHConfig { const remainingKeys = (Object.keys(caseInsensitiveOverrides) as Array).sort() remainingKeys.forEach((key) => { const correctKey = caseInsensitiveOverrides[key] - lines.push(this.withIndentation(`${correctKey} ${overrides[correctKey]}`)) + const value = overrides[correctKey] + // Only write the value if it is not empty. + if (value !== "") { + lines.push(this.withIndentation(`${correctKey} ${value}`)) + } }) lines.push(this.endBlockComment)