I’m very excited about the new accounts-2fa package, so i upgraded to meteor 2.7 and tried it out.
I got the qrcode working and enabled with and the appropriate user.service.twoFactorAuthentication.type set to otp. Accounts.has2faEnabled tells me it’s enabled for that user.
But when i just do a Meteor.loginWithPassword() i get no error and just logged in.
I expected a no-2fa-code error, as mentioned in the docs.
Meteor.loginWithPasswordAnd2faCode() does the same thing, no matter what the code is.
Is it possible for you to provide a repository on GitHub reproducing this scenario? If this is really happening, it’s a bug, but I couldn’t reproduce it.
I can try to reduce my project to the minimum and create a repo if it still happens.
I really wouldn’t see which other package could interfere with that core login function though.
While i do that… here is the full list of packages i use… maybe it’s something obvious?
The problem is that the users you’re creating don’t have an email property in the first level of the user object. So in this point the query Meteor.users({email:"something"}) doesn’t return a user.
Thank you so much for looking into this.
Glad it was an actual bug and not my mind going crazy
And thank you for solving this (kind of major) security flaw swiftly
But am i reading it right that, either i have to create the first level email property or i won’t be able to use the email for login? I seem to remember the emails array was the default meteor way(?), and i don’t see a nice way of getting the username if they didn’t even log in yet.
Given how critical this package is for security, I think a timeout and a proper code review would be in order here. Since people at MDG are currently changing, it probbaly did not receive the attention it would have needed.
@nooitaf you’re correct. It should be {"emails.address":{$in:[ selector ]}}.
We did test it as much as we could, but unfortunately, this slipped away.
We’re adjusting things on this package in this PR. So, for example, this chunk of code doesn’t even exist anymore. The function now uses Meteor.user() instead to Meteor.users.findOne.
There are other changes there, and @zodern is also reviewing this PR with us. His input is helping a lot.
If any of you want to help review and test this PR, feel free to do so, it would be a great help.
The plan was to release a new version today, but we decided to wait until tomorrow so we can get more time to review and test those changes.
As soon as the new release is live, I’ll let you all know.
I also like the extra check on the recreation of the qrcode, that was a little odd indeed.
I’m not sure if some sort of choice for the (account) in the qrcode could be practical?
Either automatically, how it is now, or something custom (optional)?
I might be overthinking this.
In one project we use emails to login, and usernames are there too, and in most cases i would expect the “login” to be used as “account” in the authenticator, which would not be possible now.
I didn’t familiarize myself with enough the node-2fa package, or top in general, but i would assume it’s just a textfield, unrelated to any actual data.
My authenticator (accounts) from other big companies are all over the place. Twitch’s account name is just (Twitch), which obviously is not my username, so i guess there is no real standard. Or twitch chose to not reveal the username, which is not a bad idea either.
It’s just a minor visual thing/feature request for concideration
