Closed babolivier closed 8 years ago
@babolivier Thank you for this second step. If you can add this in a doc folder, it would be nice. If you can post it on the forum as a tech topic, it would be nice too.
@frankrousseau Sure thing, will do as soon as I have a stable codebase.
After discussing with @m4dz, I changed the way the QR Code is being generated. Instead a heavy Node module ran server-side, we now generate the QR Code client-side via qr.js, which fixes the problem with the tests not running, and allowed me to remove a couple of functions and a server route.
I've also added the doc in a separate .md
file.
@frankrousseau, can you please tell me if there's still anything that troubles you?
Yes the WIP tag. Should we still consider it's a work in progress PR or can we proceed to the functional review?
I think we faced a potential security leak with the secret key: according to the RFC, it should be stored in a cryptographic manner and be deciphered only to check token validity, then re-encrypted immediately after to avoid RAM compromission.
I suggest we store the key the same manner than we do for other sensitive data: using the user password. This way, we decipher the key to check the token using the prompted password at auth, then flush the deciphered password immediately.
@babolivier, any though about this strategy?
@frankrousseau The WIP tag is still here only because I haven't added unit tests yet. All the rest can be reviewed. But if you prefer to wait for them in order to review all at once, I'm fine with this.
@m4dz I must admit I have troubles visualizing the concrete implementation, and I think we should have a talk about that. However, I always agree with the highest security :)
@babolivier are you still working on it? Do you want we finish the job?
Hi @frankrousseau,
I've just put this on stand by until the PR on the DS is merged, in order to give some time to my non-profit. But I didn't give up the project and will get back on it soon.
Ok I merged the PR you mentioned. Thank you again for what you do for the security of Cozy!
Pleasure's all mine! I got the notification about the DS PR. I'll get back on this one tonight (if possible, if not, tomorrow), and hopefully will have everything working here before the end of the week.
@frankrousseau Finally, I consider the PR complete and ready, tests are written and green, and have removed the WIP tag! Ready for the final review!
Activation and configuration panel for the two-factors authentication. Directly follows up https://github.com/cozy/cozy-proxy/pull/260.
This pull request adds a panel to the Settings page under the password change one. The user selects the authentication strategy (algorithm) of its choice between the available ones (only HOTP and TOTP for now), then clicks the button. The server will then edit the current user in the DS: it will generate an OTP key (10-char-long random string) and will fill the auth type accordingly ("hotp" or "totp").
Once the user has clicked the button, the page will display a success message and reload after 2s. The Settings page now displays this as the 2FA panel:
When the home loads, it checks wether 2FA is enabled to know which panel it has to display. It also retrieve the 2FA token the user will have to enter into its app or device, as long as the corresponding QR code.
The configuration panel for HOTP is a bit different. Indeed, there's an issue we can have with the fact that we store the highest counter encountered until now in order to invalidate any previously used authentication code: when the user switches from authentication app or device to another, its generation will start all the way back to the very first code, with counter 0 (because the new app or device has no way to know the counter we have in the DS nor the corresponding code), which means that, if an user used more than a thousand codes on the old app/device, it'll have to generate as much codes to be able to log back into Cozy. To address that matter, I've added a button that only shows when HOTP is enabled, which resets the HOTP counter: (We can also see in the last two screenshots that the OTP key is regenerated each time we disable/enable back 2FA)
At the time of this writing, I have the process fully working. I now have to:
Once all of this is done, I'll remove the [WIP] tag.
If you have any question, or anything to say about the PR, what it brings, and the process, please let me know.
Also, @frankrousseau, do you want me to copy all of the above explanation in a doc folder like I did with the proxy?
Edit: Travis is failing, but it doesn't seem that I caused it (the error comes up on the
npm install
command), can I have a hint on that?