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

feat: check new owner email domain before transferring form ownership #6500

Closed
wants to merge 4 commits into from

Conversation

jia1
Copy link
Member

@jia1 jia1 commented Jul 2, 2023

Problem

(Don't know if this issue has been worked on, but I've been working on a related feature (#34) which might have expected this issue to be part of the scope at first.)

Problem description: Unlike updateFormCollaborators, in transferFormOwnership, the new owner's email domain is not checked against the AgencyModel. We should check.

Might resolve #5971

Solution

Copy the email domain check from updateFormCollaborators but do it at the start of transferFormOwnership.

Breaking Changes

  • Yes - this PR contains breaking changes
    • Details ...
  • No - this PR is backwards compatible

Improvements:

  • Consolidated all .split('@').pop() with the same intent into a utility function getEmailDomainFromEmail.

Tests

TBC

@jia1 jia1 self-assigned this Jul 2, 2023
@jia1 jia1 added the contribute free for contributors to pick up label Jul 2, 2023
@kevin9foong
Copy link
Contributor

Hi @jia1, are you still working on this? I am looking to clean up abandoned PRs and wondering if there's any interest to continue on this. Otherwise, will close the PR!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
contribute free for contributors to pick up
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Transferring ownership to emails from non-whitelisted domains is allowed
2 participants