Open magnacartatron opened 10 months ago
I think I've found the issue @mtrezza
The class MFAAdapter extends AuthAdapter
For the file mfa.js async validateUpdate(authData, _, req) {
the second parameter is _ and the third parameter is the request
But when looking at the AuthAdapter it extends we can see that
validateUpdate(authData, req, options) { return Promise.resolve({}); }
The second parameter is the request and the third parameter is the options.
So this particular function in the mfa.js file the request is all out of whack
async validateUpdate(authData, _, req) {
if (req.master) {
return;
}
if (authData.mobile && this.sms) {
if (!authData.token) {
throw 'MFA is already set up on this account';
}
return this.confirmSMSOTP(authData, req.original.get('authData')?.mfa || {});
}
if (this.totp) {
await this.validateLogin({ token: authData.old }, null, req);
return this.validateSetUp(authData);
}
throw 'Invalid MFA data';
}
Same applies for the validateLogin functions, looks like the parameters our out of order.
@mtrezza Actually the issue above is something else altogether.
I know what the problem is, I was using the masterKey instead of the sessionToken.
The issue is that if you don't pass in any param for token the error you get back is"MFA is already set up on this account" when really you should get back something like "token is missing"
Another issue is that if the initial token expires for setting up SMS on the account, then there's no way to send another token without first unlinking the MFA and then sending it again.
Interesting, would you mind opening a PR to improve that?
No problem.
Changed this from bug to feature improvement.
Hi sorry, about the delay I'll try again.
The issue is this. If you're registered and you type in incorrect token you get an error saying "Invalid MFA token 1" and if it's expired you get an error saying "Invalid MFA token 2". It's easy to distinguish between these two and take appropriate action like send another token etc.
However if you've just added the MFA adapter and you pass in the wrong token or the token has expired the only error you get back is "Invalid MFA token".
The issue is this 1) I get the same error wether it's the incorrect token or the token has expired 2) There's no way to send another token unless you unlink and relink MFA adapter
This means that if they get the token incorrect, my only course of action is to unlink and relink and send another token, so now they're getting bombarded with messages every time they get their token wrong.
If there was a way to differentiate between incorrect token and expired token it would make this a lot easier.
Not sure why the same error messages aren't applied on linking MFA as they are once it's in use.
One more thing to note is that the pending object used to clear out but now it stays but it's empty e.g. pending: {}
If you want I can fix this it would take me a couple of hours with testing.
@mtrezza I'd be more than happy to work on documentation and type fixes and general bugs. I've been working with Parse since 2015 and had to go deep in the last 2 years to get past some of the idiosyncrasies of the inner workings. We have a platform with over 100,000 users using parse and have come into just about every problem you can imagine. I have to document most of this stuff for my development team anyway so this would be easier. Can I just read the CONTRIBUTE.md file and go from there? Do you need assistance with all the bugs?
Sure we'll be happy to review your PRs.
New Issue Checklist
Issue Description
MFA sms authentication method ignores initial signup token
Steps to reproduce
Setup mfa auth adapter. e.g.
Perform initial signup and link to mfa
The log should produce
{ mfa: { status: 'disabled' } }
On the server in the logs you should see something like this
SMSCODE: 211392 NUMBER: +11111111111
If you had an SMS service linked this code would be send to your mobile.
Issue arises when you have to verify this code. You have to verify this code otherwise the adapter status will remain disabled.
Looking at spec file in order to do this you need to run
await user.save({ authData: { mfa: { mobile: '+11111111111', token: '848722' } } });
The issue is that token there, can be anything, you can put anything in then token field and the mfa authData will be set to enabled. There's no verification at this step. So if I get an SMS with 123456 for the token, and I put in the text 'donkey' in there it will still work.After doing this the Auth Adapter works as it should, you have to use 'request' for token and then you have to use the actual token to login. It's just that initial enabling of SMS auth that seems to be broken.
Actual Outcome
The setup token for validation isn't verified before enabling SMS mfa for user.
Expected Outcome
For the token to be verified at this step.
Environment
NodeJS: 18.6.1
Server
6.4.0-alpha.6
MacOS 14.0
Local
Database
6.0
Client
JavaScript SDK
4.3.0-alpha.4
Logs