From 212a7939da0c1cf06e633c1ed9dc553742241422 Mon Sep 17 00:00:00 2001 From: Pierre Brisorgueil Date: Thu, 2 Jul 2026 14:08:08 +0200 Subject: [PATCH] fix(auth): hash new password in updatePassword to stop plaintext storage + self-lockout (#3925) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. --- .../controllers/auth.password.controller.js | 3 ++- .../tests/user.account.integration.tests.js | 24 +++++++++++++++++-- 2 files changed, 24 insertions(+), 3 deletions(-) diff --git a/modules/auth/controllers/auth.password.controller.js b/modules/auth/controllers/auth.password.controller.js index 2becdc718..8782277eb 100644 --- a/modules/auth/controllers/auth.password.controller.js +++ b/modules/auth/controllers/auth.password.controller.js @@ -163,7 +163,8 @@ const updatePassword = async (req, res) => { if (!(await AuthService.comparePassword(req.body.currentPassword, user.password))) return responses.error(res, 422, 'Unprocessable Entity', 'Current password is incorrect')(); if (req.body.newPassword !== req.body.verifyPassword) return responses.error(res, 422, 'Unprocessable Entity', 'Passwords do not match')(); - password = AuthService.checkPassword(req.body.newPassword); + const checkedPassword = AuthService.checkPassword(req.body.newPassword); + password = await AuthService.hashPassword(checkedPassword); user = await UserService.update(user, { password }, 'recover'); return res .status(200) diff --git a/modules/users/tests/user.account.integration.tests.js b/modules/users/tests/user.account.integration.tests.js index 63d2514d2..686432924 100644 --- a/modules/users/tests/user.account.integration.tests.js +++ b/modules/users/tests/user.account.integration.tests.js @@ -76,12 +76,13 @@ describe('User integration tests:', () => { }); test('should be able to change user own password successfully', async () => { + const newPassword = 'Waos.azs@^:SA3$&2'; try { const result = await agent .post('/api/users/password') .send({ - newPassword: 'Waos.azs@^:SA3$&2', - verifyPassword: 'Waos.azs@^:SA3$&2', + newPassword, + verifyPassword: newPassword, currentPassword: credentials[0].password, }) .expect(200); @@ -89,6 +90,25 @@ describe('User integration tests:', () => { } catch (err) { expect(err).toBeFalsy(); } + + // Regression guard (#3925): the STORED password must be a bcrypt hash, + // never the raw plaintext. updatePassword previously persisted the + // plaintext verbatim (the model has no pre-save hash hook), which both + // stored credentials in the clear and locked the user out on next signin. + // This shipped undetected precisely because no test read the persisted + // value — assert the hash format at rest, then confirm via signin oracle. + try { + const stored = await UserService.getBrut({ id: user.id }); + expect(stored.password).toMatch(/^\$2[aby]\$/); + expect(stored.password).not.toBe(newPassword); + + // Runtime oracle: the NEW password authenticates, the OLD one does not. + await request(app).post('/api/auth/signin').send({ email: credentials[0].email, password: newPassword }).expect(200); + await request(app).post('/api/auth/signin').send({ email: credentials[0].email, password: credentials[0].password }).expect(401); + } catch (err) { + console.log(err); + expect(err).toBeFalsy(); + } }); test('should not be able to change user own password if wrong verifyPassword is given', async () => {