From cc57c6b0760c55c036dbf0e0acf05ff76ab8e6c0 Mon Sep 17 00:00:00 2001 From: Bhaskar Reddy Yasa Date: Mon, 21 Nov 2016 22:16:47 +0530 Subject: [PATCH 1/3] password history support in passwordPolicy Refactor RestWrite.transformUser --- README.md | 1 + spec/PasswordPolicy.spec.js | 300 ++++++++++++++++++ src/Adapters/Storage/Mongo/MongoTransform.js | 1 + .../Postgres/PostgresStorageAdapter.js | 5 +- src/Config.js | 4 + src/Controllers/DatabaseController.js | 2 +- src/RestWrite.js | 252 +++++++++------ 7 files changed, 474 insertions(+), 91 deletions(-) diff --git a/README.md b/README.md index d95dbd7ef0..cdcd23dd29 100644 --- a/README.md +++ b/README.md @@ -288,6 +288,7 @@ var server = ParseServer({ validatorCallback: (password) => { return validatePassword(password) }, doNotAllowUsername: true, // optional setting to disallow username in passwords maxPasswordAge: 90, // optional setting in days for password expiry. Login fails if user does not reset the password within this period after signup/last reset. + passwordHistory: 5, // optional setting to prevent reuse of previous n passwords. Maximum value that can be specified is 20. Not specifying it or specifying 0 will not enforce history. //optional setting to set a validity duration for password reset links (in seconds) resetTokenValidityDuration: 24*60*60, // expire after 24 hours } diff --git a/spec/PasswordPolicy.spec.js b/spec/PasswordPolicy.spec.js index 028ead228b..ff163c398d 100644 --- a/spec/PasswordPolicy.spec.js +++ b/spec/PasswordPolicy.spec.js @@ -1010,4 +1010,304 @@ describe("Password Policy: ", () => { }); }); + it('should fail if passwordPolicy.passwordHistory is not a number', done => { + reconfigureServer({ + appName: 'passwordPolicy', + passwordPolicy: { + passwordHistory: "not a number" + }, + publicServerURL: "http://localhost:8378/1" + }).then(() => { + fail('passwordPolicy.passwordHistory "not a number" test failed'); + done(); + }).catch(err => { + expect(err).toEqual('passwordPolicy.passwordHistory must be an integer ranging 0 - 20'); + done(); + }); + }); + + it('should fail if passwordPolicy.passwordHistory is a negative number', done => { + reconfigureServer({ + appName: 'passwordPolicy', + passwordPolicy: { + passwordHistory: -10 + }, + publicServerURL: "http://localhost:8378/1" + }).then(() => { + fail('passwordPolicy.passwordHistory negative number test failed'); + done(); + }).catch(err => { + expect(err).toEqual('passwordPolicy.passwordHistory must be an integer ranging 0 - 20'); + done(); + }); + }); + + it('should fail if passwordPolicy.passwordHistory is greater than 20', done => { + reconfigureServer({ + appName: 'passwordPolicy', + passwordPolicy: { + passwordHistory: 21 + }, + publicServerURL: "http://localhost:8378/1" + }).then(() => { + fail('passwordPolicy.passwordHistory negative number test failed'); + done(); + }).catch(err => { + expect(err).toEqual('passwordPolicy.passwordHistory must be an integer ranging 0 - 20'); + done(); + }); + }); + + it('should fail if the new password is same as the last password', done => { + const user = new Parse.User(); + const emailAdapter = { + sendVerificationEmail: () => Promise.resolve(), + sendPasswordResetEmail: options => { + requestp.get({ + uri: options.link, + followRedirect: false, + simple: false, + resolveWithFullResponse: true + }).then(response => { + expect(response.statusCode).toEqual(302); + const re = /http:\/\/localhost:8378\/1\/apps\/choose_password\?token=([a-zA-Z0-9]+)\&id=test\&username=user1/; + const match = response.body.match(re); + if (!match) { + fail("should have a token"); + return Promise.reject("Invalid password link"); + } + return Promise.resolve(match[1]); // token + }).then(token => { + return new Promise((resolve, reject) => { + requestp.post({ + uri: "http://localhost:8378/1/apps/test/request_password_reset", + body: `new_password=user1&token=${token}&username=user1`, + headers: { + 'Content-Type': 'application/x-www-form-urlencoded' + }, + followRedirect: false, + simple: false, + resolveWithFullResponse: true + }).then(response => { + resolve([response, token]); + }).catch(error => { + reject(error); + }); + }); + }).then(data => { + const response = data[0]; + const token = data[1]; + expect(response.statusCode).toEqual(302); + expect(response.body).toEqual(`Found. Redirecting to http://localhost:8378/1/apps/choose_password?username=user1&token=${token}&id=test&error=New%20password%20should%20not%20be%20the%20same%20as%20last%201%20passwords.&app=passwordPolicy`); + done(); + return Promise.resolve(); + }).catch(error => { + jfail(error); + fail("Repeat password test failed"); + done(); + }); + }, + sendMail: () => { + } + }; + reconfigureServer({ + appName: 'passwordPolicy', + verifyUserEmails: false, + emailAdapter: emailAdapter, + passwordPolicy: { + passwordHistory: 1 + }, + publicServerURL: "http://localhost:8378/1" + }).then(() => { + user.setUsername("user1"); + user.setPassword("user1"); + user.set('email', 'user1@parse.com'); + user.signUp().then(() => { + return Parse.User.logOut(); + }).then(() => { + return Parse.User.requestPasswordReset('user1@parse.com'); + }).catch(error => { + jfail(error); + fail("SignUp or reset request failed"); + done(); + }); + }); + }); + + it('should fail if the new password is same as 5th oldest password in history when policy does not allow last 5 passwords', done => { + const user = new Parse.User(); + let pwCount = 1; + const emailAdapter = { + sendVerificationEmail: () => Promise.resolve(), + sendPasswordResetEmail: options => { + pwCount++; // counter for password generation like user2, user3, .... (user1 is used during signup) + + requestp.get({ + uri: options.link, + followRedirect: false, + simple: false, + resolveWithFullResponse: true + }).then(response => { + expect(response.statusCode).toEqual(302); + const re = /http:\/\/localhost:8378\/1\/apps\/choose_password\?token=([a-zA-Z0-9]+)\&id=test\&username=user1/; + const match = response.body.match(re); + if (!match) { + fail("should have a token"); + return Promise.reject("Invalid password link"); + } + return Promise.resolve(match[1]); // token + }).then(token => { + let newpw = `user${pwCount}`; + if(pwCount === 6) // try to reuse the first password which is in history + newpw = 'user1'; + return new Promise((resolve, reject) => { + requestp.post({ + uri: "http://localhost:8378/1/apps/test/request_password_reset", + body: `new_password=${newpw}&token=${token}&username=user1`, + headers: { + 'Content-Type': 'application/x-www-form-urlencoded' + }, + followRedirect: false, + simple: false, + resolveWithFullResponse: true + }).then(response => { + resolve([response, token]); + }).catch(error => { + reject(error); + }); + }); + }).then(data => { + const response = data[0]; + const token = data[1]; + if(pwCount === 6){ // repeating password 'user1' - must fail + expect(response.statusCode).toEqual(302); + expect(response.body).toEqual(`Found. Redirecting to http://localhost:8378/1/apps/choose_password?username=user1&token=${token}&id=test&error=New%20password%20should%20not%20be%20the%20same%20as%20last%205%20passwords.&app=passwordPolicy`); + done(); + } else { // continue with resets + expect(response.statusCode).toEqual(302); + expect(response.body).toEqual('Found. Redirecting to http://localhost:8378/1/apps/password_reset_success.html?username=user1'); + Parse.User.requestPasswordReset('user1@parse.com'); + } + return Promise.resolve(); + }).catch(error => { + jfail(error); + fail("Repeat password test failed"); + done(); + }); + }, + sendMail: () => { + } + }; + reconfigureServer({ + appName: 'passwordPolicy', + verifyUserEmails: false, + emailAdapter: emailAdapter, + passwordPolicy: { + passwordHistory: 5 + }, + publicServerURL: "http://localhost:8378/1" + }).then(() => { + user.setUsername("user1"); + user.setPassword("user1"); + user.set('email', 'user1@parse.com'); + user.signUp().then(() => { + return Parse.User.logOut(); + }).then(() => { + // initiate a sequence of password resets + return Parse.User.requestPasswordReset('user1@parse.com'); + }).catch(error => { + jfail(error); + fail("SignUp or reset request failed"); + done(); + }); + }); + }); + + it('should succeed if the a password beyond the history limit is reused', done => { + const user = new Parse.User(); + let pwCount = 1; + const emailAdapter = { + sendVerificationEmail: () => Promise.resolve(), + sendPasswordResetEmail: options => { + pwCount++; // counter for password sequence (user2, user3, ...) + + requestp.get({ + uri: options.link, + followRedirect: false, + simple: false, + resolveWithFullResponse: true + }).then(response => { + expect(response.statusCode).toEqual(302); + const re = /http:\/\/localhost:8378\/1\/apps\/choose_password\?token=([a-zA-Z0-9]+)\&id=test\&username=user1/; + const match = response.body.match(re); + if (!match) { + fail("should have a token"); + return Promise.reject("Invalid password link"); + } + return Promise.resolve(match[1]); // token + }).then(token => { + let newpw = `user${pwCount}`; + if(pwCount === 7) // reuse the first password now + newpw = 'user1'; + return new Promise((resolve, reject) => { + requestp.post({ + uri: "http://localhost:8378/1/apps/test/request_password_reset", + body: `new_password=${newpw}&token=${token}&username=user1`, + headers: { + 'Content-Type': 'application/x-www-form-urlencoded' + }, + followRedirect: false, + simple: false, + resolveWithFullResponse: true + }).then(response => { + resolve([response, token]); + }).catch(error => { + reject(error); + }); + }); + }).then(data => { + const response = data[0]; + const token = data[1]; + expect(response.statusCode).toEqual(302); + expect(response.body).toEqual('Found. Redirecting to http://localhost:8378/1/apps/password_reset_success.html?username=user1'); + if(pwCount === 7){ // end it now + done(); + } else { //continue with more resets + Parse.User.requestPasswordReset('user1@parse.com'); + } + return Promise.resolve(); + }).catch(error => { + jfail(error); + fail("Repeat password test failed"); + done(); + }); + }, + sendMail: () => { + } + }; + reconfigureServer({ + appName: 'passwordPolicy', + verifyUserEmails: false, + emailAdapter: emailAdapter, + passwordPolicy: { + passwordHistory: 5 + }, + publicServerURL: "http://localhost:8378/1" + }).then(() => { + user.setUsername("user1"); + user.setPassword("user1"); + user.set('email', 'user1@parse.com'); + user.signUp().then(() => { + return Parse.User.logOut(); + }).then(() => { + // initiate sequence of password resets + return Parse.User.requestPasswordReset('user1@parse.com'); + }).catch(error => { + jfail(error); + fail("SignUp or reset request failed"); + done(); + }); + }); + }); + }) diff --git a/src/Adapters/Storage/Mongo/MongoTransform.js b/src/Adapters/Storage/Mongo/MongoTransform.js index 14a6972e68..0494a83c04 100644 --- a/src/Adapters/Storage/Mongo/MongoTransform.js +++ b/src/Adapters/Storage/Mongo/MongoTransform.js @@ -787,6 +787,7 @@ const mongoObjectToParseObject = (className, mongoObject, schema) => { case '_email_verify_token_expires_at': case '_account_lockout_expires_at': case '_failed_login_count': + case '_old_passwords': // Those keys will be deleted if needed in the DB Controller restObject[key] = mongoObject[key]; break; diff --git a/src/Adapters/Storage/Postgres/PostgresStorageAdapter.js b/src/Adapters/Storage/Postgres/PostgresStorageAdapter.js index ee5760fcc6..0284c80b85 100644 --- a/src/Adapters/Storage/Postgres/PostgresStorageAdapter.js +++ b/src/Adapters/Storage/Postgres/PostgresStorageAdapter.js @@ -108,6 +108,7 @@ const toPostgresSchema = (schema) => { schema.fields._rperm = {type: 'Array', contents: {type: 'String'}} if (schema.className === '_User') { schema.fields._hashed_password = {type: 'String'}; + schema.fields._old_passwords = {type: 'Array'}; } return schema; } @@ -471,6 +472,7 @@ export class PostgresStorageAdapter { fields._perishable_token = {type: 'String'}; fields._perishable_token_expires_at = {type: 'Date'}; fields._password_changed_at = {type: 'Date'}; + fields._old_passwords = { type: 'Array'}; } let index = 2; let relations = []; @@ -683,7 +685,8 @@ export class PostgresStorageAdapter { if (!schema.fields[fieldName] && className === '_User') { if (fieldName === '_email_verify_token' || fieldName === '_failed_login_count' || - fieldName === '_perishable_token') { + fieldName === '_perishable_token' || + fieldName === '_old_passwords'){ valuesArray.push(object[fieldName]); } diff --git a/src/Config.js b/src/Config.js index d2fa2726e9..8575e8880f 100644 --- a/src/Config.js +++ b/src/Config.js @@ -138,6 +138,10 @@ export class Config { if(passwordPolicy.doNotAllowUsername && typeof passwordPolicy.doNotAllowUsername !== 'boolean') { throw 'passwordPolicy.doNotAllowUsername must be a boolean value.'; } + + if (passwordPolicy.passwordHistory && (!Number.isInteger(passwordPolicy.passwordHistory) || passwordPolicy.passwordHistory <= 0 || passwordPolicy.passwordHistory > 20)) { + throw 'passwordPolicy.passwordHistory must be an integer ranging 0 - 20'; + } } } diff --git a/src/Controllers/DatabaseController.js b/src/Controllers/DatabaseController.js index 4acbefa393..696eff05b5 100644 --- a/src/Controllers/DatabaseController.js +++ b/src/Controllers/DatabaseController.js @@ -190,7 +190,7 @@ const filterSensitiveData = (isMaster, aclGroup, className, object) => { // acl: a list of strings. If the object to be updated has an ACL, // one of the provided strings must provide the caller with // write permissions. -const specialKeysForUpdate = ['_hashed_password', '_perishable_token', '_email_verify_token', '_email_verify_token_expires_at', '_account_lockout_expires_at', '_failed_login_count', '_perishable_token_expires_at', '_password_changed_at']; +const specialKeysForUpdate = ['_hashed_password', '_perishable_token', '_email_verify_token', '_email_verify_token_expires_at', '_account_lockout_expires_at', '_failed_login_count', '_perishable_token_expires_at', '_password_changed_at', '_old_passwords']; const isSpecialUpdateKey = key => { return specialKeysForUpdate.indexOf(key) >= 0; diff --git a/src/RestWrite.js b/src/RestWrite.js index 0d0bcf0ac1..7323040c95 100644 --- a/src/RestWrite.js +++ b/src/RestWrite.js @@ -352,14 +352,16 @@ RestWrite.prototype.transformUser = function() { if (this.query) { // If we're updating a _User object, we need to clear out the cache for that user. Find all their // session tokens, and remove them from the cache. - promise = new RestQuery(this.config, Auth.master(this.config), '_Session', { user: { - __type: "Pointer", - className: "_User", - objectId: this.objectId(), - }}).execute() - .then(results => { - results.results.forEach(session => this.config.cacheController.user.del(session.sessionToken)); - }); + promise = new RestQuery(this.config, Auth.master(this.config), '_Session', { + user: { + __type: "Pointer", + className: "_User", + objectId: this.objectId(), + } + }).execute() + .then(results => { + results.results.forEach(session => this.config.cacheController.user.del(session.sessionToken)); + }); } return promise.then(() => { @@ -368,44 +370,12 @@ RestWrite.prototype.transformUser = function() { return; } - let defer = Promise.resolve(); - - // check if the password conforms to the defined password policy if configured - if (this.config.passwordPolicy) { - const policyError = 'Password does not meet the Password Policy requirements.'; - - // check whether the password conforms to the policy - if (this.config.passwordPolicy.patternValidator && !this.config.passwordPolicy.patternValidator(this.data.password) || - this.config.passwordPolicy.validatorCallback && !this.config.passwordPolicy.validatorCallback(this.data.password)) { - return Promise.reject(new Parse.Error(Parse.Error.VALIDATION_ERROR, policyError)); - } - - // check whether password contain username - if (this.config.passwordPolicy.doNotAllowUsername === true) { - if (this.data.username) { // username is not passed during password reset - if (this.data.password.indexOf(this.data.username) >= 0) - return Promise.reject(new Parse.Error(Parse.Error.VALIDATION_ERROR, policyError)); - - } else { // retrieve the User object using objectId during password reset - defer = this.config.database.find('_User', {objectId: this.objectId()}) - .then(results => { - if (results.length != 1) { - throw undefined; - } - if (this.data.password.indexOf(results[0].username) >= 0) - return Promise.reject(new Parse.Error(Parse.Error.VALIDATION_ERROR, policyError)); - return Promise.resolve(); - }); - } - } - } - if (this.query && !this.auth.isMaster) { this.storage['clearSessions'] = true; this.storage['generateNewSession'] = true; } - return defer.then(() => { + return this._validatePasswordPolicy().then(() => { return passwordCrypto.hash(this.data.password).then((hashedPassword) => { this.data._hashed_password = hashedPassword; delete this.data.password; @@ -413,51 +383,130 @@ RestWrite.prototype.transformUser = function() { }); }).then(() => { - // Check for username uniqueness - if (!this.data.username) { - if (!this.query) { - this.data.username = cryptoUtils.randomString(25); - this.responseShouldHaveUsername = true; - } - return; + return this._validateUserName(); + }).then(() => { + return this._validateEmail(); + }); +}; + +RestWrite.prototype._validateUserName = function () { + // Check for username uniqueness + if (!this.data.username) { + if (!this.query) { + this.data.username = cryptoUtils.randomString(25); + this.responseShouldHaveUsername = true; } - // We need to a find to check for duplicate username in case they are missing the unique index on usernames - // TODO: Check if there is a unique index, and if so, skip this query. - return this.config.database.find( - this.className, - { username: this.data.username, objectId: {'$ne': this.objectId()} }, - { limit: 1 } - ) - .then(results => { - if (results.length > 0) { - throw new Parse.Error(Parse.Error.USERNAME_TAKEN, 'Account already exists for this username.'); - } - return; - }); - }) - .then(() => { - if (!this.data.email || this.data.email.__op === 'Delete') { - return; + return Promise.resolve(); + } + // We need to a find to check for duplicate username in case they are missing the unique index on usernames + // TODO: Check if there is a unique index, and if so, skip this query. + return this.config.database.find( + this.className, + {username: this.data.username, objectId: {'$ne': this.objectId()}}, + {limit: 1} + ).then(results => { + if (results.length > 0) { + throw new Parse.Error(Parse.Error.USERNAME_TAKEN, 'Account already exists for this username.'); + } + return; + }); +}; + +RestWrite.prototype._validateEmail = function() { + if (!this.data.email || this.data.email.__op === 'Delete') { + return Promise.resolve(); + } + // Validate basic email address format + if (!this.data.email.match(/^.+@.+$/)) { + return Promise.reject(new Parse.Error(Parse.Error.INVALID_EMAIL_ADDRESS, 'Email address format is invalid.')); + } + // Same problem for email as above for username + return this.config.database.find( + this.className, + {email: this.data.email, objectId: {'$ne': this.objectId()}}, + {limit: 1} + ).then(results => { + if (results.length > 0) { + throw new Parse.Error(Parse.Error.EMAIL_TAKEN, 'Account already exists for this email address.'); } - // Validate basic email address format - if (!this.data.email.match(/^.+@.+$/)) { - throw new Parse.Error(Parse.Error.INVALID_EMAIL_ADDRESS, 'Email address format is invalid.'); + // We updated the email, send a new validation + this.storage['sendVerificationEmail'] = true; + this.config.userController.setEmailVerifyToken(this.data); + }); +}; + +RestWrite.prototype._validatePasswordPolicy = function() { + if (!this.config.passwordPolicy) + return Promise.resolve(); + return this._validatePasswordRequirements().then(() => { + return this._validatePasswordHistory(); + }); +}; + + +RestWrite.prototype._validatePasswordRequirements = function() { + // check if the password conforms to the defined password policy if configured + const policyError = 'Password does not meet the Password Policy requirements.'; + + // check whether the password meets the password strength requirements + if (this.config.passwordPolicy.patternValidator && !this.config.passwordPolicy.patternValidator(this.data.password) || + this.config.passwordPolicy.validatorCallback && !this.config.passwordPolicy.validatorCallback(this.data.password)) { + return Promise.reject(new Parse.Error(Parse.Error.VALIDATION_ERROR, policyError)); + } + + // check whether password contain username + if (this.config.passwordPolicy.doNotAllowUsername === true) { + if (this.data.username) { // username is not passed during password reset + if (this.data.password.indexOf(this.data.username) >= 0) + return Promise.reject(new Parse.Error(Parse.Error.VALIDATION_ERROR, policyError)); + } else { // retrieve the User object using objectId during password reset + return this.config.database.find('_User', {objectId: this.objectId()}) + .then(results => { + if (results.length != 1) { + throw undefined; + } + if (this.data.password.indexOf(results[0].username) >= 0) + return Promise.reject(new Parse.Error(Parse.Error.VALIDATION_ERROR, policyError)); + return Promise.resolve(); + }); } - // Same problem for email as above for username - return this.config.database.find( - this.className, - { email: this.data.email, objectId: {'$ne': this.objectId()} }, - { limit: 1 } - ) - .then(results => { - if (results.length > 0) { - throw new Parse.Error(Parse.Error.EMAIL_TAKEN, 'Account already exists for this email address.'); - } - // We updated the email, send a new validation - this.storage['sendVerificationEmail'] = true; - this.config.userController.setEmailVerifyToken(this.data); - }); - }) + } + return Promise.resolve(); +}; + +RestWrite.prototype._validatePasswordHistory = function() { + // check whether password is repeating from specified history + if (this.query && this.config.passwordPolicy.passwordHistory) { + return this.config.database.find('_User', {objectId: this.objectId()}, {keys: ["_old_passwords", "_hashed_password"]}) + .then(results => { + if (results.length != 1) { + throw undefined; + } + const user = results[0]; + let oldPasswords = []; + if (user._old_passwords) + oldPasswords = _.take(user._old_passwords, this.config.passwordPolicy.passwordHistory - 1); + oldPasswords.push(user.password); + const newPassword = this.data.password; + // compare the new password hash with all old password hashes + let promises = oldPasswords.map(function (hash) { + return passwordCrypto.compare(newPassword, hash).then((result) => { + if (result) // reject if there is a match + return Promise.reject("REPEAT_PASSWORD"); + return Promise.resolve(); + }) + }); + // wait for all comparisons to complete + return Promise.all(promises).then(() => { + return Promise.resolve(); + }).catch(err => { + if (err === "REPEAT_PASSWORD") // a match was found + return Promise.reject(new Parse.Error(Parse.Error.VALIDATION_ERROR, `New password should not be the same as last ${this.config.passwordPolicy.passwordHistory} passwords.`)); + throw err; + }); + }); + } + return Promise.resolve(); }; RestWrite.prototype.createSessionTokenIfNeeded = function() { @@ -851,12 +900,37 @@ RestWrite.prototype.runDatabaseOperation = function() { // Ignore createdAt when update delete this.data.createdAt; - // Run an update - return this.config.database.update(this.className, this.query, this.data, this.runOptions) - .then(response => { - response.updatedAt = this.updatedAt; - this._updateResponseWithData(response, this.data); - this.response = { response }; + let defer = Promise.resolve(); + // if password history is enabled then save the current password to history + if (this.className === '_User' && this.data._hashed_password && this.config.passwordPolicy && this.config.passwordPolicy.passwordHistory) { + defer = this.config.database.find('_User', {objectId: this.objectId()}, + {keys: ["_old_passwords", "_hashed_password"]}).then(results => { + + if (results.length != 1) { + throw undefined; + } + const user = results[0]; + let oldPasswords = []; + if (user._old_passwords) { + oldPasswords = _.take(user._old_passwords, this.config.passwordPolicy.passwordHistory); + } + //n-1 passwords go into history including last password + while (oldPasswords.length > this.config.passwordPolicy.passwordHistory - 2) { + oldPasswords.shift(); + } + oldPasswords.push(user.password); + this.data._old_passwords = oldPasswords; + }); + } + + return defer.then(() => { + // Run an update + return this.config.database.update(this.className, this.query, this.data, this.runOptions) + .then(response => { + response.updatedAt = this.updatedAt; + this._updateResponseWithData(response, this.data); + this.response = { response }; + }); }); } else { // Set the default ACL and password timestamp for the new _User From 9f5bac35d1c5c74ad8e6464d2fdcf11cead08add Mon Sep 17 00:00:00 2001 From: Bhaskar Reddy Yasa Date: Tue, 29 Nov 2016 11:55:18 +0530 Subject: [PATCH 2/3] fix eslint issues --- spec/PasswordPolicy.spec.js | 1 - src/RestWrite.js | 4 +--- 2 files changed, 1 insertion(+), 4 deletions(-) diff --git a/spec/PasswordPolicy.spec.js b/spec/PasswordPolicy.spec.js index ff163c398d..2b9599775f 100644 --- a/spec/PasswordPolicy.spec.js +++ b/spec/PasswordPolicy.spec.js @@ -1267,7 +1267,6 @@ describe("Password Policy: ", () => { }); }).then(data => { const response = data[0]; - const token = data[1]; expect(response.statusCode).toEqual(302); expect(response.body).toEqual('Found. Redirecting to http://localhost:8378/1/apps/password_reset_success.html?username=user1'); if(pwCount === 7){ // end it now diff --git a/src/RestWrite.js b/src/RestWrite.js index 7323040c95..41e5413838 100644 --- a/src/RestWrite.js +++ b/src/RestWrite.js @@ -903,9 +903,7 @@ RestWrite.prototype.runDatabaseOperation = function() { let defer = Promise.resolve(); // if password history is enabled then save the current password to history if (this.className === '_User' && this.data._hashed_password && this.config.passwordPolicy && this.config.passwordPolicy.passwordHistory) { - defer = this.config.database.find('_User', {objectId: this.objectId()}, - {keys: ["_old_passwords", "_hashed_password"]}).then(results => { - + defer = this.config.database.find('_User', {objectId: this.objectId()}, {keys: ["_old_passwords", "_hashed_password"]}).then(results => { if (results.length != 1) { throw undefined; } From 75d28f6535148a02e7388b5c7b66cc7b92432172 Mon Sep 17 00:00:00 2001 From: Bhaskar Reddy Yasa Date: Tue, 29 Nov 2016 13:20:53 +0530 Subject: [PATCH 3/3] - renamed "passwordHistory" setting to "maxPasswordHistory" - renamed field "_old_passwords" to "_password_history" - simplified passwordHistory test cases without using emailAdapter --- README.md | 2 +- spec/PasswordPolicy.spec.js | 237 +++++++----------- src/Adapters/Storage/Mongo/MongoTransform.js | 2 +- .../Postgres/PostgresStorageAdapter.js | 6 +- src/Config.js | 4 +- src/Controllers/DatabaseController.js | 2 +- src/RestWrite.js | 30 +-- 7 files changed, 111 insertions(+), 172 deletions(-) diff --git a/README.md b/README.md index cdcd23dd29..7295402ea5 100644 --- a/README.md +++ b/README.md @@ -288,7 +288,7 @@ var server = ParseServer({ validatorCallback: (password) => { return validatePassword(password) }, doNotAllowUsername: true, // optional setting to disallow username in passwords maxPasswordAge: 90, // optional setting in days for password expiry. Login fails if user does not reset the password within this period after signup/last reset. - passwordHistory: 5, // optional setting to prevent reuse of previous n passwords. Maximum value that can be specified is 20. Not specifying it or specifying 0 will not enforce history. + maxPasswordHistory: 5, // optional setting to prevent reuse of previous n passwords. Maximum value that can be specified is 20. Not specifying it or specifying 0 will not enforce history. //optional setting to set a validity duration for password reset links (in seconds) resetTokenValidityDuration: 24*60*60, // expire after 24 hours } diff --git a/spec/PasswordPolicy.spec.js b/spec/PasswordPolicy.spec.js index 2b9599775f..4f385821ae 100644 --- a/spec/PasswordPolicy.spec.js +++ b/spec/PasswordPolicy.spec.js @@ -1010,55 +1010,55 @@ describe("Password Policy: ", () => { }); }); - it('should fail if passwordPolicy.passwordHistory is not a number', done => { + it('should fail if passwordPolicy.maxPasswordHistory is not a number', done => { reconfigureServer({ appName: 'passwordPolicy', passwordPolicy: { - passwordHistory: "not a number" + maxPasswordHistory: "not a number" }, publicServerURL: "http://localhost:8378/1" }).then(() => { - fail('passwordPolicy.passwordHistory "not a number" test failed'); + fail('passwordPolicy.maxPasswordHistory "not a number" test failed'); done(); }).catch(err => { - expect(err).toEqual('passwordPolicy.passwordHistory must be an integer ranging 0 - 20'); + expect(err).toEqual('passwordPolicy.maxPasswordHistory must be an integer ranging 0 - 20'); done(); }); }); - it('should fail if passwordPolicy.passwordHistory is a negative number', done => { + it('should fail if passwordPolicy.maxPasswordHistory is a negative number', done => { reconfigureServer({ appName: 'passwordPolicy', passwordPolicy: { - passwordHistory: -10 + maxPasswordHistory: -10 }, publicServerURL: "http://localhost:8378/1" }).then(() => { - fail('passwordPolicy.passwordHistory negative number test failed'); + fail('passwordPolicy.maxPasswordHistory negative number test failed'); done(); }).catch(err => { - expect(err).toEqual('passwordPolicy.passwordHistory must be an integer ranging 0 - 20'); + expect(err).toEqual('passwordPolicy.maxPasswordHistory must be an integer ranging 0 - 20'); done(); }); }); - it('should fail if passwordPolicy.passwordHistory is greater than 20', done => { + it('should fail if passwordPolicy.maxPasswordHistory is greater than 20', done => { reconfigureServer({ appName: 'passwordPolicy', passwordPolicy: { - passwordHistory: 21 + maxPasswordHistory: 21 }, publicServerURL: "http://localhost:8378/1" }).then(() => { - fail('passwordPolicy.passwordHistory negative number test failed'); + fail('passwordPolicy.maxPasswordHistory negative number test failed'); done(); }).catch(err => { - expect(err).toEqual('passwordPolicy.passwordHistory must be an integer ranging 0 - 20'); + expect(err).toEqual('passwordPolicy.maxPasswordHistory must be an integer ranging 0 - 20'); done(); }); }); - it('should fail if the new password is same as the last password', done => { + it('should fail to reset if the new password is same as the last password', done => { const user = new Parse.User(); const emailAdapter = { sendVerificationEmail: () => Promise.resolve(), @@ -1115,7 +1115,7 @@ describe("Password Policy: ", () => { verifyUserEmails: false, emailAdapter: emailAdapter, passwordPolicy: { - passwordHistory: 1 + maxPasswordHistory: 1 }, publicServerURL: "http://localhost:8378/1" }).then(() => { @@ -1134,76 +1134,15 @@ describe("Password Policy: ", () => { }); }); - it('should fail if the new password is same as 5th oldest password in history when policy does not allow last 5 passwords', done => { + + it('should fail if the new password is same as the previous one', done => { const user = new Parse.User(); - let pwCount = 1; - const emailAdapter = { - sendVerificationEmail: () => Promise.resolve(), - sendPasswordResetEmail: options => { - pwCount++; // counter for password generation like user2, user3, .... (user1 is used during signup) - requestp.get({ - uri: options.link, - followRedirect: false, - simple: false, - resolveWithFullResponse: true - }).then(response => { - expect(response.statusCode).toEqual(302); - const re = /http:\/\/localhost:8378\/1\/apps\/choose_password\?token=([a-zA-Z0-9]+)\&id=test\&username=user1/; - const match = response.body.match(re); - if (!match) { - fail("should have a token"); - return Promise.reject("Invalid password link"); - } - return Promise.resolve(match[1]); // token - }).then(token => { - let newpw = `user${pwCount}`; - if(pwCount === 6) // try to reuse the first password which is in history - newpw = 'user1'; - return new Promise((resolve, reject) => { - requestp.post({ - uri: "http://localhost:8378/1/apps/test/request_password_reset", - body: `new_password=${newpw}&token=${token}&username=user1`, - headers: { - 'Content-Type': 'application/x-www-form-urlencoded' - }, - followRedirect: false, - simple: false, - resolveWithFullResponse: true - }).then(response => { - resolve([response, token]); - }).catch(error => { - reject(error); - }); - }); - }).then(data => { - const response = data[0]; - const token = data[1]; - if(pwCount === 6){ // repeating password 'user1' - must fail - expect(response.statusCode).toEqual(302); - expect(response.body).toEqual(`Found. Redirecting to http://localhost:8378/1/apps/choose_password?username=user1&token=${token}&id=test&error=New%20password%20should%20not%20be%20the%20same%20as%20last%205%20passwords.&app=passwordPolicy`); - done(); - } else { // continue with resets - expect(response.statusCode).toEqual(302); - expect(response.body).toEqual('Found. Redirecting to http://localhost:8378/1/apps/password_reset_success.html?username=user1'); - Parse.User.requestPasswordReset('user1@parse.com'); - } - return Promise.resolve(); - }).catch(error => { - jfail(error); - fail("Repeat password test failed"); - done(); - }); - }, - sendMail: () => { - } - }; reconfigureServer({ appName: 'passwordPolicy', verifyUserEmails: false, - emailAdapter: emailAdapter, passwordPolicy: { - passwordHistory: 5 + maxPasswordHistory: 5 }, publicServerURL: "http://localhost:8378/1" }).then(() => { @@ -1211,85 +1150,28 @@ describe("Password Policy: ", () => { user.setPassword("user1"); user.set('email', 'user1@parse.com'); user.signUp().then(() => { - return Parse.User.logOut(); + // try to set the same password as the previous one + user.setPassword('user1'); + return user.save(); }).then(() => { - // initiate a sequence of password resets - return Parse.User.requestPasswordReset('user1@parse.com'); + fail("should have failed because the new password is same as the old"); + done(); }).catch(error => { - jfail(error); - fail("SignUp or reset request failed"); + expect(error.message).toEqual('New password should not be the same as last 5 passwords.'); + expect(error.code).toEqual(Parse.Error.VALIDATION_ERROR); done(); }); }); }); - it('should succeed if the a password beyond the history limit is reused', done => { + it('should fail if the new password is same as the 5th oldest one and policy does not allow the previous 5', done => { const user = new Parse.User(); - let pwCount = 1; - const emailAdapter = { - sendVerificationEmail: () => Promise.resolve(), - sendPasswordResetEmail: options => { - pwCount++; // counter for password sequence (user2, user3, ...) - requestp.get({ - uri: options.link, - followRedirect: false, - simple: false, - resolveWithFullResponse: true - }).then(response => { - expect(response.statusCode).toEqual(302); - const re = /http:\/\/localhost:8378\/1\/apps\/choose_password\?token=([a-zA-Z0-9]+)\&id=test\&username=user1/; - const match = response.body.match(re); - if (!match) { - fail("should have a token"); - return Promise.reject("Invalid password link"); - } - return Promise.resolve(match[1]); // token - }).then(token => { - let newpw = `user${pwCount}`; - if(pwCount === 7) // reuse the first password now - newpw = 'user1'; - return new Promise((resolve, reject) => { - requestp.post({ - uri: "http://localhost:8378/1/apps/test/request_password_reset", - body: `new_password=${newpw}&token=${token}&username=user1`, - headers: { - 'Content-Type': 'application/x-www-form-urlencoded' - }, - followRedirect: false, - simple: false, - resolveWithFullResponse: true - }).then(response => { - resolve([response, token]); - }).catch(error => { - reject(error); - }); - }); - }).then(data => { - const response = data[0]; - expect(response.statusCode).toEqual(302); - expect(response.body).toEqual('Found. Redirecting to http://localhost:8378/1/apps/password_reset_success.html?username=user1'); - if(pwCount === 7){ // end it now - done(); - } else { //continue with more resets - Parse.User.requestPasswordReset('user1@parse.com'); - } - return Promise.resolve(); - }).catch(error => { - jfail(error); - fail("Repeat password test failed"); - done(); - }); - }, - sendMail: () => { - } - }; reconfigureServer({ appName: 'passwordPolicy', verifyUserEmails: false, - emailAdapter: emailAdapter, passwordPolicy: { - passwordHistory: 5 + maxPasswordHistory: 5 }, publicServerURL: "http://localhost:8378/1" }).then(() => { @@ -1297,16 +1179,73 @@ describe("Password Policy: ", () => { user.setPassword("user1"); user.set('email', 'user1@parse.com'); user.signUp().then(() => { - return Parse.User.logOut(); + // build history + user.setPassword('user2'); + return user.save(); }).then(() => { - // initiate sequence of password resets - return Parse.User.requestPasswordReset('user1@parse.com'); + user.setPassword('user3'); + return user.save(); + }).then(() => { + user.setPassword('user4'); + return user.save(); + }).then(() => { + user.setPassword('user5'); + return user.save(); + }).then(() => { + // set the same password as the initial one + user.setPassword('user1'); + return user.save(); + }).then(() => { + fail("should have failed because the new password is same as the old"); + done(); }).catch(error => { - jfail(error); - fail("SignUp or reset request failed"); + expect(error.message).toEqual('New password should not be the same as last 5 passwords.'); + expect(error.code).toEqual(Parse.Error.VALIDATION_ERROR); done(); }); }); }); + it('should succeed if the new password is same as the 6th oldest one and policy does not allow only previous 5', done => { + const user = new Parse.User(); + + reconfigureServer({ + appName: 'passwordPolicy', + verifyUserEmails: false, + passwordPolicy: { + maxPasswordHistory: 5 + }, + publicServerURL: "http://localhost:8378/1" + }).then(() => { + user.setUsername("user1"); + user.setPassword("user1"); + user.set('email', 'user1@parse.com'); + user.signUp().then(() => { + // build history + user.setPassword('user2'); + return user.save(); + }).then(() => { + user.setPassword('user3'); + return user.save(); + }).then(() => { + user.setPassword('user4'); + return user.save(); + }).then(() => { + user.setPassword('user5'); + return user.save(); + }).then(() => { + user.setPassword('user6'); // this pushes initial password out of history + return user.save(); + }).then(() => { + // set the same password as the initial one + user.setPassword('user1'); + return user.save(); + }).then(() => { + done(); + }).catch(() => { + fail("should have succeeded because the new password is not in history"); + done(); + }); + }); + }); }) diff --git a/src/Adapters/Storage/Mongo/MongoTransform.js b/src/Adapters/Storage/Mongo/MongoTransform.js index 0494a83c04..dd45f3dd57 100644 --- a/src/Adapters/Storage/Mongo/MongoTransform.js +++ b/src/Adapters/Storage/Mongo/MongoTransform.js @@ -787,7 +787,7 @@ const mongoObjectToParseObject = (className, mongoObject, schema) => { case '_email_verify_token_expires_at': case '_account_lockout_expires_at': case '_failed_login_count': - case '_old_passwords': + case '_password_history': // Those keys will be deleted if needed in the DB Controller restObject[key] = mongoObject[key]; break; diff --git a/src/Adapters/Storage/Postgres/PostgresStorageAdapter.js b/src/Adapters/Storage/Postgres/PostgresStorageAdapter.js index 0284c80b85..3135c71ea6 100644 --- a/src/Adapters/Storage/Postgres/PostgresStorageAdapter.js +++ b/src/Adapters/Storage/Postgres/PostgresStorageAdapter.js @@ -108,7 +108,7 @@ const toPostgresSchema = (schema) => { schema.fields._rperm = {type: 'Array', contents: {type: 'String'}} if (schema.className === '_User') { schema.fields._hashed_password = {type: 'String'}; - schema.fields._old_passwords = {type: 'Array'}; + schema.fields._password_history = {type: 'Array'}; } return schema; } @@ -472,7 +472,7 @@ export class PostgresStorageAdapter { fields._perishable_token = {type: 'String'}; fields._perishable_token_expires_at = {type: 'Date'}; fields._password_changed_at = {type: 'Date'}; - fields._old_passwords = { type: 'Array'}; + fields._password_history = { type: 'Array'}; } let index = 2; let relations = []; @@ -686,7 +686,7 @@ export class PostgresStorageAdapter { if (fieldName === '_email_verify_token' || fieldName === '_failed_login_count' || fieldName === '_perishable_token' || - fieldName === '_old_passwords'){ + fieldName === '_password_history'){ valuesArray.push(object[fieldName]); } diff --git a/src/Config.js b/src/Config.js index 8575e8880f..a7d9de9d79 100644 --- a/src/Config.js +++ b/src/Config.js @@ -139,8 +139,8 @@ export class Config { throw 'passwordPolicy.doNotAllowUsername must be a boolean value.'; } - if (passwordPolicy.passwordHistory && (!Number.isInteger(passwordPolicy.passwordHistory) || passwordPolicy.passwordHistory <= 0 || passwordPolicy.passwordHistory > 20)) { - throw 'passwordPolicy.passwordHistory must be an integer ranging 0 - 20'; + if (passwordPolicy.maxPasswordHistory && (!Number.isInteger(passwordPolicy.maxPasswordHistory) || passwordPolicy.maxPasswordHistory <= 0 || passwordPolicy.maxPasswordHistory > 20)) { + throw 'passwordPolicy.maxPasswordHistory must be an integer ranging 0 - 20'; } } } diff --git a/src/Controllers/DatabaseController.js b/src/Controllers/DatabaseController.js index 696eff05b5..1cd3157efe 100644 --- a/src/Controllers/DatabaseController.js +++ b/src/Controllers/DatabaseController.js @@ -190,7 +190,7 @@ const filterSensitiveData = (isMaster, aclGroup, className, object) => { // acl: a list of strings. If the object to be updated has an ACL, // one of the provided strings must provide the caller with // write permissions. -const specialKeysForUpdate = ['_hashed_password', '_perishable_token', '_email_verify_token', '_email_verify_token_expires_at', '_account_lockout_expires_at', '_failed_login_count', '_perishable_token_expires_at', '_password_changed_at', '_old_passwords']; +const specialKeysForUpdate = ['_hashed_password', '_perishable_token', '_email_verify_token', '_email_verify_token_expires_at', '_account_lockout_expires_at', '_failed_login_count', '_perishable_token_expires_at', '_password_changed_at', '_password_history']; const isSpecialUpdateKey = key => { return specialKeysForUpdate.indexOf(key) >= 0; diff --git a/src/RestWrite.js b/src/RestWrite.js index 41e5413838..e94b5d771d 100644 --- a/src/RestWrite.js +++ b/src/RestWrite.js @@ -343,12 +343,12 @@ RestWrite.prototype.handleAuthData = function(authData) { // The non-third-party parts of User transformation RestWrite.prototype.transformUser = function() { + var promise = Promise.resolve(); + if (this.className !== '_User') { - return; + return promise; } - var promise = Promise.resolve(); - if (this.query) { // If we're updating a _User object, we need to clear out the cache for that user. Find all their // session tokens, and remove them from the cache. @@ -367,7 +367,7 @@ RestWrite.prototype.transformUser = function() { return promise.then(() => { // Transform the password if (!this.data.password) { - return; + return Promise.resolve(); } if (this.query && !this.auth.isMaster) { @@ -476,16 +476,16 @@ RestWrite.prototype._validatePasswordRequirements = function() { RestWrite.prototype._validatePasswordHistory = function() { // check whether password is repeating from specified history - if (this.query && this.config.passwordPolicy.passwordHistory) { - return this.config.database.find('_User', {objectId: this.objectId()}, {keys: ["_old_passwords", "_hashed_password"]}) + if (this.query && this.config.passwordPolicy.maxPasswordHistory) { + return this.config.database.find('_User', {objectId: this.objectId()}, {keys: ["_password_history", "_hashed_password"]}) .then(results => { if (results.length != 1) { throw undefined; } const user = results[0]; let oldPasswords = []; - if (user._old_passwords) - oldPasswords = _.take(user._old_passwords, this.config.passwordPolicy.passwordHistory - 1); + if (user._password_history) + oldPasswords = _.take(user._password_history, this.config.passwordPolicy.maxPasswordHistory - 1); oldPasswords.push(user.password); const newPassword = this.data.password; // compare the new password hash with all old password hashes @@ -501,7 +501,7 @@ RestWrite.prototype._validatePasswordHistory = function() { return Promise.resolve(); }).catch(err => { if (err === "REPEAT_PASSWORD") // a match was found - return Promise.reject(new Parse.Error(Parse.Error.VALIDATION_ERROR, `New password should not be the same as last ${this.config.passwordPolicy.passwordHistory} passwords.`)); + return Promise.reject(new Parse.Error(Parse.Error.VALIDATION_ERROR, `New password should not be the same as last ${this.config.passwordPolicy.maxPasswordHistory} passwords.`)); throw err; }); }); @@ -902,22 +902,22 @@ RestWrite.prototype.runDatabaseOperation = function() { let defer = Promise.resolve(); // if password history is enabled then save the current password to history - if (this.className === '_User' && this.data._hashed_password && this.config.passwordPolicy && this.config.passwordPolicy.passwordHistory) { - defer = this.config.database.find('_User', {objectId: this.objectId()}, {keys: ["_old_passwords", "_hashed_password"]}).then(results => { + if (this.className === '_User' && this.data._hashed_password && this.config.passwordPolicy && this.config.passwordPolicy.maxPasswordHistory) { + defer = this.config.database.find('_User', {objectId: this.objectId()}, {keys: ["_password_history", "_hashed_password"]}).then(results => { if (results.length != 1) { throw undefined; } const user = results[0]; let oldPasswords = []; - if (user._old_passwords) { - oldPasswords = _.take(user._old_passwords, this.config.passwordPolicy.passwordHistory); + if (user._password_history) { + oldPasswords = _.take(user._password_history, this.config.passwordPolicy.maxPasswordHistory); } //n-1 passwords go into history including last password - while (oldPasswords.length > this.config.passwordPolicy.passwordHistory - 2) { + while (oldPasswords.length > this.config.passwordPolicy.maxPasswordHistory - 2) { oldPasswords.shift(); } oldPasswords.push(user.password); - this.data._old_passwords = oldPasswords; + this.data._password_history = oldPasswords; }); }