From 62b619865a409439c205678b988696518367e786 Mon Sep 17 00:00:00 2001 From: Laurent Cozic Date: Sun, 11 Jul 2021 16:28:07 +0100 Subject: [PATCH] Server: Added support for resetting user password --- .../src/middleware/notificationHandler.ts | 3 +- packages/server/src/models/EmailModel.ts | 4 + packages/server/src/models/TokenModel.ts | 20 ++++- packages/server/src/models/UserModel.ts | 38 ++++++--- .../server/src/routes/index/password.test.ts | 63 ++++++++++++++ packages/server/src/routes/index/password.ts | 85 +++++++++++++++++++ packages/server/src/routes/index/signup.ts | 4 +- packages/server/src/routes/index/users.ts | 11 +-- packages/server/src/routes/routes.ts | 2 + packages/server/src/utils/urlUtils.ts | 21 ++++- .../server/src/views/index/login.mustache | 1 + .../src/views/index/password/forgot.mustache | 24 ++++++ .../src/views/index/password/reset.mustache | 45 ++++++++++ .../server/src/views/partials/navbar.mustache | 2 +- 14 files changed, 300 insertions(+), 23 deletions(-) create mode 100644 packages/server/src/routes/index/password.test.ts create mode 100644 packages/server/src/routes/index/password.ts create mode 100644 packages/server/src/views/index/password/forgot.mustache create mode 100644 packages/server/src/views/index/password/reset.mustache diff --git a/packages/server/src/middleware/notificationHandler.ts b/packages/server/src/middleware/notificationHandler.ts index ecdec8bff3..0b97e12713 100644 --- a/packages/server/src/middleware/notificationHandler.ts +++ b/packages/server/src/middleware/notificationHandler.ts @@ -6,6 +6,7 @@ import Logger from '@joplin/lib/Logger'; import * as MarkdownIt from 'markdown-it'; import config from '../config'; import { NotificationKey } from '../models/NotificationModel'; +import { profileUrl } from '../utils/urlUtils'; const logger = Logger.create('notificationHandler'); @@ -20,7 +21,7 @@ async function handleChangeAdminPasswordNotification(ctx: AppContext) { ctx.joplin.owner.id, NotificationKey.ChangeAdminPassword, NotificationLevel.Important, - _('The default admin password is insecure and has not been changed! [Change it now](%s)', ctx.joplin.models.user().profileUrl()) + _('The default admin password is insecure and has not been changed! [Change it now](%s)', profileUrl()) ); } else { await notificationModel.markAsRead(ctx.joplin.owner.id, NotificationKey.ChangeAdminPassword); diff --git a/packages/server/src/models/EmailModel.ts b/packages/server/src/models/EmailModel.ts index 89d21d2384..df52665048 100644 --- a/packages/server/src/models/EmailModel.ts +++ b/packages/server/src/models/EmailModel.ts @@ -30,4 +30,8 @@ export default class EmailModel extends BaseModel { return this.db(this.tableName).where('sent_time', '=', 0); } + public async deleteAll() { + await this.db(this.tableName).delete(); + } + } diff --git a/packages/server/src/models/TokenModel.ts b/packages/server/src/models/TokenModel.ts index 1976c4a6e4..1ee9bcc8ef 100644 --- a/packages/server/src/models/TokenModel.ts +++ b/packages/server/src/models/TokenModel.ts @@ -1,5 +1,5 @@ -import { Token, Uuid } from '../db'; -import { ErrorForbidden } from '../utils/errors'; +import { Token, User, Uuid } from '../db'; +import { ErrorForbidden, ErrorNotFound } from '../utils/errors'; import uuidgen from '../utils/uuidgen'; import BaseModel from './BaseModel'; @@ -37,6 +37,22 @@ export default class TokenModel extends BaseModel { .first(); } + private async byToken(tokenValue: string): Promise { + return this + .db(this.tableName) + .select(['user_id', 'value']) + .where('value', '=', tokenValue) + .first(); + } + + public async userFromToken(tokenValue: string): Promise { + const token = await this.byToken(tokenValue); + if (!token) throw new ErrorNotFound(`No such token: ${tokenValue}`); + const user = this.models().user().load(token.user_id); + if (!user) throw new ErrorNotFound('No user associated with this token'); + return user; + } + public async isValid(userId: string, tokenValue: string): Promise { const token = await this.byUser(userId, tokenValue); return !!token; diff --git a/packages/server/src/models/UserModel.ts b/packages/server/src/models/UserModel.ts index 6d4181b0d5..25468a9818 100644 --- a/packages/server/src/models/UserModel.ts +++ b/packages/server/src/models/UserModel.ts @@ -8,6 +8,8 @@ import { formatBytes, GB, MB } from '../utils/bytes'; import { itemIsEncrypted } from '../utils/joplinUtils'; import { getMaxItemSize, getMaxTotalItemSize } from './utils/user'; import * as zxcvbn from 'zxcvbn'; +import { confirmUrl, resetPasswordUrl } from '../utils/urlUtils'; +import { checkRepeatPassword, CheckRepeatPasswordInput } from '../routes/index/users'; export enum AccountType { Default = 0, @@ -226,14 +228,6 @@ export default class UserModel extends BaseModel { return !!s[0].length && !!s[1].length; } - public profileUrl(): string { - return `${this.baseUrl}/users/me`; - } - - public confirmUrl(userId: Uuid, validationToken: string): string { - return `${this.baseUrl}/users/${userId}/confirm?token=${validationToken}`; - } - public async delete(id: string): Promise { const shares = await this.models().share().sharesByUser(id); @@ -256,7 +250,7 @@ export default class UserModel extends BaseModel { public async sendAccountConfirmationEmail(user: User) { const validationToken = await this.models().token().generate(user.id); - const confirmUrl = encodeURI(this.confirmUrl(user.id, validationToken)); + const url = encodeURI(confirmUrl(user.id, validationToken)); await this.models().email().push({ sender_id: EmailSender.NoReply, @@ -264,10 +258,34 @@ export default class UserModel extends BaseModel { recipient_email: user.email, recipient_name: user.full_name || '', subject: `Please setup your ${this.appName} account`, - body: `Your new ${this.appName} account is almost ready to use!\n\nPlease click on the following link to finish setting up your account:\n\n[Complete your account](${confirmUrl})`, + body: `Your new ${this.appName} account is almost ready to use!\n\nPlease click on the following link to finish setting up your account:\n\n[Complete your account](${url})`, }); } + public async sendResetPasswordEmail(email: string) { + const user = await this.loadByEmail(email); + if (!user) throw new ErrorNotFound(`No such user: ${email}`); + + const validationToken = await this.models().token().generate(user.id); + const url = resetPasswordUrl(validationToken); + + await this.models().email().push({ + sender_id: EmailSender.NoReply, + recipient_id: user.id, + recipient_email: user.email, + recipient_name: user.full_name || '', + subject: `Reset your ${this.appName} password`, + body: `Somebody asked to reset your password on ${this.appName}\n\nIf it was not you, you can safely ignore this email.\n\nClick the following link to choose a new password:\n\n${url}`, + }); + } + + public async resetPassword(token: string, fields: CheckRepeatPasswordInput) { + checkRepeatPassword(fields, true); + const user = await this.models().token().userFromToken(token); + await this.models().user().save({ id: user.id, password: fields.password }); + await this.models().token().deleteByValue(user.id, token); + } + private formatValues(user: User): User { const output: User = { ...user }; if ('email' in output) output.email = user.email.trim().toLowerCase(); diff --git a/packages/server/src/routes/index/password.test.ts b/packages/server/src/routes/index/password.test.ts new file mode 100644 index 0000000000..70cb355f01 --- /dev/null +++ b/packages/server/src/routes/index/password.test.ts @@ -0,0 +1,63 @@ +import { beforeAllDb, afterAllTests, beforeEachDb, createUserAndSession, models, expectHttpError } from '../../utils/testing/testUtils'; +import { execRequest } from '../../utils/testing/apiUtils'; +import uuidgen from '../../utils/uuidgen'; +import { ErrorNotFound } from '../../utils/errors'; + +describe('index/password', function() { + + beforeAll(async () => { + await beforeAllDb('index/password'); + }); + + afterAll(async () => { + await afterAllTests(); + }); + + beforeEach(async () => { + await beforeEachDb(); + }); + + test('should queue an email to reset password', async function() { + const { user } = await createUserAndSession(1); + await models().email().deleteAll(); + await execRequest('', 'POST', 'password/forgot', { email: user.email }); + const emails = await models().email().all(); + expect(emails.length).toBe(1); + const match = emails[0].body.match(/(password\/reset)\?token=(.{32})/); + expect(match).toBeTruthy(); + + const newPassword = uuidgen(); + await execRequest('', 'POST', match[1], { + password: newPassword, + password2: newPassword, + }, { query: { token: match[2] } }); + + const loggedInUser = await models().user().login(user.email, newPassword); + expect(loggedInUser.id).toBe(user.id); + }); + + test('should not queue an email for non-existing emails', async function() { + await createUserAndSession(1); + await models().email().deleteAll(); + await execRequest('', 'POST', 'password/forgot', { email: 'justtryingtohackdontmindme@example.com' }); + expect((await models().email().all()).length).toBe(0); + }); + + test('should not reset the password if the token is invalid', async function() { + const { user } = await createUserAndSession(1); + await models().email().deleteAll(); + + const newPassword = uuidgen(); + + await expectHttpError(async () => { + await execRequest('', 'POST', 'password/reset', { + password: newPassword, + password2: newPassword, + }, { query: { token: 'stilltryingtohack' } }); + }, ErrorNotFound.httpCode); + + const loggedInUser = await models().user().login(user.email, newPassword); + expect(loggedInUser).toBeFalsy(); + }); + +}); diff --git a/packages/server/src/routes/index/password.ts b/packages/server/src/routes/index/password.ts new file mode 100644 index 0000000000..88ae07b554 --- /dev/null +++ b/packages/server/src/routes/index/password.ts @@ -0,0 +1,85 @@ +import { RouteHandler, SubPath } from '../../utils/routeUtils'; +import Router from '../../utils/Router'; +import { RouteType } from '../../utils/types'; +import { AppContext } from '../../utils/types'; +import { ErrorNotFound } from '../../utils/errors'; +import defaultView from '../../utils/defaultView'; +import { forgotPasswordUrl, resetPasswordUrl } from '../../utils/urlUtils'; +import { bodyFields } from '../../utils/requestUtils'; +import Logger from '@joplin/lib/Logger'; + +const logger = Logger.create('index/password'); + +const router: Router = new Router(RouteType.Web); +router.public = true; + +interface ForgotPasswordFields { + email: string; +} + +interface ResetPasswordFields { + password: string; + password2: string; +} + +const subRoutes: Record = { + forgot: async (_path: SubPath, ctx: AppContext) => { + let confirmationMessage: string = ''; + + if (ctx.method === 'POST') { + const fields = await bodyFields(ctx.req); + try { + await ctx.joplin.models.user().sendResetPasswordEmail(fields.email || ''); + } catch (error) { + logger.warn(`Could not send reset email for ${fields.email}`, error); + } + + confirmationMessage = 'If we have an account that matches your email, you should receive an email with instructions on how to reset your password shortly.'; + } + + const view = defaultView('password/forgot', 'Reset password'); + view.content = { + postUrl: forgotPasswordUrl(), + confirmationMessage, + }; + return view; + }, + + reset: async (_path: SubPath, ctx: AppContext) => { + let successMessage: string = ''; + let error: Error = null; + const token = ctx.query.token; + + if (ctx.method === 'POST') { + const fields = await bodyFields(ctx.req); + + try { + await ctx.joplin.models.user().resetPassword(token, fields); + successMessage = 'Your password was successfully reset.'; + } catch (e) { + error = e; + } + } + + const view = defaultView('password/reset', 'Reset password'); + view.content = { + postUrl: resetPasswordUrl(token), + error, + successMessage, + }; + view.jsFiles.push('zxcvbn'); + return view; + }, +}; + +router.get('password/:id', async (path: SubPath, ctx: AppContext) => { + if (!subRoutes[path.id]) throw new ErrorNotFound(`Not found: password/${path.id}`); + return subRoutes[path.id](path, ctx); +}); + +router.post('password/:id', async (path: SubPath, ctx: AppContext) => { + if (!subRoutes[path.id]) throw new ErrorNotFound(`Not found: password/${path.id}`); + return subRoutes[path.id](path, ctx); +}); + +export default router; diff --git a/packages/server/src/routes/index/signup.ts b/packages/server/src/routes/index/signup.ts index 04e410f84b..d713313980 100644 --- a/packages/server/src/routes/index/signup.ts +++ b/packages/server/src/routes/index/signup.ts @@ -6,7 +6,7 @@ import { bodyFields } from '../../utils/requestUtils'; import config from '../../config'; import defaultView from '../../utils/defaultView'; import { View } from '../../services/MustacheService'; -import { checkPassword } from './users'; +import { checkRepeatPassword } from './users'; import { NotificationKey } from '../../models/NotificationModel'; import { AccountType } from '../../models/UserModel'; import { ErrorForbidden } from '../../utils/errors'; @@ -41,7 +41,7 @@ router.post('signup', async (_path: SubPath, ctx: AppContext) => { try { const formUser = await bodyFields(ctx.req); - const password = checkPassword(formUser, true); + const password = checkRepeatPassword(formUser, true); const user = await ctx.joplin.models.user().save({ account_type: AccountType.Basic, diff --git a/packages/server/src/routes/index/users.ts b/packages/server/src/routes/index/users.ts index 3903bd0f24..ae4b6db481 100644 --- a/packages/server/src/routes/index/users.ts +++ b/packages/server/src/routes/index/users.ts @@ -15,13 +15,14 @@ import uuidgen from '../../utils/uuidgen'; import { formatMaxItemSize, formatMaxTotalSize, formatTotalSize, formatTotalSizePercent, yesOrNo } from '../../utils/strings'; import { getCanShareFolder, totalSizeClass } from '../../models/utils/user'; import { yesNoDefaultOptions } from '../../utils/views/select'; +import { confirmUrl } from '../../utils/urlUtils'; -interface CheckPasswordInput { +export interface CheckRepeatPasswordInput { password: string; password2: string; } -export function checkPassword(fields: CheckPasswordInput, required: boolean): string { +export function checkRepeatPassword(fields: CheckRepeatPasswordInput, required: boolean): string { if (fields.password) { if (fields.password !== fields.password2) throw new ErrorUnprocessableEntity('Passwords do not match'); return fields.password; @@ -57,7 +58,7 @@ function makeUser(isNew: boolean, fields: any): User { if ('can_share_folder' in fields) user.can_share_folder = boolOrDefaultToValue(fields, 'can_share_folder'); if ('account_type' in fields) user.account_type = Number(fields.account_type); - const password = checkPassword(fields, false); + const password = checkRepeatPassword(fields, false); if (password) user.password = password; if (!isNew) user.id = fields.id; @@ -174,7 +175,7 @@ router.get('users/:id/confirm', async (path: SubPath, ctx: AppContext, error: Er user, error, token, - postUrl: ctx.joplin.models.user().confirmUrl(userId, token), + postUrl: confirmUrl(userId, token), }, navbar: false, }; @@ -207,7 +208,7 @@ router.post('users/:id/confirm', async (path: SubPath, ctx: AppContext) => { const fields = await bodyFields(ctx.req); await ctx.joplin.models.token().checkToken(userId, fields.token); - const password = checkPassword(fields, true); + const password = checkRepeatPassword(fields, true); await ctx.joplin.models.user().save({ id: userId, password, must_set_password: 0 }); await ctx.joplin.models.token().deleteByValue(userId, fields.token); diff --git a/packages/server/src/routes/routes.ts b/packages/server/src/routes/routes.ts index 6c90109989..c31acfd5c0 100644 --- a/packages/server/src/routes/routes.ts +++ b/packages/server/src/routes/routes.ts @@ -17,6 +17,7 @@ import indexItems from './index/items'; import indexLogin from './index/login'; import indexLogout from './index/logout'; import indexNotifications from './index/notifications'; +import indexPassword from './index/password'; import indexSignup from './index/signup'; import indexShares from './index/shares'; import indexUsers from './index/users'; @@ -41,6 +42,7 @@ const routes: Routers = { 'changes': indexChanges, 'home': indexHome, 'items': indexItems, + 'password': indexPassword, 'login': indexLogin, 'logout': indexLogout, 'notifications': indexNotifications, diff --git a/packages/server/src/utils/urlUtils.ts b/packages/server/src/utils/urlUtils.ts index e3d52ec8f4..c73db089f5 100644 --- a/packages/server/src/utils/urlUtils.ts +++ b/packages/server/src/utils/urlUtils.ts @@ -1,6 +1,6 @@ -/* eslint-disable import/prefer-default-export */ - import { URL } from 'url'; +import config from '../config'; +import { Uuid } from '../db'; export function setQueryParameters(url: string, query: any): string { if (!query) return url; @@ -13,3 +13,20 @@ export function setQueryParameters(url: string, query: any): string { return u.toString(); } + +export function resetPasswordUrl(token: string): string { + return `${config().baseUrl}/password/reset${token ? `?token=${token}` : ''}`; +} + +export function forgotPasswordUrl(): string { + return `${config().baseUrl}/password/forgot`; +} + + +export function profileUrl(): string { + return `${config().baseUrl}/users/me`; +} + +export function confirmUrl(userId: Uuid, validationToken: string): string { + return `${config().baseUrl}/users/${userId}/confirm?token=${validationToken}`; +} diff --git a/packages/server/src/views/index/login.mustache b/packages/server/src/views/index/login.mustache index 8092bfab0d..b2e5a21611 100644 --- a/packages/server/src/views/index/login.mustache +++ b/packages/server/src/views/index/login.mustache @@ -16,6 +16,7 @@
+

I forgot my password

diff --git a/packages/server/src/views/index/password/forgot.mustache b/packages/server/src/views/index/password/forgot.mustache new file mode 100644 index 0000000000..be2c2f4815 --- /dev/null +++ b/packages/server/src/views/index/password/forgot.mustache @@ -0,0 +1,24 @@ +
+
+ {{#confirmationMessage}} +
+ {{confirmationMessage}} +
+ {{/confirmationMessage}} + +

Reset your password

+

Enter your email address, and we'll send you a password reset email.

+ +
+
+ +
+ +
+
+
+ +
+
+
+
diff --git a/packages/server/src/views/index/password/reset.mustache b/packages/server/src/views/index/password/reset.mustache new file mode 100644 index 0000000000..13c9ba75b8 --- /dev/null +++ b/packages/server/src/views/index/password/reset.mustache @@ -0,0 +1,45 @@ +
+
+ {{#successMessage}} +
+ {{successMessage}} +
+ {{/successMessage}} + + {{#error}} +
+ {{error.message}} +
+ {{/error}} + +

Reset your password

+

Enter your new password below:

+ +
+
+ +
+ +
+

+
+ +
+ +
+ +
+
+ +
+ +
+
+
+
+ + diff --git a/packages/server/src/views/partials/navbar.mustache b/packages/server/src/views/partials/navbar.mustache index 11056bb585..51b00048e8 100644 --- a/packages/server/src/views/partials/navbar.mustache +++ b/packages/server/src/views/partials/navbar.mustache @@ -2,7 +2,7 @@