From 0baf237548c3c29100d55e9ef59d69d04b82439e Mon Sep 17 00:00:00 2001 From: Adolfo Reyna Date: Fri, 20 Feb 2026 20:07:26 -0500 Subject: [PATCH 1/5] 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); From c6d9dfd3c13d2499b32e4ff221581f30e93a8c7c Mon Sep 17 00:00:00 2001 From: Adolfo Reyna Date: Fri, 20 Feb 2026 20:09:29 -0500 Subject: [PATCH 2/5] fix(auth): enforce POST body credentials and generic auth errors --- auth/authEmail.js | 68 +++++++++++++++++++++++++---------------------- index.js | 8 +++--- 2 files changed, 40 insertions(+), 36 deletions(-) diff --git a/auth/authEmail.js b/auth/authEmail.js index 19a655c..ef18e14 100644 --- a/auth/authEmail.js +++ b/auth/authEmail.js @@ -9,18 +9,17 @@ const Notifications = require("../notifications"); // Object Definitions const Post = require("../def/post.js") const Profile = require("../def/profile.js"); +const DUMMY_BCRYPT_HASH = '$2b$10$2zQfAaxK0cN13N7V2Q5hAOL3wxY5E9OQj1YxDCEV4VpWw2X2gYd6C'; // Function to Singup new users. An user is a combination of a user obj and a profile. // 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; - const profile = req.query.profile || req.body.profile; + // SECURITY FIX (#2): only accept credentials from request body. + const username = (req.body.username || "").trim().toLowerCase(); + const password = req.body.password; + const email = (req.body.email || "").trim().toLowerCase(); + const profile = req.body.profile; if (!username || !password || !email) return res.json({ status: "Incomplete information!" }); // Check if the new user has an invitation. const DB = await MongoDB.getDB; @@ -42,8 +41,8 @@ const signup = async function (req, res) { // 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(), - email: email.toLowerCase(), + username, + email, password: hashedPassword }); // If newUserObject it's an error message, we check by looking toLowerCase function @@ -83,29 +82,26 @@ 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 invalidCredentials = () => res.status(401).json({ status: "Invalid credentials" }); + // SECURITY FIX (#2): only accept credentials from request body. + const username = (req.body.username || req.body.email || "").trim().toLowerCase(); + const password = req.body.password || ""; + if (!username || !password) return invalidCredentials(); 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', - event: 'server@' + req.method + '@' + req.originalUrl + '@userNotFound', - properties: { - username: username, - } + event: 'server@' + req.method + '@' + req.originalUrl + '@invalidCredentials', + properties: { username }, }); - return res.json({ status: "user not founded" }); } // 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" }); + // SECURITY FIX (#4): compare against dummy hash when user doesn't exist to reduce timing side-channel. + const isSamePassword = await bcrypt.compare(password, user?.password || DUMMY_BCRYPT_HASH); + // SECURITY FIX (#4): same response for non-existing user and wrong password. + if (!user || !isSamePassword) return invalidCredentials(); try { // Store a new session loging on DB, and use ID as session ID const sessionObj = await DB.newSession(user._id); @@ -193,12 +189,23 @@ const resetPassword = async function (req, res) { // 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. + // SECURITY FIX (#4): return generic response regardless of account existence. + const genericResetResponse = { + status: "ok", + details: "If the account exists, check your email for next steps" + }; // Logic for non-logged in users. - const username = req.body.username; + const username = (req.body.username || req.body.email || "").trim().toLowerCase(); + if (!username) return res.json(genericResetResponse); const user = await DB.getUser(username); - if (!user) return res.json({ status: "user not founded" }); + if (!user) { + client_logger.capture({ + distinctId: 'app_level', + event: 'server@' + req.method + '@' + req.originalUrl + '@resetRequestedUnknownUser', + properties: { username } + }); + return res.json(genericResetResponse); + } const password = generatePassword(); const hashedPassword = await bcrypt.hash(password, 10); // TODO: Add salt to password here as well. @@ -220,10 +227,7 @@ const resetPassword = async function (req, res) { username: username, } }); - return res.json({ - status: "ok", - details: 'Check your email for new password' // Enum of details? - }); + return res.json(genericResetResponse); } diff --git a/index.js b/index.js index acccfac..120ae91 100644 --- a/index.js +++ b/index.js @@ -75,9 +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); +// SECURITY FIX (#2): POST-only signup to avoid query-string credential leakage. +app.post('/signup', signup); /** * @swagger * /login: @@ -110,7 +109,8 @@ app.route('/signup').get(signup).post(signup); * 401: * description: Invalid credentials. */ -app.route('/login').get(login).post(login); +// SECURITY FIX (#2): POST-only login to avoid query-string credential leakage. +app.post('/login', login); /** * @swagger * /logout: From 469962d03ca47581ea7eadc989bdcd86671dea01 Mon Sep 17 00:00:00 2001 From: Adolfo Reyna Date: Fri, 20 Feb 2026 20:20:40 -0500 Subject: [PATCH 3/5] fix(auth): add single-use token login recovery flow --- SECURITY_PASSWORD_PLAN.md | 22 +++--- auth/authEmail.js | 139 ++++++++++++++++++++------------------ index.js | 32 +++++++-- mongoDB.js | 43 ++++++++++-- 4 files changed, 150 insertions(+), 86 deletions(-) diff --git a/SECURITY_PASSWORD_PLAN.md b/SECURITY_PASSWORD_PLAN.md index 1564ec2..55baab2 100644 --- a/SECURITY_PASSWORD_PLAN.md +++ b/SECURITY_PASSWORD_PLAN.md @@ -6,19 +6,19 @@ - `auth/authEmail.js` - `mongoDB.js` -## 1. Replace insecure reset flow with token-based reset +## 1. Replace insecure reset flow with single-use token login - Problem: - Current flow resets by username and emails a plaintext temporary password. - Implementation: - - Add `POST /password/request-reset`: - - Accept identifier (email). + - Keep `POST /resetPassword` as token request endpoint: + - Accept identifier (email/username). - 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. + - If account exists, create one-time login token with short TTL (15-30 min), store hashed token, email link. + - Add `POST /password/token-login`: + - Accept token. + - Validate token (exists, not expired, unused), mark used atomically, then create normal auth session cookies. - Data model: - - New collection `password_reset_tokens` with fields: + - New collection `password_login_tokens` with fields: - `userId`, `tokenHash`, `expiresAt`, `usedAt`, `createdAt`, `requestMeta`. - Add TTL index on `expiresAt`. @@ -60,7 +60,7 @@ - On login, detect outdated hash params and rehash after successful auth. ## Suggested rollout order -1. Tokenized reset flow (new endpoints + DB token store). +1. Tokenized login flow (new endpoint + DB token store). 2. POST-only auth route enforcement. 3. Generic auth/reset responses. 4. Dedicated auth rate limiting. @@ -68,10 +68,10 @@ ## Validation checklist - Unit/integration tests: - - reset token creation, expiry, one-time use, invalid token paths. + - 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. + - verify token cannot be reused after first successful consumption. diff --git a/auth/authEmail.js b/auth/authEmail.js index ef18e14..c764550 100644 --- a/auth/authEmail.js +++ b/auth/authEmail.js @@ -10,6 +10,38 @@ const Notifications = require("../notifications"); const Post = require("../def/post.js") const Profile = require("../def/profile.js"); const DUMMY_BCRYPT_HASH = '$2b$10$2zQfAaxK0cN13N7V2Q5hAOL3wxY5E9OQj1YxDCEV4VpWw2X2gYd6C'; +const PASSWORD_TOKEN_TTL_MINUTES = parseInt(process.env.PASSWORD_TOKEN_TTL_MINUTES || '20', 10); +const PASSWORD_TOKEN_PATH = process.env.PASSWORD_TOKEN_PATH || '/token-login'; +const FRONTEND_URL = (process.env.FRONTEND_URL || 'https://social.emmint.com').replace(/\/+$/, ''); + +const createPasswordTokenHash = (rawToken) => + crypto.createHash('sha256').update(rawToken).digest('hex'); + +const createSessionFromUser = async ({ DB, user, req, res }) => { + const sessionObj = await DB.newSession(user._id); + res.cookie('user_sid', user._id, cookiesOptions); + res.cookie('session_id', sessionObj.insertedId, cookiesOptions); + const latestUpdatedProfile = await DB.latestProfile(user._id); + if (latestUpdatedProfile && latestUpdatedProfile._id) { + res.cookie('profile_id', latestUpdatedProfile._id, cookiesOptions); + } + client_logger.identify({ + distinctId: user._id, + properties: { + name: latestUpdatedProfile?.profile?.firstName || '', + } + }); + client_logger.capture({ + distinctId: user._id, + event: 'server@' + req.method + '@' + req.originalUrl, + }); + return { + status: "ok", + user_sid: user._id, + session_id: sessionObj.insertedId, + profile_id: latestUpdatedProfile?._id + }; +}; // Function to Singup new users. An user is a combination of a user obj and a profile. // When new users are subscribed, they have a single profile, which is the personal one. @@ -103,30 +135,7 @@ const login = async function (req, res) { // SECURITY FIX (#4): same response for non-existing user and wrong password. if (!user || !isSamePassword) return invalidCredentials(); try { - // Store a new session loging on DB, and use ID as session ID - const sessionObj = await DB.newSession(user._id); - // Create coockies with information for Auth - res.cookie('user_sid', user._id, cookiesOptions); - res.cookie('session_id', sessionObj.insertedId, cookiesOptions); - // Chooses the most recent update profile as current active profile - const latestUpdatedProfile = await DB.latestProfile(user._id); - res.cookie('profile_id', latestUpdatedProfile._id, cookiesOptions); - client_logger.identify({ - distinctId: user._id, - properties: { - name: latestUpdatedProfile.profile.firstName, - } - }); - client_logger.capture({ - distinctId: user._id, - event: 'server@' + req.method + '@' + req.originalUrl, - }); - return res.json({ - status: "ok", - user_sid: user._id, - session_id: sessionObj.insertedId, - profile_id: latestUpdatedProfile._id - }); + return res.json(await createSessionFromUser({ DB, user, req, res })); } catch (error) { console.error(error); client_logger.capture({ @@ -158,38 +167,10 @@ const logout = async function (req, res) { } } -// Util function for generating new random password for users. -function generatePassword(length = 12) { - const charset = "abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789!@#$%^&*()-_=+"; - return Array.from(crypto.randomFillSync(new Uint8Array(length))) - .map((x) => charset[x % charset.length]) - .join(""); -} - const resetPassword = async function (req, res) { - const session_id = getSessionId(req); - const user_sid = getUserId(req); const DB = await MongoDB.getDB; - if (session_id && user_sid) { - // Sadly reusing this endpoint to change password to legged in users. - // TODO: Move change password logic to its own endpoint. - const userInfo = await DB.checkSessionOnDB(session_id, user_sid); - if (userInfo) { - const password = req.body.password; - const hashedPassword = await bcrypt.hash(password, 10); - // TODO: Add salt to password here as well. - DB.resetUserPassword(userInfo.username, hashedPassword); - return res.json({ - status: "ok", - details: 'password changed!' // This should be an enum that syncs with clients. - }); - } - } - // 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 FIX (#4): return generic response regardless of account existence. + + // SECURITY FIX (#1): issue a single-use token instead of sending/changing passwords. const genericResetResponse = { status: "ok", details: "If the account exists, check your email for next steps" @@ -206,20 +187,24 @@ const resetPassword = async function (req, res) { }); return res.json(genericResetResponse); } - const password = generatePassword(); - const hashedPassword = await bcrypt.hash(password, 10); - // TODO: Add salt to password here as well. - // TODO: We need to limit this to every 2 hours or something like this. - // TODO: Move this template to the Notif file. - DB.resetUserPassword(username, hashedPassword); - Notifications.sendEmail(username, "Your new credentials", + const rawToken = crypto.randomBytes(32).toString('hex'); + const tokenHash = createPasswordTokenHash(rawToken); + const expiresAt = new Date(Date.now() + PASSWORD_TOKEN_TTL_MINUTES * 60 * 1000); + const tokenStored = await DB.createPasswordLoginToken(user._id, tokenHash, expiresAt); + if (!tokenStored) { + return res.json(genericResetResponse); + } + const loginUrl = `${FRONTEND_URL}${PASSWORD_TOKEN_PATH}?token=${rawToken}`; + Notifications.sendEmail(username, "Your secure sign-in link", ` -

