From 2086648c874173ea649cee70e56b392515619cd2 Mon Sep 17 00:00:00 2001 From: Asher Date: Mon, 23 Mar 2020 14:51:58 -0500 Subject: [PATCH] Only handle exact domain matches This simplifies the logic a bit. --- src/node/app/proxy.ts | 39 +++++++++++++++++++++++---------------- src/node/http.ts | 29 +++++++++-------------------- 2 files changed, 32 insertions(+), 36 deletions(-) diff --git a/src/node/app/proxy.ts b/src/node/app/proxy.ts index dd8f6bda..912d94e1 100644 --- a/src/node/app/proxy.ts +++ b/src/node/app/proxy.ts @@ -6,8 +6,15 @@ import { HttpProvider, HttpProviderOptions, HttpProxyProvider, HttpResponse, Rou * Proxy HTTP provider. */ export class ProxyHttpProvider extends HttpProvider implements HttpProxyProvider { + /** + * Proxy domains are stored here without the leading `*.` + */ public readonly proxyDomains: string[] + /** + * Domains can be provided in the form `coder.com` or `*.coder.com`. Either + * way, `.coder.com` will be proxied to `number`. + */ public constructor(options: HttpProviderOptions, proxyDomains: string[] = []) { super(options) this.proxyDomains = proxyDomains.map((d) => d.replace(/^\*\./, "")).filter((d, i, arr) => arr.indexOf(d) === i) @@ -29,12 +36,14 @@ export class ProxyHttpProvider extends HttpProvider implements HttpProxyProvider throw new HttpError("Not found", HttpCode.NotFound) } - public getProxyDomain(host?: string): string | undefined { - if (!host || !this.proxyDomains) { - return undefined - } - - return this.proxyDomains.find((d) => host.endsWith(d)) + public getCookieDomain(host: string): string { + let current: string | undefined + this.proxyDomains.forEach((domain) => { + if (host.endsWith(domain) && (!current || domain.length < current.length)) { + current = domain + } + }) + return current || host } public maybeProxy(request: http.IncomingMessage): HttpResponse | undefined { @@ -44,23 +53,21 @@ export class ProxyHttpProvider extends HttpProvider implements HttpProxyProvider return undefined } + // At minimum there needs to be sub.domain.tld. const host = request.headers.host - const proxyDomain = this.getProxyDomain(host) - if (!host || !proxyDomain) { + const parts = host && host.split(".") + if (!parts || parts.length < 3) { return undefined } - const proxyDomainLength = proxyDomain.split(".").length - const portStr = host - .split(".") - .slice(0, -proxyDomainLength) - .pop() - - if (!portStr) { + // There must be an exact match. + const port = parts.shift() + const proxyDomain = parts.join(".") + if (!port || !this.proxyDomains.includes(proxyDomain)) { return undefined } - return this.proxy(portStr) + return this.proxy(port) } private proxy(portStr: string): HttpResponse { diff --git a/src/node/http.ts b/src/node/http.ts index 4303ae02..411e3ada 100644 --- a/src/node/http.ts +++ b/src/node/http.ts @@ -397,27 +397,20 @@ export interface HttpProvider3 { export interface HttpProxyProvider { /** * Return a response if the request should be proxied. Anything that ends in a - * proxy domain and has a subdomain should be proxied. The port is found in - * the top-most subdomain. + * proxy domain and has a *single* subdomain should be proxied. Anything else + * should return `undefined` and will be handled as normal. * - * For example, if the proxy domain is `coder.com` then `8080.coder.com` and - * `test.8080.coder.com` will both proxy to `8080` but `8080.test.coder.com` - * will have an error because `test` isn't a port. If the proxy domain was - * `test.coder.com` then it would work. + * For example if `coder.com` is specified `8080.coder.com` will be proxied + * but `8080.test.coder.com` and `test.8080.coder.com` will not. */ maybeProxy(request: http.IncomingMessage): HttpResponse | undefined /** - * Get the matching proxy domain based on the provided host. + * Get the domain that should be used for setting a cookie. This will allow + * the user to authenticate only once. This will return the highest level + * domain (e.g. `coder.com` over `test.coder.com` if both are specified). */ - getProxyDomain(host: string): string | undefined - - /** - * Domains can be provided in the form `coder.com` or `*.coder.com`. Either - * way, `.coder.com` will be proxied to `number`. The domains are - * stored here without the `*.`. - */ - readonly proxyDomains: string[] + getCookieDomain(host: string): string | undefined } /** @@ -560,12 +553,8 @@ export class HttpServer { "Set-Cookie": [ `${payload.cookie.key}=${payload.cookie.value}`, `Path=${normalize(payload.cookie.path || "/", true)}`, - // Set the cookie against the host so it can be used in - // subdomains. Use a matching proxy domain if possible so - // requests to any of those subdomains will already be - // authenticated. request.headers.host - ? `Domain=${(this.proxy && this.proxy.getProxyDomain(request.headers.host)) || request.headers.host}` + ? `Domain=${(this.proxy && this.proxy.getCookieDomain(request.headers.host)) || request.headers.host}` : undefined, // "HttpOnly", "SameSite=strict",