Skip to content

Commit 59db017

Browse files
committed
Fix issues surrounding sessions when accessed from a proxy.
- Updated vscode args to match latest upstream. - Fixed issues surrounding trailing slashes affecting base paths. - Updated cookie names to better match upstream's usage, debuggability.
1 parent 9e2c7a7 commit 59db017

File tree

6 files changed

+30
-22
lines changed

6 files changed

+30
-22
lines changed

src/node/http.ts

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -62,6 +62,10 @@ export const replaceTemplates = <T extends object>(
6262
.replace("{{OPTIONS}}", () => escapeJSON(serverOptions))
6363
}
6464

65+
export enum Cookie {
66+
SessionKey = "code-server-session",
67+
}
68+
6569
/**
6670
* Throw an error if not authorized. Call `next` if provided.
6771
*/
@@ -93,7 +97,7 @@ export const authenticated = async (req: express.Request): Promise<boolean> => {
9397
const passwordMethod = getPasswordMethod(hashedPasswordFromArgs)
9498
const isCookieValidArgs: IsCookieValidArgs = {
9599
passwordMethod,
96-
cookieKey: sanitizeString(req.cookies.key),
100+
cookieKey: sanitizeString(req.cookies[Cookie.SessionKey]),
97101
passwordFromArgs: req.args.password || "",
98102
hashedPasswordFromArgs: req.args["hashed-password"],
99103
}

src/node/main.ts

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
import { field, logger } from "@coder/logger"
2-
import * as os from "os"
32
import http from "http"
3+
import * as os from "os"
44
import path from "path"
55
import { Disposable } from "../common/emitter"
66
import { plural } from "../common/util"
@@ -37,8 +37,11 @@ export const runVsCodeCli = async (args: DefaultedArgs): Promise<void> => {
3737
try {
3838
await spawnCli({
3939
...args,
40-
// For some reason VS Code takes the port as a string.
41-
port: typeof args.port !== "undefined" ? args.port.toString() : undefined,
40+
/** Type casting. */
41+
"accept-server-license-terms": true,
42+
help: !!args.help,
43+
version: !!args.version,
44+
port: args.port?.toString(),
4245
})
4346
} catch (error: any) {
4447
logger.error("Got error from VS Code", error)

src/node/routes/login.ts

Lines changed: 4 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -4,13 +4,9 @@ import { RateLimiter as Limiter } from "limiter"
44
import * as os from "os"
55
import * as path from "path"
66
import { rootPath } from "../constants"
7-
import { authenticated, getCookieDomain, redirect, replaceTemplates } from "../http"
7+
import { authenticated, Cookie, getCookieDomain, redirect, replaceTemplates } from "../http"
88
import { getPasswordMethod, handlePasswordValidation, humanPath, sanitizeString, escapeHtml } from "../util"
99

10-
export enum Cookie {
11-
Key = "key",
12-
}
13-
1410
// RateLimiter wraps around the limiter library for logins.
1511
// It allows 2 logins every minute plus 12 logins every hour.
1612
export class RateLimiter {
@@ -62,7 +58,7 @@ router.get("/", async (req, res) => {
6258
res.send(await getRoot(req))
6359
})
6460

65-
router.post("/", async (req, res) => {
61+
router.post<{}, string, { password: string; base?: string }, { to?: string }>("/", async (req, res) => {
6662
const password = sanitizeString(req.body.password)
6763
const hashedPasswordFromArgs = req.args["hashed-password"]
6864

@@ -87,13 +83,13 @@ router.post("/", async (req, res) => {
8783
if (isPasswordValid) {
8884
// The hash does not add any actual security but we do it for
8985
// obfuscation purposes (and as a side effect it handles escaping).
90-
res.cookie(Cookie.Key, hashedPassword, {
86+
res.cookie(Cookie.SessionKey, hashedPassword, {
9187
domain: getCookieDomain(req.headers.host || "", req.args["proxy-domain"]),
9288
// Browsers do not appear to allow cookies to be set relatively so we
9389
// need to get the root path from the browser since the proxy rewrites
9490
// it out of the path. Otherwise code-server instances hosted on
9591
// separate sub-paths will clobber each other.
96-
path: req.body.base ? path.posix.join(req.body.base, "..") : "/",
92+
path: req.body.base ? path.posix.join(req.body.base, "..", "/") : "/",
9793
sameSite: "lax",
9894
})
9995

src/node/routes/logout.ts

Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,17 +1,19 @@
11
import { Router } from "express"
2-
import { getCookieDomain, redirect } from "../http"
3-
import { Cookie } from "./login"
2+
import { Cookie, getCookieDomain, redirect } from "../http"
3+
import { sanitizeString } from "../util"
44

55
export const router = Router()
66

7-
router.get("/", async (req, res) => {
7+
router.get<{}, undefined, undefined, { base?: string; to?: string }>("/", async (req, res) => {
8+
const path = sanitizeString(req.query.base) || "/"
9+
const to = sanitizeString(req.query.to) || "/"
10+
811
// Must use the *identical* properties used to set the cookie.
9-
res.clearCookie(Cookie.Key, {
12+
res.clearCookie(Cookie.SessionKey, {
1013
domain: getCookieDomain(req.headers.host || "", req.args["proxy-domain"]),
11-
path: req.query.base || "/",
14+
path: decodeURIComponent(path),
1215
sameSite: "lax",
1316
})
1417

15-
const to = (typeof req.query.to === "string" && req.query.to) || "/"
1618
return redirect(req, res, to, { to: undefined, base: undefined })
1719
})

src/node/routes/vscode.ts

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,7 @@ export class CodeServerRouteWrapper {
2424
const isAuthenticated = await authenticated(req)
2525

2626
if (!isAuthenticated) {
27-
return redirect(req, res, "login", {
27+
return redirect(req, res, "login/", {
2828
// req.baseUrl can be blank if already at the root.
2929
to: req.baseUrl && req.baseUrl !== "/" ? req.baseUrl : undefined,
3030
})
@@ -88,9 +88,12 @@ export class CodeServerRouteWrapper {
8888

8989
try {
9090
this._codeServerMain = await createVSServer(null, {
91-
connectionToken: "0000",
91+
"connection-token": "0000",
92+
"accept-server-license-terms": true,
9293
...args,
93-
// For some reason VS Code takes the port as a string.
94+
/** Type casting. */
95+
help: !!args.help,
96+
version: !!args.version,
9497
port: args.port?.toString(),
9598
})
9699
} catch (createServerError) {

src/node/util.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -321,7 +321,7 @@ export async function isCookieValid({
321321
* - greater than 0 characters
322322
* - trims whitespace
323323
*/
324-
export function sanitizeString(str: string): string {
324+
export function sanitizeString(str: unknown): string {
325325
// Very basic sanitization of string
326326
// Credit: https://stackoverflow.com/a/46719000/3015595
327327
return typeof str === "string" && str.trim().length > 0 ? str.trim() : ""

0 commit comments

Comments
 (0)