-
-
Notifications
You must be signed in to change notification settings - Fork 266
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Added TOTP MFA support per user. #756
Conversation
Added the option to enable/disable TOTP MFA per user using QR code or manually entering a key.
Thanks for the PR. I will test it in my local dev first before merge it. |
Thanks for reviewing it! Also, since this adds new functionality is it possible for other contributors to add to the documentation on https://docs.htmly.com/ or possibly adding the Wiki feature to this GitHub repo so we can build out more documentation as new things like this get added? Thanks again! |
The |
Secret Key is in hash/encrypted format in user.ini or plain text? If the latter, then it needs to be secured. Also, I will look into the wiki/docs in a few days time and add with proper snapshots. Edit: I remember docs/wiki pages on Github. Unable to locate those now. Odd. @danpros please provide the code to add/update the guides. |
@vdbhb59 this still in master branch and not yet released. And the development still ongoing so I will add the docs later after we tidy it up. |
Also, why suggest only the foogle 2fa, why not some opensource app like FreeOTP+ from F-Droid? |
I can only suggest the things I know, never heard of that one. Feel free to use it as well. |
I think a good solution would be to encrypt the secret with the user's password so it's unencrypted at login with that password. I should be able to get this feature updated to make sure it's stored encrypted in the user.ini file. |
I know, just thought that with time people would have realized the demon foogle is. :( |
You don't have to use Google, you can use any auth app to scan the QR code or enter the code manually. They all work with that library. You don't even need a Google account to implement it. I just picked that library because it's still being maintained (not by Google) and was easy to use. |
No no. I know that. I was just trying to refer to the suggestion part of it. Nothing otherwise. I use FreeOTP+ and KeepassDX which makes it best solution for me. :) |
@KuJoe Yes we should secure the secret key. And you can remove the verify password actually. I try blank or different password and the password also changed. Remove or use During login we missing the variable $message (error message), try logout and use random MFA code. This should fix the error: $message['error'] = '';
$message['error'] .= '<li class="alert alert-danger">' . i18n('MFA_Error') . '</li>'; Thank you for this feature. |
Reading a bunch of MFA article whether the MFA secret should encrypted or not.. its fine we leave the MFA secret as is. The MFA is for second line of defense, usually against brute force. So we should not use the user password to encrypt it because it the hacker can decrypt it than they can get the user password. Edit: except we use another password/seed to encrypt it of course. |
Ideally any 2FA/MFA is separate from the 1FA/OFA. Also, it would be beneficial to encrypt 1FA/OFA separately in itself. Probably by 256bits. |
Doesn't the MFA secret need to be accessible to the MFA library to check the TOTP code? If you encrypt it, where will you store the decryption key? |
@KuJoe docs added to github: https://github.com/danpros/htmly-docs |
This PR adds the ability to enabled and disable TOTP MFA for individual users. You can add the MFA token by scanning a QR code or manually typing in the secret key. The secret key is saved in the user.ini file and can be reset by changing the value to "disabled". This can probably be done more cleanly but I wanted to implement it without having to rewrite too many core modules or functions.
The libraries used are "pragmarx/google2fa" and "bacon/bacon-qr-code", but they do require PHP 8.0 or higher to work properly making all 5.x and 7.x PHP versions incompatible.
Additionally the php-gd module is required to generate the QR code.
Should resolve issues #589 and #449