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

fix(lms): do not create a duplicative authentication option for teachers #57999

Merged
merged 1 commit into from Apr 15, 2024

Conversation

stephenliang
Copy link
Contributor

@stephenliang stephenliang commented Apr 12, 2024

Previously during LMS new user initialization for teachers via NRPS, the email address of the teacher was added using User.update_primary_contact_info. This function will look up the user and check for an existing auth option, for which none will be found for a new user. As a fallback, this function will create a new auth option using credential type EMAIL and return.

Subsequently, the initialize_lti_user_from_nrps function will add the LTI authentication option with the same email. This results in a two authentication options with the same email which fails the AuthenticationOption email_must_be_unique validation causing a 422 error to be thrown back to the user.

In the SSO user creation flow, PartialRegistration updates the primary contact info to the first authentication option, causing the subsequent call to update_primary_contact_info to select the LTI auth option.

This PR updates the logic to set the primary contact info to the LTI authentication option directly bypassing the second auth option that was created.

Links

Testing story

  1. Added test to emulate duplicate user condition
  2. Reproduced issue on Canvas and was able to sync successfully with teacher created

Deployment strategy

Follow-up work

Privacy

Security

Caching

PR Checklist:

  • Tests provide adequate coverage
  • Privacy and Security impacts have been assessed
  • Code is well-commented
  • New features are translatable or updates will not break translations
  • Relevant documentation has been added or updated
  • User impact is well-understood and desirable
  • Pull Request is labeled appropriately
  • Follow-up work items (including potential tech debt) are tracked and linked

@stephenliang stephenliang requested a review from a team April 12, 2024 20:58
Copy link
Contributor

@carl-codeorg carl-codeorg left a comment

Choose a reason for hiding this comment

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

LGTM!

@stephenliang stephenliang merged commit 0d48efa into staging Apr 15, 2024
2 checks passed
@stephenliang stephenliang deleted the stephen/lms-duplicate branch April 15, 2024 14:42
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.

None yet

2 participants