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

Conflict on batch insert users when adding addVerification hook #16

Closed
letminin opened this issue Mar 23, 2017 · 2 comments
Closed

Conflict on batch insert users when adding addVerification hook #16

letminin opened this issue Mar 23, 2017 · 2 comments

Comments

@letminin
Copy link

letminin commented Mar 23, 2017

This issue is the produce of a brief talk on the feathers slack help channel with @eddyystop, the problem appear on the hook.data param, as described on the documentation, a service should have a data param considered as a single object or an array, when using the service as a batch service like :
app.service('users').create([userobject1, userobject2])

The hook addVerification will not work in this case, it will simply add the token properties after the array, we need to correlate what we already have in hashPassword hook in feathers-authentication like :


if (Array.isArray(hook.data)) {
      // make sure we actually have password fields
      var dataToCheck = [].concat(hook.data);
      dataToCheck.filter(function (item) {
        return item.hasOwnProperty(options.passwordField);
      });
      if (dataToCheck.length > 0) {
        // set it to the array so we can iterate later on it
        password = hook.data;
      }
    } else {
      password = hook.data[options.passwordField];
    }

Instead we got :

module.exports.addVerification = function (path) {
  return function (hook) {

    utils.checkContext(hook, 'before', 'create');

    return Promise.resolve().then(function () {
      return hook.app.service(path || 'authManagement').create({ action: 'options' });
    }).then(function (options) {
      return Promise.all([options, getLongToken(options.longTokenLen), getShortToken(options.shortTokenLen, options.shortTokenDigits)]);
    }).then(function (_ref) {
      var _ref2 = _slicedToArray(_ref, 3),
          options = _ref2[0],
          longToken = _ref2[1],
          shortToken = _ref2[2];

      hook.data.isVerified = false;
      hook.data.verifyExpires = Date.now() + options.delay;
      hook.data.verifyToken = longToken;
      hook.data.verifyShortToken = shortToken;
      hook.data.verifyChanges = {};

      return hook;
    }).catch(function (err) {
      throw new errors.GeneralError(err);
    });
  };
};`
`

Expected behavior would be to test hook.data if it's an array, and iterate on it.
@beeplin
Copy link

beeplin commented Mar 23, 2017

yes I have encountered this bug too. Now I just create single user once a time as a workaround.

@eddyystop
Copy link
Collaborator

This issue has been fixed in the a-l-m rewrite. Complete details at https://github.com/feathers-plus/authentication-local-management/blob/master/misc/upgrading.md

Please add any comments in a-l-m.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants