From f3a782a3603a1106b29f10e9a3f6597211036a22 Mon Sep 17 00:00:00 2001 From: Adolfo Reyna Date: Fri, 20 Feb 2026 21:22:47 -0500 Subject: [PATCH] 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: