Server: Check password complexity

.eslintrc.js

@@ -37,6 +37,9 @@ module.exports = {
 
 		// Server admin UI global variables
 		'onDocumentReady': 'readonly',
+		'setupPasswordStrengthHandler': 'readonly',
+		'$': 'readonly',
+		'zxcvbn': 'readonly',
 
 		'tinymce': 'readonly',
 	},

packages/server/package-lock.json

@@ -1466,6 +1466,12 @@
       "integrity": "sha512-gCubfBUZ6KxzoibJ+SCUc/57Ms1jz5NjHe4+dI2krNmU5zCPAphyLJYyTOg06ueIyfj+SaCUqmzun7ImlxDcKg==",
       "dev": true
     },
+    "@types/zxcvbn": {
+      "version": "4.4.1",
+      "resolved": "https://registry.npmjs.org/@types/zxcvbn/-/zxcvbn-4.4.1.tgz",
+      "integrity": "sha512-3NoqvZC2W5gAC5DZbTpCeJ251vGQmgcWIHQJGq2J240HY6ErQ9aWKkwfoKJlHLx+A83WPNTZ9+3cd2ILxbvr1w==",
+      "dev": true
+    },
     "abab": {
       "version": "2.0.5",
       "resolved": "https://registry.npmjs.org/abab/-/abab-2.0.5.tgz",
@@ -5375,6 +5381,11 @@
         }
       }
     },
+    "jquery": {
+      "version": "3.6.0",
+      "resolved": "https://registry.npmjs.org/jquery/-/jquery-3.6.0.tgz",
+      "integrity": "sha512-JVzAR/AjBvVt2BmYhxRCSYysDsPcssdmTFnzyLEts9qNwmjmu4JTAMYubEfwVOSwpQ1I1sKKFcxhZCI2buerfw=="
+    },
     "js-tokens": {
       "version": "4.0.0",
       "resolved": "https://registry.npmjs.org/js-tokens/-/js-tokens-4.0.0.tgz",
@@ -8535,6 +8546,11 @@
       "version": "1.2.1",
       "resolved": "https://registry.npmjs.org/ylru/-/ylru-1.2.1.tgz",
       "integrity": "sha512-faQrqNMzcPCHGVC2aaOINk13K+aaBDUPjGWl0teOXywElLjyVAB6Oe2jj62jHYtwsU49jXhScYbvPENK+6zAvQ=="
+    },
+    "zxcvbn": {
+      "version": "4.4.2",
+      "resolved": "https://registry.npmjs.org/zxcvbn/-/zxcvbn-4.4.2.tgz",
+      "integrity": "sha1-KOwXzwl0PtyrBW3dixsGJizHPDA="
     }
   }
 }

packages/server/package.json

@@ -29,6 +29,7 @@
     "formidable": "^1.2.2",
     "fs-extra": "^8.1.0",
     "html-entities": "^1.3.1",
+    "jquery": "^3.6.0",
     "knex": "0.95.4",
     "koa": "^2.8.1",
     "markdown-it": "^12.0.4",
@@ -44,7 +45,8 @@
     "raw-body": "^2.4.1",
     "sqlite3": "^4.1.0",
     "stripe": "^8.150.0",
-    "yargs": "^14.0.0"
+    "yargs": "^14.0.0",
+    "zxcvbn": "^4.4.2"
   },
   "devDependencies": {
     "@joplin/tools": "^1.0.9",
@@ -57,6 +59,7 @@
     "@types/mustache": "^0.8.32",
     "@types/nodemailer": "^6.4.1",
     "@types/yargs": "^13.0.2",
+    "@types/zxcvbn": "^4.4.1",
     "jest": "^26.6.3",
     "jsdom": "^16.4.0",
     "node-mocks-http": "^1.10.0",

packages/server/public/js/main.js

@@ -6,3 +6,39 @@ function onDocumentReady(fn) {
 		document.addEventListener('DOMContentLoaded', fn);
 	}
 }
