Merge pull request #308 from NHSDigital/DTOSS-10523-permission-rules

[DTOSS-10523] define some permissions to distinguish the different roles

Author: MatMoore

manage_breast_screening/auth/demo_views.py

@@ -11,8 +11,8 @@ def persona_login(request):
     users = _get_users(request.user)
 
     if request.method == "POST":
-        username = get_object_or_404(users, username=request.POST["username"])
-        login(request, username)
+        user = get_object_or_404(users, username=request.POST["username"])
+        login(request, user, backend="django.contrib.auth.backends.ModelBackend")
         return redirect(_next_page(request))
 
     else:

manage_breast_screening/auth/models.py

@@ -1,4 +1,16 @@
 from dataclasses import dataclass
+from enum import StrEnum
+
+
+class Role(StrEnum):
+    ADMINISTRATIVE = "Administrative"
+    CLINICAL = "Clinical"
+    SUPERUSER = "Superuser"
+
+
+class Permission(StrEnum):
+    VIEW_PARTICIPANT_DATA = "participants.view_participant_data"
+    PERFORM_MAMMOGRAM_APPOINTMENT = "mammograms.perform_mammogram_appointment"
 
 
 @dataclass
@@ -12,7 +24,7 @@ def username(self):
         return f"{self.first_name.lower()}_{self.last_name.lower()}"
 
 
-ADMINISTRATIVE_PERSONA = Persona("Anna", "Davies", "Administrative")
-CLINICAL_PERSONA = Persona("Chloë", "Robinson", "Clinical")
-SUPERUSER_PERSONA = Persona("Simon", "O'Brien", "Superuser")
+ADMINISTRATIVE_PERSONA = Persona("Anna", "Davies", Role.ADMINISTRATIVE)
+CLINICAL_PERSONA = Persona("Chloë", "Robinson", Role.CLINICAL)
+SUPERUSER_PERSONA = Persona("Simon", "O'Brien", Role.SUPERUSER)
 PERSONAS = [ADMINISTRATIVE_PERSONA, CLINICAL_PERSONA, SUPERUSER_PERSONA]

manage_breast_screening/auth/rules.py

@@ -0,0 +1,28 @@
+"""
+Define rules and permissions for access control.
+
+This file is automatically imported by rules.apps.AutodiscoverRulesConfig
+"""
+
+import rules
+
+from .models import Permission, Role
+
+
+def user_has_any_role(role, *other_roles):
+    roles = [role, *other_roles, Role.SUPERUSER]
+
+    @rules.predicate
+    def check(user):
+        # TODO: customise user model to add a cachable role attribute
+        return any(group.name in roles for group in user.groups.all())
+
+    return check
+
+
+# fmt: off
+
+rules.add_perm(Permission.VIEW_PARTICIPANT_DATA, user_has_any_role(Role.CLINICAL, Role.ADMINISTRATIVE))
+rules.add_perm(Permission.PERFORM_MAMMOGRAM_APPOINTMENT, user_has_any_role(Role.CLINICAL))
+
+# fmt: on

manage_breast_screening/auth/tests/factories.py

@@ -0,0 +1,35 @@
+from django.contrib.auth import get_user_model
+from django.contrib.auth.models import Group
+from factory.declarations import Sequence
+from factory.django import DjangoModelFactory
+from factory.helpers import post_generation
+
+from manage_breast_screening.auth.rules import Role
+
+
+class UserFactory(DjangoModelFactory):
+    class Meta:
+        model = get_user_model()
+        django_get_or_create = ("first_name", "last_name")
+        skip_postgeneration_save = True
+
+    username = Sequence(lambda n: "alice%d" % n)
+    first_name = Sequence(lambda n: "alice%d" % n)
+    last_name = "Lastname"
+
+    @post_generation
+    def groups(self, create, extracted, **kwargs):
+        if not create:
+            return
+
+        if extracted:
+            self.groups.set(extracted)
+
+        if kwargs.get("administrative"):
+            self.groups.add(Group.objects.get(name=Role.ADMINISTRATIVE))
+
+        if kwargs.get("clinical"):
+            self.groups.add(Group.objects.get(name=Role.CLINICAL))
+
+        if kwargs.get("superuser"):
+            self.groups.add(Group.objects.get(name=Role.SUPERUSER))

manage_breast_screening/auth/tests/test_rules.py

