From 2086648c874173ea649cee70e56b392515619cd2 Mon Sep 17 00:00:00 2001
From: Asher <ash@coder.com>
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, `<number>.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<A1, A2, A3, T> {
 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, `<number>.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",
-- 
GitLab