From 79808a4ccf498fb1d4bc635308d21c1ec1ef94c4 Mon Sep 17 00:00:00 2001
From: Kyle Deal <kdeal@kyledeal.com>
Date: Fri, 6 Oct 2023 19:02:57 +0000
Subject: [PATCH] Fix for users getting redirected to the wrong path

Users were no longer getting redirected back to the correct page after
logging in. This was broken because passport started clearing the
session store for users after authenticating them for security reasons
in newer versions. This means the session.oauth2return is always empty
after login, so users are always redirected to the home page. The oauth
flow does allow us to send a state variable that will be forwarded to
the callback url. It is recommended to either make the state variable a
random string and store the state elsewhere or sign the data that you
put into it. It seemed easier to use a random string and store the state
in app.locals, so I went that route. The app.locals is in memory, so if
it gets restarted the state will be lost. This is already true of our
session store, so it shouldn't be any worse then our current state.
---
 frontend/routes/authRoutes.js | 36 +++++++++++++++++++++++------------
 frontend/webapp.js            |  7 +++++--
 2 files changed, 29 insertions(+), 14 deletions(-)

diff --git a/frontend/routes/authRoutes.js b/frontend/routes/authRoutes.js
index 11e43aea..abc05839 100644
--- a/frontend/routes/authRoutes.js
+++ b/frontend/routes/authRoutes.js
@@ -1,24 +1,36 @@
+const crypto = require("crypto");
 const passport = require("passport");
 
 module.exports = (app) => {
-  app.get(
-    "/auth/google",
-    (req, res, next) => {
-      if (req.query.return) {
-        req.session.oauth2return = req.query.return;
+  app.get("/auth/google", (req, res, next) => {
+    crypto.randomBytes(128, (err, buff) => {
+      if (err) {
+        throw err;
       }
-      next();
-    },
-    passport.authenticate("google", { scope: ["email"] }),
-  );
+      const state = buff.toString("hex");
+      app.locals.stateStore.set(state, { returnUrl: req.query.return || "/" });
+      const authenticator = passport.authenticate("google", {
+        scope: ["email"],
+        state,
+      });
+      authenticator(req, res, next);
+    });
+  });
 
   app.get(
     "/auth/google/callback",
     passport.authenticate("google"),
     (req, res) => {
-      const redirect = req.session.oauth2return || "/";
-      delete req.session.oauth2return;
-      res.redirect(redirect);
+      const { state } = req.query;
+      if (app.locals.stateStore.has(state)) {
+        const { redirectUrl } = app.locals.stateStore.get(state).returnUrl;
+        app.locals.stateStore.delete(state);
+        res.redirect(redirectUrl);
+      } else {
+        // If we don't have anything in the state store then treat
+        // the login as invalid
+        res.sendStatus(403);
+      }
     },
   );
 
diff --git a/frontend/webapp.js b/frontend/webapp.js
index f37dba79..2fb12f69 100644
--- a/frontend/webapp.js
+++ b/frontend/webapp.js
@@ -8,6 +8,9 @@ const config = require("./lib/config");
 
 const app = express();
 
+// Initialize the state store
+app.locals.stateStore = new Map();
+
 const corsOptions = {
   origin: [`${config.get("PROJECT")}.appspot.com`, "localhost:5000"],
   allowedHeaders: ["Content-Type"],
@@ -30,8 +33,8 @@ app.use(session(sessionConfig));
 /* eslint-disable consistent-return */
 const authRequired = function authRequired(req, res, next) {
   if (!req.user) {
-    req.session.oauth2return = req.originalUrl;
-    return res.redirect("/auth/google");
+    const params = new URLSearchParams({ return: req.originalUrl });
+    return res.redirect(`/auth/google?${params.toString()}`);
   }
   next();
 };