Handling emails with invalid characters (#4245)

* Handling bad emails for user roles page

* Filling in name/email inputs

* Creating get_user_role_management_context

* Lint

* Removing duplicate code

* Validating emails for step 3

* Lint

* Fixing unit test

* Email validation for gen form

* Fixing unit test

* Adding 'scroll down' messages

* Remembering auditee and auditor step 3 info

* Fixing some field values

* Lint

* Forgot to add this

backend/api/views.py

@@ -183,7 +183,27 @@ def access_and_submission_check(user, data):
 
         return {"report_id": sac.report_id, "next": "TBD"}
 
-    return {"errors": serializer.errors}
+    return {
+        "errors": serializer.errors,
+        "certifying_auditee_contact_fullname": data.get(
+            "certifying_auditee_contact_fullname"
+        ),
+        "certifying_auditee_contact_email": data.get(
+            "certifying_auditee_contact_email"
+        ),
+        "certifying_auditee_contact_re_email": data.get(
+            "certifying_auditee_contact_re_email"
+        ),
+        "certifying_auditor_contact_fullname": data.get(
+            "certifying_auditor_contact_fullname"
+        ),
+        "certifying_auditor_contact_email": data.get(
+            "certifying_auditor_contact_email"
+        ),
+        "certifying_auditor_contact_re_email": data.get(
+            "certifying_auditor_contact_re_email"
+        ),
+    }
 
 
 class Sprite(generic.View):

backend/audit/templates/audit/manage-submission-change-access.html

@@ -31,23 +31,26 @@ <h1 id="role-management">Add Audit Editor</h1>
                             For the changes to take effect, users must log out and log back in to their account.
                         </p>
                     {% endif %}
-
-                    <hr />
-
                     {% if errors %}
-                        <ul>
-                            {% for error in errors %}<li class="usa-error-message">{{ error }}</li>{% endfor %}
-                        </ul>
+                        <span class="usa-error-message" role="alert">There were errors when attempting to submit the form. Scroll down for more details.</span>
                     {% endif %}
-
+
+                    <hr />
+
                     <label class="usa-label" for="fullname">New name:</label>
-                    <input class="usa-input" id="name" name="fullname" placeholder="[name]" />
-
+                    <input class="usa-input"
+                           id="name"
+                           name="fullname"
+                           placeholder="[name]"
+                           value="{{ certifier_name }}"/>
+
                     <label class="usa-label" for="email">New email address:</label>
                     <input class="usa-input"
                            id="email"
                            name="email"
-                           placeholder="[email address]" />
+                           placeholder="[email address]"
+                           value="{{ email }}"/>
+                    <span class="usa-error-message" id="email-error-message" role="alert">{{ errors.email }}</span>
                 </fieldset>
                 <button class="usa-button margin-top-5" id="continue">Submit</button>
                 <a class="usa-button usa-button--unstyled margin-left-2"

backend/audit/views/manage_submission_access.py

@@ -1,3 +1,4 @@
+import logging
 from types import SimpleNamespace
 from django import forms
 from django.db import transaction
@@ -13,6 +14,8 @@
     SingleAuditChecklist,
 )
 
+logger = logging.getLogger(__name__)
+
 
 def _get_friendly_role(role):
     return dict(ACCESS_ROLES)[role]
@@ -54,29 +57,7 @@ def get(self, request, *args, **kwargs):
         """
         report_id = kwargs["report_id"]
         sac = SingleAuditChecklist.objects.get(report_id=report_id)
-        context = {
-            "role": self.role,
-            "friendly_role": None,
-            "auditee_uei": sac.general_information["auditee_uei"],
-            "auditee_name": sac.general_information.get("auditee_name"),
-            "certifier_name": None,
-            "email": None,
-            "report_id": report_id,
-            "errors": [],
-        }
-        if self.role != "editor":
-            try:
-                access = Access.objects.get(sac=sac, role=self.role)
-            except Access.DoesNotExist:
-                access = SimpleNamespace(
-                    fullname="UNASSIGNED ROLE", email="UNASSIGNED ROLE", role=self.role
-                )
-
-            context = context | {
-                "friendly_role": _get_friendly_role(access.role),
-                "certifier_name": access.fullname,
-                "email": access.email,
-            }
+        context = self.get_user_role_management_context(sac)
 
         return render(request, self.template, context)
 
@@ -88,7 +69,23 @@ def post(self, request, *args, **kwargs):
         report_id = kwargs["report_id"]
         sac = SingleAuditChecklist.objects.get(report_id=report_id)
         form = ChangeAccessForm(request.POST)
+        context = self.get_user_role_management_context(sac)
+
         form.full_clean()
+        if not form.is_valid():
+            context = (
+                context
+                | form.cleaned_data
+                | {
+                    "errors": form.errors,
+                }
+            )
+
+            for field, errors in form.errors.items():
+                logger.warning(f"Error {field}: {errors}")
+
+            return render(request, self.template, context, status=400)
+
         url = reverse("audit:ManageSubmission", kwargs={"report_id": report_id})
         fullname = form.cleaned_data["fullname"]
         email = form.cleaned_data["email"]
@@ -122,9 +119,9 @@ def post(self, request, *args, **kwargs):
                 "certifier_name": access.fullname,
                 "email": access.email,
                 "report_id": report_id,
-                "errors": [
-                    "Cannot use the same email address for both certifying officials."
-                ],
+                "errors": {
+                    "email": "Cannot use the same email address for both certifying officials."
+                },
             }
             return render(request, self.template, context, status=400)
 
@@ -141,6 +138,34 @@ def post(self, request, *args, **kwargs):
 
         return redirect(url)
 
+    def get_user_role_management_context(self, sac):
+        context = {
+            "role": self.role,
+            "friendly_role": None,
+            "auditee_uei": sac.general_information["auditee_uei"],
+            "auditee_name": sac.general_information.get("auditee_name"),
+            "certifier_name": None,
+            "email": None,
+            "report_id": sac.report_id,
+            "errors": [],
+        }
+
+        if self.role != "editor":
+            try:
+                access = Access.objects.get(sac=sac, role=self.role)
+            except Access.DoesNotExist:
+                access = SimpleNamespace(
+                    fullname="UNASSIGNED ROLE", email="UNASSIGNED ROLE", role=self.role
+                )
+
+            context = context | {
+                "friendly_role": _get_friendly_role(access.role),
+                "certifier_name": access.fullname,
+                "email": access.email,
+            }
+
+        return context
+
 
 class ChangeAuditorCertifyingOfficialView(ChangeOrAddRoleView):
     """

backend/report_submission/forms.py

@@ -127,7 +127,7 @@ class GeneralInformationForm(forms.Form):
         required=False,
     )
     auditee_phone = forms.CharField(required=False, validators=[phone_validator])
