codex/password-security-plan-comments #2
+11
-11
@@ -6,19 +6,19 @@
|
|||||||
- `auth/authEmail.js`
|
- `auth/authEmail.js`
|
||||||
- `mongoDB.js`
|
- `mongoDB.js`
|
||||||
|
|
||||||
## 1. Replace insecure reset flow with token-based reset
|
## 1. Replace insecure reset flow with single-use token login
|
||||||
- Problem:
|
- Problem:
|
||||||
- Current flow resets by username and emails a plaintext temporary password.
|
- Current flow resets by username and emails a plaintext temporary password.
|
||||||
- Implementation:
|
- Implementation:
|
||||||
- Add `POST /password/request-reset`:
|
- Keep `POST /resetPassword` as token request endpoint:
|
||||||
- Accept identifier (email).
|
- Accept identifier (email/username).
|
||||||
- Always return generic success response.
|
- 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.
|
- If account exists, create one-time login token with short TTL (15-30 min), store hashed token, email link.
|
||||||
- Add `POST /password/confirm-reset`:
|
- Add `POST /password/token-login`:
|
||||||
- Accept token + new password.
|
- Accept token.
|
||||||
- Validate token (exists, not expired, unused), then rotate password hash and invalidate all active sessions.
|
- Validate token (exists, not expired, unused), mark used atomically, then create normal auth session cookies.
|
||||||
- Data model:
|
- Data model:
|
||||||
- New collection `password_reset_tokens` with fields:
|
- New collection `password_login_tokens` with fields:
|
||||||
- `userId`, `tokenHash`, `expiresAt`, `usedAt`, `createdAt`, `requestMeta`.
|
- `userId`, `tokenHash`, `expiresAt`, `usedAt`, `createdAt`, `requestMeta`.
|
||||||
- Add TTL index on `expiresAt`.
|
- Add TTL index on `expiresAt`.
|
||||||
|
|
||||||
@@ -60,7 +60,7 @@
|
|||||||
- On login, detect outdated hash params and rehash after successful auth.
|
- On login, detect outdated hash params and rehash after successful auth.
|
||||||
|
|
||||||
## Suggested rollout order
|
## 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.
|
2. POST-only auth route enforcement.
|
||||||
3. Generic auth/reset responses.
|
3. Generic auth/reset responses.
|
||||||
4. Dedicated auth rate limiting.
|
4. Dedicated auth rate limiting.
|
||||||
@@ -68,10 +68,10 @@
|
|||||||
|
|
||||||
## Validation checklist
|
## Validation checklist
|
||||||
- Unit/integration tests:
|
- 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.
|
- login generic error response behavior.
|
||||||
- auth rate limiter trigger and cooldown.
|
- auth rate limiter trigger and cooldown.
|
||||||
- query credential rejection.
|
- query credential rejection.
|
||||||
- Manual:
|
- Manual:
|
||||||
- verify no plaintext password emails are sent.
|
- verify no plaintext password emails are sent.
|
||||||
- verify existing sessions are revoked after password reset.
|
- verify token cannot be reused after first successful consumption.
|
||||||
|
|||||||
+74
-65
@@ -10,6 +10,38 @@ const Notifications = require("../notifications");
|
|||||||
const Post = require("../def/post.js")
|
const Post = require("../def/post.js")
|
||||||
const Profile = require("../def/profile.js");
|
const Profile = require("../def/profile.js");
|
||||||
const DUMMY_BCRYPT_HASH = '$2b$10$2zQfAaxK0cN13N7V2Q5hAOL3wxY5E9OQj1YxDCEV4VpWw2X2gYd6C';
|
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.
|
// 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.
|
// 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.
|
// SECURITY FIX (#4): same response for non-existing user and wrong password.
|
||||||
if (!user || !isSamePassword) return invalidCredentials();
|
if (!user || !isSamePassword) return invalidCredentials();
|
||||||
try {
|
try {
|
||||||
// Store a new session loging on DB, and use ID as session ID
|
return res.json(await createSessionFromUser({ DB, user, req, res }));
|
||||||
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
|
|
||||||
});
|
|
||||||
} catch (error) {
|
} catch (error) {
|
||||||
console.error(error);
|
console.error(error);
|
||||||
client_logger.capture({
|
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 resetPassword = async function (req, res) {
|
||||||
const session_id = getSessionId(req);
|
|
||||||
const user_sid = getUserId(req);
|
|
||||||
const DB = await MongoDB.getDB;
|
const DB = await MongoDB.getDB;
|
||||||
if (session_id && user_sid) {
|
|
||||||
// Sadly reusing this endpoint to change password to legged in users.
|
// SECURITY FIX (#1): issue a single-use token instead of sending/changing passwords.
|
||||||
// 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.
|
|
||||||
const genericResetResponse = {
|
const genericResetResponse = {
|
||||||
status: "ok",
|
status: "ok",
|
||||||
details: "If the account exists, check your email for next steps"
|
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);
|
return res.json(genericResetResponse);
|
||||||
}
|
}
|
||||||
const password = generatePassword();
|
const rawToken = crypto.randomBytes(32).toString('hex');
|
||||||
const hashedPassword = await bcrypt.hash(password, 10);
|
const tokenHash = createPasswordTokenHash(rawToken);
|
||||||
// TODO: Add salt to password here as well.
|
const expiresAt = new Date(Date.now() + PASSWORD_TOKEN_TTL_MINUTES * 60 * 1000);
|
||||||
// TODO: We need to limit this to every 2 hours or something like this.
|
const tokenStored = await DB.createPasswordLoginToken(user._id, tokenHash, expiresAt);
|
||||||
// TODO: Move this template to the Notif file.
|
if (!tokenStored) {
|
||||||
DB.resetUserPassword(username, hashedPassword);
|
return res.json(genericResetResponse);
|
||||||
Notifications.sendEmail(username, "Your new credentials",
|
}
|
||||||
|
const loginUrl = `${FRONTEND_URL}${PASSWORD_TOKEN_PATH}?token=${rawToken}`;
|
||||||
|
Notifications.sendEmail(username, "Your secure sign-in link",
|
||||||
`
|
`
|
||||||
<p> Hello,</p>
|
<p>Hello,</p>
|
||||||
<p> This is your new password: ${password}</p>
|
<p>Use this one-time sign-in link to access your account:</p>
|
||||||
<p><a href="https://social.emmint.com/">Log in</a></p>
|
<p><a href="${loginUrl}">${loginUrl}</a></p>
|
||||||
|
<p>This link expires in ${PASSWORD_TOKEN_TTL_MINUTES} minutes and can only be used once.</p>
|
||||||
|
<p>If you did not request this, you can ignore this email.</p>
|
||||||
<p>Blessings</p>
|
<p>Blessings</p>
|
||||||
<p>Emmanuel International Ministries</p>
|
<p>Emmanuel International Ministries</p>
|
||||||
`)
|
`);
|
||||||
client_logger.capture({
|
client_logger.capture({
|
||||||
distinctId: user._id,
|
distinctId: user._id,
|
||||||
event: 'server@' + req.method + '@' + req.originalUrl,
|
event: 'server@' + req.method + '@' + req.originalUrl,
|
||||||
@@ -230,10 +215,34 @@ const resetPassword = async function (req, res) {
|
|||||||
return res.json(genericResetResponse);
|
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 = {
|
module.exports = {
|
||||||
signup,
|
signup,
|
||||||
login,
|
login,
|
||||||
logout,
|
logout,
|
||||||
resetPassword,
|
resetPassword,
|
||||||
|
loginWithPasswordToken,
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -42,7 +42,7 @@ app.use(limiter);
|
|||||||
// Use tighter thresholds than the global limiter and key by account+IP.
|
// Use tighter thresholds than the global limiter and key by account+IP.
|
||||||
|
|
||||||
// Authentication
|
// Authentication
|
||||||
const { signup, login, logout, resetPassword } = require('./auth/authEmail.js');
|
const { signup, login, logout, resetPassword, loginWithPasswordToken } = require('./auth/authEmail.js');
|
||||||
/**
|
/**
|
||||||
* @swagger
|
* @swagger
|
||||||
* /signup:
|
* /signup:
|
||||||
@@ -133,7 +133,7 @@ app.get('/logout', logout);
|
|||||||
* @swagger
|
* @swagger
|
||||||
* /resetPassword:
|
* /resetPassword:
|
||||||
* post:
|
* post:
|
||||||
* summary: Resets a user's password
|
* summary: Sends a one-time sign-in link if the account exists
|
||||||
* tags: [Auth]
|
* tags: [Auth]
|
||||||
* requestBody:
|
* requestBody:
|
||||||
* required: true
|
* required: true
|
||||||
@@ -159,10 +159,30 @@ app.get('/logout', logout);
|
|||||||
* description: Bad request.
|
* description: Bad request.
|
||||||
*/
|
*/
|
||||||
app.route('/resetPassword').post(resetPassword);
|
app.route('/resetPassword').post(resetPassword);
|
||||||
// SECURITY PLAN (point #1):
|
// SECURITY FIX (#1):
|
||||||
// Replace /resetPassword with request/confirm reset token endpoints:
|
// Single-use token login endpoint for password recovery flow.
|
||||||
// POST /password/request-reset
|
/**
|
||||||
// POST /password/confirm-reset
|
* @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
|
// Routes
|
||||||
const profileRoute = require('./routes/profile.js');
|
const profileRoute = require('./routes/profile.js');
|
||||||
|
|||||||
+39
-4
@@ -41,10 +41,9 @@ const getDB = new Promise((resolve, reject) => {
|
|||||||
DB.usersCol = db.db(DBName).collection("users");
|
DB.usersCol = db.db(DBName).collection("users");
|
||||||
DB.tokensCol = db.db(DBName).collection("tokens");
|
DB.tokensCol = db.db(DBName).collection("tokens");
|
||||||
DB.invitationCol = db.db(DBName).collection("invitation");
|
DB.invitationCol = db.db(DBName).collection("invitation");
|
||||||
// SECURITY PLAN (point #1):
|
DB.passwordLoginTokensCol = db.db(DBName).collection("password_login_tokens");
|
||||||
// Add password reset token collection + TTL index, e.g.:
|
DB.passwordLoginTokensCol.createIndex({ expiresAt: 1 }, { expireAfterSeconds: 0 }).catch(console.error);
|
||||||
// DB.passwordResetTokensCol = db.db(DBName).collection("password_reset_tokens");
|
DB.passwordLoginTokensCol.createIndex({ tokenHash: 1 }, { unique: true }).catch(console.error);
|
||||||
// DB.passwordResetTokensCol.createIndex({ expiresAt: 1 }, { expireAfterSeconds: 0 });
|
|
||||||
|
|
||||||
DB.checkSessionOnDB = async (session_id, user_sid)=>{
|
DB.checkSessionOnDB = async (session_id, user_sid)=>{
|
||||||
const temp_id = new mongo.ObjectID(session_id);
|
const temp_id = new mongo.ObjectID(session_id);
|
||||||
@@ -82,6 +81,42 @@ const getDB = new Promise((resolve, reject) => {
|
|||||||
return DB.usersCol.findOne({ _id });
|
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 = {}
|
let usernamesCache = {}
|
||||||
DB.getUsernameByIdCache = async (userid)=>{
|
DB.getUsernameByIdCache = async (userid)=>{
|
||||||
if(!userid) return {};
|
if(!userid) return {};
|
||||||
|
|||||||
Reference in New Issue
Block a user