From 8c2bb61af9ef3c7e6824a1f12e6ffbb815118bb6 Mon Sep 17 00:00:00 2001 From: Joe Previte Date: Thu, 3 Jun 2021 16:37:46 -0700 Subject: [PATCH] refactor: parse options with multiple = in cli There was a case with the hashed-password which had multiple equal signs in the value and it wasn't being parsed correctly. This uses a new function and adds a few tests. --- docs/FAQ.md | 66 ++++++++++++++++++++---------------------- package.json | 2 -- src/node/cli.ts | 23 ++------------- src/node/http.ts | 9 ++++-- src/node/util.ts | 6 ++-- test/package.json | 2 -- test/unit/cli.test.ts | 29 ++++++++++++++----- typings/pluginapi.d.ts | 6 +++- 8 files changed, 71 insertions(+), 72 deletions(-) diff --git a/docs/FAQ.md b/docs/FAQ.md index 0e11308d9..b7d57a8cf 100644 --- a/docs/FAQ.md +++ b/docs/FAQ.md @@ -2,39 +2,38 @@ # FAQ -- [FAQ](#faq) - - [Questions?](#questions) - - [iPad Status?](#ipad-status) - - [Community Projects (awesome-code-server)](#community-projects-awesome-code-server) - - [How can I reuse my VS Code configuration?](#how-can-i-reuse-my-vs-code-configuration) - - [Differences compared to VS Code?](#differences-compared-to-vs-code) - - [Installing an extension](#installing-an-extension) - - [How can I request a missing extension?](#how-can-i-request-a-missing-extension) - - [Installing an extension manually](#installing-an-extension-manually) - - [How do I configure the marketplace URL?](#how-do-i-configure-the-marketplace-url) - - [Where are extensions stored?](#where-are-extensions-stored) - - [How is this different from VS Code Codespaces?](#how-is-this-different-from-vs-code-codespaces) - - [How should I expose code-server to the internet?](#how-should-i-expose-code-server-to-the-internet) - - [Can I store my password hashed?](#can-i-store-my-password-hashed) - - [How do I securely access web services?](#how-do-i-securely-access-web-services) - - [Sub-paths](#sub-paths) - - [Sub-domains](#sub-domains) - - [Why does the code-server proxy strip `/proxy/` from the request path?](#why-does-the-code-server-proxy-strip-proxyport-from-the-request-path) - - [Proxying to Create React App](#proxying-to-create-react-app) - - [Multi-tenancy](#multi-tenancy) - - [Docker in code-server container?](#docker-in-code-server-container) - - [How can I disable telemetry?](#how-can-i-disable-telemetry) - - [How does code-server decide what workspace or folder to open?](#how-does-code-server-decide-what-workspace-or-folder-to-open) - - [How do I debug issues with code-server?](#how-do-i-debug-issues-with-code-server) - - [Heartbeat File](#heartbeat-file) - - [Healthz endpoint](#healthz-endpoint) - - [How does the config file work?](#how-does-the-config-file-work) - - [Isn't an install script piped into sh insecure?](#isnt-an-install-script-piped-into-sh-insecure) - - [How do I make my keyboard shortcuts work?](#how-do-i-make-my-keyboard-shortcuts-work) - - [How do I access my Documents/Downloads/Desktop folders in code-server on OSX?](#how-do-i-access-my-documentsdownloadsdesktop-folders-in-code-server-on-osx) - - [Differences compared to Theia?](#differences-compared-to-theia) - - [`$HTTP_PROXY`, `$HTTPS_PROXY`, `$NO_PROXY`](#http_proxy-https_proxy-no_proxy) - - [Enterprise](#enterprise) +- [Questions?](#questions) +- [iPad Status?](#ipad-status) +- [Community Projects (awesome-code-server)](#community-projects-awesome-code-server) +- [How can I reuse my VS Code configuration?](#how-can-i-reuse-my-vs-code-configuration) +- [Differences compared to VS Code?](#differences-compared-to-vs-code) + - [Installing an extension](#installing-an-extension) +- [How can I request a missing extension?](#how-can-i-request-a-missing-extension) +- [Installing an extension manually](#installing-an-extension-manually) +- [How do I configure the marketplace URL?](#how-do-i-configure-the-marketplace-url) +- [Where are extensions stored?](#where-are-extensions-stored) +- [How is this different from VS Code Codespaces?](#how-is-this-different-from-vs-code-codespaces) +- [How should I expose code-server to the internet?](#how-should-i-expose-code-server-to-the-internet) +- [Can I store my password hashed?](#can-i-store-my-password-hashed) +- [How do I securely access web services?](#how-do-i-securely-access-web-services) + - [Sub-paths](#sub-paths) + - [Sub-domains](#sub-domains) +- [Why does the code-server proxy strip `/proxy/` from the request path?](#why-does-the-code-server-proxy-strip-proxyport-from-the-request-path) + - [Proxying to Create React App](#proxying-to-create-react-app) +- [Multi-tenancy](#multi-tenancy) +- [Docker in code-server container?](#docker-in-code-server-container) +- [How can I disable telemetry?](#how-can-i-disable-telemetry) +- [How does code-server decide what workspace or folder to open?](#how-does-code-server-decide-what-workspace-or-folder-to-open) +- [How do I debug issues with code-server?](#how-do-i-debug-issues-with-code-server) +- [Heartbeat File](#heartbeat-file) +- [Healthz endpoint](#healthz-endpoint) +- [How does the config file work?](#how-does-the-config-file-work) +- [Isn't an install script piped into sh insecure?](#isnt-an-install-script-piped-into-sh-insecure) +- [How do I make my keyboard shortcuts work?](#how-do-i-make-my-keyboard-shortcuts-work) +- [How do I access my Documents/Downloads/Desktop folders in code-server on OSX?](#how-do-i-access-my-documentsdownloadsdesktop-folders-in-code-server-on-osx) +- [Differences compared to Theia?](#differences-compared-to-theia) +- [`$HTTP_PROXY`, `$HTTPS_PROXY`, `$NO_PROXY`](#http_proxy-https_proxy-no_proxy) +- [Enterprise](#enterprise) @@ -209,7 +208,6 @@ Yes you can! Set the value of `hashed-password` instead of `password`. Generate ```shell echo -n "password" | npx argon2-cli -e $argon2i$v=19$m=4096,t=3,p=1$wst5qhbgk2lu1ih4dmuxvg$ls1alrvdiwtvzhwnzcm1dugg+5dto3dt1d5v9xtlws4 - ``` Of course replace `thisismypassword` with your actual password and **remember to put it inside quotes**! diff --git a/package.json b/package.json index 32faa1ad1..4ca3ef36e 100644 --- a/package.json +++ b/package.json @@ -36,7 +36,6 @@ "main": "out/node/entry.js", "devDependencies": { "@schemastore/package": "^0.0.6", - "@types/bcrypt": "^5.0.0", "@types/body-parser": "^1.19.0", "@types/compression": "^1.7.0", "@types/cookie-parser": "^1.4.2", @@ -90,7 +89,6 @@ "dependencies": { "@coder/logger": "1.1.16", "argon2": "^0.28.0", - "bcrypt": "^5.0.1", "body-parser": "^1.19.0", "compression": "^1.7.4", "cookie-parser": "^1.4.5", diff --git a/src/node/cli.ts b/src/node/cli.ts index 01c933495..144e45485 100644 --- a/src/node/cli.ts +++ b/src/node/cli.ts @@ -247,14 +247,8 @@ export function splitOnFirstEquals(str: string): string[] { // $argon2i$v=19$m=4096,t=3,p=1$0qR/o+0t00hsbJFQCKSfdQ$oFcM4rL6o+B7oxpuA4qlXubypbBPsf+8L531U7P9HYY // 2 means return two items // Source: https://stackoverflow.com/a/4607799/3015595 - const split = str.split(/=(.+)/, 2) - - // It should always return two elements - // because it's used in a place where - // it expected two elements - if (split.length === 1) { - split.push("") - } + // We use the ? to say the the substr after the = is optional + const split = str.split(/=(.+)?/, 2) return split } @@ -289,17 +283,9 @@ export const parse = ( let key: keyof Args | undefined let value: string | undefined if (arg.startsWith("--")) { - // TODO fix this - const split = arg.replace(/^--/, "").split("=", 2) + const split = splitOnFirstEquals(arg.replace(/^--/, "")) key = split[0] as keyof Args value = split[1] - } else { - const short = arg.replace(/^-/, "") - const pair = Object.entries(options).find(([, v]) => v.short === short) - if (pair) { - key = pair[0] as keyof Args - } - } if (!key || !options[key]) { throw error(`Unknown option ${arg}`) @@ -563,7 +549,6 @@ export function parseConfigFile(configFile: string, configPath: string): ConfigA const config = yaml.load(configFile, { filename: configPath, }) - console.log("what is this config", config) if (!config || typeof config === "string") { throw new Error(`invalid config: ${config}`) } @@ -576,11 +561,9 @@ export function parseConfigFile(configFile: string, configPath: string): ConfigA } return `--${optName}=${opt}` }) - console.log("what are the configFileArgv", configFileArgv) const args = parse(configFileArgv, { configFile: configPath, }) - console.log(args, "args") return { ...args, config: configPath, diff --git a/src/node/http.ts b/src/node/http.ts index c907dd928..8b4c9d8c8 100644 --- a/src/node/http.ts +++ b/src/node/http.ts @@ -63,9 +63,10 @@ export const ensureAuthenticated = async ( */ export const authenticated = async (req: express.Request): Promise => { switch (req.args.auth) { - case AuthType.None: + case AuthType.None: { return true - case AuthType.Password: + } + case AuthType.Password: { // The password is stored in the cookie after being hashed. const hashedPasswordFromArgs = req.args["hashed-password"] const passwordMethod = getPasswordMethod(hashedPasswordFromArgs) @@ -77,8 +78,10 @@ export const authenticated = async (req: express.Request): Promise => { } return await isCookieValid(isCookieValidArgs) - default: + } + default: { throw new Error(`Unsupported auth type ${req.args.auth}`) + } } } diff --git a/src/node/util.ts b/src/node/util.ts index f575eb524..5a3d0b701 100644 --- a/src/node/util.ts +++ b/src/node/util.ts @@ -1,15 +1,15 @@ +import { logger } from "@coder/logger" +import * as argon2 from "argon2" import * as cp from "child_process" import * as crypto from "crypto" -import * as argon2 from "argon2" import envPaths from "env-paths" import { promises as fs } from "fs" import * as net from "net" import * as os from "os" import * as path from "path" +import safeCompare from "safe-compare" import * as util from "util" import xdgBasedir from "xdg-basedir" -import safeCompare from "safe-compare" -import { logger } from "@coder/logger" export interface Paths { data: string diff --git a/test/package.json b/test/package.json index 773c043f4..842cf4175 100644 --- a/test/package.json +++ b/test/package.json @@ -17,7 +17,5 @@ }, "resolutions": { "@playwright/test/playwright": "^1.11.0-next-alpha-apr-13-2021" - }, - "dependencies": { } } diff --git a/test/unit/cli.test.ts b/test/unit/cli.test.ts index 252bc56c3..2d6e252ad 100644 --- a/test/unit/cli.test.ts +++ b/test/unit/cli.test.ts @@ -349,6 +349,21 @@ describe("parser", () => { ], }) }) + it("should parse options with double-dash and multiple equal signs ", async () => { + const args = parse( + [ + "--hashed-password=$argon2i$v=19$m=4096,t=3,p=1$0qr/o+0t00hsbjfqcksfdq$ofcm4rl6o+b7oxpua4qlxubypbbpsf+8l531u7p9hyy", + ], + { + configFile: "/pathtoconfig", + }, + ) + expect(args).toEqual({ + _: [], + "hashed-password": + "$argon2i$v=19$m=4096,t=3,p=1$0qr/o+0t00hsbjfqcksfdq$ofcm4rl6o+b7oxpua4qlxubypbbpsf+8l531u7p9hyy", + }) + }) }) describe("cli", () => { @@ -426,25 +441,25 @@ describe("cli", () => { describe("splitOnFirstEquals", () => { it("should split on the first equals", () => { - const testStr = "--enabled-proposed-api=test=value" + const testStr = "enabled-proposed-api=test=value" const actual = splitOnFirstEquals(testStr) - const expected = ["--enabled-proposed-api", "test=value"] + const expected = ["enabled-proposed-api", "test=value"] expect(actual).toEqual(expect.arrayContaining(expected)) }) it("should split on first equals regardless of multiple equals signs", () => { const testStr = - "--hashed-password=$argon2i$v=19$m=4096,t=3,p=1$0qR/o+0t00hsbJFQCKSfdQ$oFcM4rL6o+B7oxpuA4qlXubypbBPsf+8L531U7P9HYY" + "hashed-password=$argon2i$v=19$m=4096,t=3,p=1$0qR/o+0t00hsbJFQCKSfdQ$oFcM4rL6o+B7oxpuA4qlXubypbBPsf+8L531U7P9HYY" const actual = splitOnFirstEquals(testStr) const expected = [ - "--hashed-password", + "hashed-password", "$argon2i$v=19$m=4096,t=3,p=1$0qR/o+0t00hsbJFQCKSfdQ$oFcM4rL6o+B7oxpuA4qlXubypbBPsf+8L531U7P9HYY", ] expect(actual).toEqual(expect.arrayContaining(expected)) }) - it("should always return two elements", () => { - const testStr = "" + it("should always return the first element before an equals", () => { + const testStr = "auth=" const actual = splitOnFirstEquals(testStr) - const expected = ["", ""] + const expected = ["auth"] expect(actual).toEqual(expect.arrayContaining(expected)) }) }) diff --git a/typings/pluginapi.d.ts b/typings/pluginapi.d.ts index 881aa1fb4..17f3ae197 100644 --- a/typings/pluginapi.d.ts +++ b/typings/pluginapi.d.ts @@ -145,7 +145,11 @@ export const proxy: ProxyServer /** * Middleware to ensure the user is authenticated. Throws if they are not. */ -export function ensureAuthenticated(req: express.Request, res?: express.Response, next?: express.NextFunction): Promise +export function ensureAuthenticated( + req: express.Request, + res?: express.Response, + next?: express.NextFunction, +): Promise /** * Returns true if the user is authenticated.