refactor(users,auth): extract password hashing to lib/helpers to break auth↔users cycle

lib/helpers/password.js

@@ -0,0 +1,25 @@
+/**
+ * Password hashing helpers (bcrypt). Dependency-free leaf — breaks the auth↔users service cycle.
+ * Source of truth for hashPassword / comparePassword; both auth.service and users.service import from here.
+ */
+import bcrypt from 'bcrypt';
+
+const saltRounds = 10;
+
+/**
+ * @desc Hash a plaintext password using bcrypt.
+ * @param {string|number} password - The plaintext password to hash.
+ * @returns {Promise<string>} The bcrypt hash.
+ */
+const hashPassword = (password) => bcrypt.hash(String(password), saltRounds);
+
+/**
+ * @desc Compare a plaintext password against a stored bcrypt hash.
+ * @param {string|number} userPassword - The plaintext candidate password.
+ * @param {string} storedPassword - The stored bcrypt hash.
+ * @returns {Promise<boolean>} True if the password matches the hash.
+ */
+const comparePassword = async (userPassword, storedPassword) => bcrypt.compare(String(userPassword), String(storedPassword));
+
+export { hashPassword, comparePassword };
+export default { hashPassword, comparePassword };

lib/helpers/tests/password.unit.tests.js

@@ -0,0 +1,67 @@
+/**
+ * Unit tests for the password helper (bcrypt wrappers).
+ * Dependency-free leaf — mocks only bcrypt.
+ */
+import { jest, describe, test, expect, beforeEach } from '@jest/globals';
+
+const mockBcryptHash = jest.fn();
+const mockBcryptCompare = jest.fn();
+
+jest.unstable_mockModule('bcrypt', () => ({
+  default: {
+    hash: (...args) => mockBcryptHash(...args),
+    compare: (...args) => mockBcryptCompare(...args),
+  },
+}));
+
+const { default: passwordHelper, hashPassword, comparePassword } = await import('../password.js');
+
+describe('password helper', () => {
+  beforeEach(() => {
+    mockBcryptHash.mockReset();
+    mockBcryptCompare.mockReset();
+  });
+
+  describe('hashPassword()', () => {
+    test('hashes the stringified password with saltRounds = 10', async () => {
+      mockBcryptHash.mockResolvedValueOnce('$2b$hashed');
+      const result = await hashPassword('mypassword');
+      expect(result).toBe('$2b$hashed');
+      expect(mockBcryptHash).toHaveBeenCalledWith('mypassword', 10);
+    });
+
+    test('coerces a non-string password to a string before hashing', async () => {
+      mockBcryptHash.mockResolvedValueOnce('$2b$num');
+      await hashPassword(12345);
+      expect(mockBcryptHash).toHaveBeenCalledWith('12345', 10);
+    });
+  });
+
+  describe('comparePassword()', () => {
+    test('compares the stringified candidate against the stored hash', async () => {
+      mockBcryptCompare.mockResolvedValueOnce(true);
+      const result = await comparePassword('plain', 'hashed');
+      expect(result).toBe(true);
+      expect(mockBcryptCompare).toHaveBeenCalledWith('plain', 'hashed');
+    });
+
+    test('coerces non-string args to strings before comparing', async () => {
+      mockBcryptCompare.mockResolvedValueOnce(false);
+      await comparePassword(12345, 67890);
+      expect(mockBcryptCompare).toHaveBeenCalledWith('12345', '67890');
+    });
+
+    test('returns false when the candidate does not match the stored hash', async () => {
+      mockBcryptCompare.mockResolvedValueOnce(false);
+      const result = await comparePassword('wrong', 'hashed');
+      expect(result).toBe(false);
+    });
+  });
+
+  describe('default export', () => {
+    test('exposes hashPassword and comparePassword', () => {
+      expect(typeof passwordHelper.hashPassword).toBe('function');
+      expect(typeof passwordHelper.comparePassword).toBe('function');
+    });
+  });
+});

modules/auth/services/auth.service.js

@@ -2,16 +2,14 @@
  * Module dependencies
  */
 import _ from 'lodash';
-import bcrypt from 'bcrypt';
 import generatePassword from 'generate-password';
 import zxcvbn from 'zxcvbn';
 
 import config from '../../../config/index.js';
 import AppError from '../../../lib/helpers/AppError.js';
+import { hashPassword, comparePassword } from '../../../lib/helpers/password.js';
 import UserService from '../../users/services/users.service.js';
 
