fix: MFA NewFactor to default to creating unverfied factors #1692

J0 · 2024-07-30T16:37:09Z

What kind of change does this PR introduce?

Split NewFactor into NewPhoneFactor() and NewTOTPFactor().
All New<Type>Factor methods will now create unverified factors.
Additionally, also guards the Challenge endpoint when verification is disabled for a factor.

The hope is to reduce cognitive load and the chance of creating a factor in an undesired state

Should one wish to obtain a Verified Factor (say for tests) they can call UpdateStatus. It is unlikely for this to be a common use case though.

Someone might have brought this up prior but only getting to it now

coveralls · 2024-07-30T17:02:57Z

Pull Request Test Coverage Report for Build 10166630443

Details

14 of 20 (70.0%) changed or added relevant lines in 2 files are covered.
No unchanged relevant lines lost coverage.
Overall coverage decreased (-0.02%) to 58.2%

Changes Missing Coverage	Covered Lines	Changed/Added Lines	%
internal/api/mfa.go	9	15	60.0%

Totals
Change from base Build 10157498258:	-0.02%
Covered Lines:	9163
Relevant Lines:	15744

💛 - Coveralls

internal/api/mfa.go

🤖 I have created a release *beep* *boop* --- ## [2.158.0](v2.157.0...v2.158.0) (2024-07-31) ### Features * add hook log entry with `run_hook` action ([#1684](#1684)) ([46491b8](46491b8)) * MFA (Phone) ([#1668](#1668)) ([ae091aa](ae091aa)) ### Bug Fixes * maintain backward compatibility for asymmetric JWTs ([#1690](#1690)) ([0ad1402](0ad1402)) * MFA NewFactor to default to creating unverfied factors ([#1692](#1692)) ([3d448fa](3d448fa)) * minor spelling errors ([#1688](#1688)) ([6aca52b](6aca52b)), closes [#1682](#1682) * treat `GOTRUE_MFA_ENABLED` as meaning TOTP enabled on enroll and verify ([#1694](#1694)) ([8015251](8015251)) * update mfa phone migration to be idempotent ([#1687](#1687)) ([fdff1e7](fdff1e7)) --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please). Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>

…#1692) ## What kind of change does this PR introduce? - Split `NewFactor` into `NewPhoneFactor()` and `NewTOTPFactor()`. - All `New<Type>Factor` methods will now create unverified factors. - Additionally, also guards the `Challenge` endpoint when verification is disabled for a factor. The hope is to reduce cognitive load and the chance of creating a factor in an undesired state Should one wish to obtain a Verified Factor (say for tests) they can call `UpdateStatus`. It is unlikely for this to be a common use case though. Someone might have brought this up prior but only getting to it now

🤖 I have created a release *beep* *boop* --- ## [2.158.0](supabase/auth@v2.157.0...v2.158.0) (2024-07-31) ### Features * add hook log entry with `run_hook` action ([supabase#1684](supabase#1684)) ([46491b8](supabase@46491b8)) * MFA (Phone) ([supabase#1668](supabase#1668)) ([ae091aa](supabase@ae091aa)) ### Bug Fixes * maintain backward compatibility for asymmetric JWTs ([supabase#1690](supabase#1690)) ([0ad1402](supabase@0ad1402)) * MFA NewFactor to default to creating unverfied factors ([supabase#1692](supabase#1692)) ([3d448fa](supabase@3d448fa)) * minor spelling errors ([supabase#1688](supabase#1688)) ([6aca52b](supabase@6aca52b)), closes [supabase#1682](supabase#1682) * treat `GOTRUE_MFA_ENABLED` as meaning TOTP enabled on enroll and verify ([supabase#1694](supabase#1694)) ([8015251](supabase@8015251)) * update mfa phone migration to be idempotent ([supabase#1687](supabase#1687)) ([fdff1e7](supabase@fdff1e7)) --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please). Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>

…#1692) ## What kind of change does this PR introduce? - Split `NewFactor` into `NewPhoneFactor()` and `NewTOTPFactor()`. - All `New<Type>Factor` methods will now create unverified factors. - Additionally, also guards the `Challenge` endpoint when verification is disabled for a factor. The hope is to reduce cognitive load and the chance of creating a factor in an undesired state Should one wish to obtain a Verified Factor (say for tests) they can call `UpdateStatus`. It is unlikely for this to be a common use case though. Someone might have brought this up prior but only getting to it now

