Enforce academic email requirements for students, postdocs, and academic researchers

[mscanlan-git]

This PR addresses #2129 by adding checks to credential applications to ensure users who select 'student, graduate student, postdoc, academic researcher' as their research category have their email address verified.

• I added a "new" validator, which was a previously used one but adjusted the text to fit the email criteria. It also briefly explains how to change your primary email.

One final thing that needs to be added is including the error banner at the top of the page instead of having it linger at the bottom of class PersonalCAF(forms.ModelForm)

[tompollard]

@mscanlan-git I just took a quick look at the failed test:

======================================================================
FAIL: test_credential_application (user.test_integration.TestCredentialing)
Test submission of a credential application.
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/code/physionet-django/user/test_integration.py", line 222, in test_credential_application
    self.assertTrue(u.credential_applications.exists())
AssertionError: False is not true

This suggests that the current test case is failing because you have added a new requirement for credentialing applications, and the test application does not know about it (and fails the test).

You probably need to update the test to ensure the application meets your new requirement (e.g. ensure it is using an academic email address, which presumably it is not?).

[mscanlan-git]

Updated the PR with the error message being properly displayed under the researcher category:

Overall, this was a better approach compared to the previous idea of implementing an email field in the form, which was a bit too complicated given what the goal of the PR is trying to accomplish. Let me know if you think there's anything else I should consider adding or changing @tompollard

[lbulgarelli]

I think it would be more interesting to keep a list of approved educational domains. .edu domains could be automatically added. Other domains (likely international) would need to be approved by an admin. I think this won't add much work since those are easy to check and will end up being more helpful in addressing the credentialing issue. It would also give us a nice clean list of institutions use PhysioNet as well.

[bemoody]

First of all, please don't tell people they need to set their primary address to this or that. If they have a verified academic address, that should be sufficient.

We can update the admin console to prominently display their academic/institutional address(es), if that would be helpful.

Second, we don't want to duplicate this logic. Don't copy and paste the existing function into another place. Instead, we want to reuse the existing function, or if that's not possible for some reason, adapt the existing function to work in a new setting.

I also don't think it's right to report error messages related to the person's email address(es) by attaching them to the researcher_category form field. I think it would make more sense to put this check in CredentialApplicationForm.clean (line 587) and raise a ValidationError there, just like all the other specialized category-dependent checks.

[bemoody]

we don't want to duplicate this logic

We could do something like this

        if category in [0, 1, 2, 7]:
            has_institutional_email = False
            for email in self.user.get_emails():
                try:
                    validate_institutional_email(email)
                    has_institutional_email = True
                except ValidationError:
                    pass
            if not has_institutional_email:
                raise ValidationError(
                    'Please provide your academic email address'
                    ' in your email settings.'
                )

[bemoody]

I think it would be more interesting to keep a list of approved educational domains. .edu domains could be automatically added. Other domains (likely international) would need to be approved by an admin.

This is a good idea. However, we don't want to block people from unknown domains - they should be able to submit their application, and then the admin can decide to mark their domain as a "known institution" (green light in the future) or "known public mail provider" (block that domain in the future).

I think this could be fairly automated and save a lot of time, but it is a bit more complicated than this PR.

[mscanlan-git]

Thanks for your feedback @bemoody, I think this is a much better approach overall and makes more sense, thanks for including the code!

I like this idea @lbulgarelli as it would allow us to know easily what domains are trusted and what are not, but like Benjamin commented, it would need to be set up in such a way so non-approved domains are not blocked initially from applying. This could definitely automate some of that for sure, which long-term is a good idea.