@@ -0,0 +1,28 @@
+import pytest
+
+from manage_breast_screening.auth.models import Permission
+
+
+@pytest.mark.django_db
+class TestRules:
+    def test_rule_requiring_clinical_role(
+        self, user, administrative_user, clinical_user, superuser
+    ):
+        user.groups.add()
+
+        assert not user.has_perm(Permission.PERFORM_MAMMOGRAM_APPOINTMENT)
+        assert not administrative_user.has_perm(
+            Permission.PERFORM_MAMMOGRAM_APPOINTMENT
+        )
+        assert clinical_user.has_perm(Permission.PERFORM_MAMMOGRAM_APPOINTMENT)
+        assert superuser.has_perm(Permission.PERFORM_MAMMOGRAM_APPOINTMENT)
+
+    def test_rule_requiring_clinical_or_administrative_role(
+        self, user, administrative_user, clinical_user, superuser
+    ):
+        user.groups.add()
+
+        assert not user.has_perm(Permission.VIEW_PARTICIPANT_DATA)
+        assert administrative_user.has_perm(Permission.VIEW_PARTICIPANT_DATA)
+        assert clinical_user.has_perm(Permission.VIEW_PARTICIPANT_DATA)
+        assert superuser.has_perm(Permission.VIEW_PARTICIPANT_DATA)

manage_breast_screening/config/settings.py

@@ -67,6 +67,7 @@ def boolean_env(key, default=None):
     "manage_breast_screening.notifications",
     "manage_breast_screening.participants",
     "manage_breast_screening.mammograms",
+    "rules.apps.AutodiscoverRulesConfig",
 ]
 
 if getenv("DJANGO_ENV", "production") != "production":
@@ -101,6 +102,11 @@ def boolean_env(key, default=None):
     INSTALLED_APPS.append("debug_toolbar")
     MIDDLEWARE.insert(0, "debug_toolbar.middleware.DebugToolbarMiddleware")
 
+AUTHENTICATION_BACKENDS = (
+    "rules.permissions.ObjectPermissionBackend",
+    "django.contrib.auth.backends.ModelBackend",
+)
+
 ROOT_URLCONF = "manage_breast_screening.core.urls"
 
 TEMPLATES = [

manage_breast_screening/config/settings_test.py

@@ -15,17 +15,10 @@
     },
 }
 
-
 MIDDLEWARE.remove(
     "whitenoise.middleware.WhiteNoiseMiddleware",
 )
 
-# Make authentication optional until we've updated the system tests to account
-# for different roles
-MIDDLEWARE.remove(
-    "django.contrib.auth.middleware.LoginRequiredMiddleware",
-)
-
 if DEBUG:
     INSTALLED_APPS.remove("debug_toolbar")
     MIDDLEWARE.remove("debug_toolbar.middleware.DebugToolbarMiddleware")

manage_breast_screening/conftest.py

@@ -1,18 +1,50 @@
 from unittest import TestCase
 
 import pytest
-from django.contrib.auth import get_user_model
+from django.test.client import Client
+
+from manage_breast_screening.auth.tests.factories import UserFactory
 
 # Show long diffs in failed test output
 TestCase.maxDiff = None
 
 
 @pytest.fixture
 def user():
-    return get_user_model().objects.create_user(username="user1", password="123")
+    return UserFactory.create(username="user1")
+
+
+@pytest.fixture
+def administrative_user():
+    return UserFactory.create(username="administrative1", groups__administrative=True)
+
+
+@pytest.fixture
+def clinical_user():
+    return UserFactory.create(username="clinical1", groups__clinical=True)
+
+
+@pytest.fixture
+def superuser():
+    return UserFactory.create(username="superuser1", groups__superuser=True)
+
+
+@pytest.fixture
+def clinical_user_client(clinical_user):
+    client = Client()
+    client.force_login(clinical_user)
+    return client
+
+
+@pytest.fixture
+def administrative_user_client(administrative_user):
+    client = Client()
+    client.force_login(administrative_user)
+    return client
 
 
 @pytest.fixture
-def logged_in_client(user, client):
-    client.force_login(user)
+def superuser_client(superuser):
+    client = Client()
+    client.force_login(superuser)
     return client

manage_breast_screening/mammograms/tests/views/test_appointment_flow.py

@@ -9,8 +9,10 @@
 
 @pytest.mark.django_db
 class TestShowAppointment:
-    def test_redirects_if_in_progress(self, logged_in_client, appointment):
-        response = logged_in_client.get(
+    def test_redirects_to_show_screening_if_in_progress(
+        self, clinical_user_client, appointment
+    ):
+        response = clinical_user_client.get(
             reverse("mammograms:show_appointment", kwargs={"pk": appointment.pk})
         )
         assertRedirects(
@@ -21,8 +23,16 @@ def test_redirects_if_in_progress(self, logged_in_client, appointment):
             ),
         )
 
