Do not require token auth with mTLS (#378)

Author: code-asher

package.json

@@ -74,17 +74,17 @@
           "default": ""
         },
         "coder.tlsCertFile": {
-          "markdownDescription": "Path to file for TLS client cert",
+          "markdownDescription": "Path to file for TLS client cert. When specified, token authorization will be skipped.",
           "type": "string",
           "default": ""
         },
         "coder.tlsKeyFile": {
-          "markdownDescription": "Path to file for TLS client key",
+          "markdownDescription": "Path to file for TLS client key. When specified, token authorization will be skipped.",
           "type": "string",
           "default": ""
         },
         "coder.tlsCaFile": {
-          "markdownDescription": "Path to file for TLS certificate authority",
+          "markdownDescription": "Path to file for TLS certificate authority.",
           "type": "string",
           "default": ""
         },

src/api.ts

@@ -10,6 +10,21 @@ import { getProxyForUrl } from "./proxy"
 import { Storage } from "./storage"
 import { expandPath } from "./util"
 
+/**
+ * Return whether the API will need a token for authorization.
+ * If mTLS is in use (as specified by the cert or key files being set) then
+ * token authorization is disabled.  Otherwise, it is enabled.
+ */
+export function needToken(): boolean {
+  const cfg = vscode.workspace.getConfiguration()
+  const certFile = expandPath(String(cfg.get("coder.tlsCertFile") ?? "").trim())
+  const keyFile = expandPath(String(cfg.get("coder.tlsKeyFile") ?? "").trim())
+  return !certFile && !keyFile
+}
+
+/**
+ * Create a new agent based off the current settings.
+ */
 async function createHttpAgent(): Promise<ProxyAgent> {
   const cfg = vscode.workspace.getConfiguration()
   const insecure = Boolean(cfg.get("coder.insecure"))
@@ -32,7 +47,16 @@ async function createHttpAgent(): Promise<ProxyAgent> {
   })
 }
 
+// The agent is a singleton so we only have to listen to the configuration once
+// (otherwise we would have to carefully dispose agents to remove their
+// configuration listeners), and to share the connection pool.
 let agent: Promise<ProxyAgent> | undefined = undefined
