Skip to content

Commit 893e135

Browse files
committed
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.
1 parent 4ae0ef6 commit 893e135

3 files changed

Lines changed: 23 additions & 3 deletions

File tree

backend/config/passportConfig.js

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -35,9 +35,12 @@ passport.serializeUser((user, done) => {
3535
});
3636

3737
// Deserialize user (retrieve user from session)
38+
// Select only the fields needed for request handling. Excluding password
39+
// prevents the bcrypt hash from being attached to req.user and accidentally
40+
// serialized into API responses.
3841
passport.deserializeUser(async (id, done) => {
3942
try {
40-
const user = await User.findById(id);
43+
const user = await User.findById(id).select('-password -__v').lean();
4144
done(null, user);
4245
} catch (err) {
4346
done(err, null);

backend/routes/auth.js

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -36,14 +36,23 @@ router.post("/login", validateRequest(loginSchema), passport.authenticate('local
3636
});
3737

3838
// Logout route
39-
router.get("/logout", (req, res) => {
39+
// POST is required here: GET is a safe/idempotent method that browsers trigger
40+
// automatically (img src, prefetch, etc.), which would allow any third-party
41+
// page to force-logout authenticated users via a passive CSRF request.
42+
router.post("/logout", (req, res) => {
4043

4144
req.logout((err) => {
4245

4346
if (err)
4447
return res.status(500).json({ message: 'Logout failed', error: err.message });
45-
else
48+
49+
req.session.destroy((destroyErr) => {
50+
if (destroyErr) {
51+
return res.status(500).json({ message: 'Session cleanup failed', error: destroyErr.message });
52+
}
53+
res.clearCookie('connect.sid');
4654
res.status(200).json({ message: 'Logged out successfully' });
55+
});
4756
});
4857
});
4958

backend/server.js

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -28,10 +28,18 @@ app.use(cors({
2828

2929
// Middleware
3030
app.use(bodyParser.json());
31+
const isProduction = process.env.NODE_ENV === 'production';
32+
3133
app.use(session({
3234
secret: process.env.SESSION_SECRET,
3335
resave: false,
3436
saveUninitialized: false,
37+
cookie: {
38+
httpOnly: true,
39+
secure: isProduction,
40+
sameSite: 'strict',
41+
maxAge: 24 * 60 * 60 * 1000, // 24 hours
42+
},
3543
}));
3644
app.use(passport.initialize());
3745
app.use(passport.session());

0 commit comments

Comments
 (0)