Server: throwing an error if the password being saved already seems to be hashed (#8637)

packages/server/src/models/UserModel.test.ts

@@ -1,6 +1,6 @@
 import { createUserAndSession, beforeAllDb, afterAllTests, beforeEachDb, models, checkThrowAsync, expectThrow } from '../utils/testing/testUtils';
 import { EmailSender, UserFlagType } from '../services/database/types';
-import { ErrorUnprocessableEntity } from '../utils/errors';
+import { ErrorBadRequest, ErrorUnprocessableEntity } from '../utils/errors';
 import { betaUserDateRange, stripeConfig } from '../utils/stripe';
 import { accountByType, AccountType } from './UserModel';
 import { failedPaymentFinalAccount, failedPaymentWarningInterval } from './SubscriptionModel';
@@ -425,4 +425,13 @@ describe('UserModel', () => {
 		expect((await models().user().load(user1.id)).enabled).toBe(0);
 	});
 
+	test('should throw an error if the password being saved seems to be hashed', async () => {
+		const passwordSimilarToHash = '$2a$10';
+
+		const error = await checkThrowAsync(async () => await models().user().save({ password: passwordSimilarToHash }));
+
+		expect(error.message).toBe('Unable to save user because password already seems to be hashed. User id: undefined');
+		expect(error instanceof ErrorBadRequest).toBe(true);
+	});
+
 });

packages/server/src/models/UserModel.ts

@@ -6,7 +6,7 @@ import { ModelType } from '@joplin/lib/BaseModel';
 import { _ } from '@joplin/lib/locale';
 import { formatBytes, GB, MB } from '../utils/bytes';
 import { itemIsEncrypted } from '../utils/joplinUtils';
-import { getMaxItemSize, getMaxTotalItemSize } from './utils/user';
+import { getMaxItemSize, getMaxTotalItemSize, isHashedPassword } from './utils/user';
 import * as zxcvbn from 'zxcvbn';
 import { confirmUrl, resetPasswordUrl } from '../utils/urlUtils';
 import { checkRepeatPassword, CheckRepeatPasswordInput } from '../routes/index/users';
@@ -636,6 +636,9 @@ export default class UserModel extends BaseModel<User> {
 		const user = this.formatValues(object);
 
 		if (user.password) {
+			if (isHashedPassword(user.password)) {
+				throw new ErrorBadRequest(`Unable to save user because password already seems to be hashed. User id: ${user.id}`);
+			}
 			if (!options.skipValidation) this.validatePassword(user.password);
 			user.password = auth.hashPassword(user.password);
 		}

packages/server/src/models/utils/user.test.ts

@@ -0,0 +1,19 @@
+import { isHashedPassword } from './user';
+
+describe('isHashedPassword', () => {
+
+	it('should be true if password starts with $2a$10', () => {
+		expect(isHashedPassword('$2a$10$LMKVPiNOWDZhtw9NizNIEuNGLsjOxQAcrwQJ0lnKuiaOtyFgZEnwO')).toBe(true);
+	});
+
+	it.each(
+		[
+			'password',
+			'123456',
+			'simple-password-that-takes-is-long',
+			'nuXUhqecx!RzK3wv6^xYaVEP%9fc$T%$E2k%9Q&TKvtDhR#2PUw3kA8KX3w2baAD8m#N9@52!DvfYn*X6hP#uAvpGF57*H9avcoePbR&4Q2XzckJnSW*EVm4G@a#YvnR',
+		]
+	)('should be false if password starts with $2a$10: %', (password) => {
+		expect(isHashedPassword(password)).toBe(false);
+	});
+});

packages/server/src/models/utils/user.ts

@@ -37,3 +37,7 @@ export function totalSizeClass(user: User) {
 	if (d >= .7) return 'is-warning';
 	return '';
 }
+
+export const isHashedPassword = (password: string) => {
+	return password.startsWith('$2a$10');
+};

packages/server/src/utils/auth.test.ts

@@ -0,0 +1,18 @@
+import { hashPassword } from './auth';
+
+describe('hashPassword', () => {
+
+	it.each(
+		[
+			'password',
+			'123456',
+			'simple-password-that-takes-is-long',
+			'nuXUhqecx!RzK3wv6^xYaVEP%9fc$T%$E2k%9Q&TKvtDhR#2PUw3kA8KX3w2baAD8m#N9@52!DvfYn*X6hP#uAvpGF57*H9avcoePbR&4Q2XzckJnSW*EVm4G@a#YvnR',
+			'$2a$10',
+			'$2a$10$LMKVPiNOWDZhtw9NizNIEuNGLsjOxQAcrwQJ0lnKuiaOtyFgZEnwO',
+		]
+	)('should return a string that starts with $2a$10 for the password: %', async (plainText) => {
+		expect(hashPassword(plainText).startsWith('$2a$10')).toBe(true);
+	});
+
+});