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: