From 1266c3874db06bdcbe85c5c97024ba4e28b7391b Mon Sep 17 00:00:00 2001 From: Joe Heffer Date: Fri, 5 Jun 2026 16:04:44 +0100 Subject: [PATCH 1/5] feat: suspend user accounts from the staff console (GDPR Art. 18) Implements the Right to Restriction of Processing: staff can temporarily freeze a user account so they cannot log in, while retaining all their data. - Add ConsoleSuspendUserView (confirm + suspend) and ConsoleUnsuspendUserView to the staff console, reusing Django's is_active flag (no migration, no cascade delete). Guards prevent suspending yourself or a superuser. - Show all users in the console list with an Active/Suspended status column, and add suspend/lift-suspension actions on the user detail page. - Show a clear "account has been suspended" message on login for suspended users with valid credentials, instead of the generic error. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../console/suspend_user_confirm.html | 53 ++++++++++++++ home/templates/console/user_detail.html | 10 ++- home/templates/console/users.html | 10 ++- home/tests/test_console_views.py | 72 ++++++++++++++++++- home/tests/test_user_views.py | 23 ++++++ home/urls.py | 10 +++ home/views/__init__.py | 4 ++ home/views/auth.py | 15 +++- home/views/console.py | 43 ++++++++++- 9 files changed, 235 insertions(+), 5 deletions(-) create mode 100644 home/templates/console/suspend_user_confirm.html diff --git a/home/templates/console/suspend_user_confirm.html b/home/templates/console/suspend_user_confirm.html new file mode 100644 index 00000000..3b9aac66 --- /dev/null +++ b/home/templates/console/suspend_user_confirm.html @@ -0,0 +1,53 @@ +{% extends "base_console.html" %} +{% block title %}| Admin — Suspend account{% endblock %} + +{% block content %} +
+ + + +
+
+
+
+

Suspend account

+
+
+

Are you sure you want to suspend {{ viewed_user }}?

+

+ While suspended, this user cannot log in or submit data. Their personal data + and survey responses are retained, not deleted, and you can + lift the suspension at any time. +

+
+ + +
+
+
+ Cancel +
+ {% csrf_token %} + +
+
+ +
+
+
+ +
+{% endblock %} diff --git a/home/templates/console/user_detail.html b/home/templates/console/user_detail.html index 97ac1256..84b81d1c 100644 --- a/home/templates/console/user_detail.html +++ b/home/templates/console/user_detail.html @@ -24,7 +24,15 @@

{{ viewed_user }}

Superuser {% endif %} {% if not viewed_user.is_active %} - Inactive + Suspended + {% endif %} + {% if not viewed_user.is_active %} +
+ {% csrf_token %} + +
+ {% elif viewed_user != request.user and not viewed_user.is_superuser %} + Suspend account {% endif %} diff --git a/home/templates/console/users.html b/home/templates/console/users.html index aa9db127..fb9061c8 100644 --- a/home/templates/console/users.html +++ b/home/templates/console/users.html @@ -11,6 +11,7 @@

Users

Name Email + Status Organisations Joined @@ -20,12 +21,19 @@

Users