-    def test_renders_response(self, logged_in_client, completed_appointment):
-        response = logged_in_client.get(
+    def test_doesnt_redirect_if_not_permitted(
+        self, administrative_user_client, appointment
+    ):
+        response = administrative_user_client.get(
+            reverse("mammograms:show_appointment", kwargs={"pk": appointment.pk})
+        )
+        assert response.status_code == 200
+
+    def test_renders_response(self, clinical_user_client, completed_appointment):
+        response = clinical_user_client.get(
             reverse(
                 "mammograms:show_appointment", kwargs={"pk": completed_appointment.pk}
             )
@@ -32,8 +42,8 @@ def test_renders_response(self, logged_in_client, completed_appointment):
 
 @pytest.mark.django_db
 class TestStartScreening:
-    def test_appointment_continued(self, logged_in_client, appointment):
-        response = logged_in_client.post(
+    def test_appointment_continued(self, clinical_user_client, appointment):
+        response = clinical_user_client.post(
             reverse("mammograms:start_screening", kwargs={"pk": appointment.pk}),
             {"decision": "continue"},
         )
@@ -45,8 +55,8 @@ def test_appointment_continued(self, logged_in_client, appointment):
             ),
         )
 
-    def test_appointment_stopped(self, logged_in_client, appointment):
-        response = logged_in_client.post(
+    def test_appointment_stopped(self, clinical_user_client, appointment):
+        response = clinical_user_client.post(
             reverse("mammograms:start_screening", kwargs={"pk": appointment.pk}),
             {"decision": "dropout"},
         )
@@ -59,9 +69,9 @@ def test_appointment_stopped(self, logged_in_client, appointment):
         )
 
     def test_already_completed_appointment_redirects(
-        self, logged_in_client, completed_appointment
+        self, clinical_user_client, completed_appointment
     ):
-        response = logged_in_client.get(
+        response = clinical_user_client.get(
             reverse(
                 "mammograms:start_screening", kwargs={"pk": completed_appointment.pk}
             )
@@ -74,8 +84,8 @@ def test_already_completed_appointment_redirects(
             ),
         )
 
-    def test_renders_invalid_form(self, logged_in_client, appointment):
-        response = logged_in_client.post(
+    def test_renders_invalid_form(self, clinical_user_client, appointment):
+        response = clinical_user_client.post(
             reverse("mammograms:start_screening", kwargs={"pk": appointment.pk}),
             {},
         )
@@ -84,8 +94,8 @@ def test_renders_invalid_form(self, logged_in_client, appointment):
 
 @pytest.mark.django_db
 class TestAskForMedicalInformation:
-    def test_continue_to_record(self, logged_in_client, appointment):
-        response = logged_in_client.post(
+    def test_continue_to_record(self, clinical_user_client, appointment):
+        response = clinical_user_client.post(
             reverse(
                 "mammograms:ask_for_medical_information",
                 kwargs={"pk": appointment.pk},
@@ -100,8 +110,8 @@ def test_continue_to_record(self, logged_in_client, appointment):
             ),
         )
 
-    def test_continue_to_imaging(self, logged_in_client, appointment):
-        response = logged_in_client.post(
+    def test_continue_to_imaging(self, clinical_user_client, appointment):
+        response = clinical_user_client.post(
             reverse(
                 "mammograms:ask_for_medical_information",
                 kwargs={"pk": appointment.pk},
@@ -116,8 +126,8 @@ def test_continue_to_imaging(self, logged_in_client, appointment):
             ),
         )
 
-    def test_renders_invalid_form(self, logged_in_client, appointment):
-        response = logged_in_client.post(
+    def test_renders_invalid_form(self, clinical_user_client, appointment):
+        response = clinical_user_client.post(
             reverse(
                 "mammograms:ask_for_medical_information",
                 kwargs={"pk": appointment.pk},
@@ -129,17 +139,17 @@ def test_renders_invalid_form(self, logged_in_client, appointment):
 
 @pytest.mark.django_db
 class TestCheckIn:
-    def test_known_redirect(self, logged_in_client, appointment):
-        response = logged_in_client.post(
+    def test_known_redirect(self, clinical_user_client, appointment):
+        response = clinical_user_client.post(
             reverse("mammograms:check_in", kwargs={"pk": appointment.pk})
         )
         assertRedirects(
             response,
             reverse("mammograms:start_screening", kwargs={"pk": appointment.pk}),
         )
 
-    def test_audit(self, logged_in_client, appointment):
-        logged_in_client.post(
+    def test_audit(self, clinical_user_client, appointment):
+        clinical_user_client.post(
             reverse("mammograms:check_in", kwargs={"pk": appointment.pk})
         )
         assert (
@@ -153,8 +163,8 @@ def test_audit(self, logged_in_client, appointment):
 
 @pytest.mark.django_db
 class TestAppointmentCannotGoAhead:
-    def test_audit(self, logged_in_client, appointment):
-        logged_in_client.post(
+    def test_audit(self, clinical_user_client, appointment):
+        clinical_user_client.post(
             reverse(
                 "mammograms:appointment_cannot_go_ahead", kwargs={"pk": appointment.pk}
             ),