fix: move permission list validation from model level to service layer

[justynoh]

Problem

Whenever agencies change email domains, we remove their domain from the Agency collection. However, this means that if they had any collaborators in their form collaborator list whose emails were from the old domain, their form is forever locked. They cannot make changes to their form anymore, since the form document cannot be saved with invalid collaborators.

The reason for this occurring is validation that is applied at the model level. The validation is thus run on every document save, and the database rejects the invalid save on change.

Solution

Even though we would like for there to be model-level validation, this is not practical or informative. The model validations should specify invariants of the model over all operations throughout the system. However, the agency collection is always subject to change, and the validator will certainly not be invariant over time. Therefore, we should not in principle validate based on the agency collection at the model level. All we can guarantee is that the list of collaborators are emails. What we would really like is that all new collaborators are whitelisted.

Therefore, this PR removes the model-level validation based on the agency collection. In replacement, the validation occurs at the service layer. Any new collaborators added are extracted and checked for validity, while old collaborators are left untouched.

Breaking Changes

• No - this PR is backwards compatible

Screenshots

Screen.Recording.2023-03-20.at.4.54.57.PM.mov

Screen.Recording.2023-03-20.at.5.00.24.PM.mov

Tests

• Ensure that the email domain data.gov.sg is not in the govtech agency. Try to add [email protected] email as a collaborator. This should fail.
• Add the data.gov.sg email domain to the govtech agency. Now add [email protected] and [email protected] email as collaborators. Then, remove the data.gov.sg email domain from the govtech agency. Perform the following actions in order.
  • Try modifying some form fields. This should work.
  • Remove [email protected] as a collaborator. This should work.
  • Modify the read/write permission of [email protected]. This should fail.
  • Add [email protected] as a collaborator. This should work.
  • Remove [email protected] as a collaborator. This should work.

Notes

Separately, I found an existing bug with transfer ownership. We currently allow ownership to be transferred to users whose emails are no longer valid domains belonging to any agency. You can see this in the second video clip, right at the end. I'll open a ticket for it after looking through the existing issues to see if it's duplicated.

[LinHuiqing]

lgtm!

[timotheeg]

Just checking: is normalization of the emails (trim + lowercasing) done before this filtering occurs? (question is also relevant to the agency search below)

[justynoh]

these are both done in the frontend. we have validation middleware requiring that the emails be emails (which means they must already be trimmed), but i don't think this enforces a lowercase restriction. we should do it here too. Thanks for catching!

I'll open a PR to force lowercase validation in the middleware