🤖 I have created a release *beep* *boop* --- ## [2.158.0](supabase/auth@v2.157.0...v2.158.0) (2024-07-31) ### Features * add hook log entry with `run_hook` action ([supabase#1684](supabase#1684)) ([46491b8](supabase@46491b8)) * MFA (Phone) ([supabase#1668](supabase#1668)) ([ae091aa](supabase@ae091aa)) ### Bug Fixes * maintain backward compatibility for asymmetric JWTs ([supabase#1690](supabase#1690)) ([0ad1402](supabase@0ad1402)) * MFA NewFactor to default to creating unverfied factors ([supabase#1692](supabase#1692)) ([3d448fa](supabase@3d448fa)) * minor spelling errors ([supabase#1688](supabase#1688)) ([6aca52b](supabase@6aca52b)), closes [supabase#1682](supabase#1682) * treat `GOTRUE_MFA_ENABLED` as meaning TOTP enabled on enroll and verify ([supabase#1694](supabase#1694)) ([8015251](supabase@8015251)) * update mfa phone migration to be idempotent ([supabase#1687](supabase#1687)) ([fdff1e7](supabase@fdff1e7)) --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please). Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>

…#1692) ## What kind of change does this PR introduce? - Split `NewFactor` into `NewPhoneFactor()` and `NewTOTPFactor()`. - All `New<Type>Factor` methods will now create unverified factors. - Additionally, also guards the `Challenge` endpoint when verification is disabled for a factor. The hope is to reduce cognitive load and the chance of creating a factor in an undesired state Should one wish to obtain a Verified Factor (say for tests) they can call `UpdateStatus`. It is unlikely for this to be a common use case though. Someone might have brought this up prior but only getting to it now

🤖 I have created a release *beep* *boop* --- ## [2.158.0](supabase/auth@v2.157.0...v2.158.0) (2024-07-31) ### Features * add hook log entry with `run_hook` action ([supabase#1684](supabase#1684)) ([46491b8](supabase@46491b8)) * MFA (Phone) ([supabase#1668](supabase#1668)) ([ae091aa](supabase@ae091aa)) ### Bug Fixes * maintain backward compatibility for asymmetric JWTs ([supabase#1690](supabase#1690)) ([0ad1402](supabase@0ad1402)) * MFA NewFactor to default to creating unverfied factors ([supabase#1692](supabase#1692)) ([3d448fa](supabase@3d448fa)) * minor spelling errors ([supabase#1688](supabase#1688)) ([6aca52b](supabase@6aca52b)), closes [supabase#1682](supabase#1682) * treat `GOTRUE_MFA_ENABLED` as meaning TOTP enabled on enroll and verify ([supabase#1694](supabase#1694)) ([8015251](supabase@8015251)) * update mfa phone migration to be idempotent ([supabase#1687](supabase#1687)) ([fdff1e7](supabase@fdff1e7)) --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please). Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>

fix: explicitly specify factor type

719a6e4

J0 requested a review from a team as a code owner July 30, 2024 16:37

J0 changed the title ~~fix: change interface for the Factor to default to unverified~~ fix: MFA NewFactor to default to creating unverfied factors Jul 30, 2024

J0 added 3 commits July 30, 2024 18:41

fix: correct typo

7c9732b

fix: also restrict challenge when verify is disabled

1e1fa88

fix: remove unused methods

8570d8a

J0 commented Jul 30, 2024

View reviewed changes

internal/api/mfa.go Show resolved Hide resolved

hf approved these changes Jul 31, 2024

View reviewed changes

hf merged commit 3d448fa into master Jul 31, 2024
2 checks passed

hf deleted the j0/phone_mfa_refactors branch July 31, 2024 13:07

github-actions bot mentioned this pull request Jul 31, 2024

chore(master): release 2.158.0 #1686

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: MFA NewFactor to default to creating unverfied factors #1692

fix: MFA NewFactor to default to creating unverfied factors #1692

J0 commented Jul 30, 2024 •

edited

Loading

coveralls commented Jul 30, 2024 •

edited

Loading

fix: MFA NewFactor to default to creating unverfied factors #1692

fix: MFA NewFactor to default to creating unverfied factors #1692

Conversation

J0 commented Jul 30, 2024 • edited Loading

What kind of change does this PR introduce?

coveralls commented Jul 30, 2024 • edited Loading

Pull Request Test Coverage Report for Build 10166630443

Details

💛 - Coveralls

J0 commented Jul 30, 2024 •

edited

Loading

coveralls commented Jul 30, 2024 •

edited

Loading