Skip to content

Commit 0e78a14

Browse files
authored
feat: github-auth flag (#4926)
* feat: github-auth flag This will allow injecting credentials into code-server if you already have them. * Update Code Contains the GitHub auth changes. * Add e2e test for GitHub token
1 parent 3f3a489 commit 0e78a14

14 files changed

+127
-16
lines changed

src/node/cli.ts

+20-1
Original file line numberDiff line numberDiff line change
@@ -87,6 +87,7 @@ export interface UserProvidedArgs {
8787
"locate-extension"?: string[]
8888
"show-versions"?: boolean
8989
category?: string
90+
"github-auth"?: string
9091
}
9192

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

344+
if (key === "github-auth" && !opts?.configFile) {
345+
throw new Error("--github-auth can only be set in the config file or passed in via $GITHUB_TOKEN")
346+
}
347+
339348
const option = options[key]
340349
if (option.type === "boolean") {
341350
;(args[key] as boolean) = true
@@ -409,7 +418,12 @@ export const parse = (
409418

410419
logger.debug(() => [
411420
`parsed ${opts?.configFile ? "config" : "command line"}`,
412-
field("args", { ...args, password: undefined }),
421+
field("args", {
422+
...args,
423+
password: args.password ? "<redacted>" : undefined,
424+
"hashed-password": args["hashed-password"] ? "<redacted>" : undefined,
425+
"github-auth": args["github-auth"] ? "<redacted>" : undefined,
426+
}),
413427
])
414428

415429
return args
@@ -530,9 +544,14 @@ export async function setDefaults(cliArgs: UserProvidedArgs, configArgs?: Config
530544
usingEnvPassword = false
531545
}
532546

547+
if (process.env.GITHUB_TOKEN) {
548+
args["github-auth"] = process.env.GITHUB_TOKEN
549+
}
550+
533551
// Ensure they're not readable by child processes.
534552
delete process.env.PASSWORD
535553
delete process.env.HASHED_PASSWORD
554+
delete process.env.GITHUB_TOKEN
536555

537556
// Filter duplicate proxy domains and remove any leading `*.`.
538557
const proxyDomains = new Set((args["proxy-domain"] || []).map((d) => d.replace(/^\*\./, "")))

test/e2e/baseFixture.ts

+2-1
Original file line numberDiff line numberDiff line change
@@ -13,11 +13,12 @@ export const describe = (
1313
name: string,
1414
includeCredentials: boolean,
1515
codeServerArgs: string[],
16+
codeServerEnv: NodeJS.ProcessEnv,
1617
fn: (codeServer: CodeServer) => void,
1718
) => {
1819
test.describe(name, () => {
1920
// This will spawn on demand so nothing is necessary on before.
20-
const codeServer = new CodeServer(name, codeServerArgs)
21+
const codeServer = new CodeServer(name, codeServerArgs, codeServerEnv)
2122

2223
// Kill code-server after the suite has ended. This may happen even without
2324
// doing it explicitly but it seems prudent to be sure.

test/e2e/codeServer.test.ts

+1-1
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@ import { promises as fs } from "fs"
22
import * as path from "path"
33
import { describe, test, expect } from "./baseFixture"
44

5-
describe("CodeServer", true, [], () => {
5+
describe("CodeServer", true, [], {}, () => {
66
test("should navigate to home page", async ({ codeServerPage }) => {
77
// We navigate codeServer before each test
88
// and we start the test with a storage state

test/e2e/extensions.test.ts

+5-2
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
import * as path from "path"
12
import { describe, test } from "./baseFixture"
23

34
function runTestExtensionTests() {
@@ -11,10 +12,12 @@ function runTestExtensionTests() {
1112
})
1213
}
1314

14-
describe("Extensions", true, [], () => {
15+
const flags = ["--extensions-dir", path.join(__dirname, "./extensions")]
16+
17+
describe("Extensions", true, flags, {}, () => {
1518
runTestExtensionTests()
1619
})
1720

18-
describe("Extensions with --cert", true, ["--cert"], () => {
21+
describe("Extensions with --cert", true, [...flags, "--cert"], {}, () => {
1922
runTestExtensionTests()
2023
})

test/e2e/github.test.ts

+38
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,38 @@
1+
import { test as base } from "@playwright/test"
2+
import { describe, expect, test } from "./baseFixture"
3+
4+
if (process.env.GITHUB_TOKEN) {
5+
describe("GitHub token", true, [], {}, () => {
6+
test("should be logged in to pull requests extension", async ({ codeServerPage }) => {
7+
await codeServerPage.exec("git init")
8+
await codeServerPage.exec("git remote add origin https://github.com/coder/code-server")
9+
await codeServerPage.installExtension("GitHub.vscode-pull-request-github")
10+
await codeServerPage.executeCommandViaMenus("View: Show Github")
11+
await codeServerPage.page.click("text=Sign in")
12+
await codeServerPage.page.click("text=Allow")
13+
// It should ask to select an account, one of which will be the one we
14+
// pre-injected.
15+
expect(await codeServerPage.page.isVisible("text=Select an account")).toBe(false)
16+
})
17+
})
18+
19+
describe("No GitHub token", true, [], { GITHUB_TOKEN: "" }, () => {
20+
test("should not be logged in to pull requests extension", async ({ codeServerPage }) => {
21+
await codeServerPage.exec("git init")
22+
await codeServerPage.exec("git remote add origin https://github.com/coder/code-server")
23+
await codeServerPage.installExtension("GitHub.vscode-pull-request-github")
24+
await codeServerPage.executeCommandViaMenus("View: Show Github")
25+
await codeServerPage.page.click("text=Sign in")
26+
await codeServerPage.page.click("text=Allow")
27+
// Since there is no account it will ask directly for the token (because
28+
// we are on localhost; otherwise it would initiate the oauth flow).
29+
expect(await codeServerPage.page.isVisible("text=GitHub Personal Access Token")).toBe(false)
30+
})
31+
})
32+
} else {
33+
base.describe("GitHub token", () => {
34+
base.skip("skipped because GITHUB_TOKEN is not set", () => {
35+
// Playwright will not show this without a function.
36+
})
37+
})
38+
}

test/e2e/globalSetup.test.ts

+1-1
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@ import { describe, test, expect } from "./baseFixture"
22

33
// This test is to make sure the globalSetup works as expected
44
// meaning globalSetup ran and stored the storageState
5-
describe("globalSetup", true, [], () => {
5+
describe("globalSetup", true, [], {}, () => {
66
test("should keep us logged in using the storageState", async ({ codeServerPage }) => {
77
// Make sure the editor actually loaded
88
expect(await codeServerPage.isEditorVisible()).toBe(true)

test/e2e/login.test.ts

+1-1
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
import { PASSWORD } from "../utils/constants"
22
import { describe, test, expect } from "./baseFixture"
33

4-
describe("login", false, [], () => {
4+
describe("login", false, [], {}, () => {
55
test("should see the login page", async ({ codeServerPage }) => {
66
// It should send us to the login page
77
expect(await codeServerPage.page.title()).toBe("code-server login")

test/e2e/logout.test.ts

+1-1
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
// NOTE@jsjoeio commenting out until we can figure out what's wrong
22
// import { describe, test, expect } from "./baseFixture"
33

4-
// describe("logout", true, [], () => {
4+
// describe("logout", true, [], {}, () => {
55
// test("should be able logout", async ({ codeServerPage }) => {
66
// // Recommended by Playwright for async navigation
77
// // https://github.com/microsoft/playwright/issues/1987#issuecomment-620182151

test/e2e/models/CodeServer.ts

+29-3
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ import * as cp from "child_process"
33
import { promises as fs } from "fs"
44
import * as path from "path"
55
import { Page } from "playwright"
6+
import util from "util"
67
import { logError, plural } from "../../../src/common/util"
78
import { onLine } from "../../../src/node/util"
89
import { PASSWORD, workspaceDir } from "../../utils/constants"
@@ -39,7 +40,11 @@ export class CodeServer {
3940
private closed = false
4041
private _workspaceDir: Promise<string> | undefined
4142

42-
constructor(name: string, private readonly codeServerArgs: string[]) {
43+
constructor(
44+
name: string,
45+
private readonly codeServerArgs: string[],
46+
private readonly codeServerEnv: NodeJS.ProcessEnv,
47+
) {
4348
this.logger = logger.named(name)
4449
}
4550

@@ -96,6 +101,8 @@ export class CodeServer {
96101
"node",
97102
[
98103
process.env.CODE_SERVER_TEST_ENTRY || ".",
104+
"--extensions-dir",
105+
path.join(dir, "extensions"),
99106
...this.codeServerArgs,
100107
// Using port zero will spawn on a random port.
101108
"--bind-addr",
@@ -107,15 +114,14 @@ export class CodeServer {
107114
path.join(dir, "config.yaml"),
108115
"--user-data-dir",
109116
dir,
110-
"--extensions-dir",
111-
path.join(__dirname, "../extensions"),
112117
// The last argument is the workspace to open.
113118
dir,
114119
],
115120
{
116121
cwd: path.join(__dirname, "../../.."),
117122
env: {
118123
...process.env,
124+
...this.codeServerEnv,
119125
PASSWORD,
120126
},
121127
},
@@ -462,4 +468,24 @@ export class CodeServerPage {
462468
await this.reloadUntilEditorIsReady()
463469
}
464470
}
471+
472+
/**
473+
* Execute a command in t root of the instance's workspace directory.
474+
*/
475+
async exec(command: string): Promise<void> {
476+
await util.promisify(cp.exec)(command, {
477+
cwd: await this.workspaceDir,
478+
})
479+
}
480+
481+
/**
482+
* Install an extension by ID to the instance's temporary extension
483+
* directory.
484+
*/
485+
async installExtension(id: string): Promise<void> {
486+
const dir = path.join(await this.workspaceDir, "extensions")
487+
await util.promisify(cp.exec)(`node . --install-extension ${id} --extensions-dir ${dir}`, {
488+
cwd: path.join(__dirname, "../../.."),
489+
})
490+
}
465491
}

test/e2e/openHelpAbout.test.ts

+1-1
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
import { describe, test, expect } from "./baseFixture"
22

3-
describe("Open Help > About", true, [], () => {
3+
describe("Open Help > About", true, [], {}, () => {
44
test("should see code-server version in about dialog", async ({ codeServerPage }) => {
55
// Open using the menu.
66
await codeServerPage.navigateMenus(["Help", "About"])

test/e2e/terminal.test.ts

+1-1
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@ import util from "util"
44
import { clean, tmpdir } from "../utils/helpers"
55
import { describe, expect, test } from "./baseFixture"
66

7-
describe("Integrated Terminal", true, [], () => {
7+
describe("Integrated Terminal", true, [], {}, () => {
88
const testName = "integrated-terminal"
99
test.beforeAll(async () => {
1010
await clean(testName)

test/unit/node/cli.test.ts

+24
Original file line numberDiff line numberDiff line change
@@ -313,6 +313,19 @@ describe("parser", () => {
313313
})
314314
})
315315

316+
it("should use env var github token", async () => {
317+
process.env.GITHUB_TOKEN = "ga-foo"
318+
const args = parse([])
319+
expect(args).toEqual({})
320+
321+
const defaultArgs = await setDefaults(args)
322+
expect(defaultArgs).toEqual({
323+
...defaults,
324+
"github-auth": "ga-foo",
325+
})
326+
expect(process.env.GITHUB_TOKEN).toBe(undefined)
327+
})
328+
316329
it("should error if password passed in", () => {
317330
expect(() => parse(["--password", "supersecret123"])).toThrowError(
318331
"--password can only be set in the config file or passed in via $PASSWORD",
@@ -325,6 +338,12 @@ describe("parser", () => {
325338
)
326339
})
327340

341+
it("should error if github-auth passed in", () => {
342+
expect(() => parse(["--github-auth", "fdas423fs8a"])).toThrowError(
343+
"--github-auth can only be set in the config file or passed in via $GITHUB_TOKEN",
344+
)
345+
})
346+
328347
it("should filter proxy domains", async () => {
329348
const args = parse(["--proxy-domain", "*.coder.com", "--proxy-domain", "coder.com", "--proxy-domain", "coder.org"])
330349
expect(args).toEqual({
@@ -374,6 +393,11 @@ describe("parser", () => {
374393
it("should ignore optional strings set to false", async () => {
375394
expect(parse(["--cert=false"])).toEqual({})
376395
})
396+
it("should use last flag", async () => {
397+
expect(parse(["--port", "8081", "--port", "8082"])).toEqual({
398+
port: 8082,
399+
})
400+
})
377401
})
378402

379403
describe("cli", () => {

vendor/package.json

+1-1
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,6 @@
77
"postinstall": "./postinstall.sh"
88
},
99
"devDependencies": {
10-
"code-oss-dev": "coder/vscode#6337ee490d16b7dfd8854d22c998f58d6cd21ef5"
10+
"code-oss-dev": "coder/vscode#0fd21e4078ac1dddb26be024f5d4224a4b86da93"
1111
}
1212
}

vendor/yarn.lock

+2-2
Original file line numberDiff line numberDiff line change
@@ -274,9 +274,9 @@ clone-response@^1.0.2:
274274
dependencies:
275275
mimic-response "^1.0.0"
276276

277-
code-oss-dev@coder/vscode#6337ee490d16b7dfd8854d22c998f58d6cd21ef5:
277+
code-oss-dev@coder/vscode#0fd21e4078ac1dddb26be024f5d4224a4b86da93:
278278
version "1.63.0"
279-
resolved "https://codeload.github.com/coder/vscode/tar.gz/6337ee490d16b7dfd8854d22c998f58d6cd21ef5"
279+
resolved "https://codeload.github.com/coder/vscode/tar.gz/0fd21e4078ac1dddb26be024f5d4224a4b86da93"
280280
dependencies:
281281
"@microsoft/applicationinsights-web" "^2.6.4"
282282
"@parcel/watcher" "2.0.3"

0 commit comments

Comments
 (0)