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

Accounts.forgotPassword; Error: Did not check() #5676

Closed
ubald opened this Issue Nov 18, 2015 · 6 comments

Comments

Projects
None yet
6 participants
@ubald

ubald commented Nov 18, 2015

When using audit-argument-checks Meteor throws:

Exception while invoking method 'forgotPassword' Error: Did not check() all arguments during call to 'forgotPassword'

when calling Accounts.forgotPassword({email:'...'})

@stubailo

This comment has been minimized.

Contributor

stubailo commented Nov 24, 2015

That's very interesting, since we do in fact check that argument:

Meteor.methods({forgotPassword: function (options) {

Do you have a reproduction of an app where this happens?

@ubald

This comment has been minimized.

ubald commented Nov 25, 2015

I'll try to come up with a sample app, but meanwhile I re-tested and I realized that it's only doing that when called from the console in the browser.

@ubald

This comment has been minimized.

ubald commented Nov 25, 2015

Or not... anyway, I made this simple test using the following packages in addition to the default ones when doing meteor create test

audit-argument-checks
accounts-password

And only this in test.js:

if (Meteor.isClient) {
  Meteor.startup(function () {
    Accounts.forgotPassword({email:'test@test.com'});
  });
}

if (Meteor.isServer) {
  Meteor.startup(function () {
    if ( !Meteor.users.findOne({username:'test'}) ) {
      Accounts.createUser({username:'test', email:'test@test.com'});
    }
  });
}

And when accessing http://localhost:3000/ I see the mail message in the console, but right after it:

Exception while invoking method 'forgotPassword' Error: Did not check() all arguments during call to 'forgotPassword'
at [object Object]._.extend.throwUnlessAllArgumentsHaveBeenChecked (packages/check/match.js:411:1)
at Object.Match._failIfArgumentsAreNotAllChecked (packages/check/match.js:106:1)
at maybeAuditArgumentChecks (livedata_server.js:1695:18)
at livedata_server.js:708:19
at [object Object]._.extend.withValue (packages/meteor/dynamics_nodejs.js:56:1)
at livedata_server.js:706:40
at [object Object]._.extend.withValue (packages/meteor/dynamics_nodejs.js:56:1)
at livedata_server.js:704:46
at tryCallTwo (/Users/ubald/.meteor/packages/promise/.0.5.1.8idxpg++os+web.browser+web.cordova/npm/node_modules/meteor-promise/node_modules/promise/lib/core.js:45:5)
at doResolve (/Users/ubald/.meteor/packages/promise/.0.5.1.8idxpg++os+web.browser+web.cordova/npm/node_modules/meteor-promise/node_modules/promise/lib/core.js:171:13)

@zol zol added bug and removed needs-repro labels May 3, 2016

@hwillson

This comment has been minimized.

Member

hwillson commented Oct 21, 2016

Hi guys - this is still an issue; I've put together a repro showing this here.

I've traced through the code a bit, and can see where the issue is. When the

Accounts.forgotPassword({ email: 'test@test.com' });

call is made without a callback function, the args parameter passed into the maybeAuditArgumentChecks function (in packages/ddp-server/livedata_server.js) looks like

[ { email: 'test@test.com' }, null ]

In the forgotPassword Method (packages/accounts-password/password_server.js) the check being made looks like

check(options, {email: String});

This means the null parameter is missed by a check call, which leads to the Did not check() ... error.

If a callback is added to the Accounts.forgotPassword call like

Accounts.forgotPassword({ email: 'test@test.com' }, () => {
  console.log('Email sent');
});

then the args parameter of the maybeAuditArgumentChecks is correct, looking like:

[ { email: 'test@test.com' } ]

This in turn gets rid of the Did not check() ... error. So adding the callback prevents the error from happening. I haven't had a chance to look into where the extra null is coming from yet, but I'll keep digging.

@hwillson

This comment has been minimized.

Member

hwillson commented Oct 26, 2016

Hi all - I've traced this further. When a callback isn't included in Accounts.forgotPassword, it passes undefined into its Accounts.connection.call("forgotPassword", options, callback); call. This translates into a DDP message that looks like the following (notice the second null element in the params array):

{ 
  msg: 'method',
  method: 'forgotPassword',
  params: [
    { email: 'test@octonary.com' },
    null 
  ],
  id: '1'
}

The params from this message get sent into the maybeAuditArgumentChecks function for verification, then fail because the forgotPassword Method is only checking the options object with the email; it's not expecting a second null parameter.

I believe the easiest fix for this is to avoid making the forgotPassword Method call with an undefined callback value. So we'd change Accounts.forgotPassword to something like:

Accounts.forgotPassword = function(options, callback) {
  if (!options.email) {
    return reportError(new Meteor.Error(400, "Must pass options.email"), callback);
  }
  if (callback) {
    Accounts.connection.call("forgotPassword", options, callback);
  } else {
    Accounts.connection.call("forgotPassword", options);
  }
};

I've submitted PR #7968 to help take care of this. Please fire over any feedback - thanks!

hwillson added a commit to hwillson/meteor that referenced this issue Nov 1, 2016

@tmeasday tmeasday closed this Nov 8, 2016

@tmeasday

This comment has been minimized.

Contributor

tmeasday commented Nov 8, 2016

#7968 is merged!

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