-    auditee_email = forms.CharField(
+    auditee_email = forms.EmailField(
         min_length=CHARACTER_LIMITS_GENERAL["auditee_email"]["min"],
         max_length=CHARACTER_LIMITS_GENERAL["auditee_email"]["max"],
         required=False,
@@ -172,7 +172,7 @@ class GeneralInformationForm(forms.Form):
         required=False,
     )
     auditor_phone = forms.CharField(required=False, validators=[phone_validator])
-    auditor_email = forms.CharField(
+    auditor_email = forms.EmailField(
         min_length=CHARACTER_LIMITS_GENERAL["auditor_email"]["min"],
         max_length=CHARACTER_LIMITS_GENERAL["auditor_email"]["max"],
         required=False,

backend/report_submission/templates/report_submission/step-3.html

@@ -18,6 +18,9 @@
                     <p class="required-explanation">
                         <abbr title="required" class="usa-hint usa-hint--required">*</abbr> Indicates a required field.
                     </p>
+                    {% if errors %}
+                        <span class="usa-error-message" role="alert">There were errors when attempting to submit the form. Scroll down for more details.</span>
+                    {% endif %}
                     <fieldset class="usa-fieldset question"
                               aria-labelledby="certifying_auditee_contact_legend certifying_auditee_contact_instruction">
                         <legend id="certifying_auditee_contact_legend" class="usa-legend">
@@ -46,7 +49,8 @@
                                            aria-required="true"
                                            required
                                            data-validate-not-null=""
-                                           data-validate-length="<= 100 >= 2"/>
+                                           data-validate-length="<= 100 >= 2"
+                                           value="{{ certifying_auditee_contact_fullname | default_if_none:'' }}" />
                                 </div>
                             </div>
                             <div class="grid-row grid-gap">
@@ -74,7 +78,8 @@
                                                data-validate-not-null=""
                                                data-validate-email=""
                                                data-validate-must-not-match="certifying_auditor_contact_email"
-                                               data-validate-length="<= 340 >= 6"/>
+                                               data-validate-length="<= 340 >= 6"
+                                               value="{{ certifying_auditee_contact_email | default_if_none:'' }}" />
                                     </div>
                                 </div>
                                 <div class="tablet:grid-col-fill">
@@ -95,10 +100,14 @@
                                                aria-required="true"
                                                required
                                                data-validate-must-match="certifying_auditee_contact_email"
-                                               data-validate-length="<= 340 >= 6"/>
+                                               data-validate-length="<= 340 >= 6"
+                                               value="{{ certifying_auditee_contact_re_email | default_if_none:'' }}" />
                                     </div>
                                 </div>
                             </div>
+                            {% if errors.certifying_auditee_contact_email %}
+                            <span class="usa-error-message" id="certifying_auditee_contact_email-error-message" role="alert">Enter a valid email address.</span>
+                            {% endif %}
                         </div>
                     </fieldset>
 
@@ -130,7 +139,8 @@
                                            aria-required="true"
                                            required
                                            data-validate-not-null=""
-                                           data-validate-length="<= 100 >= 2"/>
+                                           data-validate-length="<= 100 >= 2"
+                                           value="{{ certifying_auditor_contact_fullname | default_if_none:'' }}" />
                                 </div>
                             </div>
                             <div class="grid-row grid-gap">
@@ -158,7 +168,8 @@
                                                data-validate-not-null=""
                                                data-validate-email=""
                                                data-validate-must-not-match="certifying_auditee_contact_email"
-                                               data-validate-length="<= 340 >= 6"/>
+                                               data-validate-length="<= 340 >= 6"
+                                               value="{{ certifying_auditor_contact_email | default_if_none:'' }}" />
                                     </div>
                                 </div>
                                 <div class="tablet:grid-col-fill">
@@ -179,13 +190,17 @@
                                                aria-required="true"
                                                required
                                                data-validate-must-match="certifying_auditor_contact_email"
-                                               data-validate-length="<= 340 >= 6"/>
+                                               data-validate-length="<= 340 >= 6"
+                                               value="{{ certifying_auditor_contact_re_email | default_if_none:'' }}" />
                                     </div>
                                 </div>
                             </div>
+                            {% if errors.certifying_auditor_contact_email %}
+                            <span class="usa-error-message" id="certifying_auditor_contact_email-error-message" role="alert">Enter a valid email address.</span>
+                            {% endif %}
                         </div>
                     </fieldset>
-                    
+
                     <fieldset class="usa-fieldset question"
                               aria-labelledby="auditee_contacts_legend dynamic_fieldset_instruction_auditee">
                         <legend id="auditee_contacts_legend" class="usa-legend">
@@ -255,6 +270,9 @@
                                     </div>
                                 </div>
                             </div>
+                            {% if errors.auditee_contacts_email %}
+                            <span class="usa-error-message" id="auditee_contacts_email-error-message" role="alert">Enter a valid email address.</span>
+                            {% endif %}
                         </div>
                         <template id="auditee_contacts-template">
                             <div id="auditee_contacts" class="grid-container auditee_contacts additional_contacts">
@@ -406,6 +424,9 @@
                                     </div>
                                 </div>
                             </div>
+                            {% if errors.auditor_contacts_email %}
+                            <span class="usa-error-message" id="auditor_contacts_email-error-message" role="alert">Enter a valid email address.</span>
+                            {% endif %}
                         </div>
                         <template id="auditor_contacts-template">
                             <div id="auditor_contacts" class="grid-container auditor_contacts additional_contacts">

backend/report_submission/test_views.py

@@ -385,8 +385,7 @@ def test_step_three_accessandsubmission_submission_fail(self):
 
         data = {}
         response = self.client.post(url, data=data)
-        self.assertEqual(response.status_code, 302)
-        self.assertEqual(response.url, "/report_submission/accessandsubmission/")
+        self.assertEqual(response.status_code, 400)
 
     def test_reportsubmissionredirectview_get_redirects(self):
         url = reverse("report_submission:report_submission")
@@ -777,7 +776,7 @@ def test_post_gsa_migration_error(self):
 
         self.assertIn("errors", response.context)
         self.assertIn(
-            "GSA_MIGRATION not permitted outside of migrations",
+            "Enter a valid email address.",
             response.context["errors"],
         )
 

backend/report_submission/views.py

@@ -114,20 +114,17 @@ def get(self, request):
             args["step"] = 3
             return render(request, "report_submission/step-3.html", args)
 
-    # render access-submission form
-
-    # gather/save step 3 info, redirect to step ...4?
-    def post(self, post_request):
-        result = api.views.access_and_submission_check(
-            post_request.user, post_request.POST
-        )
+    def post(self, request):
+        result = api.views.access_and_submission_check(request.user, request.POST)
 
         report_id = result.get("report_id")
 
         if report_id:
             return redirect(f"/report_submission/general-information/{report_id}")
         else:
-            return redirect(reverse("report_submission:accessandsubmission"))
+            return render(
+                request, "report_submission/step-3.html", context=result, status=400
+            )
 
 
 class GeneralInformationFormView(LoginRequiredMixin, View):