-
-
Notifications
You must be signed in to change notification settings - Fork 4.8k
New issue
Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? # to your account
New SMS MFA Auth Adapter not verifying setup token #8785
Comments
Thanks for opening this issue!
|
I think I've found the issue @mtrezza The class MFAAdapter extends AuthAdapter For the file mfa.js But when looking at the AuthAdapter it extends we can see that
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
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
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 # token
Steps to reproduce
Setup mfa auth adapter. e.g.
Perform initial # 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
The text was updated successfully, but these errors were encountered: