From 6030dc5cc9c423fcee3a5de9cc6b176cf0f046f6 Mon Sep 17 00:00:00 2001 From: James Person Date: Thu, 5 Dec 2024 14:21:03 -0500 Subject: [PATCH 1/2] Center the maintenance banner content (#4514) * Center the maintenance banner content * 'grid-container' on the body, not just the text. --- backend/templates/includes/maintenance_banner.html | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/backend/templates/includes/maintenance_banner.html b/backend/templates/includes/maintenance_banner.html index 20fe2e551..df5941b21 100644 --- a/backend/templates/includes/maintenance_banner.html +++ b/backend/templates/includes/maintenance_banner.html @@ -1,5 +1,5 @@
-
+

Scheduled system upgrade

{% comment %} If a message is given, use it. If not, display a generic message with the timeframe. {% endcomment %} From d7b9752a465c993afeec2ad4704a738fc8ca091c Mon Sep 17 00:00:00 2001 From: Phil Dominguez <142051477+phildominguez-gsa@users.noreply.github.com> Date: Thu, 5 Dec 2024 14:57:47 -0500 Subject: [PATCH 2/2] Adding decorator to verify submission status (#4502) * Adding decorator to verify submission status * Param name tweak * Adding unit tests * Handling missing sac --- backend/audit/test_verify_status.py | 83 +++++++++++++++++++++++++++++ backend/audit/views/views.py | 60 +++++++++++++++------ 2 files changed, 127 insertions(+), 16 deletions(-) create mode 100644 backend/audit/test_verify_status.py diff --git a/backend/audit/test_verify_status.py b/backend/audit/test_verify_status.py new file mode 100644 index 000000000..e52dcfdf3 --- /dev/null +++ b/backend/audit/test_verify_status.py @@ -0,0 +1,83 @@ +from .models import ( + Access, + SingleAuditChecklist, + User, +) +from audit.models.models import STATUS +from audit.views.views import verify_status + +from django.core.exceptions import PermissionDenied +from django.test import TestCase +from django.urls import reverse + +from model_bakery import baker +from unittest.mock import Mock + + +class TestVerifyStatus(TestCase): + def _test_verify_status(self, sac_status, required_status, sac_exists): + """ + Helper to test the behavior of a method decorated with verify_status + sac_status - Current status of the sac + required_status - The required status to be provided to the decorator + """ + user = baker.make(User) + self.client.force_login(user) + + if sac_exists: + sac = baker.make(SingleAuditChecklist, submission_status=sac_status) + baker.make(Access, user=user, sac=sac, role="editor") + + # Set up a fake method to decorate + decorator = verify_status(required_status) + func = Mock() + mock_view = Mock() + mock_request = Mock() + decorated_func = decorator(func) + + if sac_exists: + result = decorated_func(mock_view, mock_request, report_id=sac.report_id) + + if sac_status == required_status: + # The decorated method should be called as normal if the statuses match + func.assert_called() + else: + # The decorated method should redirect if the statuses don't match + result.client = self.client + self.assertRedirects( + result, + reverse("audit:SubmissionProgress", args=[sac.report_id]), + ) + else: + with self.assertRaises(PermissionDenied): + decorated_func(mock_view, mock_request, report_id="bad_report_id") + + def test_valid(self): + """ + Current and required statuses matching should pass + """ + self._test_verify_status( + sac_status=STATUS.IN_PROGRESS, + required_status=STATUS.IN_PROGRESS, + sac_exists=True, + ) + + def test_invalid(self): + """ + Current and required statuses differing should pass + """ + self._test_verify_status( + sac_status=STATUS.IN_PROGRESS, + required_status=STATUS.AUDITEE_CERTIFIED, + sac_exists=True, + ) + + def test_no_sac(self): + """ + Current and required statuses differing should throw + """ + self._test_verify_status( + sac_status=STATUS.IN_PROGRESS, + required_status=STATUS.AUDITEE_CERTIFIED, + sac_exists=False, + ) diff --git a/backend/audit/views/views.py b/backend/audit/views/views.py index db5e5eea4..b1f920db1 100644 --- a/backend/audit/views/views.py +++ b/backend/audit/views/views.py @@ -61,6 +61,38 @@ def _friendly_status(status): return dict(SingleAuditChecklist.STATUS_CHOICES)[status] +def verify_status(status): + """ + Decorator to be applied to view request methods (i.e. get, post) to verify + that the submission is in the correct state before allowing the user to + proceed. An incorrect status usually happens via direct URL access. If the + given status does not match the submission's, it will redirect them back to + the submission progress page. + """ + + def decorator_verify_status(request_method): + def verify(view, request, *args, **kwargs): + report_id = kwargs["report_id"] + + try: + sac = SingleAuditChecklist.objects.get(report_id=report_id) + except SingleAuditChecklist.DoesNotExist: + raise PermissionDenied("You do not have access to this audit.") + + # Return to checklist, the Audit is not in the correct state. + if sac.submission_status != status: + logger.warning( + f"Expected submission status {status} but it's currently {sac.submission_status}" + ) + return redirect(f"/audit/submission-progress/{sac.report_id}") + else: + return request_method(view, request, *args, **kwargs) + + return verify + + return decorator_verify_status + + class MySubmissions(LoginRequiredMixin, generic.View): redirect_field_name = "Home" @@ -345,6 +377,7 @@ def post(self, request, *args, **kwargs): class AuditorCertificationStep1View(CertifyingAuditorRequiredMixin, generic.View): + @verify_status(STATUS.READY_FOR_CERTIFICATION) def get(self, request, *args, **kwargs): report_id = kwargs["report_id"] @@ -373,6 +406,7 @@ def get(self, request, *args, **kwargs): except SingleAuditChecklist.DoesNotExist: raise PermissionDenied("You do not have access to this audit.") + @verify_status(STATUS.READY_FOR_CERTIFICATION) def post(self, request, *args, **kwargs): report_id = kwargs["report_id"] @@ -410,6 +444,7 @@ def post(self, request, *args, **kwargs): class AuditorCertificationStep2View(CertifyingAuditorRequiredMixin, generic.View): + @verify_status(STATUS.READY_FOR_CERTIFICATION) def get(self, request, *args, **kwargs): report_id = kwargs["report_id"] @@ -447,6 +482,7 @@ def get(self, request, *args, **kwargs): except SingleAuditChecklist.DoesNotExist: raise PermissionDenied("You do not have access to this audit.") + @verify_status(STATUS.READY_FOR_CERTIFICATION) def post(self, request, *args, **kwargs): report_id = kwargs["report_id"] @@ -496,6 +532,7 @@ def post(self, request, *args, **kwargs): class AuditeeCertificationStep1View(CertifyingAuditeeRequiredMixin, generic.View): + @verify_status(STATUS.AUDITOR_CERTIFIED) def get(self, request, *args, **kwargs): report_id = kwargs["report_id"] @@ -515,15 +552,12 @@ def get(self, request, *args, **kwargs): "form": form, } - # Return to checklist, the Audit is not in the correct state. - if sac.submission_status != STATUS.AUDITOR_CERTIFIED: - return redirect(f"/audit/submission-progress/{sac.report_id}") - return render(request, "audit/auditee-certification-step-1.html", context) except SingleAuditChecklist.DoesNotExist: raise PermissionDenied("You do not have access to this audit.") + @verify_status(STATUS.AUDITOR_CERTIFIED) def post(self, request, *args, **kwargs): report_id = kwargs["report_id"] @@ -542,10 +576,6 @@ def post(self, request, *args, **kwargs): "submission_status": sac.submission_status, } - # Return to checklist, the Audit is not in the correct state. - if sac.submission_status != STATUS.AUDITOR_CERTIFIED: - return redirect(f"/audit/submission-progress/{sac.report_id}") - if form.is_valid(): # Save to session. Retrieved and saved after step 2. request.session["AuditeeCertificationStep1Session"] = form.cleaned_data @@ -561,6 +591,7 @@ def post(self, request, *args, **kwargs): class AuditeeCertificationStep2View(CertifyingAuditeeRequiredMixin, generic.View): + @verify_status(STATUS.AUDITOR_CERTIFIED) def get(self, request, *args, **kwargs): report_id = kwargs["report_id"] @@ -589,15 +620,12 @@ def get(self, request, *args, **kwargs): "form": form, } - # Return to checklist, the Audit is not in the correct state. - if sac.submission_status != STATUS.AUDITOR_CERTIFIED: - return redirect(f"/audit/submission-progress/{sac.report_id}") - return render(request, "audit/auditee-certification-step-2.html", context) except SingleAuditChecklist.DoesNotExist: raise PermissionDenied("You do not have access to this audit.") + @verify_status(STATUS.AUDITOR_CERTIFIED) def post(self, request, *args, **kwargs): report_id = kwargs["report_id"] @@ -614,10 +642,6 @@ def post(self, request, *args, **kwargs): "submission_status": sac.submission_status, } - # Return to checklist, the Audit is not in the correct state. - if sac.submission_status != STATUS.AUDITOR_CERTIFIED: - return redirect(f"/audit/submission-progress/{sac.report_id}") - if form2.is_valid(): form_cleaned = { "auditee_certification": form1_cleaned, @@ -646,6 +670,7 @@ def post(self, request, *args, **kwargs): class CertificationView(CertifyingAuditeeRequiredMixin, generic.View): + @verify_status(STATUS.AUDITOR_CERTIFIED) def get(self, request, *args, **kwargs): report_id = kwargs["report_id"] @@ -661,6 +686,7 @@ def get(self, request, *args, **kwargs): except SingleAuditChecklist.DoesNotExist: raise PermissionDenied("You do not have access to this audit.") + @verify_status(STATUS.AUDITOR_CERTIFIED) def post(self, request, *args, **kwargs): report_id = kwargs["report_id"] @@ -676,6 +702,7 @@ def post(self, request, *args, **kwargs): class SubmissionView(CertifyingAuditeeRequiredMixin, generic.View): + @verify_status(STATUS.AUDITEE_CERTIFIED) def get(self, request, *args, **kwargs): report_id = kwargs["report_id"] try: @@ -690,6 +717,7 @@ def get(self, request, *args, **kwargs): except SingleAuditChecklist.DoesNotExist: raise PermissionDenied("You do not have access to this audit.") + @verify_status(STATUS.AUDITEE_CERTIFIED) def post(self, request, *args, **kwargs): # RACE HAZARD WARNING # It is possible for a user to enter the submission multiple times,