Skip to content

Commit

Permalink
Fix for users getting redirected to the wrong path
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
kdeal committed Oct 6, 2023
1 parent 5888702 commit 62b7490
Show file tree
Hide file tree
Showing 2 changed files with 30 additions and 15 deletions.
36 changes: 24 additions & 12 deletions frontend/routes/authRoutes.js
Original file line number Diff line number Diff line change
@@ -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);
}
},
);

Expand Down
9 changes: 6 additions & 3 deletions frontend/webapp.js
Original file line number Diff line number Diff line change
Expand Up @@ -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"],
Expand All @@ -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();
};
Expand Down Expand Up @@ -67,7 +70,7 @@ app.get("/admin/subscriptions/:id", (req, res) => {
});

app.get("/email", (req, res) => {
res.send({ email: req.user });
res.send({ email: "[email protected]" });
});

apiRoutes(app);
Expand Down

0 comments on commit 62b7490

Please sign in to comment.