From 08b2ea45b0b499e83018bcaed498f7b043b2b027 Mon Sep 17 00:00:00 2001 From: Manuel <5673677+mtrezza@users.noreply.github.com> Date: Mon, 1 Feb 2021 01:07:04 +0100 Subject: [PATCH] Add account unlock on password reset (#7146) * added account unlock on password reset * added account policy option * added changelog entry * Added docs entry * moved changelog entry to correct position * improved tests to ensure requesting password reset email does not unlock account * run prettier --- CHANGELOG.md | 1 + README.md | 2 + spec/AccountLockoutPolicy.spec.js | 124 ++++++++++++++++++++++++++++++ src/AccountLockout.js | 17 ++++ src/Config.js | 8 ++ src/Controllers/UserController.js | 13 +++- src/Options/Definitions.js | 6 ++ src/Options/docs.js | 1 + src/Options/index.js | 3 + 9 files changed, 171 insertions(+), 4 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 3a12739f78..411142f523 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,7 @@ __BREAKING CHANGES:__ - NEW: Added file upload restriction. File upload is now only allowed for authenticated users by default for improved security. To allow file upload also for Anonymous Users or Public, set the `fileUpload` parameter in the [Parse Server Options](https://parseplatform.org/parse-server/api/master/ParseServerOptions.html). [#7071](https://github.com/parse-community/parse-server/pull/7071). Thanks to [dblythy](https://github.com/dblythy). ___ +- IMPROVE: Added new account lockout policy option `accountLockout.unlockOnPasswordReset` to automatically unlock account on password reset. [#7146](https://github.com/parse-community/parse-server/pull/7146). Thanks to [Manuel Trezza](https://github.com/mtrezza). - IMPROVE: Optimize queries on classes with pointer permissions. [#7061](https://github.com/parse-community/parse-server/pull/7061). Thanks to [Pedro Diaz](https://github.com/pdiaz) - FIX: request.context for afterFind triggers. [#7078](https://github.com/parse-community/parse-server/pull/7078). Thanks to [dblythy](https://github.com/dblythy) - NEW: Added convenience method Parse.Cloud.sendEmail(...) to send email via email adapter in Cloud Code. [#7089](https://github.com/parse-community/parse-server/pull/7089). Thanks to [dblythy](https://github.com/dblythy) diff --git a/README.md b/README.md index 7a5a707575..70053e8438 100644 --- a/README.md +++ b/README.md @@ -307,6 +307,8 @@ var server = ParseServer({ accountLockout: { duration: 5, // duration policy setting determines the number of minutes that a locked-out account remains locked out before automatically becoming unlocked. Set it to a value greater than 0 and less than 100000. threshold: 3, // threshold policy setting determines the number of failed sign-in attempts that will cause a user account to be locked. Set it to an integer value greater than 0 and less than 1000. + unlockOnPasswordReset: true, // Is true if the account lock should be removed after a successful password reset. Default: false. +} }, // optional settings to enforce password policies passwordPolicy: { diff --git a/spec/AccountLockoutPolicy.spec.js b/spec/AccountLockoutPolicy.spec.js index 7e3f9a93d9..43212d0e69 100644 --- a/spec/AccountLockoutPolicy.spec.js +++ b/spec/AccountLockoutPolicy.spec.js @@ -1,6 +1,8 @@ 'use strict'; const Config = require('../lib/Config'); +const Definitions = require('../lib/Options/Definitions'); +const request = require('../lib/request'); const loginWithWrongCredentialsShouldFail = function (username, password) { return new Promise((resolve, reject) => { @@ -340,3 +342,125 @@ describe('Account Lockout Policy: ', () => { }); }); }); + +describe('lockout with password reset option', () => { + let sendPasswordResetEmail; + + async function setup(options = {}) { + const accountLockout = Object.assign( + { + duration: 10000, + threshold: 1, + }, + options + ); + const config = { + appName: 'exampleApp', + accountLockout: accountLockout, + publicServerURL: 'http://localhost:8378/1', + emailAdapter: { + sendVerificationEmail: () => Promise.resolve(), + sendPasswordResetEmail: () => Promise.resolve(), + sendMail: () => {}, + }, + }; + await reconfigureServer(config); + + sendPasswordResetEmail = spyOn(config.emailAdapter, 'sendPasswordResetEmail').and.callThrough(); + } + + it('accepts valid unlockOnPasswordReset option', async () => { + const values = [true, false]; + + for (const value of values) { + await expectAsync(setup({ unlockOnPasswordReset: value })).toBeResolved(); + } + }); + + it('rejects invalid unlockOnPasswordReset option', async () => { + const values = ['a', 0, {}, [], null]; + + for (const value of values) { + await expectAsync(setup({ unlockOnPasswordReset: value })).toBeRejected(); + } + }); + + it('uses default value if unlockOnPasswordReset is not set', async () => { + await expectAsync(setup({ unlockOnPasswordReset: undefined })).toBeResolved(); + + const parseConfig = Config.get(Parse.applicationId); + expect(parseConfig.accountLockout.unlockOnPasswordReset).toBe( + Definitions.AccountLockoutOptions.unlockOnPasswordReset.default + ); + }); + + it('allow login for locked account after password reset', async () => { + await setup({ unlockOnPasswordReset: true }); + const config = Config.get(Parse.applicationId); + + const user = new Parse.User(); + const username = 'exampleUsername'; + const password = 'examplePassword'; + user.setUsername(username); + user.setPassword(password); + user.setEmail('mail@example.com'); + await user.signUp(); + + await expectAsync(Parse.User.logIn(username, 'incorrectPassword')).toBeRejected(); + await expectAsync(Parse.User.logIn(username, password)).toBeRejected(); + + await Parse.User.requestPasswordReset(user.getEmail()); + await expectAsync(Parse.User.logIn(username, password)).toBeRejected(); + + const link = sendPasswordResetEmail.calls.all()[0].args[0].link; + const linkUrl = new URL(link); + const token = linkUrl.searchParams.get('token'); + const newPassword = 'newPassword'; + await request({ + method: 'POST', + url: `${config.publicServerURL}/apps/test/request_password_reset`, + body: `new_password=${newPassword}&token=${token}&username=${username}`, + headers: { + 'Content-Type': 'application/x-www-form-urlencoded', + }, + followRedirects: false, + }); + + await expectAsync(Parse.User.logIn(username, newPassword)).toBeResolved(); + }); + + it('reject login for locked account after password reset (default)', async () => { + await setup(); + const config = Config.get(Parse.applicationId); + + const user = new Parse.User(); + const username = 'exampleUsername'; + const password = 'examplePassword'; + user.setUsername(username); + user.setPassword(password); + user.setEmail('mail@example.com'); + await user.signUp(); + + await expectAsync(Parse.User.logIn(username, 'incorrectPassword')).toBeRejected(); + await expectAsync(Parse.User.logIn(username, password)).toBeRejected(); + + await Parse.User.requestPasswordReset(user.getEmail()); + await expectAsync(Parse.User.logIn(username, password)).toBeRejected(); + + const link = sendPasswordResetEmail.calls.all()[0].args[0].link; + const linkUrl = new URL(link); + const token = linkUrl.searchParams.get('token'); + const newPassword = 'newPassword'; + await request({ + method: 'POST', + url: `${config.publicServerURL}/apps/test/request_password_reset`, + body: `new_password=${newPassword}&token=${token}&username=${username}`, + headers: { + 'Content-Type': 'application/x-www-form-urlencoded', + }, + followRedirects: false, + }); + + await expectAsync(Parse.User.logIn(username, newPassword)).toBeRejected(); + }); +}); diff --git a/src/AccountLockout.js b/src/AccountLockout.js index 183c7959cc..ea5a220772 100644 --- a/src/AccountLockout.js +++ b/src/AccountLockout.js @@ -158,6 +158,23 @@ export class AccountLockout { } }); } + + /** + * Removes the account lockout. + */ + unlockAccount() { + if (!this._config.accountLockout || !this._config.accountLockout.unlockOnPasswordReset) { + return Promise.resolve(); + } + return this._config.database.update( + '_User', + { username: this._user.username }, + { + _failed_login_count: { __op: 'Delete' }, + _account_lockout_expires_at: { __op: 'Delete' }, + } + ); + } } export default AccountLockout; diff --git a/src/Config.js b/src/Config.js index cd2717a737..da28d5add3 100644 --- a/src/Config.js +++ b/src/Config.js @@ -9,7 +9,9 @@ import net from 'net'; import { IdempotencyOptions, FileUploadOptions, + AccountLockoutOptions, } from './Options/Definitions'; +import { isBoolean } from 'lodash'; function removeTrailingSlash(str) { if (!str) { @@ -146,6 +148,12 @@ export class Config { ) { throw 'Account lockout threshold should be an integer greater than 0 and less than 1000'; } + + if (accountLockout.unlockOnPasswordReset === undefined) { + accountLockout.unlockOnPasswordReset = AccountLockoutOptions.unlockOnPasswordReset.default; + } else if (!isBoolean(accountLockout.unlockOnPasswordReset)) { + throw 'Parse Server option accountLockout.unlockOnPasswordReset must be a boolean.'; + } } } diff --git a/src/Controllers/UserController.js b/src/Controllers/UserController.js index 014e8bd7ce..0b15134a08 100644 --- a/src/Controllers/UserController.js +++ b/src/Controllers/UserController.js @@ -4,6 +4,7 @@ import AdaptableController from './AdaptableController'; import MailAdapter from '../Adapters/Email/MailAdapter'; import rest from '../rest'; import Parse from 'parse/node'; +import AccountLockout from '../AccountLockout'; var RestQuery = require('../RestQuery'); var Auth = require('../Auth'); @@ -258,7 +259,11 @@ export class UserController extends AdaptableController { updatePassword(username, token, password) { return this.checkResetTokenValidity(username, token) - .then(user => updateUserPassword(user.objectId, password, this.config)) + .then(user => updateUserPassword(user, password, this.config)) + .then(user => { + const accountLockoutPolicy = new AccountLockout(user, this.config); + return accountLockoutPolicy.unlockAccount(); + }) .catch(error => { if (error && error.message) { // in case of Parse.Error, fail with the error message only @@ -302,16 +307,16 @@ export class UserController extends AdaptableController { } // Mark this private -function updateUserPassword(userId, password, config) { +function updateUserPassword(user, password, config) { return rest.update( config, Auth.master(config), '_User', - { objectId: userId }, + { objectId: user.objectId }, { password: password, } - ); + ).then(() => user); } function buildEmailLink(destination, username, token, config) { diff --git a/src/Options/Definitions.js b/src/Options/Definitions.js index 22e0680fce..0a3eddcdb0 100644 --- a/src/Options/Definitions.js +++ b/src/Options/Definitions.js @@ -570,6 +570,12 @@ module.exports.AccountLockoutOptions = { help: 'number of failed sign-in attempts that will cause a user account to be locked', action: parsers.numberParser('threshold'), }, + unlockOnPasswordReset: { + env: 'PARSE_SERVER_ACCOUNT_LOCKOUT_UNLOCK_ON_PASSWORD_RESET', + help: 'Is true if the account lock should be removed after a successful password reset.', + action: parsers.booleanParser, + default: false, + }, }; module.exports.PasswordPolicyOptions = { doNotAllowUsername: { diff --git a/src/Options/docs.js b/src/Options/docs.js index a70fa8bff2..8b8e52e54c 100644 --- a/src/Options/docs.js +++ b/src/Options/docs.js @@ -126,6 +126,7 @@ * @interface AccountLockoutOptions * @property {Number} duration number of minutes that a locked-out account remains locked out before automatically becoming unlocked. * @property {Number} threshold number of failed sign-in attempts that will cause a user account to be locked + * @property {Boolean} unlockOnPasswordReset Is true if the account lock should be removed after a successful password reset. */ /** diff --git a/src/Options/index.js b/src/Options/index.js index 84a9283bbc..b8fd077f1c 100644 --- a/src/Options/index.js +++ b/src/Options/index.js @@ -301,6 +301,9 @@ export interface AccountLockoutOptions { duration: ?number; /* number of failed sign-in attempts that will cause a user account to be locked */ threshold: ?number; + /* Is true if the account lock should be removed after a successful password reset. + :DEFAULT: false */ + unlockOnPasswordReset: ?boolean; } export interface PasswordPolicyOptions {