Skip to content
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
merged 233 commits into from
Oct 18, 2022
Merged

feat: add MFA support (disabled by default) #736

merged 233 commits into from
Oct 18, 2022

Conversation

J0
Copy link
Contributor

@J0 J0 commented Oct 5, 2022

What kind of change does this PR introduce?

Adds support for TOTP MFA. Summary of changes:

  • Adds enrollment, challenge and verify APIs.
  • Adds management APIs.
  • Exposes aal claim in JWTs that contain aal1, aal2 levels.
  • Exposes amr claim in JWTs that contains a list of all authentication methods used by the user on that particular browsing session.

PRs involved:

joel@joellee.org and others added 30 commits June 14, 2022 14:44
J0 and others added 11 commits October 4, 2022 14:31
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>
@J0 J0 marked this pull request as ready for review October 10, 2022 13:56
@J0 J0 requested a review from a team as a code owner October 10, 2022 13:56
hf and others added 9 commits October 11, 2022 11:28
* 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 J0 changed the title feat: add MFA application code behind feature flag feat: add MFA(disabled by default) Oct 18, 2022
@J0 J0 changed the title feat: add MFA(disabled by default) feat: add MFA support(disabled by default) Oct 18, 2022
@hf hf changed the title feat: add MFA support(disabled by default) feat: add MFA support (disabled by default) Oct 18, 2022
Copy link
Contributor

@hf hf left a 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!

@J0 J0 merged commit 940f582 into master Oct 18, 2022
@J0 J0 deleted the mfa branch October 18, 2022 17:04
@github-actions
Copy link
Contributor

🎉 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
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants