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.

Sign up for GitHub

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

Jump to bottom

task(recovery-phone): Add support for confirming code on sign in #18207

Merged
merged 1 commit into from
Jan 14, 2025
Merged

task(recovery-phone): Add support for confirming code on sign in #18207

merged 1 commit into from
Jan 14, 2025

Conversation

dschom
Copy link
Contributor

@dschom dschom commented Jan 10, 2025 •
edited
Loading

Because

  • We want a sms code from valid recovery phone to unlock the session

This pull request

  • Changes the end point to /recovery-phone/signin/confirm for clarity
  • Differentiates between confirming a signup code and a signin code
  • Cleans up codes after they are confirmed
  • Cleans up database table factories by moving them into libs/shared/db/mysql factories
  • Adds sql spec for sessionTokens
  • Adds sql spec for unverifiedTokens
  • Adds ability to verify a session through the AccountManager
  • Registers the AccountManager in the DI container in auth server

Issue that this pull request solves

Closes: FXA-10945

Checklist

Put an x in the boxes that apply

  • My commit is GPG signed.
  • If applicable, I have modified or added tests which pass locally.
  • I have added necessary documentation (if appropriate).
  • I have verified that my changes render correctly in RTL (if appropriate).

Screenshots (Optional)

Please attach the screenshots of the changes made in case of change in user interface.

Other information (Optional)

Any other information that is important to this pull request.

@dschom dschom closed this Jan 10, 2025
@dschom dschom reopened this Jan 10, 2025
@dschom dschom changed the title task(recovery-phone): Allow recovery to verify session task(recovery-phone): Add support for confirming code on sign in Jan 10, 2025
@dschom dschom marked this pull request as ready for review January 10, 2025 22:18
@dschom dschom requested a review from a team as a code owner January 10, 2025 22:18
vbudhram
vbudhram approved these changes Jan 13, 2025
View reviewed changes
Copy link
Contributor

@vbudhram vbudhram left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@dschom LGTM!

libs/accounts/recovery-phone/src/lib/recovery-phone.errors.ts Outdated Show resolved Hide resolved
libs/shared/account/account/src/lib/account.repository.ts Outdated Show resolved Hide resolved
packages/fxa-auth-server/lib/routes/recovery-phone.ts Outdated Show resolved Hide resolved
packages/fxa-auth-server/test/remote/recovery_phone_tests.js Show resolved Hide resolved
@dschom dschom force-pushed the FXA-10945 branch 2 times, most recently from fc1e5e7 to d91395a Compare January 14, 2025 05:25
@dschom
task(recovery-phone): Add support for confirming code on sign in
c9ab0fa 
Because:
- We want a sms code from valid recovery phone to unlock the session

This Commit:
- Changes the end point to /recovery-phone/signin/confirm for clarity
- Differentiates between confirming a signup code and a signin code
- Cleans up codes after they are confirmed
- Cleans up factories by moving them into libs/shared/db/mysql factories
- Adds sql spec for sessionTokens
- Adds sql spec for unverifiedTokens
- Adds ability to verify a session through the AccountManager
- Registers the AccountManager in the DI container in auth server
@dschom dschom merged commit 05e05f4 into main Jan 14, 2025
25 checks passed
@dschom dschom deleted the FXA-10945 branch January 14, 2025 18:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@vbudhram vbudhram vbudhram approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@dschom @vbudhram