Accounts.validateLoginAttempt woes

help
1

So I’m using Accounts.validateLoginAttempt() in order to manually allow certain login attempts to be successful, if the password matches a “super user one-time-password”… Now I’ve got all of the code in there and seemingly working ok; when the OTP is detected, the validateLoginAttempt function returns a true value.

However, on the client, the Meteor.loginWithPassword(username, password, (error) => {}); function still gets an error in the callback stating that the password was invalid! Is there any way to override this!? Or have I missed something!?

2

I can see that there’s a lot of code here: https://github.com/meteor/meteor/blob/master/packages/accounts-base/accounts_server.js#L138-L162

That’s purpose seems to be ensuring that previous failed login attempt’s error messages (such as the initial password check) don’t get overridden, but in this case that’s EXACTLY what I need to do… :confused:

3

In fact… the cloneAttemptWithConnection code basically makes it impossible to override an already invalid attempt… Damn.

4

So, it is impossible? I’m trying similar thing and having the same issue.

5

If you create a local copy of the accounts-base package, change the clone function to just create the expected object and NOT clone it… Then in the validate function you can set error to undefined and valid to true, which will override the old values.

1 Like
6

Ignore that… seems it’s not quite that simple as you’ll start getting Exception while invoking method 'login' Error: setUserId must be called on string or null, not undefined errors.

This is because the initial login attempt won’t return a userId IF there is an error… :confused:

EDIT: This isn’t as bad as it first seemed, it’ll only error if there’s a “bigger” issue… ie. the user has no password set at all, etc.

7

I think this package intentionally doesn’t allow overrides to make the login less secure, which is kind of what you are trying to do.

I think you need to write a new login handler - if you look at the way accounts-password is implemented (and the other accounts packages) you can basically just write a method that sets the userId on the connection. That would be much better than hacking the existing package which wasn’t designed to do this.

1 Like
8

Did you try to override Accounts._checkPassword from accounts-password package? It seems to me that this is a better place to look to achieve what you want.

Edit: Or better yet, as @sashko proposed, use Accounts.registerLoginHandler to create your own handler.

Just make sure you test the hell out of this code, because one simple mistake here can lead to nasty security holes!

1 Like
9

@M4v3R is there a simple way I can just override this function (Accounts._checkPassword) without having to clone the whole package locally and changing it?

10

Well you could try to monkey-patch it since everything can be overridden in Javascript, so if you just do the following in your server code (preferably in the startup block I guess) it should work.

Accounts._checkPassword = function(...) {
...
}
```

But this is not really recommended, as when the package gets updated when you update the whole app this can easily brake without you knowing it.
11

I tried that, I put:

const originalCheckPassword = Accounts._checkPassword;
Accounts._checkPassword = (user, password) => {
  const result = originalCheckPassword(user, password);
  if (!result.error) return result;

  const hashedPassword = 'something to check for';

  // If the password matches...
  if (hashedPassword === password) {
    // Remove any error's as the login is fine!
    delete result.error;
  }
  return result;
};

But it didn’t ever get fired… :confused:

12

Ah, my bad, that won’t work because there’s actually a safeguard in the accounts-password package code against that. So I guess the only thing you can (and should in your case) do is to clone the package locally and modify the code there.

13

Ahh good spot. Darn…

Cloning seems to be the way to go then!