Skip to content
7 changes: 6 additions & 1 deletion SORT/settings.py
Original file line number Diff line number Diff line change
Expand Up @@ -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",
)

Expand Down
6 changes: 4 additions & 2 deletions home/constants.py
Original file line number Diff line number Diff line change
Expand Up @@ -34,5 +34,7 @@ class OrganisationMembershipRole:
(PERMISSION_EDIT, "View and Edit"),
]

# Email domain used to mark accounts anonymised for GDPR erasure (see UserService.anonymise)
DELETED_EMAIL_DOMAIN = "deleted.invalid"
# Email domain used to anonymise a user's address on GDPR erasure (see
# UserService.anonymise). Used to distinguish erased accounts from merely
# suspended ones, since both share is_active=False.
DELETED_ACCOUNT_EMAIL_DOMAIN = "deleted.invalid"
11 changes: 5 additions & 6 deletions home/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@
from django.dispatch import receiver
from django.urls import reverse

from .constants import DELETED_EMAIL_DOMAIN, ROLE_ADMIN, ROLE_PROJECT_MANAGER, ROLES
from .constants import DELETED_ACCOUNT_EMAIL_DOMAIN, ROLE_ADMIN, ROLE_PROJECT_MANAGER, ROLES


class UserManager(BaseUserManager):
Expand Down Expand Up @@ -59,11 +59,10 @@ def __str__(self):

@property
def is_deleted(self) -> bool:
"""
Whether this account has been anonymised for GDPR erasure (see UserService.anonymise),
as opposed to just having a blank name.
"""
return self.email.endswith(f"@{DELETED_EMAIL_DOMAIN}")
"""True once this account has been anonymised via GDPR erasure (see
UserService.anonymise), as opposed to merely suspended — both set
is_active=False."""
return self.email.endswith(f"@{DELETED_ACCOUNT_EMAIL_DOMAIN}")

@property
def active_projects(self) -> int:
Expand Down
4 changes: 2 additions & 2 deletions home/services/user.py
Original file line number Diff line number Diff line change
@@ -1,14 +1,14 @@
import uuid

from ..constants import DELETED_EMAIL_DOMAIN
from ..constants import DELETED_ACCOUNT_EMAIL_DOMAIN
from ..models import OrganisationMembership, User


class UserService:
def anonymise(self, user: User) -> None:
user.first_name = "Deleted"
user.last_name = "User"
user.email = f"deleted-{uuid.uuid4().hex}@{DELETED_EMAIL_DOMAIN}"
user.email = f"deleted-{uuid.uuid4().hex}@{DELETED_ACCOUNT_EMAIL_DOMAIN}"
user.is_active = False
user.set_unusable_password()
user.save()
Expand Down
53 changes: 53 additions & 0 deletions home/templates/console/suspend_user_confirm.html
Original file line number Diff line number Diff line change
@@ -0,0 +1,53 @@
{% extends "base_console.html" %}
{% block title %}| Admin — Suspend account{% endblock %}

{% block content %}
<div class="container py-4">

<nav aria-label="breadcrumb" class="mb-3">
<ol class="breadcrumb">
<li class="breadcrumb-item"><a href="{% url 'admin_users' %}">Users</a></li>
<li class="breadcrumb-item"><a href="{% url 'admin_user_detail' viewed_user.pk %}">{{ viewed_user }}</a></li>
<li class="breadcrumb-item active" aria-current="page">Suspend account</li>
</ol>
</nav>

<div class="row justify-content-center">
<div class="col-md-6">
<div class="card border-danger shadow-sm">
<div class="card-header bg-white text-danger">
<h1 class="h5 mb-0">Suspend account</h1>
</div>
<div class="card-body">
<p>Are you sure you want to suspend <strong>{{ viewed_user }}</strong>?</p>
<p class="text-muted small">
While suspended, this user cannot log in or submit data. Their personal data
and survey responses are <strong>retained, not deleted</strong>, and you can
lift the suspension at any time.
</p>
<div class="mt-3">
<label for="confirm-input" class="form-label small">
To confirm, type <strong>{{ viewed_user.email }}</strong> below:
</label>
<input type="text" id="confirm-input" class="form-control form-control-sm" autocomplete="off">
</div>
</div>
<div class="card-footer bg-white d-flex gap-2 justify-content-end">
<a href="{% url 'admin_user_detail' viewed_user.pk %}" class="btn btn-outline-secondary">Cancel</a>
<form method="post">
{% csrf_token %}
<button type="submit" id="confirm-submit" class="btn btn-danger" disabled>Suspend account</button>
</form>
</div>
<script>
document.getElementById('confirm-input').addEventListener('input', function () {
document.getElementById('confirm-submit').disabled =
this.value !== '{{ viewed_user.email|escapejs }}';
});
</script>
</div>
</div>
</div>

