From 3b1a726a23f44b7cca2c9716452927346ab9f28b Mon Sep 17 00:00:00 2001
From: pedr <pedrlz.frn@gmail.com>
Date: Tue, 8 Aug 2023 07:15:03 -0300
Subject: [PATCH] Server: throwing an error if the password being saved already
 seems to be hashed (#8637)

---
 packages/server/src/models/UserModel.test.ts  | 11 ++++++++++-
 packages/server/src/models/UserModel.ts       |  5 ++++-
 packages/server/src/models/utils/user.test.ts | 19 +++++++++++++++++++
 packages/server/src/models/utils/user.ts      |  4 ++++
 packages/server/src/utils/auth.test.ts        | 18 ++++++++++++++++++
 5 files changed, 55 insertions(+), 2 deletions(-)
 create mode 100644 packages/server/src/models/utils/user.test.ts
 create mode 100644 packages/server/src/utils/auth.test.ts

diff --git a/packages/server/src/models/UserModel.test.ts b/packages/server/src/models/UserModel.test.ts
index adc3183e69..fd4cadb2b2 100644
--- a/packages/server/src/models/UserModel.test.ts
+++ b/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);
+	});
+
 });
diff --git a/packages/server/src/models/UserModel.ts b/packages/server/src/models/UserModel.ts
index 22619309db..d7cc8a1735 100644
--- a/packages/server/src/models/UserModel.ts
+++ b/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);
 		}
diff --git a/packages/server/src/models/utils/user.test.ts b/packages/server/src/models/utils/user.test.ts
new file mode 100644
index 0000000000..848ff57df6
--- /dev/null
+++ b/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);
+	});
+});
diff --git a/packages/server/src/models/utils/user.ts b/packages/server/src/models/utils/user.ts
index 3d31f74ad1..7e60c8dde5 100644
--- a/packages/server/src/models/utils/user.ts
+++ b/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');
+};
diff --git a/packages/server/src/utils/auth.test.ts b/packages/server/src/utils/auth.test.ts
new file mode 100644
index 0000000000..4ff524126c
--- /dev/null
+++ b/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);
+	});
+
+});