-const saltRounds = 10;
-
 /**
  * @desc Local function to removeSensitive data from user
  * @param {Object} user
@@ -24,14 +22,6 @@ const removeSensitive = (user, conf) => {
   return _.pick(plain, keys);
 };
 
-/**
- * @desc Function to compare passwords
- * @param {String} userPassword
- * @param {String} storedPassword
- * @return {Boolean} true/false
- */
-const comparePassword = async (userPassword, storedPassword) => bcrypt.compare(String(userPassword), String(storedPassword));
-
 /**
  * @desc Check whether the user account is currently locked and throw if so
  * @param {Object} user - Mongoose user document
@@ -109,13 +99,6 @@ const authenticate = async (email, password) => {
   throw new AppError('invalid user or password.', { code: 'SERVICE_ERROR' });
 };
 
-/**
- * @desc Function to hash passwords
- * @param {String} password
- * @return {String} password hashed
- */
-const hashPassword = (password) => bcrypt.hash(String(password), saltRounds);
-
 /**
  * @desc Function to check password strength using zxcvbn
  * @param {String} password

modules/auth/tests/auth.unit.tests.js

@@ -70,44 +70,21 @@ describe('Auth service unit tests:', () => {
   });
 
   // ---------------------------------------------------------------------------
-  // comparePassword
+  // re-exported bcrypt helpers
   // ---------------------------------------------------------------------------
-  describe('comparePassword()', () => {
-    test('should return true when passwords match', async () => {
-      mockBcryptCompare.mockResolvedValueOnce(true);
-      const result = await AuthService.comparePassword('plain', 'hashed');
-      expect(result).toBe(true);
-      expect(mockBcryptCompare).toHaveBeenCalledWith('plain', 'hashed');
-    });
-
-    test('should return false when passwords do not match', async () => {
-      mockBcryptCompare.mockResolvedValueOnce(false);
-      const result = await AuthService.comparePassword('wrong', 'hashed');
-      expect(result).toBe(false);
-    });
-
-    test('should coerce arguments to strings before comparing', async () => {
-      mockBcryptCompare.mockResolvedValueOnce(true);
-      await AuthService.comparePassword(12345, 67890);
-      expect(mockBcryptCompare).toHaveBeenCalledWith('12345', '67890');
-    });
-  });
-
-  // ---------------------------------------------------------------------------
-  // hashPassword
-  // ---------------------------------------------------------------------------
-  describe('hashPassword()', () => {
-    test('should resolve with the hashed string', async () => {
+  describe('re-exported bcrypt helpers', () => {
+    test('hashPassword is re-exported and delegates to the helper', async () => {
       mockBcryptHash.mockResolvedValueOnce('$2b$hashed');
       const result = await AuthService.hashPassword('mypassword');
       expect(result).toBe('$2b$hashed');
       expect(mockBcryptHash).toHaveBeenCalledWith('mypassword', 10);
     });
 
-    test('should coerce the password to a string', async () => {
-      mockBcryptHash.mockResolvedValueOnce('$2b$hashed');
-      await AuthService.hashPassword(42);
-      expect(mockBcryptHash).toHaveBeenCalledWith('42', 10);
+    test('comparePassword is re-exported and delegates to the helper', async () => {
+      mockBcryptCompare.mockResolvedValueOnce(true);
+      const result = await AuthService.comparePassword('plain', 'hashed');
+      expect(result).toBe(true);
+      expect(mockBcryptCompare).toHaveBeenCalledWith('plain', 'hashed');
     });
   });
 

modules/users/services/users.service.js

@@ -8,7 +8,7 @@ import config from '../../../config/index.js';
 import logger from '../../../lib/services/logger.js';
 import getBaseUrl from '../../../lib/helpers/getBaseUrl.js';
 import mailer from '../../../lib/helpers/mailer/index.js';
-import AuthService from '../../auth/services/auth.service.js';
+import passwordHelper from '../../../lib/helpers/password.js';
 import UserRepository from '../repositories/users.repository.js';
 import MembershipService from '../../organizations/services/organizations.membership.service.js';
 import OrganizationsRepository from '../../organizations/repositories/organizations.repository.js';
@@ -56,7 +56,7 @@ const create = async (user) => {
     //   throw new AppError(`${validPassword.feedback.warning}. ${validPassword.feedback.suggestions.join('. ')}`);
     // }
     // When password is provided we need to make sure we are hashing it
-    user.password = await AuthService.hashPassword(user.password);
+    user.password = await passwordHelper.hashPassword(user.password);
   }
   const result = await UserRepository.create(user);
   // Remove sensitive data before return

modules/users/tests/users.service.count.unit.tests.js

@@ -26,8 +26,10 @@ jest.unstable_mockModule('../repositories/users.repository.js', () => ({
   },
 }));
 
-jest.unstable_mockModule('../../auth/services/auth.service.js', () => ({
+jest.unstable_mockModule('../../../lib/helpers/password.js', () => ({
   default: { hashPassword: jest.fn(), comparePassword: jest.fn() },
+  hashPassword: jest.fn(),
+  comparePassword: jest.fn(),
 }));
 
 jest.unstable_mockModule('../../organizations/services/organizations.membership.service.js', () => ({

modules/users/tests/users.service.remove.cascade.unit.tests.js

@@ -67,10 +67,6 @@ jest.unstable_mockModule('../../organizations/repositories/organizations.members
   },
 }));
 
-jest.unstable_mockModule('../../auth/services/auth.service.js', () => ({
-  default: { signOut: jest.fn() },
-}));
-
 jest.unstable_mockModule('../../../config/index.js', () => ({
   default: { organizations: { enabled: true } },
 }));

modules/users/tests/users.service.remove.pendingSweep.unit.tests.js

@@ -69,10 +69,6 @@ jest.unstable_mockModule('../../organizations/repositories/organizations.members
   },
 }));
 
-jest.unstable_mockModule('../../auth/services/auth.service.js', () => ({
-  default: { signOut: jest.fn() },
-}));
-
 jest.unstable_mockModule('../../../config/index.js', () => ({
   default: { organizations: { enabled: true } },
 }));