put auth token in cookies (#1457)

* put auth token in cookies fixes #1454 helps with #1456 * remove lusca because csrf is mitigated by cookie options
dyc3 · Mar 7, 2024 · cc043a5 · cc043a5
1 parent b8473a6
commit cc043a5
Show file tree

Hide file tree

Showing 6 changed files with 43 additions and 12 deletions.
diff --git a/common/models/types.ts b/common/models/types.ts
@@ -43,7 +43,6 @@ export type AuthToken = string;
 export type MySession = Session & {
 	username?: string;
 	passport?: { user?: number };
-	token?: AuthToken;
 	postLoginRedirect?: string;
 };
 

diff --git a/server/app.ts b/server/app.ts
@@ -22,6 +22,7 @@ import { initExtractor } from "./infoextractor";
 import session, { SessionOptions } from "express-session";
 import RedisStore from "connect-redis";
 import { setupPostgresMetricsCollection } from "./storage.metrics";
+import cookieparser from "cookie-parser";
 
 const app = express();
 
@@ -77,6 +78,8 @@ export async function main() {
 	process.on("SIGTERM", shutdown);
 
 	app.use(metricsMiddleware);
+	app.use(cookieparser(conf.get("session_secret")));
+
 	const server = http.createServer(app);
 	async function checkRedis() {
 		if (performance) {

diff --git a/server/auth/index.ts b/server/auth/index.ts
@@ -8,6 +8,7 @@ import nocache from "nocache";
 import usermanager from "../usermanager";
 import { OttException } from "ott-common/exceptions";
 import { requireApiKey } from "../admin";
+import { conf } from "../ott-config";
 
 const router = express.Router();
 router.use(nocache());
@@ -53,6 +54,8 @@ export async function authTokenMiddleware(
 	if (req.headers.authorization && req.headers.authorization.startsWith("Bearer")) {
 		const token: AuthToken = req.headers.authorization.split(" ")[1];
 		req.token = token;
+	} else if (req.cookies?.token) {
+		req.token = req.cookies.token;
 	}
 
 	if (!req.token || !(await tokens.validate(req.token))) {
@@ -66,8 +69,6 @@ export async function authTokenMiddleware(
 		return;
 	}
 
-	// eslint-disable-next-line @typescript-eslint/no-unnecessary-type-assertion
-	(req.session as MySession).token = req.token;
 	req.ottsession = await tokens.getSessionInfo(req.token);
 	if (req.ottsession && req.ottsession.isLoggedIn) {
 		try {
@@ -87,7 +88,11 @@ router.get("/grant", async (req, res) => {
 			const token: AuthToken = req.headers.authorization.split(" ")[1];
 			if (await tokens.validate(token)) {
 				log.debug("token is already valid");
-				res.json({
+				res.cookie("token", token, {
+					httpOnly: true,
+					sameSite: "lax",
+					secure: !conf.get("force_insecure_cookies"),
+				}).json({
 					token,
 				});
 				return;
@@ -101,7 +106,11 @@ router.get("/grant", async (req, res) => {
 	log.debug("minting new auth token...");
 	const token: AuthToken = await tokens.mint();
 	await tokens.setSessionInfo(token, createSession());
-	res.json({
+	res.cookie("token", token, {
+		httpOnly: true,
+		sameSite: "lax",
+		secure: !conf.get("force_insecure_cookies"),
+	}).json({
 		token,
 	});
 });
@@ -132,8 +141,7 @@ router.get(
 			});
 			return;
 		}
-		// eslint-disable-next-line @typescript-eslint/no-unnecessary-type-assertion
-		const token = (req.session as MySession).token;
+		const token = req.cookies?.token;
 		if (!token) {
 			res.status(400).json({
 				success: false,

diff --git a/server/package.json b/server/package.json
@@ -21,6 +21,7 @@
     "axios": "1.6.7",
     "connect-redis": "7.1.0",
     "convict": "^6.2.4",
+    "cookie-parser": "^1.4.6",
     "dayjs": "^1.10.4",
     "express": "^4.17.1",
     "express-session": "^1.17.0",
@@ -58,6 +59,7 @@
   },
   "devDependencies": {
     "@types/convict": "^6.1.1",
+    "@types/cookie-parser": "^1.4.7",
     "@types/express": "^4.17.11",
     "@types/express-session": "^1.17.3",
     "@types/lodash": "^4.14.170",

diff --git a/server/tests/unit/api/user.spec.ts b/server/tests/unit/api/user.spec.ts
@@ -274,14 +274,13 @@ describe("User API", () => {
 					.set("Authorization", `Bearer ${token}`)
 					.expect(200);
 
-				await request(app)
+				const resp = await request(app)
 					.post("/api/user/logout")
 					.set("Authorization", `Bearer ${token}`)
 					.expect("Content-Type", /json/)
-					.expect(200)
-					.then(resp => {
-						expect(resp.body.success).toBe(true);
-					});
+					.expect(200);
+
+				expect(resp.body.success).toBe(true);
 			});
 
 			it("should fail if the user is not logged in", async () => {

diff --git a/yarn.lock b/yarn.lock
@@ -3243,6 +3243,13 @@
   dependencies:
     "@types/node" "*"
 
+"@types/cookie-parser@^1.4.7":
+  version "1.4.7"
+  resolved "https://registry.yarnpkg.com/@types/cookie-parser/-/cookie-parser-1.4.7.tgz#c874471f888c72423d78d2b3c32d1e8579cf3c8f"
+  integrity sha512-Fvuyi354Z+uayxzIGCwYTayFKocfV7TuDYZClCdIP9ckhvAu/ixDtCB6qx2TT0FKjPLf1f3P/J1rgf6lPs64mw==
+  dependencies:
+    "@types/express" "*"
+
 "@types/cookiejar@*":
   version "2.1.2"
   resolved "https://registry.yarnpkg.com/@types/cookiejar/-/cookiejar-2.1.2.tgz#66ad9331f63fe8a3d3d9d8c6e3906dd10f6446e8"
@@ -6111,11 +6118,24 @@ convict@^6.2.4:
     lodash.clonedeep "^4.5.0"
     yargs-parser "^20.2.7"
 
+cookie-parser@^1.4.6:
+  version "1.4.6"
+  resolved "https://registry.yarnpkg.com/cookie-parser/-/cookie-parser-1.4.6.tgz#3ac3a7d35a7a03bbc7e365073a26074824214594"
+  integrity sha512-z3IzaNjdwUC2olLIB5/ITd0/setiaFMLYiZJle7xg5Fe9KWAceil7xszYfHHBtDFYLSgJduS2Ty0P1uJdPDJeA==
+  dependencies:
+    cookie "0.4.1"
+    cookie-signature "1.0.6"
+
 cookie-signature@1.0.6:
   version "1.0.6"
   resolved "https://registry.yarnpkg.com/cookie-signature/-/cookie-signature-1.0.6.tgz#e303a882b342cc3ee8ca513a79999734dab3ae2c"
   integrity sha512-QADzlaHc8icV8I7vbaJXJwod9HWYp8uCqf1xa4OfNu1T7JVxQIrUgOWtHdNDtPiywmFbiS12VjotIXLrKM3orQ==
 
+cookie@0.4.1:
+  version "0.4.1"
+  resolved "https://registry.yarnpkg.com/cookie/-/cookie-0.4.1.tgz#afd713fe26ebd21ba95ceb61f9a8116e50a537d1"
+  integrity sha512-ZwrFkGJxUR3EIoXtO+yVE69Eb7KlixbaeAWfBQB9vVsNn/o+Yw69gBWSSDK825hQNdN+wF8zELf3dFNl/kxkUA==
+
 cookie@0.4.2:
   version "0.4.2"
   resolved "https://registry.yarnpkg.com/cookie/-/cookie-0.4.2.tgz#0e41f24de5ecf317947c82fc789e06a884824432"