-
-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
Implement UI for 2FA setup and verification #3541
Merged
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
zoldar
added
the
deploy-to-staging
Special label that triggers a deploy to a staging environment
label
Nov 21, 2023
zoldar
force-pushed
the
feat/2fa-final
branch
11 times, most recently
from
November 28, 2023 08:37
8b844a4
to
8ef2857
Compare
8 tasks
zoldar
force-pushed
the
feat/2fa-final
branch
from
November 28, 2023 09:18
8ef2857
to
9ab5259
Compare
zoldar
force-pushed
the
feat/2fa-final
branch
5 times, most recently
from
November 29, 2023 12:55
b155e63
to
e86dc1e
Compare
zoldar
changed the title
[WIP] Implement UI for 2FA setup and verification
Implement UI for 2FA setup and verification
Nov 29, 2023
zoldar
force-pushed
the
feat/2fa-final
branch
2 times, most recently
from
November 30, 2023 08:11
8ca7763
to
7a69af4
Compare
zoldar
force-pushed
the
feat/2fa-final
branch
from
November 30, 2023 09:05
7a69af4
to
c533973
Compare
Ouch, there's some conflicts to be dealt with |
aerosol
reviewed
Dec 4, 2023
zoldar
force-pushed
the
feat/2fa-final
branch
from
December 5, 2023 11:55
9690c50
to
8755eb1
Compare
Co-authored-by: hq1 <hq@mtod.org>
Co-authored-by: hq1 <hq@mtod.org>
zoldar
force-pushed
the
feat/2fa-final
branch
from
December 5, 2023 11:58
8755eb1
to
1e7f08f
Compare
ukutaht
approved these changes
Dec 5, 2023
ukutaht
reviewed
Dec 5, 2023
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Changes
This PR implements controller level logic and UI views for 2FA.
The setting section is hidden behind a feature flag so that the visibility of the feature can be enabled any time after production deployment.
Description of changes
When implemening UI, we have stuck to standard views as LV wouldn't improve user experience in any meaningful way.
The most important part is implementation of controller level logic for 2FA. Most of it is in
AuthController
andPlausibleWeb.TwoFactor
. I have spiked a refactor extracting all things related to settings and user management fromAuthController
(creating another one, calledUserController
) but decided against doing it now as that would be too many things changing at once and 2FA implementation on its own is pretty big anyway.The controller actions lean on
Auth.TOTP
API but they also layer logic related to session and cookie management on top. The moment the user with 2FA enabled logs in with their email and password, a dedicated session cookie is created for them (session_2fa
), which records the fact that user has authenticated with first factor already but they haven't yet completed the second one. Only after the second factor is completed, the proper user session is setup on 2FA session is discarded. The lifetime of 2FA session is strictly limited to 5 minutes. If user does not verify their second factor within that timeframe, they will have to start over. This mechanism is meant to minimize risk of leaving half-authenticated session for extended period of time. The 2FA session is also invalidated the moment the user enters auth views outside second factor verification, including login. The session is stored in an encrypted cookie - a numerical user ID is stored there, so data is treated as sensitive.Another 2FA aspect managed by auth controller is remembering trusted device. When a user checks "Trust this device for 30 days" when verifying the second factor (applies only to TOTP codes, not recovery ones), a separate cookie is created with expiry time 30 days in the future. This cookie is encrypted as well and contains TOTP token set at the time when the cookie was created. If the cookie is set and token inside it matches token stored for user, 2FA validation step is skipped after login even if 2FA is enabled. The cookie is cleared when 2FA is disabled or password is reset. It's cleared also when verifying 2FA without trusting (for instance, when running 2FA verification against another account in the same browser without setting trust checkbox).
TODO before deployment
totp_token
and run it before deploying the restTests
Changelog
Documentation
Dark mode