Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Do not require token auth with mTLS #378

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -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": ""
},
Expand Down
24 changes: 24 additions & 0 deletions src/api.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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"))
Expand All @@ -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) => {
Expand Down
136 changes: 78 additions & 58 deletions src/commands.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -147,78 +147,33 @@ export class Commands {
// 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()

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, args[1])
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.",
},
Expand All @@ -234,6 +189,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.
*/
Expand Down
10 changes: 7 additions & 3 deletions src/extension.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -92,8 +92,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)
Expand Down
10 changes: 5 additions & 5 deletions src/storage.ts
Original file line number Diff line number Diff line change
Expand Up @@ -435,8 +435,8 @@ 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) {
await Promise.all([this.updateUrlForCli(label, url), this.updateTokenForCli(label, token)])
Expand All @@ -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 ?? "")
}
}

Expand Down