Closed babolivier closed 8 years ago
@babolivier Thank you for that awesome pull request. Can you add explanation about what you did, how does it work and what it brings to the platform?
Sure thing @frankrousseau. Sorry if my initial post wasn't explicit enough (actually it wasn't explicit at all).
As said above, this PR contains support for two-factors authentication in Cozy's authentication proxy.
The proxy can now authenticate using the HOTP (HMAC-based One-Time Password, or OTP with a counter) and TOTP (Timer-based One-Time Password, or OTP with a timer) algorithms through the corresponding Passport strategies in the passport_configurator.coffee
file right here.
Quick look at the algorithms and the corresponding strategies:
In the future, the user will be able to chose which algorithme to use via a configuration panel within the Cozy home.
In order to store everything we need in the database, I've added three fields to the User
doctype:
otpKey
is the key (sort of a master password) we'll pass to the chosen Passport strategy. It's a 10 characters-long randomly-generated alphanumeric string, which generations rules don't differ from HOTP to TOTP (and vice-versa), so we don't have to have two "key" fields. Note that the key isn't the longer and all-capitalized code you enter in your 2FA app/device, which is a base32 encoding of the key, so we don't need to store it.hotpCounter
is only used when using the HOTP algorithm, to store the counter. This allows us to check wether the user enters a token with a lower counter than the last recorded one (which will mean that the entered token may have already been used) and invalidate it. It's also a necessary field required by the Passport strategy.authType
stores the selected algorithms. It can be null
or even undefined
if the user hasn't enabled 2FA (or has disabled it), which will be understood as no 2FA authentication, or have a value corresponding to the selected Passport strategy (right now, only "hotp" and "totp" won't trigger a Passport error as only these ones are implemented).The authentication proceeds as such:
authType
is null or undefined, we redirect the user to the Cozy home (usual authentication)authType
has a defined value, we proceed to the corresponding authentication (HOTP or TOTP)The authType
also allows one really useful feature: When we generate and send the login page to the user, we can modulate it according to wether or not OTP-based 2FA is enabled. This way, if this feature is disabled, the user will see the usual login page:
But if the OTP-based 2FA feature is enabled (on any of the two algorithms), the user will get this login page:
I've also added a few translations (mainly for OTP-related errors) and edited this error in order for it to be accurate in both simple password authentication and 2FA.
If you have any feedback, remark or question, I'd be more than happy to read and answer them.
Another question. I don't understand how the use get its authentication code. How does he retrieve it?
Via an app such as Authy or Google Authenticator. These apps will process the base32 of the master key (which is the string we'll give to the user in the configuration panel) and output a 6 digits-long token for a limited time that the user will have to enter in the "Authentication code" field. If the chosen algorithm is HOTP, he can also retrieve the token through a dedicated device such as a Yubikey. I also see discussions about adding a token generation feature in the Cozy Android app, which could be great to make switching to 2FA easier.
To put this in perspective, it's the same process as the Steam Authenticator or Battle.net Authenticator apps (except these two are for specific services). Given the right information (in our case, the base32 of the master key), the app will give us a temporary token to enter.
Ok thank you a lot for the explanation. I would like to ask you two more things:
.md
file located in doc
folder at the root of the repository. Something similar to: https://github.com/cozy/cozy-controller/tree/master/docIs it ok for you?
Absolutely. I've just added the .md
file to my pull request, and will start workin on tests tonight.
Hi @frankrousseau,
Tests have been added (Travis's displaying them all green, whoohoo!), I've had occasions to try it myself and fix a few things, add comments where I thought them necessary and add a doc, I think the PR is now ready from here! We still need a configuration panel in the home, but that'll come after. (removing the "WIP" tag then)
Thanks a lot @babolivier for this awesome work :tada:!
@frankrousseau it' ok for me, ready to merge!
:tada: :tada: :tada:
My pleasure :smile:
@m4dz I will review it before merging. Thank you for the additional review.
OK Can you make sure that all lines are less than 80 chars wide?
@frankrousseau I made the requested edits, and no line to my knowledge is wider than 80 char. Let me know if you have any more feedback on the commit.
Thank you @babolivier for this beautiful PR. We must now run into the functional testing. We'll provide you feedback soon.
NB: Sorry for all that checking but auth is something we can't do wrong.
No problem @frankrousseau. Actually, would seem weird if changes to auth didn't require a lot of checkout before rolling out to production. And I also want this feature to be as stable and working as possible!
@babolivier : when login is wrong, is it possible to differentiate password error and auth code error ?
@poupotte Well technically we could, but I already looked into it, and, front-end, the proxy always assumes that if there's an error on the login page it's due to the password being wrong. So it would be possible, but we would have to change the whole error management on this view, and making that change wasn't a priority to me.
@poupotte @m4dz can we merge it?
It's fine for me \o/
@m4dz Did you see the error display issue. Do you think it can be easily fixed?
I saw the issue, I think we can easily parse an error response for the server, but I ask myself about two points:
Any though about it?
Ok let's merge it. @poupotte will tell if an improvement is required.
Congrats @babolivier for this awesome PR!
:tada: :tada:
Thanks! :smile: Configuration in cozy-home should have its PR open in a few days at last. :wink:
Here is the 2FA (2-factors authentication) support for the Cozy proxy. Probably needs polishing, and doesn't have a single test, that's the reason for the WIP tag.
For the 2FA support to be complete in Cozy, I consider that we still need to have:
I'll hopefully be working on both in the next few days.
When complete, will fix #123.