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

Move to email verification codes rather than links on signup #4195

Merged
merged 9 commits into from
Jul 18, 2024

Conversation

knolleary
Copy link
Member

Closes #4174

This reworks our how we verify a user's email address in the account registration flow.

On registration, we would generate a signed JWT token that we email to the user as a url for them to follow. They then open the url, and are presented with a page to click a button to verify. They then had to login.

With this change, we now email a code to them and prompt them to enter the code on the screen shown after initial registration. This means they stay in the same browser session through-out - and are logged in automatically.

Demo video of it in action (internal link) https://flowforgeworkspace.slack.com/archives/C04GW82DJFK/p1721207608385039

  • The route POST /account/verify/:token was previously used by the button the user clicks on. That is now replaced with:

    • POST /account/verify/token (note token is now part of the url, not a param), which accepts the body of. { token } - this receives the submitted verification token from the new verify page.
  • We now log the user in in response to the initial registration - this means we don't need the additional log-in step

  • If the email is sso-enabled, we still prompt them to login via SSO provider as that does the email verification step. This may change with the auto-provisioning users for SSO task that we'll look at soon.

  • The Change Email flow has not been modified - that still uses the email link approach. Changing that was out of scope for this task.

Copy link

codecov bot commented Jul 17, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 78.63%. Comparing base (5a3c176) to head (0776939).

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #4195      +/-   ##
==========================================
+ Coverage   78.60%   78.63%   +0.03%     
==========================================
  Files         286      286              
  Lines       13111    13117       +6     
  Branches     2933     2933              
==========================================
+ Hits        10306    10315       +9     
+ Misses       2805     2802       -3     
Flag Coverage Δ
backend 78.63% <100.00%> (+0.03%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link
Contributor

@Steve-Mcl Steve-Mcl left a comment

Choose a reason for hiding this comment

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

On top of the comments, I dont see any docs update, in particular: the signup workflow

forge/postoffice/templates/VerifyEmail.js Show resolved Hide resolved
forge/postoffice/templates/VerifyEmail.js Outdated Show resolved Hide resolved
forge/postoffice/templates/VerifyEmail.js Outdated Show resolved Hide resolved
frontend/src/pages/UnverifiedEmail.vue Show resolved Hide resolved
Co-authored-by: Stephen McLaughlin <44235289+Steve-Mcl@users.noreply.github.com>
@knolleary knolleary requested a review from Steve-Mcl July 17, 2024 15:03
@knolleary
Copy link
Member Author

Good spot on signup flow - will update that now. I don't think we have any user-facing docs that describe the sign-up process - but will double check.

@knolleary
Copy link
Member Author

@Steve-Mcl sign-up workflow doc updated. Cannot find any other docs that need updating.

Copy link
Contributor

@Steve-Mcl Steve-Mcl left a comment

Choose a reason for hiding this comment

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

Approved. Took the liberty of adding missing quote to mermaid diagram

@Steve-Mcl
Copy link
Contributor

@knolleary
Any specific timing on merge once tests complete?

@knolleary
Copy link
Member Author

@Steve-Mcl I'm going to merge this in the morning when I can also verify all is working as expected - I'm out of time to do that verification tonight.

@knolleary knolleary merged commit 0a914ae into main Jul 18, 2024
14 checks passed
@knolleary knolleary deleted the 4174-simplified-sign-up branch July 18, 2024 08:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Require users to insert confirmation code on signup
2 participants