</div>
{% endblock %}
15 changes: 13 additions & 2 deletions home/templates/console/user_detail.html
Original file line number Diff line number Diff line change
Expand Up @@ -23,8 +23,19 @@ <h1 class="h3 mb-1">{{ viewed_user }}</h1>
{% if viewed_user.is_superuser %}
<span class="badge bg-danger">Superuser</span>
{% endif %}
{% if not viewed_user.is_active %}
<span class="badge bg-secondary">Inactive</span>
{% if viewed_user.is_deleted %}
<span class="badge bg-secondary">Deleted</span>
{% elif not viewed_user.is_active %}
<span class="badge bg-secondary">Suspended</span>
{% endif %}
{% if viewed_user.is_deleted %}
{% elif not viewed_user.is_active %}
<form method="post" action="{% url 'admin_unsuspend_user' viewed_user.pk %}">
{% csrf_token %}
<button type="submit" class="btn btn-outline-success btn-sm">Lift suspension</button>
</form>
{% elif viewed_user != request.user and not viewed_user.is_superuser %}
<a href="{% url 'admin_suspend_user' viewed_user.pk %}" class="btn btn-outline-danger btn-sm">Suspend account</a>
{% endif %}
{% if viewed_user.is_active and not viewed_user.is_staff and not viewed_user.is_superuser and viewed_user != request.user %}
<a href="{% url 'admin_delete_user' viewed_user.pk %}" class="btn btn-sm btn-outline-danger">Delete user</a>
Expand Down
16 changes: 12 additions & 4 deletions home/templates/console/users.html
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ <h1 class="h3 mb-4">Users</h1>
<div class="d-flex gap-2 align-items-center">
<span class="text-muted text-nowrap">Status:</span>
<a href="?status=active" class="btn btn-sm {% if status_filter == 'active' %}btn-secondary{% else %}btn-outline-secondary{% endif %}">Active</a>
<a href="?status=suspended" class="btn btn-sm {% if status_filter == 'suspended' %}btn-secondary{% else %}btn-outline-secondary{% endif %}">Suspended</a>
<a href="?status=deleted" class="btn btn-sm {% if status_filter == 'deleted' %}btn-secondary{% else %}btn-outline-secondary{% endif %}">Deleted</a>
<a href="?status=all" class="btn btn-sm {% if status_filter == 'all' %}btn-secondary{% else %}btn-outline-secondary{% endif %}">All</a>
</div>
Expand All @@ -21,6 +22,7 @@ <h1 class="h3 mb-4">Users</h1>
<tr>
<th>Name</th>
<th>Email</th>
<th>Status</th>
<th>Organisations</th>
<th>Joined</th>
</tr>
Expand All @@ -38,17 +40,23 @@ <h1 class="h3 mb-4">Users</h1>
<em>(no name set)</em>
{% endif %}
</a>
{% if not user.is_active %}
<span class="badge bg-secondary ms-1">Deleted</span>
{% endif %}
</td>
<td>{{ user.email }}</td>
<td>
{% if user.is_active %}
<span class="badge bg-success">Active</span>
{% elif user.is_deleted %}
<span class="badge bg-secondary">Deleted</span>
{% else %}
<span class="badge bg-secondary">Suspended</span>
{% endif %}
</td>
<td>{{ user.organisationmembership_set.count }}</td>
<td>{{ user.date_joined|date:"d M Y" }}</td>
</tr>
{% empty %}
<tr>
<td colspan="4" class="text-muted">No users</td>
<td colspan="5" class="text-muted">No users</td>
</tr>
{% endfor %}
</tbody>
Expand Down
94 changes: 93 additions & 1 deletion home/tests/test_console_views.py
Original file line number Diff line number Diff line change
Expand Up @@ -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


Expand Down Expand Up @@ -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()
Expand Down Expand Up @@ -261,6 +272,87 @@ def test_console_remove_member_post_removes_membership(self):
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_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)
self.login_staff()
response = self.client.get("/console/users/")
self.assertContains(response, suspended.email)
self.assertContains(response, "Suspended")

# -- Delete (anonymise) user ---------------------------------------------

def test_delete_user_get_shows_confirmation(self):
"""Staff users see the delete confirmation page for a regular user."""
target = UserFactory()
Expand Down
23 changes: 23 additions & 0 deletions home/tests/test_user_views.py
Original file line number Diff line number Diff line change
Expand Up @@ -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):
Expand All @@ -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")
10 changes: 10 additions & 0 deletions home/urls.py
Original file line number Diff line number Diff line change
Expand Up @@ -160,6 +160,16 @@
path("console/users/", views.ConsoleUserListView.as_view(), name="admin_users"),
path("console/users/<int:pk>/", views.ConsoleUserDetailView.as_view(), name="admin_user_detail"),
path("console/users/<int:pk>/delete/", views.ConsoleDeleteUserView.as_view(), name="admin_delete_user"),
path(
"console/users/<int:pk>/suspend/",
views.ConsoleSuspendUserView.as_view(),
name="admin_suspend_user",
),
path(
"console/users/<int:pk>/unsuspend/",
views.ConsoleUnsuspendUserView.as_view(),
name="admin_unsuspend_user",
),
path(
"console/surveys/", views.ConsoleSurveyListView.as_view(), name="admin_surveys"
),
Expand Down
4 changes: 4 additions & 0 deletions home/views/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,8 @@
ConsoleProjectDetailView,
ConsoleProjectListView,
ConsoleRemoveMemberView,
ConsoleSuspendUserView,
ConsoleUnsuspendUserView,
ConsoleUserDetailView,
)
from .project import (
Expand Down Expand Up @@ -91,6 +93,8 @@
"ConsoleProjectDetailView",
"ConsoleProjectListView",
"ConsoleRemoveMemberView",
"ConsoleSuspendUserView",
"ConsoleUnsuspendUserView",
"ConsoleUserDetailView",
"ConsoleDataProtectionLogView",
]
Loading
Loading