From a1d6bcb8e5a5f25d94877a4029740208e36d0553 Mon Sep 17 00:00:00 2001
From: Asher <ash@coder.com>
Date: Thu, 7 Nov 2019 12:41:06 -0600
Subject: [PATCH] Handle cookies more robustly

If you visit /login/ instead of /login the cookie will be set at /login
instead of / which means the cookie can't be read at the root. It will
redirect to the login page which *can* read the cookie at /login and
redirect back resulting in an infinite loop.

The previous solution relied on setting the cookie at / (any invalid
value works) which then overrode the login page cookie since
parseCookies only kept a single value. So the login page would see the
same cookie the root was seeing and not redirect back. However, that
behavior depends on the cookies being in the right order which I'm not
sure is guaranteed.

This new method tests all available cookies and always sets the cookie
so the root path will be able to read it in case the login page is
seeing a cookie the root can't.

It also goes a step further and explicitly sets the path on the cookie
which fixes the case where there is a permanent misconfiguration
redirecting /login to /login/. Otherwise the cookie would continually be
set on /login only and you'd have another loop. It also means you only
need to delete one cookie to log out.

Lastly add some properties to make the cookies a bit more secure.
---
 src/node/server.ts | 44 +++++++++++++++++++++++++++++---------------
 1 file changed, 29 insertions(+), 15 deletions(-)

diff --git a/src/node/server.ts b/src/node/server.ts
index 92c406282..c95ad1fb5 100644
--- a/src/node/server.ts
+++ b/src/node/server.ts
@@ -98,7 +98,7 @@ export interface Response {
 }
 
 export interface LoginPayload {
-	password?: string;
+	password?: string[] | string;
 }
 
 export class HttpError extends Error {
@@ -298,10 +298,7 @@ export abstract class Server {
 						return response;
 				}
 				if (!this.authenticate(request)) {
-					return { 
-			   			redirect: "/login",
-			   			headers: { "Set-Cookie": `password=` }
-			   		};
+					return { redirect: "/login" };
 				}
 				break;
 			case "/static":
@@ -360,16 +357,22 @@ export abstract class Server {
 	}
 
 	private async tryLogin(request: http.IncomingMessage): Promise<Response> {
-		if (this.authenticate(request) && (request.method === "GET" || request.method === "POST")) {
-			return { redirect: "/" };
+		const redirect = (password?: string | string[] | true) => {
+			return {
+				redirect: "/",
+				headers: typeof password === "string"
+					? { "Set-Cookie": `password=${password}; Path=${this.options.basePath || "/"}; HttpOnly; SameSite=strict` }
+					: {},
+			};
+		};
+		const providedPassword = this.authenticate(request);
+		if (providedPassword && (request.method === "GET" || request.method === "POST")) {
+			return redirect(providedPassword);
 		}
 		if (request.method === "POST") {
 			const data = await this.getData<LoginPayload>(request);
 			if (this.authenticate(request, data)) {
-				return {
-					redirect: "/",
-					headers: { "Set-Cookie": `password=${data.password}` }
-				};
+				return redirect(data.password);
 			}
 			console.error("Failed login attempt", JSON.stringify({
 				xForwardedFor: request.headers["x-forwarded-for"],
@@ -429,7 +432,7 @@ export abstract class Server {
 			: Promise.resolve({} as T);
 	}
 
-	private authenticate(request: http.IncomingMessage, payload?: LoginPayload): boolean {
+	private authenticate(request: http.IncomingMessage, payload?: LoginPayload): string | boolean {
 		if (this.options.auth !== "password") {
 			return true;
 		}
@@ -437,15 +440,26 @@ export abstract class Server {
 		if (typeof payload === "undefined") {
 			payload = this.parseCookies<LoginPayload>(request);
 		}
-		return !!this.options.password && safeCompare(payload.password || "", this.options.password);
+		if (this.options.password && payload.password) {
+			const toTest = Array.isArray(payload.password) ? payload.password : [payload.password];
+			for (let i = 0; i < toTest.length; ++i) {
+				if (safeCompare(toTest[i], this.options.password)) {
+					return toTest[i];
+				}
+			}
+		}
+		return false;
 	}
 
 	private parseCookies<T extends object>(request: http.IncomingMessage): T {
-		const cookies: { [key: string]: string } = {};
+		const cookies: { [key: string]: string[] } = {};
 		if (request.headers.cookie) {
 			request.headers.cookie.split(";").forEach((keyValue) => {
 				const [key, value] = split(keyValue, "=");
-				cookies[key] = decodeURI(value);
+				if (!cookies[key]) {
+					cookies[key] = [];
+				}
+				cookies[key].push(decodeURI(value));
 			});
 		}
 		return cookies as T;