[SOLVED] Accounts-2fa doesn't seem to block me from login?

Hey all!

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.

Am i missing something?

accounts-2fa@1.0.0
accounts-base@2.2.2
accounts-password@2.3.0

Hi @nooitaf,

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?

meteor-base@1.5.1
mobile-experience@1.1.0
mongo@1.14.6                
blaze-html-templates    
jquery                  
reactive-var@1.0.11
tracker@1.2.0          

standard-minifier-css@1.8.0  
standard-minifier-js@2.8.0   
es5-shim@4.8.0                
ecmascript@0.16.2           
typescript@4.5.4             
shell-server@0.5.0          

hot-module-replacement@0.5.1 
blaze-hot               

markdown
email@2.2.1
session@1.2.0
underscore@1.0.10

useraccounts:core
useraccounts:flow-routing-extra
useraccounts:unstyled

aldeed:template-extension
aldeed:autoform
aldeed:collection2
communitypackages:autoform-plain

accounts-2fa
accounts-password@2.3.0
accounts-ui-unstyled@1.7.0

kadira:blaze-layout

ostrio:files
ostrio:autoform-files
ostrio:flow-router-title
ostrio:flow-router-extra

momentjs:moment
mizzao:user-status
pfafman:filesaver

Everything seems to be correct with the versions from what I’m looking at. It must be something in the code.

I created a very minimal version of this and it’s still happening. I really would not know what is having any say over loginWithPassword, lol

1 Like

Hi @nooitaf,

Thank you very much for your reproduction. It helped a lot.

I narrowed down the issue and found a solution.

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.

The right thing to do there is to throw an error when the user is not found, which I’m doing now on this PR. But also, there are other improvements to this case that will be released here Release 2.7.1 by denihs · Pull Request #11989 · meteor/meteor · GitHub.

So with version 2.7.1, you’ll not need to worry about adding the email to the first level.

We’ll work to release this new version tomorrow. For now, you can use the username instead of the email so that you can get the correct error message.

3 Likes

Thank you so much for looking into this.
Glad it was an actual bug and not my mind going crazy :smiley:

And thank you for solving this (kind of major) security flaw swiftly :wink:

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.

Wouldn’t the correct selector be…

{"emails.address":{$in:[ selector ]}}

instead of…

{email: selector}

a few lines above your fix here ?

This makes me wonder now if the package was even tested before release. :interrobang:

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.

5 Likes

Hey guys, thank you for all your concerns.

@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.

Thank you again!

7 Likes

Nice! You’re welcome and thank you too!

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 :wink:

I just checked my authenticator and large majority of the services use the format:

<App Name> : <Account reference>

e.g. Google:me@email.com

Hey @nooitaf, version 2.7.1 is out.

You can go ahead and update your Meteor version with meteor update.

The issue should be solved now.

2 Likes

Is there a link for the docs for the new 2FA API?

Yep, here you can see the 2FA Documentation and there is also a blog post explaining how to use it.

2 Likes