Skip to content

Commit

Permalink
Fix handling of user verification options
Browse files Browse the repository at this point in the history
 - 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`.
  • Loading branch information
bajtos committed Oct 9, 2017
1 parent 8488da2 commit d0a4941
Show file tree
Hide file tree
Showing 2 changed files with 35 additions and 3 deletions.
10 changes: 7 additions & 3 deletions common/models/user.js
Original file line number Diff line number Diff line change
Expand Up @@ -603,11 +603,11 @@ module.exports = function(User) {
*/

User.getVerifyOptions = function() {
const verifyOptions = {
const defaultOptions = {
type: 'email',
from: '[email protected]',
};
return this.settings.verifyOptions || verifyOptions;
return Object.assign({}, this.settings.verifyOptions || defaultOptions);
};

/**
Expand Down Expand Up @@ -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;

Expand Down
28 changes: 28 additions & 0 deletions test/user.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -1606,6 +1606,34 @@ describe('User', function() {
done();
});

it('returns same verifyOptions after verify user model', () => {
const defaultOptions = {
type: 'email',
from: '[email protected]',
};
var verifyOptions = Object.assign({}, defaultOptions);
const user = new User({
email: '[email protected]',
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: '[email protected]',
};
User.settings.verifyOptions = Object.assign({}, defaultOptions);
var verifyOptions = User.getVerifyOptions();
verifyOptions.from = '[email protected]';
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();
Expand Down

0 comments on commit d0a4941

Please sign in to comment.