Skip to content
Permalink
Browse files Browse the repository at this point in the history
fix(core): require auth key for server endpoints
  • Loading branch information
eysi09 committed Apr 7, 2022
1 parent dac340b commit 56051a5
Show file tree
Hide file tree
Showing 13 changed files with 179 additions and 43 deletions.
6 changes: 5 additions & 1 deletion core/src/cli/cli.ts
Expand Up @@ -435,7 +435,11 @@ ${renderCommands(commands)}
namespace: garden.namespace,
})

command.server.showUrl(dashboardProcess?.serverHost || undefined)
let url: string | undefined
if (dashboardProcess) {
url = `${dashboardProcess.serverHost}?key=${dashboardProcess.serverAuthKey}`
}
command.server.showUrl(url)
}
}

Expand Down
1 change: 1 addition & 0 deletions core/src/constants.ts
Expand Up @@ -71,6 +71,7 @@ export const gardenEnv = {
GARDEN_LOGGER_TYPE: env.get("GARDEN_LOGGER_TYPE").required(false).asString(),
GARDEN_PROXY_DEFAULT_ADDRESS: env.get("GARDEN_PROXY_DEFAULT_ADDRESS").required(false).asString(),
GARDEN_SERVER_PORT: env.get("GARDEN_SERVER_PORT").required(false).asPortNumber(),
GARDEN_SERVER_HOSTNAME: env.get("GARDEN_SERVER_HOSTNAME").required(false).asUrlString(),
GARDEN_SKIP_TESTS: env.get("GARDEN_SKIP_TESTS").required(false).default("").asString(),
GARDEN_HARD_CONCURRENCY_LIMIT: env.get("GARDEN_HARD_CONCURRENCY_LIMIT").required(false).default(50).asInt(),
GARDEN_TASK_CONCURRENCY_LIMIT: env.get("GARDEN_TASK_CONCURRENCY_LIMIT").required(false).default(6).asInt(),
Expand Down
2 changes: 1 addition & 1 deletion core/src/events.ts
Expand Up @@ -98,7 +98,7 @@ export interface Events extends LoggerEvents {

// Process events
serversUpdated: {
servers: { host: string; command: string }[]
servers: { host: string; command: string; serverAuthKey: string }[]
}
receivedToken: AuthTokenResponse

Expand Down
6 changes: 5 additions & 1 deletion core/src/server/dashboard-event-stream.ts
Expand Up @@ -62,7 +62,11 @@ export class DashboardEventStream extends BufferedEventStream {
this.log.debug(`Updated list of running dashboard servers: ${servers.map((p) => p.serverHost).join(", ")}`)

this.garden.events.emit("serversUpdated", {
servers: servers.map((p) => ({ command: p.command!, host: p.serverHost! })),
servers: servers.map((p) => ({
command: p.command!,
host: p.serverHost!,
serverAuthKey: p.serverAuthKey || "",
})),
})
}

Expand Down
49 changes: 31 additions & 18 deletions core/src/server/server.ts
Expand Up @@ -79,15 +79,15 @@ export class GardenServer {
this.debugLog = this.log.placeholder({ level: LogLevel.debug, childEntriesInheritLevel: true })
this.garden = undefined
this.port = port
this.authKey = randomString(64)
this.authKey = randomString(24)
this.incomingEvents = new EventBus()
this.activePersistentRequests = {}

this.serversUpdatedListener = ({ servers }) => {
// Update status log line with new `garden dashboard` server, if any
for (const { host, command } of servers) {
for (const { host, command, serverAuthKey } of servers) {
if (command === "dashboard") {
this.showUrl(host)
this.showUrl(`${host}?key=${serverAuthKey}`)
return
}
}
Expand All @@ -104,13 +104,15 @@ export class GardenServer {

this.app = await this.createApp()

const hostname = gardenEnv.GARDEN_SERVER_HOSTNAME || "localhost"

if (this.port) {
this.server = this.app.listen(this.port)
this.server = this.app.listen(this.port, hostname)
} else {
do {
try {
this.port = await getPort({ port: defaultWatchServerPort })
this.server = this.app.listen(this.port)
this.server = this.app.listen(this.port, hostname)
} catch {}
} while (!this.server)
}
Expand All @@ -119,10 +121,14 @@ export class GardenServer {
this.statusLog = this.log.placeholder()
}

getUrl() {
getBaseUrl() {
return `http://localhost:${this.port}`
}

getUrl() {
return `${this.getBaseUrl()}?key=${this.authKey}`
}

showUrl(url?: string) {
this.statusLog.setState({
emoji: "sunflower",
Expand Down Expand Up @@ -156,6 +162,16 @@ export class GardenServer {
const app = websockify(new Koa())
const http = new Router()

http.use((ctx, next) => {
const authToken = ctx.header[authTokenHeader] || ctx.query.key

if (authToken !== this.authKey) {
ctx.throw(401, `Unauthorized request`)
return
}
return next()
})

/**
* HTTP API endpoint (POST /api)
*
Expand All @@ -164,7 +180,6 @@ export class GardenServer {
* means we can keep a consistent format across mechanisms.
*/
http.post("/api", async (ctx) => {
// TODO: require auth key here from 0.13.0 onwards
if (!this.garden) {
return this.notReady(ctx)
}
Expand Down Expand Up @@ -239,15 +254,7 @@ export class GardenServer {
* The API matches that of the Garden Cloud /events endpoint.
*/
http.post("/events", async (ctx) => {
const authHeader = ctx.header[authTokenHeader]

if (authHeader !== this.authKey) {
ctx.status = 401
return
}

// TODO: validate the input

const batch = ctx.request.body as ApiEventBatch
this.debugLog.debug(`Received ${batch.events.length} events from session ${batch.sessionId}`)

Expand All @@ -258,6 +265,7 @@ export class GardenServer {
})

app.use(bodyParser())

app.use(http.routes())
app.use(http.allowedMethods())

Expand All @@ -277,7 +285,7 @@ export class GardenServer {
return app
}

private notReady(ctx: Router.IRouterContext) {
private notReady(ctx: Router.IRouterContext | Koa.ParameterizedContext) {
ctx.status = 503
ctx.response.body = notReadyMessage
}
Expand All @@ -297,8 +305,6 @@ export class GardenServer {

const connId = uuidv4()

// TODO: require auth key on connections here, from 0.13.0 onwards

// The typing for koa-websocket isn't working currently
const websocket: Koa.Context["ws"] = ctx["websocket"]

Expand All @@ -318,6 +324,13 @@ export class GardenServer {
return send("error", { message, requestId })
}

// TODO: Only allow auth key authentication
if (ctx.query.sessionId !== `${this.garden.sessionId}` && ctx.query.key !== `${this.authKey}`) {
error(`401 Unauthorized`)
websocket.terminate()
return
}

// Set up heartbeat to detect dead connections
let isAlive = true

Expand Down
4 changes: 2 additions & 2 deletions core/test/unit/src/cli/cli.ts
Expand Up @@ -420,7 +420,7 @@ describe("cli", () => {
command: "dashboard",
sessionId: serverGarden.sessionId,
persistent: true,
serverHost: server.getUrl(),
serverHost: server.getBaseUrl(),
serverAuthKey: server.authKey,
projectRoot: serverGarden.projectRoot,
projectName: serverGarden.projectName,
Expand Down Expand Up @@ -527,7 +527,7 @@ describe("cli", () => {
command: "dashboard",
sessionId: serverGarden.sessionId,
persistent: true,
serverHost: server.getUrl(),
serverHost: server.getBaseUrl(),
serverAuthKey: server.authKey,
projectRoot: serverGarden.projectRoot,
projectName: serverGarden.projectName,
Expand Down
8 changes: 5 additions & 3 deletions core/test/unit/src/server/dashboard-event-stream.ts
Expand Up @@ -70,8 +70,8 @@ describe("DashboardEventStream", () => {
streamEvents: true,
streamLogEntries: true,
targets: [
{ host: serverA.getUrl(), clientAuthToken: serverA.authKey, enterprise: false },
{ host: serverB.getUrl(), clientAuthToken: serverB.authKey, enterprise: false },
{ host: serverA.getBaseUrl(), clientAuthToken: serverA.authKey, enterprise: false },
{ host: serverB.getBaseUrl(), clientAuthToken: serverB.authKey, enterprise: false },
],
})

Expand Down Expand Up @@ -193,7 +193,9 @@ describe("DashboardEventStream", () => {
await record.setCommand(values)
await streamer.updateTargets()

garden.events.expectEvent("serversUpdated", { servers: [{ host: values.serverHost, command: "dashboard" }] })
garden.events.expectEvent("serversUpdated", {
servers: [{ host: values.serverHost, command: "dashboard", serverAuthKey: "foo" }],
})
})

it("ignores servers matching ignoreHost", async () => {
Expand Down
99 changes: 89 additions & 10 deletions core/test/unit/src/server/server.ts
Expand Up @@ -58,47 +58,83 @@ describe("GardenServer", () => {
it("should update dashboard URL with new one if another is started", async () => {
gardenServer.showUrl("http://foo")
garden.events.emit("serversUpdated", {
servers: [{ host: "http://localhost:9800", command: "dashboard" }],
servers: [{ host: "http://localhost:9800", command: "dashboard", serverAuthKey: "foo" }],
})
const line = gardenServer["statusLog"]
await sleep(1) // This is enough to let go of the control loop
const status = stripAnsi(line.getLatestMessage().msg || "")
expect(status).to.equal(`Garden dashboard running at http://localhost:9800`)
expect(status).to.equal(`Garden dashboard running at http://localhost:9800?key=foo`)
})

describe("GET /", () => {
it("should return the dashboard index page", async () => {
await request(server).get("/").expect(200)
await request(server)
.get("/")
.set({ [authTokenHeader]: gardenServer.authKey })
.expect(200)
})
})

describe("POST /api", () => {
it("returns 401 if missing auth header", async () => {
await request(server).post("/api").send({}).expect(401)
})

it("returns 401 if auth header doesn't match auth key", async () => {
await request(server)
.post("/api")
.set({ [authTokenHeader]: "foo" })
.send({})
.expect(401)
})

it("should 400 on non-JSON body", async () => {
await request(server).post("/api").send("foo").expect(400)
await request(server)
.post("/api")
.set({ [authTokenHeader]: gardenServer.authKey })
.send("foo")
.expect(400)
})

it("should 400 on invalid payload", async () => {
await request(server).post("/api").send({ foo: "bar" }).expect(400)
await request(server)
.post("/api")
.set({ [authTokenHeader]: gardenServer.authKey })
.send({ foo: "bar" })
.expect(400)
})

it("should 404 on invalid command", async () => {
await request(server).post("/api").send({ command: "foo" }).expect(404)
await request(server)
.post("/api")
.set({ [authTokenHeader]: gardenServer.authKey })
.send({ command: "foo" })
.expect(404)
})

it("should 503 when Garden instance is not set", async () => {
gardenServer["garden"] = undefined
await request(server).post("/api").send({ command: "get.config" }).expect(503)
await request(server)
.post("/api")
.set({ [authTokenHeader]: gardenServer.authKey })
.send({ command: "get.config" })
.expect(503)
})

it("should execute a command and return its results", async () => {
const res = await request(server).post("/api").send({ command: "get.config" }).expect(200)
const res = await request(server)
.post("/api")
.set({ [authTokenHeader]: gardenServer.authKey })
.send({ command: "get.config" })
.expect(200)
const config = await garden.dumpConfig({ log: garden.log })
expect(res.body.result).to.eql(deepOmitUndefined(config))
})

it("should correctly map arguments and options to commands", async () => {
const res = await request(server)
.post("/api")
.set({ [authTokenHeader]: gardenServer.authKey })
.send({
command: "build",
parameters: {
Expand All @@ -119,8 +155,23 @@ describe("GardenServer", () => {
})

describe("/dashboardPages", () => {
it("returns 401 if missing auth header", async () => {
await request(server).get("/dashboardPages/test-plugin/test").expect(401)
})

it("returns 401 if auth header doesn't match auth key", async () => {
await request(server)
.get("/dashboardPages/test-plugin/test")
.set({ [authTokenHeader]: "foo" })
.send({})
.expect(401)
})

it("should resolve the URL for the given dashboard page and redirect", async () => {
const res = await request(server).get("/dashboardPages/test-plugin/test").expect(302)
const res = await request(server)
.get("/dashboardPages/test-plugin/test")
.set({ [authTokenHeader]: gardenServer.authKey })
.expect(302)

expect(res.header.location).to.equal("http://localhost:12345/test")
})
Expand Down Expand Up @@ -162,7 +213,7 @@ describe("GardenServer", () => {
let ws: WebSocket

beforeEach((done) => {
ws = new WebSocket(`ws://localhost:${port}/ws`)
ws = new WebSocket(`ws://localhost:${port}/ws?sessionId=${garden.sessionId}`)
ws.on("open", () => {
done()
})
Expand All @@ -177,6 +228,34 @@ describe("GardenServer", () => {
ws.on("message", (msg) => cb(JSON.parse(msg.toString())))
}

it("terminates the connection if auth query params are missing", (done) => {
const badWs = new WebSocket(`ws://localhost:${port}/ws`)
badWs.on("close", () => {
done()
})
})

it("terminates the connection if key doesn't match and sessionId is missing", (done) => {
const badWs = new WebSocket(`ws://localhost:${port}/ws?key=foo`)
badWs.on("close", () => {
done()
})
})

it("terminates the connection if sessionId doesn't match and key is missing", (done) => {
const badWs = new WebSocket(`ws://localhost:${port}/ws?sessionId=foo`)
badWs.on("close", () => {
done()
})
})

it("terminates the connection if both sessionId and key are bad", (done) => {
const badWs = new WebSocket(`ws://localhost:${port}/ws?sessionId=foo&key=bar`)
badWs.on("close", () => {
done()
})
})

it("should emit events from the Garden event bus", (done) => {
onMessage((req) => {
expect(req).to.eql({ type: "event", name: "_test", payload: "foo" })
Expand Down
4 changes: 2 additions & 2 deletions dashboard/src/api/actions.ts
Expand Up @@ -36,7 +36,7 @@ import {
FetchTaskResultParams,
FetchTestResultParams,
} from "./api"
import { getTestKey } from "../util/helpers"
import { getAuthKey, getTestKey } from "../util/helpers"
import { ProviderMap } from "@garden-io/core/build/src/config/provider"
import { DashboardPage } from "@garden-io/core/build/src/types/plugin/provider/getDashboardPage"

Expand Down Expand Up @@ -113,7 +113,7 @@ function processConfigInitResult(entities: Entities, config: ConfigDump) {
path: `/provider/${provider.name}/${page.name}`,
description: page.description + ` (from provider ${provider.name})`,
// Use static URL if provided, otherwise we'll request a redirect from this API endpoint
url: page.url || `/dashboardPages/${provider.name}/${page.name}`,
url: page.url || `/dashboardPages/${provider.name}/${page.name}?key=${getAuthKey()}`,
}))
})

Expand Down

0 comments on commit 56051a5

Please sign in to comment.