Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 15 additions & 0 deletions auth.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
router.get("/logout", (req, res) => {
req.logout(err => {
if(err) return res.status(500).json({ message: "Logout failed" });
res.json({ message: "Logged out successfully" });
});
});

// Get logged-in user
router.get("/current", (req, res) => {
if (req.isAuthenticated()) {
res.json({ user: { username: req.user.username } });
} else {
res.status(401).json({ user: null });
}
});
Comment on lines +1 to +15
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

⚠️ Potential issue

Router not declared/exported; file won’t work as-is.

Add Router setup and export, and return status codes explicitly.

+import express from 'express';
+const router = express.Router();
+
 router.get("/logout", (req, res) => {
   req.logout(err => {
-    if(err) return res.status(500).json({ message: "Logout failed" });
-    res.json({ message: "Logged out successfully" });
+    if (err) return res.status(500).json({ message: "Logout failed" });
+    return res.status(200).json({ message: "Logged out successfully" });
   });
 });
 
 // Get logged-in user
 router.get("/current", (req, res) => {
   if (req.isAuthenticated()) {
-    res.json({ user: { username: req.user.username } });
+    return res.status(200).json({ user: { username: req.user.username } });
   } else {
-    res.status(401).json({ user: null });
+    return res.status(401).json({ user: null });
   }
 });
+
+export default router;
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
router.get("/logout", (req, res) => {
req.logout(err => {
if(err) return res.status(500).json({ message: "Logout failed" });
res.json({ message: "Logged out successfully" });
});
});
// Get logged-in user
router.get("/current", (req, res) => {
if (req.isAuthenticated()) {
res.json({ user: { username: req.user.username } });
} else {
res.status(401).json({ user: null });
}
});
import express from 'express';
const router = express.Router();
router.get("/logout", (req, res) => {
req.logout(err => {
if (err) return res.status(500).json({ message: "Logout failed" });
return res.status(200).json({ message: "Logged out successfully" });
});
});
// Get logged-in user
router.get("/current", (req, res) => {
if (req.isAuthenticated()) {
return res.status(200).json({ user: { username: req.user.username } });
} else {
return res.status(401).json({ user: null });
}
});
export default router;
🤖 Prompt for AI Agents
In auth.js around lines 1-15, the file uses router but never declares or exports
it and some responses lack explicit status codes; fix by importing or requiring
express and creating const router = express.Router() at the top, replace plain
res.json success responses with explicit status codes (e.g., send 200 for
successful logout and current-user responses, keep 500 for logout error and 401
for unauthorized), and export the router at the bottom (module.exports = router
or export default router consistent with the project style).

16 changes: 16 additions & 0 deletions backend/config/db.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
const mongoose = require('mongoose');

const connectDB = async () => {
try {
await mongoose.connect('mongodb://127.0.0.1:27017/githubTracker', {
useNewUrlParser: true,
useUnifiedTopology: true,
});
console.log('MongoDB connected');
Comment on lines +5 to +9
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Use env-configured Mongo URI; drop legacy options

Hard-coding the URI hinders env parity and the listed options are no longer needed on modern Mongoose. Prefer an env var and defaults.

-    await mongoose.connect('mongodb://127.0.0.1:27017/githubTracker', {
-      useNewUrlParser: true,
-      useUnifiedTopology: true,
-    });
-    console.log('MongoDB connected');
+    const uri = process.env.MONGODB_URI || 'mongodb://127.0.0.1:27017/githubTracker';
+    await mongoose.connect(uri);
+    console.log('MongoDB connected');
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
await mongoose.connect('mongodb://127.0.0.1:27017/githubTracker', {
useNewUrlParser: true,
useUnifiedTopology: true,
});
console.log('MongoDB connected');
const uri = process.env.MONGODB_URI || 'mongodb://127.0.0.1:27017/githubTracker';
await mongoose.connect(uri);
console.log('MongoDB connected');
🤖 Prompt for AI Agents
In backend/config/db.js around lines 5 to 9, replace the hard-coded MongoDB
connection string with an env-configured URI (e.g., process.env.MONGODB_URI ||
process.env.MONGO_URI || 'mongodb://127.0.0.1:27017/githubTracker') and remove
the legacy mongoose options useNewUrlParser and useUnifiedTopology (they're not
needed on modern Mongoose); call mongoose.connect with the URI only, and
preserve/emit a clear success or error log (wrap connect in a try/catch or
handle the returned promise to log failures).

} catch (err) {
console.error(err.message);
process.exit(1);
}
};

module.exports = connectDB;
37 changes: 37 additions & 0 deletions backend/config/passport.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
import passport from 'passport';
import { Strategy as LocalStrategy } from 'passport-local';
import bcrypt from 'bcryptjs';
Comment on lines +1 to +3
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Verification agent