Hello,

-

This is your new password: ${password}

-

Log in

+

Hello,

+

Use this one-time sign-in link to access your account:

+

${loginUrl}

+

This link expires in ${PASSWORD_TOKEN_TTL_MINUTES} minutes and can only be used once.

+

If you did not request this, you can ignore this email.

Blessings

Emmanuel International Ministries

-`) +`); client_logger.capture({ distinctId: user._id, event: 'server@' + req.method + '@' + req.originalUrl, @@ -230,10 +215,34 @@ const resetPassword = async function (req, res) { return res.json(genericResetResponse); } +const loginWithPasswordToken = async function (req, res) { + const DB = await MongoDB.getDB; + const token = (req.body.token || "").trim(); + if (!token || token.length < 32) { + return res.status(401).json({ status: "Invalid or expired token" }); + } + const tokenHash = createPasswordTokenHash(token); + const tokenDoc = await DB.consumePasswordLoginToken(tokenHash); + if (!tokenDoc || !tokenDoc.userId) { + return res.status(401).json({ status: "Invalid or expired token" }); + } + const user = await DB.getUserById(tokenDoc.userId); + if (!user || !user._id) { + return res.status(401).json({ status: "Invalid or expired token" }); + } + try { + return res.json(await createSessionFromUser({ DB, user, req, res })); + } catch (error) { + console.error("Token login error", error); + return res.status(500).json({ status: "Internal server error" }); + } +} + module.exports = { signup, login, logout, resetPassword, + loginWithPasswordToken, } diff --git a/index.js b/index.js index 120ae91..a4ba20e 100644 --- a/index.js +++ b/index.js @@ -42,7 +42,7 @@ app.use(limiter); // Use tighter thresholds than the global limiter and key by account+IP. // Authentication -const { signup, login, logout, resetPassword } = require('./auth/authEmail.js'); +const { signup, login, logout, resetPassword, loginWithPasswordToken } = require('./auth/authEmail.js'); /** * @swagger * /signup: @@ -133,7 +133,7 @@ app.get('/logout', logout); * @swagger * /resetPassword: * post: - * summary: Resets a user's password + * summary: Sends a one-time sign-in link if the account exists * tags: [Auth] * requestBody: * required: true @@ -159,10 +159,30 @@ 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 +// SECURITY FIX (#1): +// Single-use token login endpoint for password recovery flow. +/** + * @swagger + * /password/token-login: + * post: + * summary: Consumes a one-time password token and starts a session + * tags: [Auth] + * requestBody: + * required: true + * content: + * application/json: + * schema: + * type: object + * properties: + * token: + * type: string + * responses: + * 200: + * description: Logged in with one-time token + * 401: + * description: Invalid or expired token + */ +app.post('/password/token-login', loginWithPasswordToken); // Routes const profileRoute = require('./routes/profile.js'); diff --git a/mongoDB.js b/mongoDB.js index e68008d..d226868 100644 --- a/mongoDB.js +++ b/mongoDB.js @@ -41,10 +41,9 @@ 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.passwordLoginTokensCol = db.db(DBName).collection("password_login_tokens"); + DB.passwordLoginTokensCol.createIndex({ expiresAt: 1 }, { expireAfterSeconds: 0 }).catch(console.error); + DB.passwordLoginTokensCol.createIndex({ tokenHash: 1 }, { unique: true }).catch(console.error); DB.checkSessionOnDB = async (session_id, user_sid)=>{ const temp_id = new mongo.ObjectID(session_id); @@ -82,6 +81,42 @@ const getDB = new Promise((resolve, reject) => { return DB.usersCol.findOne({ _id }); } + DB.createPasswordLoginToken = async (userId, tokenHash, expiresAt) => { + const userObjectId = mongo.ObjectID.isValid(userId) ? new mongo.ObjectID(userId) : userId; + const tokenDoc = { + userId: userObjectId, + tokenHash, + createdAt: new Date(), + expiresAt, + usedAt: null, + }; + return DB.passwordLoginTokensCol.insertOne(tokenDoc).catch((err) => { + console.log(err); + return false; + }); + }; + + DB.consumePasswordLoginToken = async (tokenHash) => { + const now = new Date(); + const result = await DB.passwordLoginTokensCol.findOneAndUpdate( + { + tokenHash, + usedAt: null, + expiresAt: { $gt: now } + }, + { + $set: { usedAt: now } + }, + { + returnOriginal: false + } + ).catch((err) => { + console.log(err); + return false; + }); + return result?.value || null; + }; + let usernamesCache = {} DB.getUsernameByIdCache = async (userid)=>{ if(!userid) return {}; From 19d805d322ff9d872890afc8cfe50553040b257f Mon Sep 17 00:00:00 2001 From: Adolfo Reyna Date: Fri, 20 Feb 2026 21:20:21 -0500 Subject: [PATCH 4/5] fix(auth): add account+IP brute-force rate limiting --- index.js | 13 ++-- middleware/authRateLimiter.js | 116 ++++++++++++++++++++++++++++++++++ 2 files changed, 121 insertions(+), 8 deletions(-) create mode 100644 middleware/authRateLimiter.js diff --git a/index.js b/index.js index a4ba20e..44d5945 100644 --- a/index.js +++ b/index.js @@ -9,6 +9,7 @@ require('dotenv').config(); const express = require('express'); const app = express(); const port = process.env.PORT || 3000; +app.set('trust proxy', true); // -- Accept request from other origins const cors = require('cors'); const { corsOptions } = require('./config/corsOptions'); @@ -34,15 +35,11 @@ const limiter = rateLimit({ return ip.includes(":") ? ip.split(":")[0] : ip; // Remove port if present } }); -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, loginWithPasswordToken } = require('./auth/authEmail.js'); +const { authRateLimiter } = require('./middleware/authRateLimiter'); /** * @swagger * /signup: @@ -110,7 +107,7 @@ app.post('/signup', signup); * description: Invalid credentials. */ // SECURITY FIX (#2): POST-only login to avoid query-string credential leakage. -app.post('/login', login); +app.post('/login', authRateLimiter('login'), login); /** * @swagger * /logout: @@ -158,7 +155,7 @@ app.get('/logout', logout); * 400: * description: Bad request. */ -app.route('/resetPassword').post(resetPassword); +app.route('/resetPassword').post(authRateLimiter('reset'), resetPassword); // SECURITY FIX (#1): // Single-use token login endpoint for password recovery flow. /** @@ -182,7 +179,7 @@ app.route('/resetPassword').post(resetPassword); * 401: * description: Invalid or expired token */ -app.post('/password/token-login', loginWithPasswordToken); +app.post('/password/token-login', authRateLimiter('token'), loginWithPasswordToken); // Routes const profileRoute = require('./routes/profile.js'); diff --git a/middleware/authRateLimiter.js b/middleware/authRateLimiter.js new file mode 100644 index 0000000..f8f86b4 --- /dev/null +++ b/middleware/authRateLimiter.js @@ -0,0 +1,116 @@ +const crypto = require('crypto'); +const { client_logger } = require('../utils/analyticsLogger'); + +const AUTH_ATTEMPT_WINDOW_MS = Math.max(60 * 1000, parseInt(process.env.AUTH_ATTEMPT_WINDOW_MS || `${15 * 60 * 1000}`, 10)); +const AUTH_ATTEMPT_MAX = Math.max(1, parseInt(process.env.AUTH_ATTEMPT_MAX || '5', 10)); +const AUTH_BLOCK_BASE_MS = Math.max(30 * 1000, parseInt(process.env.AUTH_BLOCK_BASE_MS || `${5 * 60 * 1000}`, 10)); +const AUTH_BLOCK_MAX_MS = Math.max(AUTH_BLOCK_BASE_MS, parseInt(process.env.AUTH_BLOCK_MAX_MS || `${60 * 60 * 1000}`, 10)); + +const limiterStore = new Map(); +let lastPruneAt = 0; + +const getClientIp = (req) => { + const forwarded = req.headers['x-forwarded-for']?.split(',')[0]?.trim(); + const rawIp = forwarded || req.ip || req.connection?.remoteAddress || 'unknown'; + return rawIp.replace('::ffff:', ''); +}; + +const hashValue = (value) => + crypto.createHash('sha256').update(String(value)).digest('hex').slice(0, 16); + +const getIdentity = (req, mode) => { + if (mode === 'token') { + const token = (req.body?.token || '').trim(); + return token ? `token:${hashValue(token)}` : 'token:anonymous'; + } + const username = (req.body?.username || req.body?.email || '').trim().toLowerCase(); + return username ? `acct:${hashValue(username)}` : 'acct:anonymous'; +}; + +const getLimiterKey = (req, mode) => `${mode}:${getIdentity(req, mode)}:ip:${getClientIp(req)}`; + +const getOrInitRecord = (key, now) => { + const existing = limiterStore.get(key); + if (existing) { + return existing; + } + const record = { + count: 0, + windowStartedAt: now, + blockedUntil: 0, + blockLevel: 0, + }; + limiterStore.set(key, record); + return record; +}; + +const computeBlockMs = (blockLevel) => + Math.min(AUTH_BLOCK_BASE_MS * (2 ** Math.max(0, blockLevel - 1)), AUTH_BLOCK_MAX_MS); + +const authRateLimiter = (mode) => (req, res, next) => { + const now = Date.now(); + if (now - lastPruneAt > 5 * 60 * 1000) { + for (const [storeKey, storeValue] of limiterStore.entries()) { + const isWindowExpired = now - storeValue.windowStartedAt > AUTH_ATTEMPT_WINDOW_MS; + const isNotBlocked = storeValue.blockedUntil <= now; + if (isWindowExpired && isNotBlocked) { + limiterStore.delete(storeKey); + } + } + lastPruneAt = now; + } + const key = getLimiterKey(req, mode); + const record = getOrInitRecord(key, now); + + if (now - record.windowStartedAt > AUTH_ATTEMPT_WINDOW_MS) { + record.count = 0; + record.windowStartedAt = now; + } + + if (record.blockedUntil > now) { + const retryAfterSec = Math.ceil((record.blockedUntil - now) / 1000); + res.set('Retry-After', retryAfterSec.toString()); + client_logger.capture({ + distinctId: 'app_level', + event: 'security@auth@rate_limited', + properties: { + route: req.originalUrl, + method: req.method, + mode, + keyHash: hashValue(key), + retryAfterSec, + blockLevel: record.blockLevel, + } + }); + return res.status(429).json({ status: 'Too many attempts. Please try again later.' }); + } + + record.count += 1; + if (record.count > AUTH_ATTEMPT_MAX) { + record.blockLevel += 1; + const blockMs = computeBlockMs(record.blockLevel); + record.blockedUntil = now + blockMs; + record.count = 0; + record.windowStartedAt = now; + res.set('Retry-After', Math.ceil(blockMs / 1000).toString()); + client_logger.capture({ + distinctId: 'app_level', + event: 'security@auth@rate_limited', + properties: { + route: req.originalUrl, + method: req.method, + mode, + keyHash: hashValue(key), + retryAfterSec: Math.ceil(blockMs / 1000), + blockLevel: record.blockLevel, + } + }); + return res.status(429).json({ status: 'Too many attempts. Please try again later.' }); + } + + return next(); +}; + +module.exports = { + authRateLimiter, +}; From f3a782a3603a1106b29f10e9a3f6597211036a22 Mon Sep 17 00:00:00 2001 From: Adolfo Reyna Date: Fri, 20 Feb 2026 21:22:47 -0500 Subject: [PATCH 5/5] chore(auth): remove security plan doc and marker comments --- SECURITY_PASSWORD_PLAN.md | 77 --------------------------------------- auth/authEmail.js | 10 ----- index.js | 4 -- 3 files changed, 91 deletions(-) delete mode 100644 SECURITY_PASSWORD_PLAN.md diff --git a/SECURITY_PASSWORD_PLAN.md b/SECURITY_PASSWORD_PLAN.md deleted file mode 100644 index 55baab2..0000000 --- a/SECURITY_PASSWORD_PLAN.md +++ /dev/null @@ -1,77 +0,0 @@ -# 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 single-use token login -- Problem: - - Current flow resets by username and emails a plaintext temporary password. -- Implementation: - - Keep `POST /resetPassword` as token request endpoint: - - Accept identifier (email/username). - - Always return generic success response. - - If account exists, create one-time login token with short TTL (15-30 min), store hashed token, email link. - - Add `POST /password/token-login`: - - Accept token. - - Validate token (exists, not expired, unused), mark used atomically, then create normal auth session cookies. - - Data model: - - New collection `password_login_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 login flow (new endpoint + 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: - - 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 token cannot be reused after first successful consumption. diff --git a/auth/authEmail.js b/auth/authEmail.js index c764550..b08ffd4 100644 --- a/auth/authEmail.js +++ b/auth/authEmail.js @@ -47,7 +47,6 @@ const createSessionFromUser = async ({ DB, user, req, res }) => { // 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 FIX (#2): only accept credentials from request body. const username = (req.body.username || "").trim().toLowerCase(); const password = req.body.password; const email = (req.body.email || "").trim().toLowerCase(); @@ -68,9 +67,6 @@ 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" }); - // 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, @@ -115,7 +111,6 @@ const login = async function (req, res) { if (userInfo) return res.redirect('/'); } const invalidCredentials = () => res.status(401).json({ status: "Invalid credentials" }); - // SECURITY FIX (#2): only accept credentials from request body. const username = (req.body.username || req.body.email || "").trim().toLowerCase(); const password = req.body.password || ""; if (!username || !password) return invalidCredentials(); @@ -128,11 +123,7 @@ const login = async function (req, res) { properties: { username }, }); } - // SECURITY PLAN (point #5): - // bcrypt.compare validates salted hashes directly; no manual salt parameter is needed. - // SECURITY FIX (#4): compare against dummy hash when user doesn't exist to reduce timing side-channel. const isSamePassword = await bcrypt.compare(password, user?.password || DUMMY_BCRYPT_HASH); - // SECURITY FIX (#4): same response for non-existing user and wrong password. if (!user || !isSamePassword) return invalidCredentials(); try { return res.json(await createSessionFromUser({ DB, user, req, res })); @@ -170,7 +161,6 @@ const logout = async function (req, res) { const resetPassword = async function (req, res) { const DB = await MongoDB.getDB; - // SECURITY FIX (#1): issue a single-use token instead of sending/changing passwords. const genericResetResponse = { status: "ok", details: "If the account exists, check your email for next steps" diff --git a/index.js b/index.js index 44d5945..9d96721 100644 --- a/index.js +++ b/index.js @@ -72,7 +72,6 @@ const { authRateLimiter } = require('./middleware/authRateLimiter'); * 400: * description: Bad request. */ -// SECURITY FIX (#2): POST-only signup to avoid query-string credential leakage. app.post('/signup', signup); /** * @swagger @@ -106,7 +105,6 @@ app.post('/signup', signup); * 401: * description: Invalid credentials. */ -// SECURITY FIX (#2): POST-only login to avoid query-string credential leakage. app.post('/login', authRateLimiter('login'), login); /** * @swagger @@ -156,8 +154,6 @@ app.get('/logout', logout); * description: Bad request. */ app.route('/resetPassword').post(authRateLimiter('reset'), resetPassword); -// SECURITY FIX (#1): -// Single-use token login endpoint for password recovery flow. /** * @swagger * /password/token-login: