Skip to content

feat: github-auth flag #4926

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

Merged
merged 3 commits into from
Mar 2, 2022
Merged
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
21 changes: 20 additions & 1 deletion src/node/cli.ts
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,7 @@ export interface UserProvidedArgs {
"locate-extension"?: string[]
"show-versions"?: boolean
category?: string
"github-auth"?: string
}

interface Option<T> {
Expand Down Expand Up @@ -205,6 +206,10 @@ const options: Options<Required<UserProvidedArgs>> = {
},
"uninstall-extension": { type: "string[]", description: "Uninstall a VS Code extension by id." },
"show-versions": { type: "boolean", description: "Show VS Code extension versions." },
"github-auth": {
type: "string",
description: "GitHub authentication token (can only be passed in via $GITHUB_TOKEN or the config file).",
},
"proxy-domain": { type: "string[]", description: "Domain used for proxying ports." },
"ignore-last-opened": {
type: "boolean",
Expand Down Expand Up @@ -336,6 +341,10 @@ export const parse = (
throw new Error("--hashed-password can only be set in the config file or passed in via $HASHED_PASSWORD")
}

if (key === "github-auth" && !opts?.configFile) {
throw new Error("--github-auth can only be set in the config file or passed in via $GITHUB_TOKEN")
}

const option = options[key]
if (option.type === "boolean") {
;(args[key] as boolean) = true
Expand Down Expand Up @@ -409,7 +418,12 @@ export const parse = (

logger.debug(() => [
`parsed ${opts?.configFile ? "config" : "command line"}`,
field("args", { ...args, password: undefined }),
field("args", {
...args,
password: args.password ? "<redacted>" : undefined,
"hashed-password": args["hashed-password"] ? "<redacted>" : undefined,
"github-auth": args["github-auth"] ? "<redacted>" : undefined,
}),
])

return args
Expand Down Expand Up @@ -530,9 +544,14 @@ export async function setDefaults(cliArgs: UserProvidedArgs, configArgs?: Config
usingEnvPassword = false
}

if (process.env.GITHUB_TOKEN) {
args["github-auth"] = process.env.GITHUB_TOKEN
}

// Ensure they're not readable by child processes.
delete process.env.PASSWORD
delete process.env.HASHED_PASSWORD
delete process.env.GITHUB_TOKEN

// Filter duplicate proxy domains and remove any leading `*.`.
const proxyDomains = new Set((args["proxy-domain"] || []).map((d) => d.replace(/^\*\./, "")))
Expand Down
3 changes: 2 additions & 1 deletion test/e2e/baseFixture.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,11 +13,12 @@ export const describe = (
name: string,
includeCredentials: boolean,
codeServerArgs: string[],
codeServerEnv: NodeJS.ProcessEnv,
fn: (codeServer: CodeServer) => void,
) => {
test.describe(name, () => {
// This will spawn on demand so nothing is necessary on before.
const codeServer = new CodeServer(name, codeServerArgs)
const codeServer = new CodeServer(name, codeServerArgs, codeServerEnv)

// Kill code-server after the suite has ended. This may happen even without
// doing it explicitly but it seems prudent to be sure.
Expand Down
2 changes: 1 addition & 1 deletion test/e2e/codeServer.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import { promises as fs } from "fs"
import * as path from "path"
import { describe, test, expect } from "./baseFixture"

describe("CodeServer", true, [], () => {
describe("CodeServer", true, [], {}, () => {
test("should navigate to home page", async ({ codeServerPage }) => {
// We navigate codeServer before each test
// and we start the test with a storage state
Expand Down
7 changes: 5 additions & 2 deletions test/e2e/extensions.test.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import * as path from "path"
import { describe, test } from "./baseFixture"

function runTestExtensionTests() {
Expand All @@ -11,10 +12,12 @@ function runTestExtensionTests() {
})
}

describe("Extensions", true, [], () => {
const flags = ["--extensions-dir", path.join(__dirname, "./extensions")]

describe("Extensions", true, flags, {}, () => {
runTestExtensionTests()
})

describe("Extensions with --cert", true, ["--cert"], () => {
describe("Extensions with --cert", true, [...flags, "--cert"], {}, () => {
runTestExtensionTests()
})
38 changes: 38 additions & 0 deletions test/e2e/github.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
import { test as base } from "@playwright/test"
import { describe, expect, test } from "./baseFixture"

if (process.env.GITHUB_TOKEN) {
describe("GitHub token", true, [], {}, () => {
test("should be logged in to pull requests extension", async ({ codeServerPage }) => {
await codeServerPage.exec("git init")
await codeServerPage.exec("git remote add origin https://github.com/coder/code-server")
await codeServerPage.installExtension("GitHub.vscode-pull-request-github")
await codeServerPage.executeCommandViaMenus("View: Show Github")
await codeServerPage.page.click("text=Sign in")
await codeServerPage.page.click("text=Allow")
// It should ask to select an account, one of which will be the one we
// pre-injected.
expect(await codeServerPage.page.isVisible("text=Select an account")).toBe(false)
})
})

describe("No GitHub token", true, [], { GITHUB_TOKEN: "" }, () => {
test("should not be logged in to pull requests extension", async ({ codeServerPage }) => {
await codeServerPage.exec("git init")
await codeServerPage.exec("git remote add origin https://github.com/coder/code-server")
await codeServerPage.installExtension("GitHub.vscode-pull-request-github")
await codeServerPage.executeCommandViaMenus("View: Show Github")
await codeServerPage.page.click("text=Sign in")
await codeServerPage.page.click("text=Allow")
// Since there is no account it will ask directly for the token (because
// we are on localhost; otherwise it would initiate the oauth flow).
expect(await codeServerPage.page.isVisible("text=GitHub Personal Access Token")).toBe(false)
})
})
} else {
base.describe("GitHub token", () => {
base.skip("skipped because GITHUB_TOKEN is not set", () => {
// Playwright will not show this without a function.
})
})
}
2 changes: 1 addition & 1 deletion test/e2e/globalSetup.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import { describe, test, expect } from "./baseFixture"

// This test is to make sure the globalSetup works as expected
// meaning globalSetup ran and stored the storageState
describe("globalSetup", true, [], () => {
describe("globalSetup", true, [], {}, () => {
test("should keep us logged in using the storageState", async ({ codeServerPage }) => {
// Make sure the editor actually loaded
expect(await codeServerPage.isEditorVisible()).toBe(true)
Expand Down
2 changes: 1 addition & 1 deletion test/e2e/login.test.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import { PASSWORD } from "../utils/constants"
import { describe, test, expect } from "./baseFixture"

describe("login", false, [], () => {
describe("login", false, [], {}, () => {
test("should see the login page", async ({ codeServerPage }) => {
// It should send us to the login page
expect(await codeServerPage.page.title()).toBe("code-server login")
Expand Down
2 changes: 1 addition & 1 deletion test/e2e/logout.test.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
// NOTE@jsjoeio commenting out until we can figure out what's wrong
// import { describe, test, expect } from "./baseFixture"

// describe("logout", true, [], () => {
// describe("logout", true, [], {}, () => {
// test("should be able logout", async ({ codeServerPage }) => {
// // Recommended by Playwright for async navigation
// // https://github.com/microsoft/playwright/issues/1987#issuecomment-620182151
Expand Down
32 changes: 29 additions & 3 deletions test/e2e/models/CodeServer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import * as cp from "child_process"
import { promises as fs } from "fs"
import * as path from "path"
import { Page } from "playwright"
import util from "util"
import { logError, plural } from "../../../src/common/util"
import { onLine } from "../../../src/node/util"
import { PASSWORD, workspaceDir } from "../../utils/constants"
Expand Down Expand Up @@ -39,7 +40,11 @@ export class CodeServer {
private closed = false
private _workspaceDir: Promise<string> | undefined

constructor(name: string, private readonly codeServerArgs: string[]) {
constructor(
name: string,
private readonly codeServerArgs: string[],
private readonly codeServerEnv: NodeJS.ProcessEnv,
) {
this.logger = logger.named(name)
}

Expand Down Expand Up @@ -96,6 +101,8 @@ export class CodeServer {
"node",
[
process.env.CODE_SERVER_TEST_ENTRY || ".",
"--extensions-dir",
path.join(dir, "extensions"),
...this.codeServerArgs,
// Using port zero will spawn on a random port.
"--bind-addr",
Expand All @@ -107,15 +114,14 @@ export class CodeServer {
path.join(dir, "config.yaml"),
"--user-data-dir",
dir,
"--extensions-dir",
path.join(__dirname, "../extensions"),
// The last argument is the workspace to open.
dir,
],
{
cwd: path.join(__dirname, "../../.."),
env: {
...process.env,
...this.codeServerEnv,
PASSWORD,
},
},
Expand Down Expand Up @@ -462,4 +468,24 @@ export class CodeServerPage {
await this.reloadUntilEditorIsReady()
}
}

/**
* Execute a command in t root of the instance's workspace directory.
*/
async exec(command: string): Promise<void> {
await util.promisify(cp.exec)(command, {
cwd: await this.workspaceDir,
})
}

/**
* Install an extension by ID to the instance's temporary extension
* directory.
*/
async installExtension(id: string): Promise<void> {
const dir = path.join(await this.workspaceDir, "extensions")
await util.promisify(cp.exec)(`node . --install-extension ${id} --extensions-dir ${dir}`, {
cwd: path.join(__dirname, "../../.."),
})
}
}
2 changes: 1 addition & 1 deletion test/e2e/openHelpAbout.test.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import { describe, test, expect } from "./baseFixture"

describe("Open Help > About", true, [], () => {
describe("Open Help > About", true, [], {}, () => {
test("should see code-server version in about dialog", async ({ codeServerPage }) => {
// Open using the menu.
await codeServerPage.navigateMenus(["Help", "About"])
Expand Down
2 changes: 1 addition & 1 deletion test/e2e/terminal.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ import util from "util"
import { clean, tmpdir } from "../utils/helpers"
import { describe, expect, test } from "./baseFixture"

describe("Integrated Terminal", true, [], () => {
describe("Integrated Terminal", true, [], {}, () => {
const testName = "integrated-terminal"
test.beforeAll(async () => {
await clean(testName)
Expand Down
24 changes: 24 additions & 0 deletions test/unit/node/cli.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -313,6 +313,19 @@ describe("parser", () => {
})
})

it("should use env var github token", async () => {
process.env.GITHUB_TOKEN = "ga-foo"
const args = parse([])
expect(args).toEqual({})

const defaultArgs = await setDefaults(args)
expect(defaultArgs).toEqual({
...defaults,
"github-auth": "ga-foo",
})
expect(process.env.GITHUB_TOKEN).toBe(undefined)
})

it("should error if password passed in", () => {
expect(() => parse(["--password", "supersecret123"])).toThrowError(
"--password can only be set in the config file or passed in via $PASSWORD",
Expand All @@ -325,6 +338,12 @@ describe("parser", () => {
)
})

it("should error if github-auth passed in", () => {
expect(() => parse(["--github-auth", "fdas423fs8a"])).toThrowError(
"--github-auth can only be set in the config file or passed in via $GITHUB_TOKEN",
)
})

it("should filter proxy domains", async () => {
const args = parse(["--proxy-domain", "*.coder.com", "--proxy-domain", "coder.com", "--proxy-domain", "coder.org"])
expect(args).toEqual({
Expand Down Expand Up @@ -374,6 +393,11 @@ describe("parser", () => {
it("should ignore optional strings set to false", async () => {
expect(parse(["--cert=false"])).toEqual({})
})
it("should use last flag", async () => {
expect(parse(["--port", "8081", "--port", "8082"])).toEqual({
port: 8082,
})
})
})

describe("cli", () => {
Expand Down
2 changes: 1 addition & 1 deletion vendor/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,6 @@
"postinstall": "./postinstall.sh"
},
"devDependencies": {
"code-oss-dev": "coder/vscode#6337ee490d16b7dfd8854d22c998f58d6cd21ef5"
"code-oss-dev": "coder/vscode#0fd21e4078ac1dddb26be024f5d4224a4b86da93"
}
}
4 changes: 2 additions & 2 deletions vendor/yarn.lock
Original file line number Diff line number Diff line change
Expand Up @@ -274,9 +274,9 @@ clone-response@^1.0.2:
dependencies:
mimic-response "^1.0.0"

code-oss-dev@coder/vscode#6337ee490d16b7dfd8854d22c998f58d6cd21ef5:
code-oss-dev@coder/vscode#0fd21e4078ac1dddb26be024f5d4224a4b86da93:
version "1.63.0"
resolved "https://codeload.github.com/coder/vscode/tar.gz/6337ee490d16b7dfd8854d22c998f58d6cd21ef5"
resolved "https://codeload.github.com/coder/vscode/tar.gz/0fd21e4078ac1dddb26be024f5d4224a4b86da93"
dependencies:
"@microsoft/applicationinsights-web" "^2.6.4"
"@parcel/watcher" "2.0.3"
Expand Down