🧩 Analysis chain

Consolidate duplicate Passport configs.

Both backend/config/passport.js (ESM) and backend/config/passportConfig.js (CJS) initialize LocalStrategy. Keep one to avoid double registration and drift.


🏁 Script executed:

#!/bin/bash
# Locate all passport strategy registrations to remove duplicates.
rg -nP -C2 "\bpassport\.use\(\s*new\s+LocalStrategy"

Length of output: 0


🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "Locating passportConfig.js"
rg --files | grep passportConfig.js || echo "No passportConfig.js found"

echo
echo "Locating passport.js"
rg --files | grep 'config/passport.js' || echo "No passport.js found"

echo
echo "Searching for passport.use occurrences"
rg -n "passport\.use" || echo "No passport.use calls found"

echo
echo "Searching for new LocalStrategy"
rg -n "new LocalStrategy" || echo "No new LocalStrategy instantiations found"

echo
echo "Searching for passport-local imports"
rg -n "passport-local" || echo "No passport-local imports found"

Length of output: 1143


Consolidate Passport strategy registrations. We have three separate LocalStrategy initializations in config/passport.js, backend/config/passport.js, and backend/config/passportConfig.js. Centralize into a single module and remove the duplicates to prevent multiple registrations.

🤖 Prompt for AI Agents
In backend/config/passport.js around lines 1 to 3, there are duplicate
LocalStrategy initializations across config/passport.js,
backend/config/passport.js, and backend/config/passportConfig.js; consolidate
them into a single module: pick one canonical file (e.g.,
backend/config/passport.js), move the complete LocalStrategy registration and
related serialization/deserialization logic into that file, remove the duplicate
strategy registrations and imports from the other files (or have them import and
use the single canonical module), and ensure only the canonical module calls
passport.use(...) so the LocalStrategy is registered exactly once.

import User from '../models/User.js';