+
+/**
+ * Get the existing agent or create one if necessary.  On settings change,
+ * recreate the agent.  The agent on the client is not automatically updated;
+ * this must be called before every request to get the latest agent.
+ */
 async function getHttpAgent(): Promise<ProxyAgent> {
   if (!agent) {
     vscode.workspace.onDidChangeConfiguration((e) => {

src/commands.ts

@@ -2,7 +2,7 @@ import { Api } from "coder/site/src/api/api"
 import { getErrorMessage } from "coder/site/src/api/errors"
 import { User, Workspace, WorkspaceAgent } from "coder/site/src/api/typesGenerated"
 import * as vscode from "vscode"
-import { makeCoderSdk } from "./api"
+import { makeCoderSdk, needToken } from "./api"
 import { extractAgents } from "./api-helper"
 import { CertificateError } from "./error"
 import { Storage } from "./storage"
@@ -137,88 +137,48 @@ export class Commands {
    * ask for it first with a menu showing recent URLs and CODER_URL, if set.
    */
   public async login(...args: string[]): Promise<void> {
-    const url = await this.maybeAskUrl(args[0])
+    // Destructure would be nice but VS Code can pass undefined which errors.
+    const inputUrl = args[0]
+    const inputToken = args[1]
+    const inputLabel = args[2]
+
+    const url = await this.maybeAskUrl(inputUrl)
     if (!url) {
       return
     }
 
     // It is possible that we are trying to log into an old-style host, in which
     // case we want to write with the provided blank label instead of generating
     // a host label.
-    const label = typeof args[2] === "undefined" ? toSafeHost(url) : args[2]
-
-    // Use a temporary client to avoid messing with the global one while trying
-    // to log in.
-    const restClient = await makeCoderSdk(url, undefined, this.storage)
-
-    let user: User | undefined
-    let token: string | undefined = args[1]
-    if (!token) {
-      const opened = await vscode.env.openExternal(vscode.Uri.parse(`${url}/cli-auth`))
-      if (!opened) {
-        vscode.window.showWarningMessage("You must accept the URL prompt to generate an API key.")
-        return
-      }
-
-      token = await vscode.window.showInputBox({
-        title: "Coder API Key",
-        password: true,
-        placeHolder: "Copy your API key from the opened browser page.",
-        value: await this.storage.getSessionToken(),
-        ignoreFocusOut: true,
-        validateInput: async (value) => {
-          restClient.setSessionToken(value)
-          try {
-            user = await restClient.getAuthenticatedUser()
-            if (!user) {
-              throw new Error("Failed to get authenticated user")
-            }
-          } catch (err) {
-            // For certificate errors show both a notification and add to the
-            // text under the input box, since users sometimes miss the
-            // notification.
-            if (err instanceof CertificateError) {
-              err.showNotification()
+    const label = typeof inputLabel === "undefined" ? toSafeHost(url) : inputLabel
 
-              return {
-                message: err.x509Err || err.message,
-                severity: vscode.InputBoxValidationSeverity.Error,
-              }
-            }
-            // This could be something like the header command erroring or an
-            // invalid session token.
-            const message = getErrorMessage(err, "no response from the server")
-            return {
-              message: "Failed to authenticate: " + message,
-              severity: vscode.InputBoxValidationSeverity.Error,
-            }
-          }
-        },
-      })
-    }
-    if (!token || !user) {
-      return
+    // Try to get a token from the user, if we need one, and their user.
+    const res = await this.maybeAskToken(url, inputToken)
+    if (!res) {
+      return // The user aborted.
     }
 
-    // The URL and token are good; authenticate the global client.
+    // The URL is good and the token is either good or not required; authorize
+    // the global client.
     this.restClient.setHost(url)
-    this.restClient.setSessionToken(token)
+    this.restClient.setSessionToken(res.token)
 
     // Store these to be used in later sessions.
     await this.storage.setUrl(url)
-    await this.storage.setSessionToken(token)
+    await this.storage.setSessionToken(res.token)
 
     // Store on disk to be used by the cli.
-    await this.storage.configureCli(label, url, token)
+    await this.storage.configureCli(label, url, res.token)
 
+    // These contexts control various menu items and the sidebar.
     await vscode.commands.executeCommand("setContext", "coder.authenticated", true)
-    if (user.roles.find((role) => role.name === "owner")) {
+    if (res.user.roles.find((role) => role.name === "owner")) {
       await vscode.commands.executeCommand("setContext", "coder.isOwner", true)
     }
 
     vscode.window
       .showInformationMessage(
-        `Welcome to Coder, ${user.username}!`,
+        `Welcome to Coder, ${res.user.username}!`,
         {
           detail: "You can now use the Coder extension to manage your Coder instance.",
         },
@@ -234,6 +194,71 @@ export class Commands {
     vscode.commands.executeCommand("coder.refreshWorkspaces")
   }
 
+  /**
+   * If necessary, ask for a token, and keep asking until the token has been
+   * validated.  Return the token and user that was fetched to validate the
+   * token.
+   */
+  private async maybeAskToken(url: string, token: string): Promise<{ user: User; token: string } | null> {
+    const restClient = await makeCoderSdk(url, token, this.storage)
+    if (!needToken()) {
+      return {
+        // For non-token auth, we write a blank token since the `vscodessh`
+        // command currently always requires a token file.
+        token: "",
+        user: await restClient.getAuthenticatedUser(),
+      }
+    }
+
+    // This prompt is for convenience; do not error if they close it since
+    // they may already have a token or already have the page opened.
+    await vscode.env.openExternal(vscode.Uri.parse(`${url}/cli-auth`))
+
+    // For token auth, start with the existing token in the prompt or the last
+    // used token.  Once submitted, if there is a failure we will keep asking
+    // the user for a new token until they quit.
+    let user: User | undefined
+    const validatedToken = await vscode.window.showInputBox({
+      title: "Coder API Key",
+      password: true,
+      placeHolder: "Paste your API key.",
+      value: token || (await this.storage.getSessionToken()),
+      ignoreFocusOut: true,
+      validateInput: async (value) => {
+        restClient.setSessionToken(value)
+        try {
+          user = await restClient.getAuthenticatedUser()
+        } catch (err) {
+          // For certificate errors show both a notification and add to the
+          // text under the input box, since users sometimes miss the
+          // notification.
+          if (err instanceof CertificateError) {
+            err.showNotification()
+
+            return {
+              message: err.x509Err || err.message,
+              severity: vscode.InputBoxValidationSeverity.Error,
+            }
+          }
+          // This could be something like the header command erroring or an
+          // invalid session token.
+          const message = getErrorMessage(err, "no response from the server")
+          return {
+            message: "Failed to authenticate: " + message,
+            severity: vscode.InputBoxValidationSeverity.Error,
+          }
+        }
+      },
+    })
+
+    if (validatedToken && user) {
+      return { token: validatedToken, user }
+    }
+
+    // User aborted.
+    return null
+  }
+
   /**
    * View the logs for the currently connected workspace.
    */

src/extension.ts

@@ -3,7 +3,7 @@ import axios, { isAxiosError } from "axios"
 import { getErrorMessage } from "coder/site/src/api/errors"
 import * as module from "module"
 import * as vscode from "vscode"
-import { makeCoderSdk } from "./api"
+import { makeCoderSdk, needToken } from "./api"
 import { errToStr } from "./api-helper"
 import { Commands } from "./commands"
 import { CertificateError, getErrorDetail } from "./error"
@@ -96,8 +96,12 @@ export async function activate(ctx: vscode.ExtensionContext): Promise<void> {
         }
 
         // If the token is missing we will get a 401 later and the user will be
-        // prompted to sign in again, so we do not need to ensure it is set.
-        const token = params.get("token")
+        // prompted to sign in again, so we do not need to ensure it is set now.
+        // For non-token auth, we write a blank token since the `vscodessh`
+        // command currently always requires a token file.  However, if there is
+        // a query parameter for non-token auth go ahead and use it anyway; all
+        // that really matters is the file is created.
+        const token = needToken() ? params.get("token") : (params.get("token") ?? "")
         if (token) {
           restClient.setSessionToken(token)
           await storage.setSessionToken(token)

src/storage.ts

@@ -435,10 +435,10 @@ export class Storage {
   /**
    * Configure the CLI for the deployment with the provided label.
    *
-   * Falsey values are a no-op; we avoid unconfiguring the CLI to avoid breaking
-   * existing connections.
+   * Falsey URLs and null tokens are a no-op; we avoid unconfiguring the CLI to
+   * avoid breaking existing connections.
    */
-  public async configureCli(label: string, url: string | undefined, token: string | undefined | null) {
+  public async configureCli(label: string, url: string | undefined, token: string | null) {
     await Promise.all([this.updateUrlForCli(label, url), this.updateTokenForCli(label, token)])
   }
 
@@ -459,15 +459,15 @@ export class Storage {
   /**
    * Update the session token for a deployment with the provided label on disk
    * which can be used by the CLI via --session-token-file.  If the token is
-   * falsey, do nothing.
+   * null, do nothing.
    *
    * If the label is empty, read the old deployment-unaware config instead.
    */
   private async updateTokenForCli(label: string, token: string | undefined | null) {
-    if (token) {
+    if (token !== null) {
       const tokenPath = this.getSessionTokenPath(label)
       await fs.mkdir(path.dirname(tokenPath), { recursive: true })
-      await fs.writeFile(tokenPath, token)
+      await fs.writeFile(tokenPath, token ?? "")
     }
   }