From 893e135c272c2cfcb7131a7e964b4004dfd3d06e Mon Sep 17 00:00:00 2001 From: anshul23102 Date: Wed, 27 May 2026 20:26:02 +0530 Subject: [PATCH] fix(auth): harden session cookie, strip password from req.user, change logout to POST Fixes #554 - Add httpOnly, Secure, and SameSite=strict to the session cookie so the token is inaccessible to JavaScript and cannot be sent in cross-origin requests. The Secure flag is conditioned on NODE_ENV=production to keep local development functional over HTTP. Fixes #555 - Pass .select('-password -__v').lean() to deserializeUser so the bcrypt hash is never attached to req.user. Using .lean() also returns a plain object instead of a full Mongoose document, preventing accidental exposure of model methods on the request context. Fixes #556 - Change the logout endpoint from GET to POST. GET is a safe, idempotent method that browsers trigger automatically (img src, link prefetch, CSS url). Any third-party page could embed a zero-pixel image pointing at the logout URL to silently sign out authenticated users. POST requires an explicit fetch or form submission, which CORS and SameSite=strict will block from cross-origin callers. The handler now also calls req.session.destroy() and clears the cookie to ensure the session is fully invalidated server-side. --- backend/config/passportConfig.js | 5 ++++- backend/routes/auth.js | 13 +++++++++++-- backend/server.js | 8 ++++++++ 3 files changed, 23 insertions(+), 3 deletions(-) diff --git a/backend/config/passportConfig.js b/backend/config/passportConfig.js index 842f50ca..34d3c37a 100644 --- a/backend/config/passportConfig.js +++ b/backend/config/passportConfig.js @@ -35,9 +35,12 @@ passport.serializeUser((user, done) => { }); // Deserialize user (retrieve user from session) +// Select only the fields needed for request handling. Excluding password +// prevents the bcrypt hash from being attached to req.user and accidentally +// serialized into API responses. passport.deserializeUser(async (id, done) => { try { - const user = await User.findById(id); + const user = await User.findById(id).select('-password -__v').lean(); done(null, user); } catch (err) { done(err, null); diff --git a/backend/routes/auth.js b/backend/routes/auth.js index 7c2cda78..467313a6 100644 --- a/backend/routes/auth.js +++ b/backend/routes/auth.js @@ -36,14 +36,23 @@ router.post("/login", validateRequest(loginSchema), passport.authenticate('local }); // Logout route -router.get("/logout", (req, res) => { +// POST is required here: GET is a safe/idempotent method that browsers trigger +// automatically (img src, prefetch, etc.), which would allow any third-party +// page to force-logout authenticated users via a passive CSRF request. +router.post("/logout", (req, res) => { req.logout((err) => { if (err) return res.status(500).json({ message: 'Logout failed', error: err.message }); - else + + req.session.destroy((destroyErr) => { + if (destroyErr) { + return res.status(500).json({ message: 'Session cleanup failed', error: destroyErr.message }); + } + res.clearCookie('connect.sid'); res.status(200).json({ message: 'Logged out successfully' }); + }); }); }); diff --git a/backend/server.js b/backend/server.js index 48d6ccfb..80ad6d38 100644 --- a/backend/server.js +++ b/backend/server.js @@ -28,10 +28,18 @@ app.use(cors({ // Middleware app.use(bodyParser.json()); +const isProduction = process.env.NODE_ENV === 'production'; + app.use(session({ secret: process.env.SESSION_SECRET, resave: false, saveUninitialized: false, + cookie: { + httpOnly: true, + secure: isProduction, + sameSite: 'strict', + maxAge: 24 * 60 * 60 * 1000, // 24 hours + }, })); app.use(passport.initialize()); app.use(passport.session());