{{ user.first_name }} {{ user.last_name }} {{ user.email }} + + {% if user.is_active %} + Active + {% else %} + Suspended + {% endif %} + {{ user.organisationmembership_set.count }} {{ user.date_joined|date:"d M Y" }} {% empty %} - No users + No users {% endfor %} diff --git a/home/tests/test_console_views.py b/home/tests/test_console_views.py index 9e1259b9..da3abcee 100644 --- a/home/tests/test_console_views.py +++ b/home/tests/test_console_views.py @@ -2,7 +2,7 @@ import SORT.test.test_case from SORT.test.model_factory import OrganisationFactory, OrganisationMembershipFactory, ProjectFactory, SurveyFactory, \ - UserFactory + SuperUserFactory, UserFactory from SORT.test.model_factory.user.constants import PASSWORD @@ -216,3 +216,73 @@ def test_console_remove_member_post_removes_membership(self): self.assertRedirects(response, f"/console/organisations/{org.pk}/") from home.models import OrganisationMembership self.assertFalse(OrganisationMembership.objects.filter(pk=membership_pk).exists()) + + # -- Suspend / unsuspend user -------------------------------------------- + + def test_console_suspend_user_get_accessible_to_staff(self): + """Staff users can access the suspend confirmation page.""" + user = UserFactory() + self.login_staff() + response = self.client.get(f"/console/users/{user.pk}/suspend/") + self.assertEqual(response.status_code, HTTPStatus.OK) + + def test_console_suspend_user_get_redirects_anonymous(self): + """Anonymous users are redirected away from the suspend page.""" + user = UserFactory() + response = self.client.get(f"/console/users/{user.pk}/suspend/") + self.assertEqual(response.status_code, HTTPStatus.FOUND) + + def test_console_suspend_user_get_forbidden_for_regular_users(self): + """Regular users cannot access the suspend page.""" + user = UserFactory() + self.login() + response = self.client.get(f"/console/users/{user.pk}/suspend/") + self.assertEqual(response.status_code, HTTPStatus.FORBIDDEN) + + def test_console_suspend_user_post_suspends_without_deleting_data(self): + """POSTing to suspend sets is_active=False, leaving data intact.""" + user = UserFactory() + membership = OrganisationMembershipFactory(user=user, organisation=OrganisationFactory()) + self.login_staff() + response = self.client.post(f"/console/users/{user.pk}/suspend/") + self.assertRedirects(response, f"/console/users/{user.pk}/") + user.refresh_from_db() + self.assertFalse(user.is_active) + # No cascade: the user and their memberships still exist. + self.assertTrue(UserFactory._meta.model.objects.filter(pk=user.pk).exists()) + from home.models import OrganisationMembership + self.assertTrue(OrganisationMembership.objects.filter(pk=membership.pk).exists()) + + def test_console_suspend_user_post_unsuspends(self): + """POSTing to unsuspend restores is_active=True.""" + user = UserFactory(is_active=False) + self.login_staff() + response = self.client.post(f"/console/users/{user.pk}/unsuspend/") + self.assertRedirects(response, f"/console/users/{user.pk}/") + user.refresh_from_db() + self.assertTrue(user.is_active) + + def test_console_suspend_self_forbidden(self): + """Staff cannot suspend their own account.""" + self.login_staff() + response = self.client.post(f"/console/users/{self.staff_user.pk}/suspend/") + self.assertEqual(response.status_code, HTTPStatus.FORBIDDEN) + self.staff_user.refresh_from_db() + self.assertTrue(self.staff_user.is_active) + + def test_console_suspend_superuser_forbidden(self): + """Superuser accounts cannot be suspended.""" + target = SuperUserFactory() + self.login_staff() + response = self.client.post(f"/console/users/{target.pk}/suspend/") + self.assertEqual(response.status_code, HTTPStatus.FORBIDDEN) + target.refresh_from_db() + self.assertTrue(target.is_active) + + def test_console_user_list_shows_suspended_users(self): + """Suspended users remain visible in the console user list.""" + suspended = UserFactory(is_active=False) + self.login_staff() + response = self.client.get("/console/users/") + self.assertContains(response, suspended.email) + self.assertContains(response, "Suspended") diff --git a/home/tests/test_user_views.py b/home/tests/test_user_views.py index fe90fe04..244075d0 100644 --- a/home/tests/test_user_views.py +++ b/home/tests/test_user_views.py @@ -5,6 +5,8 @@ from http import HTTPStatus import SORT.test.test_case +from SORT.test.model_factory import UserFactory +from SORT.test.model_factory.user.constants import PASSWORD class UserViewTestCase(SORT.test.test_case.ViewTestCase): @@ -31,3 +33,24 @@ def test_login_post(self): def test_logout(self): # Expect to be redirected self.post("logout", expected_status_code=HTTPStatus.FOUND, login=True) + + def test_login_suspended_user_shows_suspension_message(self): + """A suspended user with correct credentials sees a clear message.""" + suspended = UserFactory(is_active=False) + response = self.post( + "login", + data=dict(username=suspended.email, password=PASSWORD), + login=False, + ) + self.assertContains(response, "suspended") + # They are not authenticated. + self.assertFalse(response.wsgi_request.user.is_authenticated) + + def test_login_wrong_password_shows_generic_message(self): + """An incorrect password shows the generic error, not the suspension one.""" + response = self.post( + "login", + data=dict(username=self.user.email, password="not-the-password"), + login=False, + ) + self.assertContains(response, "Invalid email or password") diff --git a/home/urls.py b/home/urls.py index cf75cbc5..a7ac059b 100644 --- a/home/urls.py +++ b/home/urls.py @@ -154,6 +154,16 @@ ), path("console/users/", views.ConsoleUserListView.as_view(), name="admin_users"), path("console/users//", views.ConsoleUserDetailView.as_view(), name="admin_user_detail"), + path( + "console/users//suspend/", + views.ConsoleSuspendUserView.as_view(), + name="admin_suspend_user", + ), + path( + "console/users//unsuspend/", + views.ConsoleUnsuspendUserView.as_view(), + name="admin_unsuspend_user", + ), path( "console/surveys/", views.ConsoleSurveyListView.as_view(), name="admin_surveys" ), diff --git a/home/views/__init__.py b/home/views/__init__.py index 2f07d498..896dff9e 100644 --- a/home/views/__init__.py +++ b/home/views/__init__.py @@ -38,6 +38,8 @@ ConsoleProjectDetailView, ConsoleProjectListView, ConsoleRemoveMemberView, + ConsoleSuspendUserView, + ConsoleUnsuspendUserView, ConsoleUserDetailView, ) from .project import ( @@ -86,5 +88,7 @@ "ConsoleProjectDetailView", "ConsoleProjectListView", "ConsoleRemoveMemberView", + "ConsoleSuspendUserView", + "ConsoleUnsuspendUserView", "ConsoleUserDetailView", ] diff --git a/home/views/auth.py b/home/views/auth.py index 805db465..7cf20459 100644 --- a/home/views/auth.py +++ b/home/views/auth.py @@ -83,7 +83,20 @@ class LoginInterfaceView(LoginView): success_url = reverse_lazy("dashboard") def form_invalid(self, form): - messages.error(self.request, "Invalid email or password.") + # A suspended account (is_active=False) is rejected by ModelBackend before + # AuthenticationForm.confirm_login_allowed runs, so it lands here with the + # generic error. Surface a clearer message — but only when the password + # actually matches, so we don't reveal which emails exist. + email = form.cleaned_data.get("username") or form.data.get("username") + password = form.cleaned_data.get("password") or form.data.get("password") + user = User.objects.filter(email=email).first() + if user and password and not user.is_active and user.check_password(password): + messages.error( + self.request, + "Your account has been suspended. Please contact your administrator.", + ) + else: + messages.error(self.request, "Invalid email or password.") return super().form_invalid(form) def dispatch(self, request, *args, **kwargs): diff --git a/home/views/console.py b/home/views/console.py index d5091da9..53e6c2e6 100644 --- a/home/views/console.py +++ b/home/views/console.py @@ -5,6 +5,7 @@ """ from django.contrib import messages +from django.core.exceptions import PermissionDenied from django.db.models import Count from django.shortcuts import get_object_or_404, redirect from django.views.generic import TemplateView, View @@ -73,7 +74,7 @@ class ConsoleUserListView(StaffRequiredMixin, TemplateView): def get_context_data(self, **kwargs): context = super().get_context_data(**kwargs) - context["users"] = User.objects.filter(is_active=True).order_by("last_name", "first_name") + context["users"] = User.objects.all().order_by("last_name", "first_name") return context @@ -204,3 +205,43 @@ def post(self, request, org_pk, membership_pk): membership.delete() messages.success(request, f"{user_display} removed from {org_name}.") return redirect("admin_organisation_detail", pk=org_pk) + + +class ConsoleSuspendUserView(StaffRequiredMixin, TemplateResponseMixin, View): + """ + Suspend a user account (UK GDPR Article 18, Right to Restriction). + + Sets ``is_active = False`` so the user can no longer log in. No data is + deleted and the action is reversible via :class:`ConsoleUnsuspendUserView`. + """ + + template_name = "console/suspend_user_confirm.html" + + def _get_suspendable_user(self, request, pk): + user = get_object_or_404(User, pk=pk) + # Guard against locking out yourself or a superuser. + if user == request.user or user.is_superuser: + raise PermissionDenied("This account cannot be suspended.") + return user + + def get(self, request, pk): + user = self._get_suspendable_user(request, pk) + return self.render_to_response({"viewed_user": user}) + + def post(self, request, pk): + user = self._get_suspendable_user(request, pk) + user.is_active = False + user.save(update_fields=["is_active"]) + messages.success(request, f"{user} has been suspended.") + return redirect("admin_user_detail", pk=pk) + + +class ConsoleUnsuspendUserView(StaffRequiredMixin, View): + """Lift the suspension on a user account, restoring login access.""" + + def post(self, request, pk): + user = get_object_or_404(User, pk=pk) + user.is_active = True + user.save(update_fields=["is_active"]) + messages.success(request, f"The suspension on {user} has been lifted.") + return redirect("admin_user_detail", pk=pk) From a7d1df9486265b93eaf0e84f639984b3ef27b66b Mon Sep 17 00:00:00 2001 From: Joe Heffer Date: Wed, 8 Jul 2026 15:06:06 +0100 Subject: [PATCH 2/5] fix: prevent staff from suspending other staff accounts Extends the console suspend-user guard to reject staff targets, not just self and superuser accounts. Co-Authored-By: Claude Sonnet 5 --- home/tests/test_console_views.py | 9 +++++++++ home/views/console.py | 7 ++++--- 2 files changed, 13 insertions(+), 3 deletions(-) diff --git a/home/tests/test_console_views.py b/home/tests/test_console_views.py index 064e40f0..0b9d598c 100644 --- a/home/tests/test_console_views.py +++ b/home/tests/test_console_views.py @@ -314,6 +314,15 @@ def test_console_suspend_superuser_forbidden(self): target.refresh_from_db() self.assertTrue(target.is_active) + def test_console_suspend_staff_forbidden(self): + """Staff cannot suspend another staff member's account.""" + target = UserFactory(is_staff=True) + self.login_staff() + response = self.client.post(f"/console/users/{target.pk}/suspend/") + self.assertEqual(response.status_code, HTTPStatus.FORBIDDEN) + target.refresh_from_db() + self.assertTrue(target.is_active) + def test_console_user_list_shows_suspended_users(self): """Suspended users remain visible in the console user list.""" suspended = UserFactory(is_active=False) diff --git a/home/views/console.py b/home/views/console.py index a5b43f8e..f5efbcfa 100644 --- a/home/views/console.py +++ b/home/views/console.py @@ -273,9 +273,10 @@ class ConsoleSuspendUserView(StaffRequiredMixin, TemplateResponseMixin, View): def _get_suspendable_user(self, request, pk): user = get_object_or_404(User, pk=pk) - # Guard against locking out yourself or a superuser, and against - # suspending an already-anonymised (deleted) account. - if user == request.user or user.is_superuser or user.is_deleted: + # Guard against locking out yourself, a fellow staff member, or a + # superuser, and against suspending an already-anonymised (deleted) + # account. + if user == request.user or user.is_staff or user.is_superuser or user.is_deleted: raise PermissionDenied("This account cannot be suspended.") return user From 33dbf190206ba6467955154048d9a2ccbfe34b27 Mon Sep 17 00:00:00 2001 From: Joe Heffer Date: Wed, 8 Jul 2026 15:18:53 +0100 Subject: [PATCH 3/5] fix: avoid timing side-channel when detecting suspended accounts Use AllowAllUsersModelBackend and confirm_login_allowed() to surface the suspended-account message from the same authenticate() call, instead of a separate check_password() call that could leak suspension status via timing. --- SORT/settings.py | 7 ++++++- home/views/auth.py | 22 ++++++++++++---------- 2 files changed, 18 insertions(+), 11 deletions(-) diff --git a/SORT/settings.py b/SORT/settings.py index fcc7f34b..87492b79 100644 --- a/SORT/settings.py +++ b/SORT/settings.py @@ -214,7 +214,12 @@ def cast_to_boolean(obj: Any) -> bool: DEFAULT_FROM_EMAIL = os.getenv("DJANGO_DEFAULT_FROM_EMAIL", "noreply@noreply.com") AUTHENTICATION_BACKENDS = ( - "django.contrib.auth.backends.ModelBackend", + # Allows authenticate() to succeed for inactive (suspended) users so that + # AuthenticationForm.confirm_login_allowed() can reject them with a specific + # error, reusing the single password hash check already performed here rather + # than requiring a second check_password() call that leaks suspension status + # via a timing side-channel. + "django.contrib.auth.backends.AllowAllUsersModelBackend", "allauth.account.auth_backends.AuthenticationBackend", ) diff --git a/home/views/auth.py b/home/views/auth.py index 6a128858..f7a34c05 100644 --- a/home/views/auth.py +++ b/home/views/auth.py @@ -13,7 +13,7 @@ PasswordResetDoneView, PasswordResetView, ) -from django.core.exceptions import PermissionDenied +from django.core.exceptions import NON_FIELD_ERRORS, PermissionDenied from django.db import IntegrityError from django.shortcuts import redirect from django.urls import reverse_lazy @@ -82,7 +82,11 @@ def form_valid(self, form): "administrator of your organisation to re-send your invitation.", ) return self.form_invalid(form) - login(self.request, user, backend="django.contrib.auth.backends.ModelBackend") + login( + self.request, + user, + backend="django.contrib.auth.backends.AllowAllUsersModelBackend", + ) return redirect(reverse_lazy("dashboard")) @@ -96,14 +100,12 @@ class LoginInterfaceView(LoginView): success_url = reverse_lazy("dashboard") def form_invalid(self, form): - # A suspended account (is_active=False) is rejected by ModelBackend before - # AuthenticationForm.confirm_login_allowed runs, so it lands here with the - # generic error. Surface a clearer message — but only when the password - # actually matches, so we don't reveal which emails exist. - email = form.cleaned_data.get("username") or form.data.get("username") - password = form.cleaned_data.get("password") or form.data.get("password") - user = User.objects.filter(email=email).first() - if user and password and not user.is_active and user.check_password(password): + # AllowAllUsersModelBackend lets authenticate() succeed for suspended + # (is_active=False) users, so confirm_login_allowed() raises this + # "inactive" error only once the password has already been verified — + # avoiding a second check_password() call that would otherwise leak + # suspension status via a timing side-channel. + if form.has_error(NON_FIELD_ERRORS, code="inactive"): messages.error( self.request, "Your account has been suspended. Please contact your administrator.", From 821f67ed5e69ac2a0bc349d83d33b0de73c9080c Mon Sep 17 00:00:00 2001 From: Joe Heffer Date: Wed, 8 Jul 2026 15:27:12 +0100 Subject: [PATCH 4/5] feat: add Suspended tab to console user list filter Suspended accounts were only distinguishable via a per-row badge under the "Active" tab, with no way to filter down to just them. --- home/templates/console/users.html | 1 + home/tests/test_console_views.py | 11 +++++++++++ home/views/console.py | 2 ++ 3 files changed, 14 insertions(+) diff --git a/home/templates/console/users.html b/home/templates/console/users.html index c0f8846e..3ff12f5e 100644 --- a/home/templates/console/users.html +++ b/home/templates/console/users.html @@ -9,6 +9,7 @@

