Skip to content

Commit c7b38e4

Browse files
committed
Only handle exact domain matches
This simplifies the logic a bit.
1 parent 198c113 commit c7b38e4

File tree

2 files changed

+32
-36
lines changed

2 files changed

+32
-36
lines changed

src/node/app/proxy.ts

+23-16
Original file line numberDiff line numberDiff line change
@@ -6,8 +6,15 @@ import { HttpProvider, HttpProviderOptions, HttpProxyProvider, HttpResponse, Rou
66
* Proxy HTTP provider.
77
*/
88
export class ProxyHttpProvider extends HttpProvider implements HttpProxyProvider {
9+
/**
10+
* Proxy domains are stored here without the leading `*.`
11+
*/
912
public readonly proxyDomains: string[]
1013

14+
/**
15+
* Domains can be provided in the form `coder.com` or `*.coder.com`. Either
16+
* way, `<number>.coder.com` will be proxied to `number`.
17+
*/
1118
public constructor(options: HttpProviderOptions, proxyDomains: string[] = []) {
1219
super(options)
1320
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
2936
throw new HttpError("Not found", HttpCode.NotFound)
3037
}
3138

32-
public getProxyDomain(host?: string): string | undefined {
33-
if (!host || !this.proxyDomains) {
34-
return undefined
35-
}
36-
37-
return this.proxyDomains.find((d) => host.endsWith(d))
39+
public getCookieDomain(host: string): string {
40+
let current: string | undefined
41+
this.proxyDomains.forEach((domain) => {
42+
if (host.endsWith(domain) && (!current || domain.length < current.length)) {
43+
current = domain
44+
}
45+
})
46+
return current || host
3847
}
3948

4049
public maybeProxy(request: http.IncomingMessage): HttpResponse | undefined {
@@ -44,23 +53,21 @@ export class ProxyHttpProvider extends HttpProvider implements HttpProxyProvider
4453
return undefined
4554
}
4655

56+
// At minimum there needs to be sub.domain.tld.
4757
const host = request.headers.host
48-
const proxyDomain = this.getProxyDomain(host)
49-
if (!host || !proxyDomain) {
58+
const parts = host && host.split(".")
59+
if (!parts || parts.length < 3) {
5060
return undefined
5161
}
5262

53-
const proxyDomainLength = proxyDomain.split(".").length
54-
const portStr = host
55-
.split(".")
56-
.slice(0, -proxyDomainLength)
57-
.pop()
58-
59-
if (!portStr) {
63+
// There must be an exact match.
64+
const port = parts.shift()
65+
const proxyDomain = parts.join(".")
66+
if (!port || !this.proxyDomains.includes(proxyDomain)) {
6067
return undefined
6168
}
6269

63-
return this.proxy(portStr)
70+
return this.proxy(port)
6471
}
6572

6673
private proxy(portStr: string): HttpResponse {

src/node/http.ts

+9-20
Original file line numberDiff line numberDiff line change
@@ -397,27 +397,20 @@ export interface HttpProvider3<A1, A2, A3, T> {
397397
export interface HttpProxyProvider {
398398
/**
399399
* Return a response if the request should be proxied. Anything that ends in a
400-
* proxy domain and has a subdomain should be proxied. The port is found in
401-
* the top-most subdomain.
400+
* proxy domain and has a *single* subdomain should be proxied. Anything else
401+
* should return `undefined` and will be handled as normal.
402402
*
403-
* For example, if the proxy domain is `coder.com` then `8080.coder.com` and
404-
* `test.8080.coder.com` will both proxy to `8080` but `8080.test.coder.com`
405-
* will have an error because `test` isn't a port. If the proxy domain was
406-
* `test.coder.com` then it would work.
403+
* For example if `coder.com` is specified `8080.coder.com` will be proxied
404+
* but `8080.test.coder.com` and `test.8080.coder.com` will not.
407405
*/
408406
maybeProxy(request: http.IncomingMessage): HttpResponse | undefined
409407

410408
/**
411-
* Get the matching proxy domain based on the provided host.
409+
* Get the domain that should be used for setting a cookie. This will allow
410+
* the user to authenticate only once. This will return the highest level
411+
* domain (e.g. `coder.com` over `test.coder.com` if both are specified).
412412
*/
413-
getProxyDomain(host: string): string | undefined
414-
415-
/**
416-
* Domains can be provided in the form `coder.com` or `*.coder.com`. Either
417-
* way, `<number>.coder.com` will be proxied to `number`. The domains are
418-
* stored here without the `*.`.
419-
*/
420-
readonly proxyDomains: string[]
413+
getCookieDomain(host: string): string | undefined
421414
}
422415

423416
/**
@@ -560,12 +553,8 @@ export class HttpServer {
560553
"Set-Cookie": [
561554
`${payload.cookie.key}=${payload.cookie.value}`,
562555
`Path=${normalize(payload.cookie.path || "/", true)}`,
563-
// Set the cookie against the host so it can be used in
564-
// subdomains. Use a matching proxy domain if possible so
565-
// requests to any of those subdomains will already be
566-
// authenticated.
567556
request.headers.host
568-
? `Domain=${(this.proxy && this.proxy.getProxyDomain(request.headers.host)) || request.headers.host}`
557+
? `Domain=${(this.proxy && this.proxy.getCookieDomain(request.headers.host)) || request.headers.host}`
569558
: undefined,
570559
// "HttpOnly",
571560
"SameSite=strict",

0 commit comments

Comments
 (0)