Skip to content

Commit e8ef982

Browse files
fix(auth): hash new password in updatePassword to stop plaintext storage + self-lockout (#3925) (#3937)
The change-password handler validated strength via checkPassword (zxcvbn only, returns the raw string) then persisted it verbatim. The user model has no pre-save hash hook, so the new password landed in the DB in plaintext — storing credentials at rest AND locking the user out on next signin (comparePassword compares the entered password against the stored plaintext, never matching). Mirror the sibling reset() path: hashPassword(checkedPassword) before persisting. Tests: strengthen the change-password success test to read the stored value and assert bcrypt format (/^$2[aby]$/), plus a runtime signin oracle (new password authenticates, old one 401). This bug shipped undetected because no test ever checked the persisted value. Verified the strengthened test fails on the pre-fix code. Follow-up (out of scope, downstream ops): remediate any already-corrupted rows with a plaintext password persisted before this fix.
1 parent 2344d4c commit e8ef982

2 files changed

Lines changed: 24 additions & 3 deletions

File tree

modules/auth/controllers/auth.password.controller.js

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -163,7 +163,8 @@ const updatePassword = async (req, res) => {
163163
if (!(await AuthService.comparePassword(req.body.currentPassword, user.password)))
164164
return responses.error(res, 422, 'Unprocessable Entity', 'Current password is incorrect')();
165165
if (req.body.newPassword !== req.body.verifyPassword) return responses.error(res, 422, 'Unprocessable Entity', 'Passwords do not match')();
166-
password = AuthService.checkPassword(req.body.newPassword);
166+
const checkedPassword = AuthService.checkPassword(req.body.newPassword);
167+
password = await AuthService.hashPassword(checkedPassword);
167168
user = await UserService.update(user, { password }, 'recover');
168169
return res
169170
.status(200)

modules/users/tests/user.account.integration.tests.js

Lines changed: 22 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -76,19 +76,39 @@ describe('User integration tests:', () => {
7676
});
7777

7878
test('should be able to change user own password successfully', async () => {
79+
const newPassword = 'Waos.azs@^:SA3$&2';
7980
try {
8081
const result = await agent
8182
.post('/api/users/password')
8283
.send({
83-
newPassword: 'Waos.azs@^:SA3$&2',
84-
verifyPassword: 'Waos.azs@^:SA3$&2',
84+
newPassword,
85+
verifyPassword: newPassword,
8586
currentPassword: credentials[0].password,
8687
})
8788
.expect(200);
8889
expect(result.body.message).toBe('Password changed successfully');
8990
} catch (err) {
9091
expect(err).toBeFalsy();
9192
}
93+
94+
// Regression guard (#3925): the STORED password must be a bcrypt hash,
95+
// never the raw plaintext. updatePassword previously persisted the
96+
// plaintext verbatim (the model has no pre-save hash hook), which both
97+
// stored credentials in the clear and locked the user out on next signin.
98+
// This shipped undetected precisely because no test read the persisted
99+
// value — assert the hash format at rest, then confirm via signin oracle.
100+
try {
101+
const stored = await UserService.getBrut({ id: user.id });
102+
expect(stored.password).toMatch(/^\$2[aby]\$/);
103+
expect(stored.password).not.toBe(newPassword);
104+
105+
// Runtime oracle: the NEW password authenticates, the OLD one does not.
106+
await request(app).post('/api/auth/signin').send({ email: credentials[0].email, password: newPassword }).expect(200);
107+
await request(app).post('/api/auth/signin').send({ email: credentials[0].email, password: credentials[0].password }).expect(401);
108+
} catch (err) {
109+
console.log(err);
110+
expect(err).toBeFalsy();
111+
}
92112
});
93113

94114
test('should not be able to change user own password if wrong verifyPassword is given', async () => {

0 commit comments

Comments
 (0)