Fix can_view_submission ignoring ReviewerSettings configured in wagtail admin (#4899)

Fixed a permission bug that ignored the
[ReviewerSettings](https://github.com/HyphaApp/hypha/blob/6b3aa1f72d6b30efa66641f968cff9813873e878/hypha/apply/funds/models/reviewer_role.py#L42)
that were set in Wagtail admin.

By default (no settings configured) a reviewer should have access to all
submissions. But in Wagtail, setting can be configured to scope a
reviewer's specific access to things like assigned applications,
specific application outcomes, etc. This was not being considered by
`can_view_submission`, which would give reviewers access to things like
attachments on applications they wouldn't otherwise have access to.

The logic for ReviewerSettings was implemented on the
`ReviewerSubmissionDetailView` alone so the detail view would account
for these settings but not the `can_view_submission` function itself.


Thank you to https://github.com/Santoshkumarpuppala for discovering and
responsibly disclosing this bug!

## Test Steps
<!-- 
If step does not require manual testing, skip/remove this section.

Give a brief overview of the steps required for a user/dev to test this
contribution. Important things to include:
 - Required user roles for where necessary (ie. "As a Staff Admin...")
- Clear & validatable expected results (ie. "Confirm the submit button
is now not clickable")
- Language that can be understood by non-technical testers if being
tested by users
-->

- [ ] Ensure that with no ReviewerSettings configured/enabled in Wagtail
admin, reviewers are able to access all submissions (both in detail view
and see them displayed in the All Submissions view)
- [ ] Ensure that with ReviewSettings configured, all permissions act as
expected (ie. if a submission cannot be viewed, confirmed associated
files also return a 403)

Author: wes-otf

hypha/apply/funds/permissions.py

@@ -1,10 +1,13 @@
+from django.apps import apps
 from django.conf import settings
 from django.core.exceptions import PermissionDenied
 from django.utils.translation import gettext as _
 from rolepermissions.permissions import register_object_checker
 
 from hypha.apply.funds.models.co_applicants import CoApplicant, CoApplicantRole
+from hypha.apply.funds.models.reviewer_role import ReviewerSettings
 from hypha.apply.funds.models.submissions import DRAFT_STATE
+from hypha.home.models import ApplyHomePage
 
 from ..users.roles import STAFF_GROUP_NAME, SUPERADMIN, TEAMADMIN_GROUP_NAME, StaffAdmin
 
@@ -213,11 +216,22 @@ def can_view_submission(user, submission):
     if (
         user.is_apply_staff
         or submission.user == user
-        or user.is_reviewer
         or submission.co_applicants.filter(user=user).exists()
     ):
         return True, ""
 
+    # By default, reviewers can see all submissions. This can be configured in Wagtail Admin > Apply > Reviewer Settings
+    if user.is_reviewer:
+        site = ApplyHomePage.objects.first().get_site()
+        reviewer_settings = ReviewerSettings.for_site(site)
+        ApplicationSubmission = apps.get_model("funds", "ApplicationSubmission")
+        if reviewer_settings.use_settings:
+            return ApplicationSubmission.objects.for_reviewer_settings(
+                user, reviewer_settings
+            ).filter(pk=submission.pk).exists(), ""
+        else:
+            return True, ""
+
     if user.is_community_reviewer and submission.community_review:
         return True, ""
 

hypha/apply/funds/tests/test_permissions.py

@@ -9,6 +9,7 @@
     CoApplicantInviteStatus,
     CoApplicantRole,
 )
+from hypha.apply.funds.models.reviewer_role import ReviewerSettings
 from hypha.apply.funds.tests.factories import ApplicationSubmissionFactory
 from hypha.apply.funds.workflows import DRAFT_STATE
 from hypha.apply.users.tests.factories import (
@@ -18,6 +19,7 @@
     StaffFactory,
     UserFactory,
 )
+from hypha.home.factories import ApplySiteFactory
 
 from ..permissions import (
     can_access_drafts,
@@ -221,12 +223,45 @@ def test_submission_owner_can_view(self):
         result, _ = can_view_submission(applicant, submission)
         self.assertTrue(result)
 
-    def test_reviewer_can_view(self):
+    def test_reviewer_can_view_by_default(self):
         reviewer = ReviewerFactory()
         submission = ApplicationSubmissionFactory()
         result, _ = can_view_submission(reviewer, submission)
         self.assertTrue(result)
 
+    # Basic tests to ensure functionality, more in-depth tests happen in hypha/apply/funds/tests/test_views.py::TestReviewerSubmissionView
+    def test_reviewer_cannot_view_unassigned_when_configured(self):
+        reviewer = ReviewerFactory()
+        apply_site = ApplySiteFactory()
+        reviewer_settings, _ = ReviewerSettings.objects.get_or_create(
+            site_id=apply_site.id
+        )
+        reviewer_settings.use_settings = True
+        reviewer_settings.submission = "all"
+        reviewer_settings.outcome = "all"
+        reviewer_settings.assigned = True
+        reviewer_settings.save()
+
+        submission = ApplicationSubmissionFactory()
+        result, _ = can_view_submission(reviewer, submission)
+        self.assertFalse(result)
+
+    def test_reviewer_can_view_assigned_when_configured(self):
+        reviewer = ReviewerFactory()
+        apply_site = ApplySiteFactory()
+        reviewer_settings, _ = ReviewerSettings.objects.get_or_create(
+            site_id=apply_site.id
+        )
+        reviewer_settings.use_settings = True
+        reviewer_settings.submission = "all"
+        reviewer_settings.outcome = "all"
+        reviewer_settings.assigned = True
+        reviewer_settings.save()
+
+        submission = ApplicationSubmissionFactory(reviewers=[reviewer])
+        result, _ = can_view_submission(reviewer, submission)
+        self.assertTrue(result)
+
     def test_unrelated_user_cannot_view(self):
         user = UserFactory()
         submission = ApplicationSubmissionFactory()

hypha/apply/funds/views/all.py

@@ -129,13 +129,11 @@ def submissions_all(
     else:
         qs = ApplicationSubmission.objects.current()
 
-    # Reviewers also have access to this view but should only see a subset of submissions.
+    # By default, reviewers can see all submissions. This can be configured in Wagtail Admin > Apply > Reviewer Settings
     if request.user.is_reviewer:
         reviewer_settings = ReviewerSettings.for_request(request)
         if reviewer_settings.use_settings:
             qs = qs.for_reviewer_settings(request.user, reviewer_settings)
-        else:
-            qs = qs.filter(reviewers=request.user)
 
     if not can_access_drafts or not show_drafts:
         qs = qs.exclude_draft()

hypha/apply/funds/views/submission_detail.py

@@ -33,7 +33,6 @@
 
 from ..models import (
     ApplicationSubmission,
-    ReviewerSettings,
 )
 from ..permissions import (
     can_alter_archived_submissions,
@@ -142,15 +141,6 @@ def dispatch(self, request, *args, **kwargs):
             "submission_view", request.user, object=submission, raise_exception=True
         )
 
-        reviewer_settings = ReviewerSettings.for_request(request)
-        if reviewer_settings.use_settings:
-            queryset = ApplicationSubmission.objects.for_reviewer_settings(
-                request.user, reviewer_settings
-            )
-            # Reviewer can't view submission which is not listed in ReviewerSubmissionsTable
-            if not queryset.filter(id=submission.id).exists():
-                raise PermissionDenied
-
         return super().dispatch(request, *args, **kwargs)