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

Ensure personal instance is created in personal team on signup #4360

Merged
merged 2 commits into from
Aug 9, 2024

Conversation

knolleary
Copy link
Member

We had received a couple reports that after invite a user to the platform, they were getting a 'verification failed' error when entering the verification code. Clicking the 'resend email' didn't do anything. However they found they could click logout and log in again and all was well.

Via the audit logs, I could see we were:

  1. creating their personal team
  2. creating an application
  3. not creating an instance

On further examination, I could create this locally based on the following conditions:

  1. An Enterprise team is created by existing user. No instances are created.
  2. An external user is invited to join the team
  3. The external user receives invite and signs up
  4. They receive the verification code, which they submit correctly
  5. The verification code is confirm and their email is marked verified
  6. The signup code then completes their signup by:
    1. Creates a personal team
    2. Accepts the invite - so they are now a member of both personal team and enterprise team
    3. Checks there are no instances in any of their team - so auto-creates a new instance
    4. The code picks the first team in their list to create the new app/instance in. However, this happens to be the Enterprise team.
    5. Because the Enterprise team is picked, the attempt to create a Starter instance fails - as starter isn't allowed in enterprise.
    6. The error bubbles up and appears in the front end as a 'verification failure' - even though the verification succeeded

This fixes that by:

  1. Guarantees that if it decides to create the starter instance, it will get created in their personal team
  2. Ensures the verify end point always reports success once its gets past the verification step. Any subsequent errors creating their team/etc are not verification issues and are logged separately.

Copy link

codecov bot commented Aug 9, 2024

Codecov Report

Attention: Patch coverage is 81.81818% with 4 lines in your changes missing coverage. Please review.

Project coverage is 78.23%. Comparing base (cff3adf) to head (509816b).
Report is 35 commits behind head on main.

Files Patch % Lines
forge/routes/auth/index.js 66.66% 4 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #4360      +/-   ##
==========================================
+ Coverage   78.17%   78.23%   +0.05%     
==========================================
  Files         292      292              
  Lines       13430    13424       -6     
  Branches     3010     3010              
==========================================
+ Hits        10499    10502       +3     
+ Misses       2931     2922       -9     
Flag Coverage Δ
backend 78.23% <81.81%> (+0.05%) ⬆️

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.

@knolleary knolleary requested a review from Steve-Mcl August 9, 2024 10:39
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.

Code review looks good and makes sense however we should probably unit test this scenario but I can see how this would be difficult to achieve and there is a more immediate need to get this fix in without additional delay. Perhaps a follow up issue around tests would be in order?

@knolleary
Copy link
Member Author

@Steve-Mcl I have added a couple tests to the existing suite that covers the auto-creation of team/instance on signup

    ✔ Creates instance in personal team when invites to other teams present (317ms)
    ✔ Skips creating instance in personal team when invited to other team with instance (211ms)

The first test covers the main scenario. The second one was a piece of existing logic I saw that was quick to add a test for.

@knolleary knolleary force-pushed the fix-signup-with-invite branch from c69384e to 509816b Compare August 9, 2024 13:03
@knolleary knolleary merged commit 524b963 into main Aug 9, 2024
14 checks passed
@knolleary knolleary deleted the fix-signup-with-invite branch August 9, 2024 13:14
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.

2 participants