Users

Status: Active + Suspended Deleted All
diff --git a/home/tests/test_console_views.py b/home/tests/test_console_views.py index 0b9d598c..4ca7f837 100644 --- a/home/tests/test_console_views.py +++ b/home/tests/test_console_views.py @@ -87,6 +87,17 @@ def test_console_users_deleted_filter_shows_only_inactive(self): self.assertNotIn(active_user, response.context["users"]) self.assertIn(deleted_user, response.context["users"]) + def test_console_users_suspended_filter_shows_only_suspended(self): + """?status=suspended shows only suspended (inactive, non-deleted) users.""" + active_user = UserFactory() + suspended_user = UserFactory(is_active=False) + deleted_user = UserFactory(is_active=False, first_name="", last_name="", email="deleted-sus@deleted.invalid") + self.login_staff() + response = self.client.get("/console/users/?status=suspended") + self.assertNotIn(active_user, response.context["users"]) + self.assertIn(suspended_user, response.context["users"]) + self.assertNotIn(deleted_user, response.context["users"]) + def test_console_users_all_filter_shows_both(self): """?status=all shows both active and deleted users.""" active_user = UserFactory() diff --git a/home/views/console.py b/home/views/console.py index f5efbcfa..0eb0a710 100644 --- a/home/views/console.py +++ b/home/views/console.py @@ -89,6 +89,8 @@ def get_context_data(self, **kwargs): deleted_filter = {"email__endswith": f"@{DELETED_ACCOUNT_EMAIL_DOMAIN}"} if status == "deleted": qs = qs.filter(**deleted_filter) + elif status == "suspended": + qs = qs.filter(is_active=False).exclude(**deleted_filter) elif status == "all": pass else: From 4f2152824019a3d920e8d7e67280e658b13b0b13 Mon Sep 17 00:00:00 2001 From: Joe Heffer Date: Wed, 8 Jul 2026 15:30:24 +0100 Subject: [PATCH 5/5] fix: use escapejs filter for email in JS string literal viewed_user.email was interpolated into a JS string comparison relying on HTML auto-escaping rather than JS-context escaping. Low risk since the field is EmailField-constrained, but escapejs is the correct filter here and for future reuse of this confirmation pattern. Co-Authored-By: Claude Sonnet 5 --- home/templates/console/suspend_user_confirm.html | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/home/templates/console/suspend_user_confirm.html b/home/templates/console/suspend_user_confirm.html index 3b9aac66..17408275 100644 --- a/home/templates/console/suspend_user_confirm.html +++ b/home/templates/console/suspend_user_confirm.html @@ -42,7 +42,7 @@

Suspend account