Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add account unlock on password reset #7146

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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