+
+// eslint-disable-next-line no-unused-vars, @typescript-eslint/no-unused-vars
+function setupPasswordStrengthHandler() {
+	$('#password_strength').hide();
+
+	const scoreToLabel = {
+		0: 'Very weak',
+		1: 'Weak',
+		2: 'Medium',
+		3: 'Strong',
+		4: 'Very strong',
+	};
+
+	function scoreToClass(score) {
+		return score < 3 ? 'has-text-danger-dark' : 'has-text-success-dark';
+	}
+
+	function checkPasswordEventHandler() {
+		const password = $(this).val();
+
+		if (!password) {
+			$('#password_strength').hide();
+		} else {
+			$('#password_strength').show();
+			const result = zxcvbn(password);
+			let msg = [`<span class="${scoreToClass(result.score)}">Strength: ${scoreToLabel[result.score]}.</span>`];
+			if (result.feedback.warning) msg.push(result.feedback.warning);
+			if (result.feedback.suggestions) msg = msg.concat(result.feedback.suggestions);
+			$('#password_strength').html(msg.join(' '));
+		}
+	}
+
+	$('#password').keydown(checkPasswordEventHandler);
+	$('#password').keyup(checkPasswordEventHandler);
+	$('#password').change(checkPasswordEventHandler);
+}

packages/server/src/middleware/notificationHandler.test.ts

