OCram85/code-server
Archived
1
0
Fork 0
Code Pull Requests 2 Activity

Add origin checks to web sockets (#6048)

Browse Source 
* Move splitOnFirstEquals to util

I will be making use of this to parse the forwarded header.

* Type splitOnFirstEquals with two items

Also add some test cases.

* Check origin header on web sockets

* Update changelog with origin check

* Fix web sockets not closing with error code
This commit is contained in:
Asher
2023-03-03 09:12:34 +00:00
committed by GitHub
parent a47cd81d8c
commit d477972c68
17 changed files with 354 additions and 102 deletions
Download Patch File Download Diff File Expand all files Collapse all files

26
test/unit/node/cli.test.ts
View File

@ -11,7 +11,6 @@ import {
readSocketPath,
setDefaults,
shouldOpenInExistingInstance,
splitOnFirstEquals,
toCodeArgs,
optionDescriptions,
options,
@ -535,31 +534,6 @@ describe("cli", () => {
})
})
describe("splitOnFirstEquals", () => {
it("should split on the first equals", () => {
const testStr = "enabled-proposed-api=test=value"
const actual = splitOnFirstEquals(testStr)
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"
const actual = splitOnFirstEquals(testStr)
const expected = [
"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 the first element before an equals", () => {
const testStr = "auth="
const actual = splitOnFirstEquals(testStr)
const expected = ["auth"]
expect(actual).toEqual(expect.arrayContaining(expected))
})
})
describe("shouldSpawnCliProcess", () => {
it("should return false if no 'extension' related args passed in", async () => {
const args = {}

149
test/unit/node/http.test.ts
View File

@ -1,55 +1,118 @@
import { getMockReq } from "@jest-mock/express"
import { constructRedirectPath, relativeRoot } from "../../../src/node/http"
import * as http from "../../../src/node/http"
import { mockLogger } from "../../utils/helpers"
describe("http", () => {
it("should construct a relative path to the root", () => {
expect(relativeRoot("/")).toStrictEqual(".")
expect(relativeRoot("/foo")).toStrictEqual(".")
expect(relativeRoot("/foo/")).toStrictEqual("./..")
expect(relativeRoot("/foo/bar ")).toStrictEqual("./..")
expect(relativeRoot("/foo/bar/")).toStrictEqual("./../..")
beforeEach(() => {
mockLogger()
})
})
describe("constructRedirectPath", () => {
it("should preserve slashes in queryString so they are human-readable", () => {
const mockReq = getMockReq({
originalUrl: "localhost:8080",
})
const mockQueryParams = { folder: "/Users/jp/dev/coder" }
const mockTo = ""
const actual = constructRedirectPath(mockReq, mockQueryParams, mockTo)
const expected = "./?folder=/Users/jp/dev/coder"
expect(actual).toBe(expected)
afterEach(() => {
jest.clearAllMocks()
})
it("should use an empty string if no query params", () => {
const mockReq = getMockReq({
originalUrl: "localhost:8080",
})
const mockQueryParams = {}
const mockTo = ""
const actual = constructRedirectPath(mockReq, mockQueryParams, mockTo)
const expected = "./"
expect(actual).toBe(expected)
it("should construct a relative path to the root", () => {
expect(http.relativeRoot("/")).toStrictEqual(".")
expect(http.relativeRoot("/foo")).toStrictEqual(".")
expect(http.relativeRoot("/foo/")).toStrictEqual("./..")
expect(http.relativeRoot("/foo/bar ")).toStrictEqual("./..")
expect(http.relativeRoot("/foo/bar/")).toStrictEqual("./../..")
})
it("should append the 'to' path relative to the originalUrl", () => {
const mockReq = getMockReq({
originalUrl: "localhost:8080",
describe("origin", () => {
;[
{
origin: "",
host: "",
expected: true,
},
{
origin: "http://localhost:8080",
host: "",
expected: false,
},
{
origin: "http://localhost:8080",
host: "localhost:8080",
expected: true,
},
{
origin: "http://localhost:8080",
host: "localhost:8081",
expected: false,
},
{
origin: "localhost:8080",
host: "localhost:8080",
expected: false, // Gets parsed as host: localhost and path: 8080.
},
{
origin: "test.org",
host: "localhost:8080",
expected: false, // Parsing fails completely.
},
].forEach((test) => {
;[
["host", test.host],
["x-forwarded-host", test.host],
["forwarded", `for=127.0.0.1, host=${test.host}, proto=http`],
["forwarded", `for=127.0.0.1;proto=http;host=${test.host}`],
["forwarded", `proto=http;host=${test.host}, for=127.0.0.1`],
].forEach(([key, value]) => {
it(`${test.origin} -> [${key}: ${value}]`, () => {
const req = getMockReq({
originalUrl: "localhost:8080",
headers: {
origin: test.origin,
[key]: value,
},
})
expect(http.authenticateOrigin(req)).toBe(test.expected)
})
})
})
const mockQueryParams = {}
const mockTo = "vscode"
const actual = constructRedirectPath(mockReq, mockQueryParams, mockTo)
const expected = "./vscode"
expect(actual).toBe(expected)
})
it("should append append queryParams after 'to' path", () => {
const mockReq = getMockReq({
originalUrl: "localhost:8080",
describe("constructRedirectPath", () => {
it("should preserve slashes in queryString so they are human-readable", () => {
const mockReq = getMockReq({
originalUrl: "localhost:8080",
})
const mockQueryParams = { folder: "/Users/jp/dev/coder" }
const mockTo = ""
const actual = http.constructRedirectPath(mockReq, mockQueryParams, mockTo)
const expected = "./?folder=/Users/jp/dev/coder"
expect(actual).toBe(expected)
})
it("should use an empty string if no query params", () => {
const mockReq = getMockReq({
originalUrl: "localhost:8080",
})
const mockQueryParams = {}
const mockTo = ""
const actual = http.constructRedirectPath(mockReq, mockQueryParams, mockTo)
const expected = "./"
expect(actual).toBe(expected)
})
it("should append the 'to' path relative to the originalUrl", () => {
const mockReq = getMockReq({
originalUrl: "localhost:8080",
})
const mockQueryParams = {}
const mockTo = "vscode"
const actual = http.constructRedirectPath(mockReq, mockQueryParams, mockTo)
const expected = "./vscode"
expect(actual).toBe(expected)
})
it("should append append queryParams after 'to' path", () => {
const mockReq = getMockReq({
originalUrl: "localhost:8080",
})
const mockQueryParams = { folder: "/Users/jp/dev/coder" }
const mockTo = "vscode"
const actual = http.constructRedirectPath(mockReq, mockQueryParams, mockTo)
const expected = "./vscode?folder=/Users/jp/dev/coder"
expect(actual).toBe(expected)
})
const mockQueryParams = { folder: "/Users/jp/dev/coder" }
const mockTo = "vscode"
const actual = constructRedirectPath(mockReq, mockQueryParams, mockTo)
const expected = "./vscode?folder=/Users/jp/dev/coder"
expect(actual).toBe(expected)
})
})

48
test/unit/node/proxy.test.ts
View File

@ -4,21 +4,26 @@ import * as http from "http"
import nodeFetch from "node-fetch"
import { HttpCode } from "../../../src/common/http"
import { proxy } from "../../../src/node/proxy"
import { getAvailablePort } from "../../utils/helpers"
import { wss, Router as WsRouter } from "../../../src/node/wsRouter"
import { getAvailablePort, mockLogger } from "../../utils/helpers"
import * as httpserver from "../../utils/httpserver"
import * as integration from "../../utils/integration"
describe("proxy", () => {
const nhooyrDevServer = new httpserver.HttpServer()
const wsApp = express.default()
const wsRouter = WsRouter()
let codeServer: httpserver.HttpServer | undefined
let proxyPath: string
let absProxyPath: string
let e: express.Express
beforeAll(async () => {
wsApp.use("/", wsRouter.router)
await nhooyrDevServer.listen((req, res) => {
e(req, res)
})
nhooyrDevServer.listenUpgrade(wsApp)
proxyPath = `/proxy/${nhooyrDevServer.port()}/wsup`
absProxyPath = proxyPath.replace("/proxy/", "/absproxy/")
})
@ -29,6 +34,7 @@ describe("proxy", () => {
beforeEach(() => {
e = express.default()
mockLogger()
})
afterEach(async () => {
@ -36,6 +42,7 @@ describe("proxy", () => {
await codeServer.dispose()
codeServer = undefined
}
jest.clearAllMocks()
})
it("should rewrite the base path", async () => {
@ -151,6 +158,35 @@ describe("proxy", () => {
expect(resp.status).toBe(500)
expect(resp.statusText).toMatch("Internal Server Error")
})
it("should pass origin check", async () => {
wsRouter.ws("/wsup", async (req) => {
wss.handleUpgrade(req, req.ws, req.head, (ws) => {
ws.send("hello")
req.ws.resume()
})
})
codeServer = await integration.setup(["--auth=none"], "")
const ws = await codeServer.wsWait(proxyPath, {
headers: {
host: "localhost:8080",
origin: "https://localhost:8080",
},
})
ws.terminate()
})
it("should fail origin check", async () => {
await expect(async () => {
codeServer = await integration.setup(["--auth=none"], "")
await codeServer.wsWait(proxyPath, {
headers: {
host: "localhost:8080",
origin: "https://evil.org",
},
})
}).rejects.toThrow()
})
})
// NOTE@jsjoeio
@ -190,18 +226,18 @@ describe("proxy (standalone)", () => {
})
// Start both servers
await proxyTarget.listen(PROXY_PORT)
await testServer.listen(PORT)
proxyTarget.listen(PROXY_PORT)
testServer.listen(PORT)
})
afterEach(async () => {
await testServer.close()
await proxyTarget.close()
testServer.close()
proxyTarget.close()
})
it("should return a 500 when proxy target errors ", async () => {
// Close the proxy target so that proxy errors
await proxyTarget.close()
proxyTarget.close()
const errorResp = await nodeFetch(`${URL}/error`)
expect(errorResp.status).toBe(HttpCode.ServerError)
expect(errorResp.statusText).toBe("Internal Server Error")

4
test/unit/node/routes/health.test.ts
View File

@ -23,7 +23,9 @@ describe("health", () => {
codeServer = await integration.setup(["--auth=none"], "")
const ws = codeServer.ws("/healthz")
const message = await new Promise((resolve, reject) => {
ws.on("error", console.error)
ws.on("error", (err) => {
console.error("[healthz]", err)
})
ws.on("message", (message) => {
try {
const j = JSON.parse(message.toString())

30
test/unit/node/routes/vscode.test.ts Normal file
View File

@ -0,0 +1,30 @@
import * as httpserver from "../../../utils/httpserver"
import * as integration from "../../../utils/integration"
import { mockLogger } from "../../../utils/helpers"
describe("vscode", () => {
let codeServer: httpserver.HttpServer | undefined
beforeEach(() => {
mockLogger()
})
afterEach(async () => {
if (codeServer) {
await codeServer.dispose()
codeServer = undefined
}
jest.clearAllMocks()
})
it("should fail origin check", async () => {
await expect(async () => {
codeServer = await integration.setup(["--auth=none"], "")
await codeServer.wsWait("/vscode", {
headers: {
host: "localhost:8080",
origin: "https://evil.org",
},
})
}).rejects.toThrow()
})
})

38
test/unit/node/util.test.ts
View File

@ -601,3 +601,41 @@ describe("constructOpenOptions", () => {
expect(urlSearch).toBe("?q=^&test")
})
})
describe("splitOnFirstEquals", () => {
const tests = [
{
name: "empty",
key: "",
value: "",
},
{
name: "split on first equals",
key: "foo",
value: "bar",
},
{
name: "split on first equals even with multiple equals",
key: "foo",
value: "bar=baz",
},
{
name: "split with empty value",
key: "foo",
value: "",
},
{
name: "split with no value",
key: "foo",
value: undefined,
},
]
tests.forEach((test) => {
it("should ${test.name}", () => {
const input = test.key && typeof test.value !== "undefined" ? `${test.key}=${test.value}` : test.key
const [key, value] = util.splitOnFirstEquals(input)
expect(key).toStrictEqual(test.key)
expect(value).toStrictEqual(test.value || undefined)
})
})
})