Skip to content

Commit

Permalink
Add account unlock on password reset (#7146)
Browse files Browse the repository at this point in the history
* 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
  • Loading branch information
mtrezza authored Feb 1, 2021
1 parent 25fb576 commit 08b2ea4
Show file tree
Hide file tree
Showing 9 changed files with 171 additions and 4 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
2 changes: 2 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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: {
Expand Down
124 changes: 124 additions & 0 deletions spec/AccountLockoutPolicy.spec.js
Original file line number Diff line number Diff line change
@@ -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) => {
Expand Down Expand Up @@ -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();
});
});
17 changes: 17 additions & 0 deletions src/AccountLockout.js
Original file line number Diff line number Diff line change
Expand Up @@ -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;
8 changes: 8 additions & 0 deletions src/Config.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,9 @@ import net from 'net';
import {
IdempotencyOptions,
FileUploadOptions,
AccountLockoutOptions,
} from './Options/Definitions';
import { isBoolean } from 'lodash';

function removeTrailingSlash(str) {
if (!str) {
Expand Down Expand Up @@ -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.';
}
}
}

Expand Down
13 changes: 9 additions & 4 deletions src/Controllers/UserController.js
Original file line number Diff line number Diff line change
Expand Up @@ -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');
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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) {
Expand Down
6 changes: 6 additions & 0 deletions src/Options/Definitions.js
Original file line number Diff line number Diff line change
Expand Up @@ -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: {
Expand Down
1 change: 1 addition & 0 deletions src/Options/docs.js
Original file line number Diff line number Diff line change
Expand Up @@ -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.
*/

/**
Expand Down
3 changes: 3 additions & 0 deletions src/Options/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down

0 comments on commit 08b2ea4

Please sign in to comment.