From 0baf237548c3c29100d55e9ef59d69d04b82439e Mon Sep 17 00:00:00 2001 From: Adolfo Reyna Date: Fri, 20 Feb 2026 20:07:26 -0500 Subject: [PATCH] docs(auth): add password security hardening plan and code markers --- SECURITY_PASSWORD_PLAN.md | 77 +++++++++++++++++++++++++++++++++++++++ auth/authEmail.js | 25 +++++++++++-- index.js | 12 +++++- mongoDB.js | 4 ++ 4 files changed, 113 insertions(+), 5 deletions(-) create mode 100644 SECURITY_PASSWORD_PLAN.md diff --git a/SECURITY_PASSWORD_PLAN.md b/SECURITY_PASSWORD_PLAN.md new file mode 100644 index 0000000..1564ec2 --- /dev/null +++ b/SECURITY_PASSWORD_PLAN.md @@ -0,0 +1,77 @@ +# Password Security Hardening Plan + +## Scope +- Applies to auth and password flows in: + - `index.js` + - `auth/authEmail.js` + - `mongoDB.js` + +## 1. Replace insecure reset flow with token-based reset +- Problem: + - Current flow resets by username and emails a plaintext temporary password. +- Implementation: + - Add `POST /password/request-reset`: + - Accept identifier (email). + - Always return generic success response. + - If account exists, create one-time reset token with short TTL (15-30 min), store hashed token, email reset link. + - Add `POST /password/confirm-reset`: + - Accept token + new password. + - Validate token (exists, not expired, unused), then rotate password hash and invalidate all active sessions. + - Data model: + - New collection `password_reset_tokens` with fields: + - `userId`, `tokenHash`, `expiresAt`, `usedAt`, `createdAt`, `requestMeta`. + - Add TTL index on `expiresAt`. + +## 2. Remove credential handling from GET/query +- Problem: + - `/signup` and `/login` accept GET and query params for credentials. +- Implementation: + - Change auth routes to `POST` only. + - Read credentials from JSON body only. + - Reject query-based credential inputs with `400`. + - Update Swagger docs and clients accordingly. + +## 3. Add account-aware brute-force protection +- Problem: + - Only global IP limiter exists; auth endpoints are not sufficiently protected. +- Implementation: + - Add dedicated limiter middleware for auth endpoints (`/login`, `/password/request-reset`, `/password/confirm-reset`): + - Combined key: normalized username/email + source IP. + - Lower thresholds and progressive backoff/lockout window. + - Add telemetry for blocked attempts. + - Optionally store counters in Redis if horizontally scaled. + +## 4. Prevent account enumeration +- Problem: + - API exposes different responses for unknown user vs wrong password. +- Implementation: + - Login: same response for invalid credentials regardless of user existence. + - Reset request: always same response regardless of account existence. + - Keep detailed reason only in internal logs/analytics. + +## 5. Keep strong password hashing policy (clarify bcrypt behavior) +- Problem: + - Existing TODO comments incorrectly state bcrypt needs manual salt handling. +- Implementation: + - Keep bcrypt (or migrate to Argon2id in a separate change set). + - Centralize hash policy: + - cost factor (benchmark-backed; start at 12 if acceptable latency). + - minimum password length and strength checks. + - On login, detect outdated hash params and rehash after successful auth. + +## Suggested rollout order +1. Tokenized reset flow (new endpoints + DB token store). +2. POST-only auth route enforcement. +3. Generic auth/reset responses. +4. Dedicated auth rate limiting. +5. Hash policy tuning + opportunistic rehash. + +## Validation checklist +- Unit/integration tests: + - reset token creation, expiry, one-time use, invalid token paths. + - login generic error response behavior. + - auth rate limiter trigger and cooldown. + - query credential rejection. +- Manual: + - verify no plaintext password emails are sent. + - verify existing sessions are revoked after password reset. diff --git a/auth/authEmail.js b/auth/authEmail.js index 65d7c17..19a655c 100644 --- a/auth/authEmail.js +++ b/auth/authEmail.js @@ -14,6 +14,9 @@ const Profile = require("../def/profile.js"); // When new users are subscribed, they have a single profile, which is the personal one. // Other profiles can be link to that user, like groups or courses. const signup = async function (req, res) { + // SECURITY PLAN (point #2): + // Stop accepting credentials from query params and enforce POST body only. + // Query-string credentials can leak through logs/history/referers. const username = req.query.username || req.body.username; const password = req.query.password || req.body.password; const email = req.query.email || req.body.email; @@ -34,8 +37,9 @@ const signup = async function (req, res) { } let isUserAlreadyRegistered = await DB.getUser(email); if (isUserAlreadyRegistered && isUserAlreadyRegistered._id) return res.json({ status: "This user is already registered" }); - // Hash password to be stored on the DB. - // TODO: I think this is missing a Salt factor to improve security + // SECURITY PLAN (point #5): + // bcrypt.hash already includes a per-password salt. + // Future hardening: centralize cost factor policy (and consider rehash-on-login). const hashedPassword = await bcrypt.hash(password, 10); const newUserObject = await DB.newUser({ username: username.toLowerCase(), @@ -79,9 +83,13 @@ const login = async function (req, res) { const userInfo = await DB.checkSessionOnDB(session_id, user_sid); if (userInfo) return res.redirect('/'); } + // SECURITY PLAN (point #2): + // Migrate to req.body-only credentials after route-level POST-only enforcement. const username = req.body.username || req.query.username; const password = req.body.password || req.query.password || ""; const user = await DB.getUser(username); + // SECURITY PLAN (point #4): + // Replace user-existence specific response with generic invalid-credentials response. if (!user) { client_logger.capture({ distinctId: 'app_level', @@ -92,8 +100,11 @@ const login = async function (req, res) { }); return res.json({ status: "user not founded" }); } - // TODO: Also add salt parameter here. + // SECURITY PLAN (point #5): + // bcrypt.compare validates salted hashes directly; no manual salt parameter is needed. const isSamePassword = await bcrypt.compare(password, user.password); + // SECURITY PLAN (point #4): + // Use the same generic auth error response for wrong password and unknown user. if (!isSamePassword) return res.json({ status: "incorrect password" }); try { // Store a new session loging on DB, and use ID as session ID @@ -178,6 +189,12 @@ const resetPassword = async function (req, res) { }); } } + // SECURITY PLAN (point #1): + // Replace this entire non-authenticated branch with reset-token flow: + // request-reset(email) -> email link -> confirm-reset(token, newPassword). + // Do not generate/send plaintext passwords by email. + // SECURITY PLAN (point #4): + // Return generic response regardless of account existence to avoid enumeration. // Logic for non-logged in users. const username = req.body.username; const user = await DB.getUser(username); @@ -215,4 +232,4 @@ module.exports = { login, logout, resetPassword, -} \ No newline at end of file +} diff --git a/index.js b/index.js index 22b8d94..acccfac 100644 --- a/index.js +++ b/index.js @@ -37,6 +37,10 @@ const limiter = rateLimit({ app.set('trust proxy', true); app.use(limiter); +// SECURITY PLAN (point #3): +// Add dedicated auth limiter(s) for /login and password reset endpoints. +// Use tighter thresholds than the global limiter and key by account+IP. + // Authentication const { signup, login, logout, resetPassword } = require('./auth/authEmail.js'); /** @@ -71,6 +75,8 @@ const { signup, login, logout, resetPassword } = require('./auth/authEmail.js'); * 400: * description: Bad request. */ +// SECURITY PLAN (point #2): +// Make signup/login POST-only once clients are aligned. app.route('/signup').get(signup).post(signup); /** * @swagger @@ -153,6 +159,10 @@ app.get('/logout', logout); * description: Bad request. */ app.route('/resetPassword').post(resetPassword); +// SECURITY PLAN (point #1): +// Replace /resetPassword with request/confirm reset token endpoints: +// POST /password/request-reset +// POST /password/confirm-reset // Routes const profileRoute = require('./routes/profile.js'); @@ -486,4 +496,4 @@ DB.getDB.then((DB) => { }); // Export the app for testing purposes -module.exports = { app, mongoDB: DB }; \ No newline at end of file +module.exports = { app, mongoDB: DB }; diff --git a/mongoDB.js b/mongoDB.js index f6711d0..e68008d 100644 --- a/mongoDB.js +++ b/mongoDB.js @@ -41,6 +41,10 @@ const getDB = new Promise((resolve, reject) => { DB.usersCol = db.db(DBName).collection("users"); DB.tokensCol = db.db(DBName).collection("tokens"); DB.invitationCol = db.db(DBName).collection("invitation"); + // SECURITY PLAN (point #1): + // Add password reset token collection + TTL index, e.g.: + // DB.passwordResetTokensCol = db.db(DBName).collection("password_reset_tokens"); + // DB.passwordResetTokensCol.createIndex({ expiresAt: 1 }, { expireAfterSeconds: 0 }); DB.checkSessionOnDB = async (session_id, user_sid)=>{ const temp_id = new mongo.ObjectID(session_id);