@@ -19,11 +19,16 @@ describe('notificationHandler', function() {
 	test('should check admin password', async function() {
 		const { session } = await createUserAndSession(1, true);
 
+		// The default admin password actually doesn't pass the complexity
+		// check, so we need to skip validation for testing here. Eventually, a
+		// better mechanism to set the initial default admin password should
+		// probably be implemented.
+
 		const admin = await models().user().save({
 			email: defaultAdminEmail,
 			password: defaultAdminPassword,
 			is_admin: 1,
-		});
+		}, { skipValidation: true });
 
 		{
 			const ctx = await koaAppContext({ sessionId: session.id });
@@ -41,7 +46,7 @@ describe('notificationHandler', function() {
 			await models().user().save({
 				id: admin.id,
 				password: 'changed!',
-			});
+			}, { skipValidation: true });
 
 			const ctx = await koaAppContext({ sessionId: session.id });
 			await notificationHandler(ctx, koaNext);

packages/server/src/models/UserModel.ts

@@ -7,6 +7,7 @@ import { _ } from '@joplin/lib/locale';
 import { formatBytes, GB, MB } from '../utils/bytes';
 import { itemIsEncrypted } from '../utils/joplinUtils';
 import { getMaxItemSize, getMaxTotalItemSize } from './utils/user';
+import * as zxcvbn from 'zxcvbn';
 
 export enum AccountType {
 	Default = 0,
@@ -186,9 +187,22 @@ export default class UserModel extends BaseModel<User> {
 		}
 	}
 
+	private validatePassword(password: string) {
+		const result = zxcvbn(password);
+		if (result.score < 3) {
+			let msg: string[] = [result.feedback.warning];
+			if (result.feedback.suggestions) {
+				msg = msg.concat(result.feedback.suggestions);
+			}
+			throw new ErrorUnprocessableEntity(msg.join(' '));
+		}
+	}
+
 	protected async validate(object: User, options: ValidateOptions = {}): Promise<User> {
 		const user: User = await super.validate(object, options);
 
+		// Note that we don't validate the password here because it's already
+		// been hashed by then.
 		if (options.isNew) {
 			if (!user.email) throw new ErrorUnprocessableEntity('email must be set');
 			if (!user.password && !user.must_set_password) throw new ErrorUnprocessableEntity('password must be set');
@@ -270,7 +284,10 @@ export default class UserModel extends BaseModel<User> {
 	public async save(object: User, options: SaveOptions = {}): Promise<User> {
 		const user = this.formatValues(object);
 
-		if (user.password) user.password = auth.hashPassword(user.password);
+		if (user.password) {
+			if (!options.skipValidation) this.validatePassword(user.password);
+			user.password = auth.hashPassword(user.password);
+		}
 
 		const isNew = await this.isNew(object, options);
 

packages/server/src/routes/api/sessions.test.ts

@@ -20,10 +20,10 @@ async function postSession(email: string, password: string): Promise<AppContext>
 	return context;
 }
 
-describe('api_sessions', function() {
+describe('api/sessions', function() {
 
 	beforeAll(async () => {
-		await beforeAllDb('api_sessions');
+		await beforeAllDb('api/sessions');
 	});
 
 	afterAll(async () => {
@@ -35,9 +35,9 @@ describe('api_sessions', function() {
 	});
 
 	test('should login user', async function() {
-		const { user } = await createUserAndSession(1, false);
+		const { user, password } = await createUserAndSession(1, false);
 
-		const context = await postSession(user.email, '123456');
+		const context = await postSession(user.email, password);
 		expect(context.response.status).toBe(200);
 		expect(!!context.response.body.id).toBe(true);
 

packages/server/src/routes/default.ts

@@ -20,6 +20,10 @@ const pathToFileMap: PathToFileMap = {
 	'css/bulma.min.css': 'node_modules/bulma/css/bulma.min.css',
 	'css/bulma-prefers-dark.min.css': 'node_modules/bulma-prefers-dark/css/bulma-prefers-dark.min.css',
 	'css/fontawesome/css/all.min.css': 'node_modules/@fortawesome/fontawesome-free/css/all.min.css',
+	'js/zxcvbn.js': 'node_modules/zxcvbn/dist/zxcvbn.js',
+	'js/zxcvbn.js.map': 'node_modules/zxcvbn/dist/zxcvbn.js.map',
+	'js/jquery.min.js': 'node_modules/jquery/dist/jquery.min.js',
+	'js/jquery.min.map': 'node_modules/jquery/dist/jquery.min.map',
 
 	// Hard-coded for now but it could be made dynamic later on
 	// 'apps/joplin/css/note.css': 'src/apps/joplin/css/note.css',

packages/server/src/routes/index/signup.test.ts

@@ -5,6 +5,7 @@ import { getCanShareFolder, getMaxItemSize } from '../../models/utils/user';
 import { MB } from '../../utils/bytes';
 import { execRequestC } from '../../utils/testing/apiUtils';
 import { beforeAllDb, afterAllTests, beforeEachDb, models } from '../../utils/testing/testUtils';
+import uuidgen from '../../utils/uuidgen';
 import { FormUser } from './signup';
 
 describe('index_signup', function() {
@@ -22,11 +23,12 @@ describe('index_signup', function() {
 	});
 
 	test('should create a new account', async function() {
+		const password = uuidgen();
 		const formUser: FormUser = {
 			full_name: 'Toto',
 			email: 'toto@example.com',
-			password: 'testing',
+			password: password,
-			password2: 'testing',
+			password2: password,
 		};
 
 		// First confirm that it doesn't work if sign up is disabled

packages/server/src/routes/index/users.test.ts

@@ -4,8 +4,11 @@ import { NotificationKey } from '../../models/NotificationModel';
 import { ErrorForbidden } from '../../utils/errors';
 import { execRequest, execRequestC } from '../../utils/testing/apiUtils';
 import { beforeAllDb, afterAllTests, beforeEachDb, koaAppContext, createUserAndSession, models, parseHtml, checkContextError, expectHttpError } from '../../utils/testing/testUtils';
+import uuidgen from '../../utils/uuidgen';
+
+export async function postUser(sessionId: string, email: string, password: string = null, props: any = null): Promise<User> {
+	password = password === null ? uuidgen() : password;
 
-export async function postUser(sessionId: string, email: string, password: string, props: any = null): Promise<User> {
 	const context = await koaAppContext({
 		sessionId: sessionId,
 		request: {
@@ -75,7 +78,8 @@ describe('index/users', function() {
 	test('should create a new user', async function() {
 		const { session } = await createUserAndSession(1, true);
 
-		await postUser(session.id, 'test@example.com', '123456', {
+		const password = uuidgen();
+		await postUser(session.id, 'test@example.com', password, {
 			max_item_size: '',
 		});
 		const newUser = await models().user().loadByEmail('test@example.com');
@@ -91,13 +95,13 @@ describe('index/users', function() {
 		const userFromModel: User = await userModel.load(newUser.id);
 
 		expect(!!userFromModel.password).toBe(true);
-		expect(userFromModel.password === '123456').toBe(false); // Password has been hashed
+		expect(userFromModel.password === password).toBe(false); // Password has been hashed
 	});
 
 	test('should create a user with null properties if they are not explicitly set', async function() {
 		const { session } = await createUserAndSession(1, true);
 
-		await postUser(session.id, 'test@example.com', '123456');
+		await postUser(session.id, 'test@example.com');
 		const newUser = await models().user().loadByEmail('test@example.com');
 
 		expect(newUser.max_item_size).toBe(null);
@@ -121,8 +125,9 @@ describe('index/users', function() {
 	test('new user should be able to login', async function() {
 		const { session } = await createUserAndSession(1, true);
 
-		await postUser(session.id, 'test@example.com', '123456');
+		const password = uuidgen();
-		const loggedInUser = await models().user().login('test@example.com', '123456');
+		await postUser(session.id, 'test@example.com', password);
+		const loggedInUser = await models().user().login('test@example.com', password);
 		expect(!!loggedInUser).toBe(true);
 		expect(loggedInUser.email).toBe('test@example.com');
 	});
@@ -132,8 +137,9 @@ describe('index/users', function() {
 
 		const { session } = await createUserAndSession(1, true);
 
-		await postUser(session.id, email, '123456');
+		const password = uuidgen();
-		const loggedInUser = await models().user().login(email, '123456');
+		await postUser(session.id, email, password);
+		const loggedInUser = await models().user().login(email, password);
 		expect(!!loggedInUser).toBe(true);
 		expect(loggedInUser.email).toBe('ilikeuppercaseandspaces@example.com');
 	});
@@ -143,13 +149,14 @@ describe('index/users', function() {
 
 		const userModel = models().user();
 
-		await postUser(session.id, 'test@example.com', '123456');
+		const password = uuidgen();
+		await postUser(session.id, 'test@example.com', password);
 
 		const beforeUserCount = (await userModel.all()).length;
 		expect(beforeUserCount).toBe(2);
 
 		try {
-			await postUser(session.id, 'test@example.com', '123456');
+			await postUser(session.id, 'test@example.com', password);
 		} catch {
 			// Ignore
 		}
@@ -173,8 +180,9 @@ describe('index/users', function() {
 
 		const userModel = models().user();
 
-		await patchUser(session.id, { id: user.id, password: 'abcdefgh', password2: 'abcdefgh' });
+		const password = uuidgen();
-		const modUser = await userModel.login('user1@localhost', 'abcdefgh');
+		await patchUser(session.id, { id: user.id, password: password, password2: password });
+		const modUser = await userModel.login('user1@localhost', password);
 		expect(!!modUser).toBe(true);
 		expect(modUser.id).toBe(user.id);
 	});
@@ -203,7 +211,7 @@ describe('index/users', function() {
 			email: 'user1@localhost',
 			must_set_password: 1,
 			email_confirmed: 0,
-			password: '123456',
+			password: uuidgen(),
 		});
 
 		const { user: user2 } = await createUserAndSession(2);
@@ -227,9 +235,10 @@ describe('index/users', function() {
 
 		// Check that we can't set the password without the token
 		{
+			const newPassword = uuidgen();
 			const context = await execRequestC('', 'POST', path, {
-				password: 'newpassword',
+				password: newPassword,
-				password2: 'newpassword',
+				password2: newPassword,
 			});
 			const sessionId = context.cookies.get('sessionId');
 			expect(sessionId).toBeFalsy();
@@ -237,19 +246,22 @@ describe('index/users', function() {
 
 		// Check that we can't set the password with someone else's token
 		{
+			const newPassword = uuidgen();
 			const token2 = (await models().token().allByUserId(user2.id))[0].value;
 			const context = await execRequestC('', 'POST', path, {
-				password: 'newpassword',
+				password: newPassword,
-				password2: 'newpassword',
+				password2: newPassword,
 				token: token2,
 			});
 			const sessionId = context.cookies.get('sessionId');
 			expect(sessionId).toBeFalsy();
 		}
 
+		const newPassword = uuidgen();
+
 		const context = await execRequestC('', 'POST', path, {
-			password: 'newpassword',
+			password: newPassword,
-			password2: 'newpassword',
+			password2: newPassword,
 			token: token,
 		});
 
@@ -259,7 +271,7 @@ describe('index/users', function() {
 		expect(session.user_id).toBe(user1.id);
 
 		// Check that the password has been set
-		const loggedInUser = await models().user().login(user1.email, 'newpassword');
+		const loggedInUser = await models().user().login(user1.email, newPassword);
 		expect(loggedInUser.id).toBe(user1.id);
 
 		// Check that the email has been verified
@@ -278,7 +290,7 @@ describe('index/users', function() {
 			email: 'user1@localhost',
 			must_set_password: 0,
 			email_confirmed: 0,
-			password: '123456',
+			password: uuidgen(),
 		});
 
 		const email = (await models().email().all()).find(e => e.recipient_id === user1.id);
@@ -316,7 +328,7 @@ describe('index/users', function() {
 		await expectHttpError(async () => execRequest(session1.id, 'GET', `users/${admin.id}`), ErrorForbidden.httpCode);
 
 		// non-admin user cannot create a new user
-		await expectHttpError(async () => postUser(session1.id, 'cantdothat@example.com', '123456'), ErrorForbidden.httpCode);
+		await expectHttpError(async () => postUser(session1.id, 'cantdothat@example.com'), ErrorForbidden.httpCode);
 
 		// non-admin user cannot update another user
 		await expectHttpError(async () => patchUser(session1.id, { id: admin.id, email: 'cantdothateither@example.com' }), ErrorForbidden.httpCode);

packages/server/src/routes/index/users.ts

@@ -143,8 +143,9 @@ router.get('users/:id', async (path: SubPath, ctx: AppContext, user: User = null
 	view.content.postUrl = postUrl;
 	view.content.showDeleteButton = !isNew && !!owner.is_admin && owner.id !== user.id;
 	view.content.showResetPasswordButton = !isNew && owner.is_admin;
-
+	view.content.canSetEmail = isNew || owner.is_admin;
 	view.content.canShareFolderOptions = yesNoDefaultOptions(user, 'can_share_folder');
+	view.jsFiles.push('zxcvbn');
 
 	if (config().accountTypesEnabled) {
 		view.content.showAccountTypes = true;
@@ -178,6 +179,8 @@ router.get('users/:id/confirm', async (path: SubPath, ctx: AppContext, error: Er
 			navbar: false,
 		};
 
+		view.jsFiles.push('zxcvbn');
+
 		return view;
 	} else {
 		await ctx.joplin.models.token().deleteByValue(userId, token);

packages/server/src/tools/dbTools.ts

@@ -44,9 +44,14 @@ export async function createDb(config: DatabaseConfig, options: CreateDbOptions
 		}
 	}
 
+	try {
 		const db = await connectDb(config);
 		await migrateDb(db);
 		await disconnectDb(db);
+	} catch (error) {
+		error.message += `: ${config.name}`;
+		throw error;
+	}
 }
 
 export async function dropDb(config: DatabaseConfig, options: DropDbOptions = null) {

packages/server/src/utils/defaultView.ts

@@ -8,5 +8,7 @@ export default function(name: string, title: string): View {
 		content: {},
 		navbar: true,
 		title: title,
+		jsFiles: [],
+		cssFiles: [],
 	};
 }

packages/server/src/utils/testing/testUtils.ts

@@ -19,6 +19,7 @@ import { FolderEntity, NoteEntity, ResourceEntity } from '@joplin/lib/services/d
 import { ModelType } from '@joplin/lib/BaseModel';
 import { initializeJoplinUtils } from '../joplinUtils';
 import MustacheService from '../../services/MustacheService';
+import uuidgen from '../uuidgen';
 
 // Takes into account the fact that this file will be inside the /dist directory
 // when it runs.
@@ -58,6 +59,8 @@ function initGlobalLogger() {
 
 let createdDbPath_: string = null;
 export async function beforeAllDb(unitName: string) {
+	unitName = unitName.replace(/\//g, '_');
+
 	createdDbPath_ = `${packageRootDir}/db-test-${unitName}.sqlite`;
 
 	const tempDir = `${packageRootDir}/temp/test-${unitName}`;
@@ -216,6 +219,7 @@ export const testAssetDir = `${packageRootDir}/assets/tests`;
 interface UserAndSession {
 	user: User;
 	session: Session;
+	password: string;
 }
 
 export function db() {
@@ -241,9 +245,11 @@ interface CreateUserAndSessionOptions {
 }
 
 export const createUserAndSession = async function(index: number = 1, isAdmin: boolean = false, options: CreateUserAndSessionOptions = null): Promise<UserAndSession> {
+	const password = uuidgen();
+
 	options = {
 		email: `user${index}@localhost`,
-		password: '123456',
+		password,
 		...options,
 	};
 
@@ -252,7 +258,8 @@ export const createUserAndSession = async function(index: number = 1, isAdmin: b
 
 	return {
 		user: await models().user().load(user.id),
-		session: session,
+		session,
+		password,
 	};
 };
 

packages/server/src/views/index/user.mustache

@@ -11,9 +11,11 @@
 	<div class="field">
 		<label class="label">Email</label>
 		<div class="control">
-			<input class="input" type="email" name="email" value="{{user.email}}" disabled/>
+			<input class="input" type="email" name="email" value="{{user.email}}" {{^canSetEmail}}disabled{{/canSetEmail}}/>
 		</div>
+		{{^canSetEmail}}
 			<p class="help">For security reasons the email cannot currently be changed. To request a change please contact {{global.supportEmail}}</p>
+		{{/canSetEmail}}	
 	</div>
 
 	{{#global.owner.is_admin}}
@@ -60,8 +62,9 @@
 	<div class="field">
 		<label class="label">Password</label>
 		<div class="control">
-			<input class="input" type="password" name="password" autocomplete="new-password"/>
+			<input id="password" class="input" type="password" name="password" autocomplete="new-password"/>
 		</div>
+		<p id="password_strength" class="help"></p>
 	</div>
 	<div class="field">
 		<label class="label">Repeat password</label>
@@ -84,12 +87,14 @@
 </form>
 
 <script>
-	onDocumentReady(function() {
+	$(() => {
 		document.getElementById("user_form").addEventListener('submit', function(event) {
 			if (event.submitter.getAttribute('name') === 'delete_button') {
 				const ok = confirm('Delete this user?');
 				if (!ok) event.preventDefault();
 			}
 		});
+
+		setupPasswordStrengthHandler();
 	});
 </script>

packages/server/src/views/index/users/confirm.mustache

@@ -15,7 +15,8 @@
 			<div class="field">
 				<label class="label">Password</label>
 				<div class="control">
-					<input class="input" type="password" name="password"/>
+					<input id="password" class="input" type="password" name="password"/>
+					<p id="password_strength" class="help"></p>
 				</div>
 			</div>
 			<div class="field">
@@ -31,3 +32,9 @@
 		</form>
 	</div>
 </section>
+
+<script>
+$(() => {
+	setupPasswordStrengthHandler();
+});
+</script>

packages/server/src/views/layouts/default.mustache

@@ -9,6 +9,7 @@
 	{{/global.prefersDarkEnabled}}
 	<link rel="stylesheet" href="{{{global.baseUrl}}}/css/main.css" crossorigin="anonymous">
 	<link rel="stylesheet" href="{{{global.baseUrl}}}/css/fontawesome/css/all.min.css" crossorigin="anonymous">
+	<script src="{{{global.baseUrl}}}/js/jquery.min.js"></script>
 	<script src="{{{global.baseUrl}}}/js/main.js"></script>
 	{{#cssFiles}}
 		<link rel="stylesheet" href="{{{.}}}" crossorigin="anonymous">