-
Notifications
You must be signed in to change notification settings - Fork 322
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
feat: add MFA support (disabled by default) #736
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
Co-authored-by: joel@joellee.org <joel@joellee.org>
* inital fixes * Update api/mfa.go * fix: rename FactorStates Co-authored-by: joel@joellee.org <joel@joellee.org>
* refactor: change json.NewDecoder -> getBodyBytes * refactor: update admin to use getBodyBytes * fix: update comments * fix: patch error message Co-authored-by: joel@joellee.org <joel@joellee.org>
…r status (#733) * refactor: change json.NewDecoder -> getBodyBytes * refactor: update admin to use getBodyBytes * fix: update comments * fix: patch error message * refactor: remove FindFactorByFriendlyName * chore: update error messages * fix: refactor unenroll test * fix: patch staticcheck * fix: patch stray staticcheck error * refactor: add test case for unverified factor * fix: correct error naming and adjust error messages Co-authored-by: joel@joellee.org <joel@joellee.org>
Co-authored-by: joel@joellee.org <joel@joellee.org>
* fix: remove belongs to association on factor * fix: extract issueRefreshToken logic for MFA * chore: run gofmt * fix: downgrade other sessions instead of deleting them * refactor: convert all aal hardcoded strings to use enum * fix: update AAL Calcuation to not duplicate claims * fix: reinstate auth_test * fix: move downgrade to be a method on factor struct * refactor: merge factor and AAL update methods * refactor: change mfa_constants to enum * refactor: change signInMethod to Enum * refactor: change hardcoded strings to test constants * refactor: rename secondary session variables * test: add check that unenrolling clears factorID from assoc session * refactor: remove unused structs * refactor: nit changes (#694) * refactor: validate totp only if challenge isn't expired * refactor: better error handling * refactor: read from DB session instead of JWT token * refactor: remove outdated test * test: add tests for calculate AAL and AMR * fix: add test to ensure claims are not duplicated * fix: add ordering condition * refactor: rename signInMethod to authenticationMethod * fix: change verify bad code return from 401->400 * chore: run gofmt * fix: re-read association from session * refactor: change methods using enum types to AuthenticationMethod * fix: change more functions to take in enum * test: add integration tests * test: add test to check AAL maintainance * fix: add test for refresh token rotation * chore: fix staticcheck * refactor: move sessionID calculation out from transaction * refactor: remove authentication method map * refactor: remove additional CalculateAALAndAMR * fix: remove requirement for tokens to be not revoked * Apply suggestions from code review Co-authored-by: Kang Ming <kang.ming1996@gmail.com> * fix: gofmt, refactor return type for enrollfactor * refactor: add comments to clarify tests, remove unused codew * fix: add index to refresh_token * refactor: minor renaming and add check on qrcode contents * fix: replace expires in to default expiry * fix: update unverified factor test Co-authored-by: joel@joellee.org <joel@joellee.org> Co-authored-by: Kang Ming <kang.ming1996@gmail.com>
* refactor: explicitly name structs * refactor: add explicit tags to user and phone * refactor: add admin related tags Co-authored-by: joel@joellee.org <joel@joellee.org>
…742) * fix: trigger token swap to ensure token is latest one * test: add additional test to check 2FA followed by 1FA sign in * Update api/token.go Co-authored-by: Stojan Dimitrovski <sdimitrovski@gmail.com> * Revert "Update api/token.go" This reverts commit f6aa576. * refactor: reduce number of params needed * refactor: move FindSessionByUserID to tests * Revert "refactor: move FindSessionByUserID to tests" This reverts commit 3b56457. Co-authored-by: joel@joellee.org <joel@joellee.org> Co-authored-by: Stojan Dimitrovski <sdimitrovski@gmail.com>
* fix: add initial flags on tests * fix: add MFA_ENABLED flag to tests * fix: checks in test admin * fix: add enabled flag * fix: properly make use of config * fix: remove stray env var addition * feat: initial prefixing * fix: prefix mfa related tests * Update models/refresh_token.go Co-authored-by: Kang Ming <kang.ming1996@gmail.com> * fix: update comparison checks at * chore: MFA_ENABLED->GOTRUE_MFA_ENABLED Co-authored-by: joel@joellee.org <joel@joellee.org> Co-authored-by: Kang Ming <kang.ming1996@gmail.com>
* refactor: add unenroll factor response * refactor: remove /user/<prefix> route for mfa
* feat: add mfa indexes * Update models/amr.go Co-authored-by: Stojan Dimitrovski <sdimitrovski@gmail.com> * Update migrations/20221011041400_add_mfa_indexes.up.sql Co-authored-by: Stojan Dimitrovski <sdimitrovski@gmail.com> Co-authored-by: joel@joellee.org <joel@joellee.org> Co-authored-by: Stojan Dimitrovski <sdimitrovski@gmail.com>
* refactor: rename errors * fix: add aal2 check * refactor: simplify amr claims * fix: add additional test for unenrolling verified factors * Apply suggestions from code review Co-authored-by: Stojan Dimitrovski <sdimitrovski@gmail.com> Co-authored-by: joel@joellee.org <joel@joellee.org> Co-authored-by: Stojan Dimitrovski <sdimitrovski@gmail.com>
Co-authored-by: joel@joellee.org <joel@joellee.org>
* refactor: remove requirement for code in order to unenroll * test: add sanity checks for secret leakage * fix: update status codes for unenroll factor Co-authored-by: joel@joellee.org <joel@joellee.org>
J0
changed the title
feat: add MFA application code behind feature flag
feat: add MFA(disabled by default)
Oct 18, 2022
J0
changed the title
feat: add MFA(disabled by default)
feat: add MFA support(disabled by default)
Oct 18, 2022
hf
changed the title
feat: add MFA support(disabled by default)
feat: add MFA support (disabled by default)
Oct 18, 2022
hf
approved these changes
Oct 18, 2022
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.
It's finally there! Make sure you squash and merge!
🎉 This PR is included in version 2.19.0 🎉 The release is available on GitHub release Your semantic-release bot 📦🚀 |
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.
What kind of change does this PR introduce?
Adds support for TOTP MFA. Summary of changes:
aal
claim in JWTs that containaal1
,aal2
levels.amr
claim in JWTs that contains a list of all authentication methods used by the user on that particular browsing session.PRs involved:
mfa
#645TruncateAll
for better readability #650GrantParams
for issuing refresh tokens #659GrantParams
object toissueRefreshToken
#662