From d0a49416680d52e122275cc84133f45fe352cfb0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Miroslav=20Bajto=C5=A1?= Date: Mon, 9 Oct 2017 13:30:28 +0200 Subject: [PATCH] Fix handling of user verification options - Fix `User.prototype.verify` to not modify properties of the supplied `verifyOptions` argument. This is needed to allow callers to supply the same options object to multiple calls of `verify`. - Fix `User.getVerifyOptions` to always return a new copy of the options object. This is needed to allow callers to modify the returned options object without affecting the result returned by subsequent calls of `getVerifyOptions`. --- common/models/user.js | 10 +++++++--- test/user.test.js | 28 ++++++++++++++++++++++++++++ 2 files changed, 35 insertions(+), 3 deletions(-) diff --git a/common/models/user.js b/common/models/user.js index c582f4b8..3e4cafc7 100644 --- a/common/models/user.js +++ b/common/models/user.js @@ -603,11 +603,11 @@ module.exports = function(User) { */ User.getVerifyOptions = function() { - const verifyOptions = { + const defaultOptions = { type: 'email', from: 'noreply@example.com', }; - return this.settings.verifyOptions || verifyOptions; + return Object.assign({}, this.settings.verifyOptions || defaultOptions); }; /** @@ -699,11 +699,15 @@ module.exports = function(User) { var user = this; var userModel = this.constructor; var registry = userModel.registry; - + verifyOptions = Object.assign({}, verifyOptions); // final assertion is performed once all options are assigned assert(typeof verifyOptions === 'object', 'verifyOptions object param required when calling user.verify()'); + // Shallow-clone the options object so that we don't override + // the global default options object + verifyOptions = Object.assign({}, verifyOptions); + // Set a default template generation function if none provided verifyOptions.templateFn = verifyOptions.templateFn || createVerificationEmailBody; diff --git a/test/user.test.js b/test/user.test.js index acfb50c8..91c5d1ed 100644 --- a/test/user.test.js +++ b/test/user.test.js @@ -1606,6 +1606,34 @@ describe('User', function() { done(); }); + it('returns same verifyOptions after verify user model', () => { + const defaultOptions = { + type: 'email', + from: 'test@example.com', + }; + var verifyOptions = Object.assign({}, defaultOptions); + const user = new User({ + email: 'example@example.com', + password: 'pass', + verificationToken: 'example-token', + }); + return user + .verify(verifyOptions) + .then(res => expect(verifyOptions).to.eql(defaultOptions)); + }); + + it('getVerifyOptions() always returns the same', () => { + const defaultOptions = { + type: 'email', + from: 'test@example.com', + }; + User.settings.verifyOptions = Object.assign({}, defaultOptions); + var verifyOptions = User.getVerifyOptions(); + verifyOptions.from = 'newTest@example.com'; + verifyOptions.test = 'test'; + expect(User.getVerifyOptions()).to.eql(defaultOptions); + }); + it('can be extended by user', function(done) { User.getVerifyOptions = function() { const base = User.base.getVerifyOptions();