-
-
Notifications
You must be signed in to change notification settings - Fork 217
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
[Feature] TOTP 2FA support #2195
base: master
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There are accessibility issues in these changes.
@jozip Welcome, and thanks for stepping up. Here is my initial feedback on the code you've written so far:
|
I found a small problem in This branch should be updated to use the 2.8.0 version of pyotp. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👏 You fixed the issue(s)! Great work.
Synopsis
This PR implements rudimentary support for Time-Based One-Time Password (TOTP) Two-Factor Authentication (2FA) that is compatible with apps such as Google Authenticator, FreeOTP and KeePass.
Rationale
Using a second factor for login greatly improves security. HOTP and TOTP are common and well-known schemes, where the latter is more widely used.
Overview
The user can enable two-factor authentication by clicking a button under the 2FA-heading in the account settings page.
This generates a TOTP-token and presents a QR-code that the user can scan using their preferred OTP-app. The setup is enabled once the user inputs a correct TOTP-code within the valid time-frame into.
The user can then use two-factor authentication when logging in using a password or via email.
Disabling 2FA is simply done by clicking the corresponding button in the account settings page.
Implementation details
I've tried to be as unintrusive as I can regarding the UX and the authentication flow, since I'm unfamiliar with the prior design considerations and I want to minimize the risk of messing up such a critical part of the system. Therefore some things may appear a bit more roundabout than they necessarily should've been.
The data related to the 2FA-logic resides in the
participants
table, since it needs to be accessible regardless of authentication method, it's not terribly secret (in that it's just a random nonce and it leaking mean little outside of this context) and I couldn't figure out ifuser_secrets
really was a suitable place.The core logic resides in the
Participants
-model with a few incisions in the authentication logic. I opted to add a couple of simplates to make enabling 2FA a bit easier, rather than trying to jack intowww/%username/settings/edit.spt
.Adding an additional, optional field for the password login was easy enough, since it's a form that is
POST
:ed. The email login flow is via aGET
, which prompted the creation of the odd javascript snippet in order to feed it an additional querystring parameter for the TOTP-code. (Again, I didn't want to up-heave the current logic with my limited knowledge.)Dependencies
The following new dependencies have been added:
pyotp==2.7.0
PyQRCode==1.2.1
pypng==0.20220715.0
A cursory glance of each library's source doesn't raise any red flags.
pip-audit
doesn't report any known vulnerabilities either.Remaining work
Participant