passport.use(
new LocalStrategy(async (username, password, done) => {
try {
// Find user by username
const user = await User.findOne({ username });
if (!user) return done(null, false, { message: 'Incorrect username' });

// Compare passwords
const isMatch = await bcrypt.compare(password, user.password);
if (!isMatch) return done(null, false, { message: 'Incorrect password' });

return done(null, user);
Comment on lines +6 to +17
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

⚠️ Potential issue

Login will fail if clients send email; set usernameField: 'email' and query by email.

Elsewhere (routes/tests) the login uses email. Update the strategy to read req.body.email and prefer the model’s comparePassword if available to keep hashing logic centralized.

-passport.use(
-  new LocalStrategy(async (username, password, done) => {
+passport.use(
+  new LocalStrategy(
+    { usernameField: 'email', passwordField: 'password', session: true },
+    async (email, password, done) => {
       try {
-      // Find user by username
-      const user = await User.findOne({ username });
-      if (!user) return done(null, false, { message: 'Incorrect username' });
+        // Find user by email
+        const user = await User.findOne({ email });
+        if (!user) return done(null, false, { message: 'Email not found' });
 
-      // Compare passwords
-      const isMatch = await bcrypt.compare(password, user.password);
-      if (!isMatch) return done(null, false, { message: 'Incorrect password' });
+        // Prefer model method if defined
+        const isMatch = typeof user.comparePassword === 'function'
+          ? await user.comparePassword(password)
+          : await bcrypt.compare(password, user.password);
+        if (!isMatch) return done(null, false, { message: 'Incorrect password' });
 
-      return done(null, user);
-    } catch (err) {
-      return done(err);
-    }
-  })
+        return done(null, user);
+      } catch (err) {
+        return done(err);
+      }
+    }
+  )
 );
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
passport.use(
new LocalStrategy(async (username, password, done) => {
try {
// Find user by username
const user = await User.findOne({ username });
if (!user) return done(null, false, { message: 'Incorrect username' });
// Compare passwords
const isMatch = await bcrypt.compare(password, user.password);
if (!isMatch) return done(null, false, { message: 'Incorrect password' });
return done(null, user);
passport.use(
new LocalStrategy(
{ usernameField: 'email', passwordField: 'password', session: true },
async (email, password, done) => {
try {
// Find user by email
const user = await User.findOne({ email });
if (!user) return done(null, false, { message: 'Email not found' });
// Prefer model method if defined
const isMatch = typeof user.comparePassword === 'function'
? await user.comparePassword(password)
: await bcrypt.compare(password, user.password);
if (!isMatch) return done(null, false, { message: 'Incorrect password' });
return done(null, user);
} catch (err) {
return done(err);
}
}
)
);
🤖 Prompt for AI Agents
In backend/config/passport.js lines 6-17, the LocalStrategy currently expects a
username and uses bcrypt.compare; change the strategy to use usernameField:
'email' (and passwordField if needed) so it reads the client's email, query User
by { email } instead of { username }, and replace direct bcrypt.compare with the
model's comparePassword method when present (e.g., await
user.comparePassword(password)) to centralize hashing logic and keep behavior
consistent with routes/tests.

} catch (err) {
return done(err);
}
})
);

// Serialize user into the session
passport.serializeUser((user, done) => {
done(null, user.id);
});

// Deserialize user from the session
passport.deserializeUser(async (id, done) => {
try {
const user = await User.findById(id).select('-password'); // Exclude password
done(null, user);
} catch (err) {
done(err);
}
});
12 changes: 7 additions & 5 deletions backend/config/passportConfig.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,23 +2,25 @@ const passport = require("passport");
const LocalStrategy = require('passport-local').Strategy;
const User = require("../models/User");

// Local strategy for login using email
passport.use(
new LocalStrategy(
{ usernameField: "email" },
{ usernameField: "email" }, // use email instead of username
async (email, password, done) => {
try {
const user = await User.findOne( {email} );
const user = await User.findOne({ email });
if (!user) {
return done(null, false, { message: 'Email is invalid '});
return done(null, false, { message: 'Email is invalid' });
}

const isMatch = await user.comparePassword(password);
if (!isMatch) {
return done(null, false, { message: 'Invalid password' });
}

// Return user object
return done(null, {
id : user._id.toString(),
id: user._id.toString(),
username: user.username,
email: user.email
});
Comment on lines +5 to 26
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Avoid credential enumeration in error messages

Different messages for unknown email vs bad password allow user enumeration. Return a single generic message.

-                if (!user) {
-                    return done(null, false, { message: 'Email is invalid' });
-                }
+                if (!user) {
+                    return done(null, false, { message: 'Invalid email or password' });
+                }
@@
-                if (!isMatch) {
-                    return done(null, false, { message: 'Invalid password' });
-                }
+                if (!isMatch) {
+                    return done(null, false, { message: 'Invalid email or password' });
+                }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
// Local strategy for login using email
passport.use(
new LocalStrategy(
{ usernameField: "email" },
{ usernameField: "email" }, // use email instead of username
async (email, password, done) => {
try {
const user = await User.findOne( {email} );
const user = await User.findOne({ email });
if (!user) {
return done(null, false, { message: 'Email is invalid '});
return done(null, false, { message: 'Email is invalid' });
}
const isMatch = await user.comparePassword(password);
if (!isMatch) {
return done(null, false, { message: 'Invalid password' });
}
// Return user object
return done(null, {
id : user._id.toString(),
id: user._id.toString(),
username: user.username,
email: user.email
});
// Local strategy for login using email
passport.use(
new LocalStrategy(
{ usernameField: "email" }, // use email instead of username
async (email, password, done) => {
try {
const user = await User.findOne({ email });
if (!user) {
return done(null, false, { message: 'Invalid email or password' });
}
const isMatch = await user.comparePassword(password);
if (!isMatch) {
return done(null, false, { message: 'Invalid email or password' });
}
// Return user object
return done(null, {
id: user._id.toString(),
username: user.username,
email: user.email
});
🤖 Prompt for AI Agents
In backend/config/passportConfig.js around lines 5 to 26, the LocalStrategy
currently returns different messages for "Email is invalid" and "Invalid
password", which enables credential enumeration; change both failure responses
to a single generic message (e.g., "Invalid email or password") and ensure
timing does not reveal existence (if user not found perform a dummy password
compare or a consistent async delay) so both unknown-email and wrong-password
cases behave identically.

Expand All @@ -29,7 +31,7 @@ passport.use(
)
);

// Serialize user (store user info in session)
// Serialize user (store user id in session)
passport.serializeUser((user, done) => {
done(null, user.id);
});
Expand Down
66 changes: 34 additions & 32 deletions backend/routes/auth.js
Original file line number Diff line number Diff line change
@@ -1,42 +1,44 @@
const express = require("express");
const passport = require("passport");
const User = require("../models/User");
const express = require('express');
const router = express.Router();
const passport = require('passport');
const User = require('../models/User');

// Signup
router.post('/signup', async (req, res) => {
const { username, email, password } = req.body;
try {
const user = new User({ username, email, password });
await user.save();
res.status(201).json({ message: 'User created successfully' });
} catch (err) {
res.status(400).json({ error: err.message });
}
});
Comment on lines +7 to +16
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Verification agent

🧩 Analysis chain

Align responses with tests and standardize JSON shape

Tests in spec/auth.routes.spec.cjs assert:

  • 201 + "User created successfully" on signup.
  • 400 + "User already exists" on duplicate.
  • 200 + "Login successful" on login.
  • 401 on wrong password.
  • 200 + "Logged out successfully" on logout.

Ensure all responses use { message: string } for messages and selected/safe user payloads only.

Run to sanity-check strings/statuses without executing tests:

Also applies to: 19-29, 32-36, 39-41


🏁 Script executed:

#!/bin/bash
rg -nC1 -e "User created successfully|User already exists|Login successful|Logged out successfully" backend/routes/auth.js
rg -nC2 -e "status\\(401\\)|status\\(400\\)" backend/routes/auth.js
rg -nC3 -e "expect\\(res.body.message\\)" spec/auth.routes.spec.cjs

Length of output: 2224


Align auth routes with spec: status codes & JSON shape

  • Signup catch: use res.status(400).json({ message: err.message }) (not error), and specifically detect duplicate‐key errors to return { message: 'User already exists' }.
  • Login failure: return res.status(401).json({ message: info.message }) for invalid credentials (not 400).
  • Login success: respond with res.status(200).json({ message: 'Login successful', user: /* sanitized user fields */ }) instead of raw res.json({ user }).
  • Logout already returns the correct 200 + { message }.
🤖 Prompt for AI Agents
In backend/routes/auth.js around lines 7-16, the signup error handling and login
responses must match the API spec: in the signup catch block replace
res.status(400).json({ error: err.message }) with res.status(400).json({
message: err.message }) and detect duplicate-key errors (MongoDB code 11000 or
err.codeName === 'DuplicateKey') to instead return res.status(400).json({
message: 'User already exists' }); for login failures return
res.status(401).json({ message: info.message }) (not 400), and on login success
respond with res.status(200).json({ message: 'Login successful', user: /*
sanitized user fields */ }) where you omit sensitive fields (e.g., password,
tokens) and only include safe fields like id, username, email; leave logout
as-is.

💡 Verification agent

🧩 Analysis chain

Signup: handle duplicate emails, validate inputs, and standardize error shape

  • No input validation; add required-field checks.
  • Duplicate email relies on DB error and returns { error: ... }, while tests expect { message: 'User already exists' } with 400.
  • Avoid leaking raw DB error strings.

Apply:

 router.post('/signup', async (req, res) => {
-  const { username, email, password } = req.body;
+  const { username, email, password } = req.body;
+  if (!username || !email || !password) {
+    return res.status(400).json({ message: 'username, email and password are required' });
+  }
   try {
     const user = new User({ username, email, password });
     await user.save();
-    res.status(201).json({ message: 'User created successfully' });
+    return res.status(201).json({ message: 'User created successfully' });
   } catch (err) {
-    res.status(400).json({ error: err.message });
+    if (err && err.code === 11000) {
+      // duplicate key (e.g., unique email)
+      return res.status(400).json({ message: 'User already exists' });
+    }
+    return res.status(400).json({ message: 'Invalid signup data' });
   }
 });

Confirm the User model enforces a unique index on email and hashes passwords (e.g., pre-save with bcrypt). If not, I can provide a patch.


🏁 Script executed:

#!/bin/bash
# Locate the User model file in backend/models
user_model_file=$(rg -l "mongoose\\.model\\(['\"]User['\"]" -g "backend/models/*.js")
echo "User model file: $user_model_file"

# Check if email field has a unique constraint
echo "--- Checking unique constraint on email ---"
rg -n "email\\s*:\\s*\\{[^}]*unique" "$user_model_file" || echo "No unique constraint found on email"

# Check for bcrypt import or usage
echo "--- Checking for bcrypt import ---"
rg -n "require\\(['\"]bcrypt['\"]\\)" "$user_model_file" || echo "No bcrypt import found"

echo "--- Checking for bcrypt usage ---"
rg -n "bcrypt\\." "$user_model_file" || echo "No bcrypt usage found"

# Check for pre-save hook for password hashing
echo "--- Checking for pre\\('save' hook ---"
rg -n "pre\\('save'" "$user_model_file" || echo "No pre-save hook found"

Length of output: 1129


🏁 Script executed:

#!/bin/bash
file=backend/models/User.js

echo "--- Searching for schema.index usage ---"
rg -n "UserSchema\\.index" "$file" || echo "No schema.index usage"

echo "--- Searching for inline unique on email ---"
rg -n "'email'\\s*:\\s*\\{[^}]*unique" "$file" || echo "No inline unique on email"

echo "--- Searching for ES6 import of bcrypt ---"
rg -n "import .*bcrypt" "$file" || echo "No ES6 import bcrypt"

echo "--- Searching for require('bcrypt') ---"
rg -n "require\\(['\"]bcrypt['\"]\\)" "$file" || echo "No require bcrypt"

Length of output: 851


Enforce unique email index in User model

  • Passwords are hashed via the pre('save') hook in backend/models/User.js.
  • The email field lacks a unique constraint—add unique: true on the field or UserSchema.index({ email: 1 }, { unique: true }) to prevent duplicates.
🤖 Prompt for AI Agents
In backend/routes/auth.js lines 7-16: the User model currently allows duplicate
emails; open backend/models/User.js and add a uniqueness constraint for email
(either set the email field option unique: true or call UserSchema.index({
email: 1 }, { unique: true })), then ensure indexes are built (e.g., run
mongoose models' ensureIndexes/createIndexes on startup or after deployment);
finally, keep existing signup error handling but optionally map Mongo duplicate
key errors (E11000) to a 409 conflict for clearer client feedback.


// Signup route
router.post("/signup", async (req, res) => {

const { username, email, password } = req.body;

try {
const existingUser = await User.findOne( {email} );

if (existingUser)
return res.status(400).json( {message: 'User already exists'} );
// Login
router.post('/login', (req, res, next) => {
passport.authenticate('local', (err, user, info) => {
if (err) return next(err);
if (!user) return res.status(400).json({ message: info.message });

const newUser = new User( {username, email, password} );
await newUser.save();
res.status(201).json( {message: 'User created successfully'} );
} catch (err) {
res.status(500).json({ message: 'Error creating user', error: err.message });
}
req.login(user, (err) => {
if (err) return next(err);
res.json({ message: 'Logged in successfully', user });
});
})(req, res, next);
});
Comment on lines +19 to 29
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Verification agent

🧩 Analysis chain

Login: return 401 on bad creds, align success message with tests, sanitize user, and prevent session fixation

  • Failure uses 400; use 401.
  • Tests expect message "Login successful" (not "Logged in successfully").
  • Returning the full user risks leaking hashed password/PII.
  • Regenerate session on login.

Apply:

 router.post('/login', (req, res, next) => {
   passport.authenticate('local', (err, user, info) => {
     if (err) return next(err);
-    if (!user) return res.status(400).json({ message: info.message });
+    if (!user) {
+      return res.status(401).json({ message: (info && info.message) || 'Invalid email or password' });
+    }
 
-    req.login(user, (err) => {
-      if (err) return next(err);
-      res.json({ message: 'Logged in successfully', user });
-    });
+    const finish = () => {
+      req.login(user, (err) => {
+        if (err) return next(err);
+        const { _id, username, email } = user;
+        return res.json({ message: 'Login successful', user: { id: _id, username, email } });
+      });
+    };
+    if (req.session && typeof req.session.regenerate === 'function') {
+      return req.session.regenerate((regenErr) => (regenErr ? next(regenErr) : finish()));
+    }
+    return finish();
   })(req, res, next);
 });

Ensure passport/session middleware is configured before these routes (express-session, passport.initialize(), passport.session()).


🏁 Script executed:

#!/bin/bash
# Check tests for expected login messages and status codes
echo "=== Login message references ==="
rg -n "Login successful" || echo 'No occurrences of "Login successful" found'
rg -n "Logged in successfully" || echo 'No occurrences of "Logged in successfully" found'
echo

echo "=== Status code expectations in tests ==="
rg -n "\.expect(400)" || echo 'No .expect(400) in tests'
rg -n "\.expect(401)" || echo 'No .expect(401) in tests'
echo

echo "=== User schema definitions ==="
rg -l "new Schema" -g "*user*.js" || echo 'No user schema files found'
rg -n "new Schema" -g "*user*.js" -C3
echo "Checking user schema fields:"
rg -nE "username|email|password" -g "*user*.js" -C1 || echo 'Unable to locate username/email/password fields in user schema'
echo

echo "=== Passport & session middleware ==="
rg -n "passport.initialize" -C2 || echo 'passport.initialize not configured'
rg -n "express-session" -C2 || echo 'express-session not configured'
rg -n "passport.session" -C2 || echo 'passport.session not configured'
echo

echo "=== Auth routes mounting ==="
rg -n "routes/auth" -C2
rg -n "use.*auth" -g "*app*.js" -C2 || echo 'Auth routes not found in app setup'

Length of output: 5862


🏁 Script executed:

# Display the beginning of the test suite to verify status code assertions for login failures
sed -n '1,200p' spec/auth.routes.spec.cjs

Length of output: 3393


backend/routes/auth.js – Login: use 401 on invalid credentials, match test message, sanitize user, regenerate session

  • Change failure status to 401 (tests expect 401 on bad creds) and provide a safe fallback on info.message.
  • Change success message to “Login successful” and return only { id, username, email } (avoid leaking hashes/PII).
  • Regenerate the session before calling req.login() to prevent session fixation.

Apply:

 router.post('/login', (req, res, next) => {
   passport.authenticate('local', (err, user, info) => {
     if (err) return next(err);
-    if (!user) return res.status(400).json({ message: info.message });
+    if (!user) {
+      return res.status(401).json({ message: (info && info.message) || 'Invalid email or password' });
+    }

-    req.login(user, (err) => {
-      if (err) return next(err);
-      res.json({ message: 'Logged in successfully', user });
-    });
+    const finish = () => {
+      req.login(user, (err) => {
+        if (err) return next(err);
+        const { _id, username, email } = user;
+        return res.json({ message: 'Login successful', user: { id: _id, username, email } });
+      });
+    };
+    if (req.session && typeof req.session.regenerate === 'function') {
+      return req.session.regenerate((regenErr) => (regenErr ? next(regenErr) : finish()));
+    }
+    return finish();
   })(req, res, next);
 });

Session and Passport middleware are already configured before these routes.

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
router.post('/login', (req, res, next) => {
passport.authenticate('local', (err, user, info) => {
if (err) return next(err);
if (!user) return res.status(400).json({ message: info.message });
const newUser = new User( {username, email, password} );
await newUser.save();
res.status(201).json( {message: 'User created successfully'} );
} catch (err) {
res.status(500).json({ message: 'Error creating user', error: err.message });
}
req.login(user, (err) => {
if (err) return next(err);
res.json({ message: 'Logged in successfully', user });
});
})(req, res, next);
});
router.post('/login', (req, res, next) => {
passport.authenticate('local', (err, user, info) => {
if (err) return next(err);
if (!user) {
return res.status(401).json({ message: (info && info.message) || 'Invalid email or password' });
}
const finish = () => {
req.login(user, (err) => {
if (err) return next(err);
const { _id, username, email } = user;
return res.json({ message: 'Login successful', user: { id: _id, username, email } });
});
};
if (req.session && typeof req.session.regenerate === 'function') {
return req.session.regenerate((regenErr) => (regenErr ? next(regenErr) : finish()));
}
return finish();
})(req, res, next);
});
🤖 Prompt for AI Agents
In backend/routes/auth.js around lines 19 to 29, the login handler must return
401 on invalid credentials and use a safe fallback for the passport
info.message; regenerate the session before calling req.login() to prevent
session fixation; on success respond with the message "Login successful" and
only return a sanitized user object containing { id, username, email } (no
password or extra PII); ensure you handle errors from req.session.regenerate and
from req.login and propagate them to next(err) or send appropriate responses.


// Login route
router.post("/login", passport.authenticate('local'), (req, res) => {
res.status(200).json( { message: 'Login successful', user: req.user } );
// Logout
router.get('/logout', (req, res) => {
req.logout(() => {
res.json({ message: 'Logged out successfully' });
});
});
Comment on lines +32 to 36
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Logout: destroy the session and clear cookie; keep GET for now but harden flow

  • Current GET route doesn’t destroy the session/cookie.
  • Keeping GET to match tests is OK, but add session destruction. Consider also exposing POST /logout to reduce CSRF risk.

Apply:

-router.get('/logout', (req, res) => {
-  req.logout(() => {
-    res.json({ message: 'Logged out successfully' });
-  });
-});
+router.get('/logout', (req, res, next) => {
+  req.logout((err) => {
+    if (err) return next(err);
+    if (req.session) {
+      return req.session.destroy(() => {
+        res.clearCookie('connect.sid');
+        return res.json({ message: 'Logged out successfully' });
+      });
+    }
+    return res.json({ message: 'Logged out successfully' });
+  });
+});

Optionally add (outside this hunk) a POST variant mirroring the above to discourage CSRFable state changes via GET.

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
router.get('/logout', (req, res) => {
req.logout(() => {
res.json({ message: 'Logged out successfully' });
});
});
router.get('/logout', (req, res, next) => {
req.logout((err) => {
if (err) return next(err);
if (req.session) {
return req.session.destroy(() => {
res.clearCookie('connect.sid');
return res.json({ message: 'Logged out successfully' });
});
}
return res.json({ message: 'Logged out successfully' });
});
});
🤖 Prompt for AI Agents
In backend/routes/auth.js around lines 32 to 36, the logout GET handler
currently calls req.logout but does not destroy the session or clear the session
cookie; update the handler to call req.logout (preserving the callback), then
call req.session.destroy with an error check and in its callback call
res.clearCookie using the session cookie name (e.g. 'connect.sid' or the app's
session cookie name) before sending the JSON response; handle errors by
returning a 500 or appropriate error response if session destroy fails. Also
optionally add a POST /logout variant (outside this hunk) that mirrors the same
destruction/clear-cookie flow to discourage CSRF via GET.


// Logout route
router.get("/logout", (req, res) => {

req.logout((err) => {

if (err)
return res.status(500).json({ message: 'Logout failed', error: err.message });
else
res.status(200).json({ message: 'Logged out successfully' });
});
// Protected route (example)
router.get('/profile', (req, res) => {
if (!req.isAuthenticated()) return res.status(401).json({ message: 'Not authenticated' });
res.json({ user: req.user });
Comment on lines +39 to +41
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

⚠️ Potential issue

Profile: don’t return the full user object

Return a minimal, safe shape to avoid leaking sensitive fields.

Apply:

 router.get('/profile', (req, res) => {
   if (!req.isAuthenticated()) return res.status(401).json({ message: 'Not authenticated' });
-  res.json({ user: req.user });
+  const { _id, username, email } = req.user;
+  return res.json({ user: { id: _id, username, email } });
 });
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
router.get('/profile', (req, res) => {
if (!req.isAuthenticated()) return res.status(401).json({ message: 'Not authenticated' });
res.json({ user: req.user });
router.get('/profile', (req, res) => {
if (!req.isAuthenticated()) return res.status(401).json({ message: 'Not authenticated' });
const { _id, username, email } = req.user;
return res.json({ user: { id: _id, username, email } });
});
🤖 Prompt for AI Agents
In backend/routes/auth.js around lines 39-41, the profile route currently
returns the full req.user which can leak sensitive fields; change the response
to return a minimal safe user shape (for example: id, email, name, and any
non-sensitive role/flags) by selecting those specific properties from req.user
before sending, explicitly excluding sensitive fields such as password, tokens,
social IDs, or internal metadata; keep the existing authentication check and
status codes but ensure the returned object is a sanitized DTO.

});

module.exports = router;
24 changes: 24 additions & 0 deletions client/.gitignore
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
# Logs
logs
*.log
npm-debug.log*
yarn-debug.log*
yarn-error.log*
pnpm-debug.log*
lerna-debug.log*

node_modules
dist
dist-ssr
*.local

# Editor directories and files
.vscode/*
!.vscode/extensions.json
.idea
.DS_Store
*.suo
*.ntvs*
*.njsproj
*.sln
*.sw?
12 changes: 12 additions & 0 deletions client/README.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
# React + Vite

This template provides a minimal setup to get React working in Vite with HMR and some ESLint rules.

Currently, two official plugins are available:

- [@vitejs/plugin-react](https://github.com/vitejs/vite-plugin-react/blob/main/packages/plugin-react) uses [Babel](https://babeljs.io/) for Fast Refresh
- [@vitejs/plugin-react-swc](https://github.com/vitejs/vite-plugin-react/blob/main/packages/plugin-react-swc) uses [SWC](https://swc.rs/) for Fast Refresh

## Expanding the ESLint configuration

If you are developing a production application, we recommend using TypeScript with type-aware lint rules enabled. Check out the [TS template](https://github.com/vitejs/vite/tree/main/packages/create-vite/template-react-ts) for information on how to integrate TypeScript and [`typescript-eslint`](https://typescript-eslint.io) in your project.
29 changes: 29 additions & 0 deletions client/eslint.config.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
import js from '@eslint/js'
import globals from 'globals'
import reactHooks from 'eslint-plugin-react-hooks'
import reactRefresh from 'eslint-plugin-react-refresh'
import { defineConfig, globalIgnores } from 'eslint/config'

export default defineConfig([
globalIgnores(['dist']),
{
files: ['**/*.{js,jsx}'],
extends: [
js.configs.recommended,
reactHooks.configs['recommended-latest'],
reactRefresh.configs.vite,
],
languageOptions: {
ecmaVersion: 2020,
globals: globals.browser,
parserOptions: {
ecmaVersion: 'latest',
ecmaFeatures: { jsx: true },
sourceType: 'module',
},
},
rules: {
'no-unused-vars': ['error', { varsIgnorePattern: '^[A-Z_]' }],
},
},
])
13 changes: 13 additions & 0 deletions client/index.html
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
<!doctype html>
<html lang="en">
<head>
<meta charset="UTF-8" />
<link rel="icon" type="image/svg+xml" href="/vite.svg" />
<meta name="viewport" content="width=device-width, initial-scale=1.0" />
<title>Vite + React</title>
</head>
<body>
<div id="root"></div>
<script type="module" src="/src/main.jsx"></script>
</body>
</html>
29 changes: 29 additions & 0 deletions client/package.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
{
"name": "client",
"private": true,
"version": "0.0.0",
"type": "module",
"scripts": {
"dev": "vite",
"build": "vite build",
"lint": "eslint .",
"preview": "vite preview"
},
"dependencies": {
"axios": "^1.11.0",
"react": "^19.1.1",
"react-dom": "^19.1.1",
"react-router-dom": "^7.8.2"
},
"devDependencies": {
"@eslint/js": "^9.33.0",
"@types/react": "^19.1.10",
"@types/react-dom": "^19.1.7",
"@vitejs/plugin-react": "^5.0.0",
"eslint": "^9.33.0",
"eslint-plugin-react-hooks": "^5.2.0",
"eslint-plugin-react-refresh": "^0.4.20",
"globals": "^16.3.0",
"vite": "^7.1.2"
}
}
1 change: 1 addition & 0 deletions client/public/vite.svg
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
42 changes: 42 additions & 0 deletions client/src/App.css
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
#root {
max-width: 1280px;
margin: 0 auto;
padding: 2rem;
text-align: center;
}

.logo {
height: 6em;
padding: 1.5em;
will-change: filter;
transition: filter 300ms;
}
.logo:hover {
filter: drop-shadow(0 0 2em #646cffaa);
}
.logo.react:hover {
filter: drop-shadow(0 0 2em #61dafbaa);
}

@keyframes logo-spin {
from {
transform: rotate(0deg);
}
to {
transform: rotate(360deg);
}
}

@media (prefers-reduced-motion: no-preference) {
a:nth-of-type(2) .logo {
animation: logo-spin infinite 20s linear;
}
}

.card {
padding: 2em;
}

.read-the-docs {
color: #888;
}
35 changes: 35 additions & 0 deletions client/src/App.jsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
import { useState } from 'react'
import reactLogo from './assets/react.svg'
import viteLogo from '/vite.svg'
import './App.css'

function App() {
const [count, setCount] = useState(0)

return (
<>
<div>
<a href="https://vite.dev" target="_blank">
<img src={viteLogo} className="logo" alt="Vite logo" />
</a>
<a href="https://react.dev" target="_blank">
<img src={reactLogo} className="logo react" alt="React logo" />
Comment on lines +12 to +16
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Add rel="noopener noreferrer" to external links

Prevents window.opener attacks.

Apply:

-        <a href="https://vite.dev" target="_blank">
+        <a href="https://vite.dev" target="_blank" rel="noopener noreferrer">
           <img src={viteLogo} className="logo" alt="Vite logo" />
         </a>
-        <a href="https://react.dev" target="_blank">
+        <a href="https://react.dev" target="_blank" rel="noopener noreferrer">
           <img src={reactLogo} className="logo react" alt="React logo" />
         </a>
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
<a href="https://vite.dev" target="_blank">
<img src={viteLogo} className="logo" alt="Vite logo" />
</a>
<a href="https://react.dev" target="_blank">
<img src={reactLogo} className="logo react" alt="React logo" />
<a href="https://vite.dev" target="_blank" rel="noopener noreferrer">
<img src={viteLogo} className="logo" alt="Vite logo" />
</a>
<a href="https://react.dev" target="_blank" rel="noopener noreferrer">
<img src={reactLogo} className="logo react" alt="React logo" />
</a>
🧰 Tools
🪛 Biome (2.1.2)

[error] 12-12: Avoid using target="_blank" without rel="noopener" or rel="noreferrer".

Opening external links in new tabs without rel="noopener" is a security risk. See the explanation for more details.
Safe fix: Add the rel="noopener" attribute.

(lint/security/noBlankTarget)


[error] 15-15: Avoid using target="_blank" without rel="noopener" or rel="noreferrer".

Opening external links in new tabs without rel="noopener" is a security risk. See the explanation for more details.
Safe fix: Add the rel="noopener" attribute.

(lint/security/noBlankTarget)

🤖 Prompt for AI Agents
In client/src/App.jsx around lines 12 to 16, the external anchor tags opening in
a new tab lack rel="noopener noreferrer"; update each <a ... target="_blank"> to
include rel="noopener noreferrer" to prevent window.opener attacks and ensure
safe external linking.

</a>
</div>
<h1>Vite + React</h1>
<div className="card">
<button onClick={() => setCount((count) => count + 1)}>
count is {count}
</button>
<p>
Edit <code>src/App.jsx</code> and save to test HMR
</p>
</div>
<p className="read-the-docs">
Click on the Vite and React logos to learn more
</p>
</>
)
}

export default App
1 change: 1 addition & 0 deletions client/src/assets/react.svg
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading