a
/
code-server
1
0
Fork 0
Code Issues Pull Requests Projects Releases Wiki Activity

Clarify some points around the cookie domain 

Also add a check that the domain has a dot. This covers the localhost
case as well, so remove that.
This commit is contained in:
Asher 2020-11-10 18:33:16 -06:00
parent 4574593664
commit 79478eb89f
No known key found for this signature in database
GPG Key ID: D63C1EF81242354A
1 changed files with 19 additions and 10 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

29
src/node/http.ts
View File

@ -98,27 +98,36 @@ export const redirect = (
/** /**
* Get the value that should be used for setting a cookie domain. This will * Get the value that should be used for setting a cookie domain. This will
* allow the user to authenticate only once. This will use the highest level * allow the user to authenticate once no matter what sub-domain they use to log
* domain (e.g. `coder.com` over `test.coder.com` if both are specified). * in. This will use the highest level proxy domain (e.g. `coder.com` over
* `test.coder.com` if both are specified).
*/ */
export const getCookieDomain = (host: string, proxyDomains: string[]): string | undefined => { export const getCookieDomain = (host: string, proxyDomains: string[]): string | undefined => {
const idx = host.lastIndexOf(":") const idx = host.lastIndexOf(":")
host = idx !== -1 ? host.substring(0, idx) : host host = idx !== -1 ? host.substring(0, idx) : host
// If any of these are true we will still set cookies but without an explicit
// `Domain` attribute on the cookie.
if ( if (
// Might be blank/missing, so there's nothing more to do. // The host can be be blank or missing so there's nothing we can set.
!host || !host ||
// IP addresses can't have subdomains so there's no value in setting the // IP addresses can't have subdomains so there's no value in setting the
// domain for them. Assume anything with a : is ipv6 (valid domain name // domain for them. Assume that anything with a : is ipv6 (valid domain name
// characters are alphanumeric or dashes). // characters are alphanumeric or dashes)...
host.includes(":") || host.includes(":") ||
// Assume anything entirely numbers and dots is ipv4 (currently tlds // ...and that anything entirely numbers and dots is ipv4 (currently tlds
// cannot be entirely numbers). // cannot be entirely numbers).
!/[^0-9.]/.test(host) || !/[^0-9.]/.test(host) ||
// localhost subdomains don't seem to work at all (browser bug?). // localhost subdomains don't seem to work at all (browser bug?). A cookie
// set at dev.localhost cannot be read by 8080.dev.localhost.
host.endsWith(".localhost") || host.endsWith(".localhost") ||
// It might be localhost (or an IP, see above) if it's a proxy and it // Domains without at least one dot (technically two since domain.tld will
// isn't setting the host header to match the access domain. // become .domain.tld) are considered invalid according to the spec so don't
host === "localhost" // set the domain for them. In my testing though localhost is the only
// problem (the browser just doesn't store the cookie at all). localhost has
// an additional problem which is that a reverse proxy might give
// code-server localhost even though the domain is really domain.tld (by
// default NGINX does this).
!host.includes(".")
) { ) {
logger.debug("no valid cookie doman", field("host", host)) logger.debug("no valid cookie doman", field("host", host))
return undefined return undefined