From 8134cc8fcd60f9fbc45d8ed0e0ef464f1115dd86 Mon Sep 17 00:00:00 2001 From: James Paul Turner Date: Fri, 20 Sep 2024 11:46:52 +0100 Subject: [PATCH 1/9] Add and use custom permissions. --- main/apps.py | 4 ++++ main/permissions.py | 30 ++++++++++++++++++++++++++++++ main/views.py | 5 ++++- 3 files changed, 38 insertions(+), 1 deletion(-) create mode 100644 main/permissions.py diff --git a/main/apps.py b/main/apps.py index ba3f08d..631af37 100644 --- a/main/apps.py +++ b/main/apps.py @@ -8,3 +8,7 @@ class MainConfig(AppConfig): default_auto_field = "django.db.models.BigAutoField" name = "main" + + def ready(self) -> None: + """Import permissions to ensure they are registered.""" + import main.permissions # noqa: F401 diff --git a/main/permissions.py b/main/permissions.py new file mode 100644 index 0000000..eeeb1e0 --- /dev/null +++ b/main/permissions.py @@ -0,0 +1,30 @@ +"""The signals module for the main app.""" + +from django.contrib.auth.models import Permission +from django.contrib.contenttypes.models import ContentType +from django.db.models.signals import post_migrate +from django.dispatch import receiver + + +@receiver(post_migrate) +def create_custom_permissions(**kwargs): + """Create custom permissions for the app.""" + content_type = ContentType.objects.get_for_model(Permission) + + Permission.objects.get_or_create( + codename="can_restart_processes", + name="Can restart processes", + content_type=content_type, + ) + + Permission.objects.get_or_create( + codename="can_flush_processes", + name="Can flush processes", + content_type=content_type, + ) + + Permission.objects.get_or_create( + codename="can_kill_processes", + name="Can kill processes", + content_type=content_type, + ) diff --git a/main/views.py b/main/views.py index 4ea9d56..f8ec81f 100644 --- a/main/views.py +++ b/main/views.py @@ -5,7 +5,7 @@ from enum import Enum import django_tables2 -from django.contrib.auth.decorators import login_required +from django.contrib.auth.decorators import login_required, permission_required from django.contrib.auth.mixins import LoginRequiredMixin from django.http import HttpRequest, HttpResponse, HttpResponseRedirect from django.shortcuts import render @@ -100,6 +100,7 @@ async def _process_call(uuid: str, action: ProcessAction) -> None: @login_required +@permission_required("main.can_restart_process", raise_exception=True) def restart_process(request: HttpRequest, uuid: uuid.UUID) -> HttpResponse: """Restart the process associated to the given UUID. @@ -116,6 +117,7 @@ def restart_process(request: HttpRequest, uuid: uuid.UUID) -> HttpResponse: @login_required +@permission_required("main.can_kill_process", raise_exception=True) def kill_process(request: HttpRequest, uuid: uuid.UUID) -> HttpResponse: """Kill the process associated to the given UUID. @@ -131,6 +133,7 @@ def kill_process(request: HttpRequest, uuid: uuid.UUID) -> HttpResponse: @login_required +@permission_required("main.can_flush_process", raise_exception=True) def flush_process(request: HttpRequest, uuid: uuid.UUID) -> HttpResponse: """Flush the process associated to the given UUID. From cb789a392efbff00f658fc54f676150ea7310d91 Mon Sep 17 00:00:00 2001 From: James Paul Turner Date: Fri, 20 Sep 2024 12:07:42 +0100 Subject: [PATCH 2/9] Permissions typo. --- main/views.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/main/views.py b/main/views.py index f8ec81f..ef7ed7f 100644 --- a/main/views.py +++ b/main/views.py @@ -100,7 +100,7 @@ async def _process_call(uuid: str, action: ProcessAction) -> None: @login_required -@permission_required("main.can_restart_process", raise_exception=True) +@permission_required("can_restart_processes", raise_exception=True) def restart_process(request: HttpRequest, uuid: uuid.UUID) -> HttpResponse: """Restart the process associated to the given UUID. @@ -117,7 +117,7 @@ def restart_process(request: HttpRequest, uuid: uuid.UUID) -> HttpResponse: @login_required -@permission_required("main.can_kill_process", raise_exception=True) +@permission_required("main.can_kill_processes", raise_exception=True) def kill_process(request: HttpRequest, uuid: uuid.UUID) -> HttpResponse: """Kill the process associated to the given UUID. @@ -133,7 +133,7 @@ def kill_process(request: HttpRequest, uuid: uuid.UUID) -> HttpResponse: @login_required -@permission_required("main.can_flush_process", raise_exception=True) +@permission_required("main.can_flush_processes", raise_exception=True) def flush_process(request: HttpRequest, uuid: uuid.UUID) -> HttpResponse: """Flush the process associated to the given UUID. From 34be3cc0bb438076324a58046326fb23f31e6109 Mon Sep 17 00:00:00 2001 From: James Paul Turner Date: Fri, 20 Sep 2024 12:13:36 +0100 Subject: [PATCH 3/9] Use correct permission namespace. --- main/views.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/main/views.py b/main/views.py index ef7ed7f..4b4ec73 100644 --- a/main/views.py +++ b/main/views.py @@ -100,7 +100,7 @@ async def _process_call(uuid: str, action: ProcessAction) -> None: @login_required -@permission_required("can_restart_processes", raise_exception=True) +@permission_required("auth.can_restart_processes", raise_exception=True) def restart_process(request: HttpRequest, uuid: uuid.UUID) -> HttpResponse: """Restart the process associated to the given UUID. @@ -117,7 +117,7 @@ def restart_process(request: HttpRequest, uuid: uuid.UUID) -> HttpResponse: @login_required -@permission_required("main.can_kill_processes", raise_exception=True) +@permission_required("auth.can_kill_processes", raise_exception=True) def kill_process(request: HttpRequest, uuid: uuid.UUID) -> HttpResponse: """Kill the process associated to the given UUID. @@ -133,7 +133,7 @@ def kill_process(request: HttpRequest, uuid: uuid.UUID) -> HttpResponse: @login_required -@permission_required("main.can_flush_processes", raise_exception=True) +@permission_required("auth.can_flush_processes", raise_exception=True) def flush_process(request: HttpRequest, uuid: uuid.UUID) -> HttpResponse: """Flush the process associated to the given UUID. From 42e6862fe900ec2d4e1a8767db4e633cdb487344 Mon Sep 17 00:00:00 2001 From: James Paul Turner Date: Fri, 20 Sep 2024 13:41:08 +0100 Subject: [PATCH 4/9] Move permissions to user model; --- main/apps.py | 4 ---- main/models.py | 11 +++++++++++ main/permissions.py | 30 ------------------------------ main/views.py | 6 +++--- 4 files changed, 14 insertions(+), 37 deletions(-) delete mode 100644 main/permissions.py diff --git a/main/apps.py b/main/apps.py index 631af37..ba3f08d 100644 --- a/main/apps.py +++ b/main/apps.py @@ -8,7 +8,3 @@ class MainConfig(AppConfig): default_auto_field = "django.db.models.BigAutoField" name = "main" - - def ready(self) -> None: - """Import permissions to ensure they are registered.""" - import main.permissions # noqa: F401 diff --git a/main/models.py b/main/models.py index 4a77c1a..9e2740c 100644 --- a/main/models.py +++ b/main/models.py @@ -1,8 +1,19 @@ """Models module for the main app.""" +from typing import ClassVar + from django.contrib.auth.models import AbstractUser from django.db import models # noqa: F401 class User(AbstractUser): """Custom user model for this project.""" + + class Meta: + """Meta class for the User model.""" + + permissions: ClassVar = [ + ("can_restart_processes", "Can restart processes"), + ("can_flush_processes", "Can flush processes"), + ("can_kill_processes", "Can kill processes"), + ] diff --git a/main/permissions.py b/main/permissions.py deleted file mode 100644 index eeeb1e0..0000000 --- a/main/permissions.py +++ /dev/null @@ -1,30 +0,0 @@ -"""The signals module for the main app.""" - -from django.contrib.auth.models import Permission -from django.contrib.contenttypes.models import ContentType -from django.db.models.signals import post_migrate -from django.dispatch import receiver - - -@receiver(post_migrate) -def create_custom_permissions(**kwargs): - """Create custom permissions for the app.""" - content_type = ContentType.objects.get_for_model(Permission) - - Permission.objects.get_or_create( - codename="can_restart_processes", - name="Can restart processes", - content_type=content_type, - ) - - Permission.objects.get_or_create( - codename="can_flush_processes", - name="Can flush processes", - content_type=content_type, - ) - - Permission.objects.get_or_create( - codename="can_kill_processes", - name="Can kill processes", - content_type=content_type, - ) diff --git a/main/views.py b/main/views.py index 4b4ec73..1b3050d 100644 --- a/main/views.py +++ b/main/views.py @@ -100,7 +100,7 @@ async def _process_call(uuid: str, action: ProcessAction) -> None: @login_required -@permission_required("auth.can_restart_processes", raise_exception=True) +@permission_required("main.can_restart_processes", raise_exception=True) def restart_process(request: HttpRequest, uuid: uuid.UUID) -> HttpResponse: """Restart the process associated to the given UUID. @@ -117,7 +117,7 @@ def restart_process(request: HttpRequest, uuid: uuid.UUID) -> HttpResponse: @login_required -@permission_required("auth.can_kill_processes", raise_exception=True) +@permission_required("main.can_kill_processes", raise_exception=True) def kill_process(request: HttpRequest, uuid: uuid.UUID) -> HttpResponse: """Kill the process associated to the given UUID. @@ -133,7 +133,7 @@ def kill_process(request: HttpRequest, uuid: uuid.UUID) -> HttpResponse: @login_required -@permission_required("auth.can_flush_processes", raise_exception=True) +@permission_required("main.can_flush_processes", raise_exception=True) def flush_process(request: HttpRequest, uuid: uuid.UUID) -> HttpResponse: """Flush the process associated to the given UUID. From baed935eccaf820ddb48371a8de1e677b2358509 Mon Sep 17 00:00:00 2001 From: James Paul Turner Date: Fri, 20 Sep 2024 14:51:17 +0100 Subject: [PATCH 5/9] Add remaining permissions and test. --- main/migrations/0002_alter_user_options.py | 17 ++++++++++ main/models.py | 2 ++ main/views.py | 6 ++-- tests/conftest.py | 22 ++++++++++++- tests/test_views.py | 37 +++++++++++++++------- 5 files changed, 70 insertions(+), 14 deletions(-) create mode 100644 main/migrations/0002_alter_user_options.py diff --git a/main/migrations/0002_alter_user_options.py b/main/migrations/0002_alter_user_options.py new file mode 100644 index 0000000..8b4ed6d --- /dev/null +++ b/main/migrations/0002_alter_user_options.py @@ -0,0 +1,17 @@ +# Generated by Django 5.1.1 on 2024-09-20 13:49 + +from django.db import migrations + + +class Migration(migrations.Migration): + + dependencies = [ + ('main', '0001_initial'), + ] + + operations = [ + migrations.AlterModelOptions( + name='user', + options={'permissions': [('can_boot_processes', 'Can boot processes'), ('can_restart_processes', 'Can restart processes'), ('can_flush_processes', 'Can flush processes'), ('can_kill_processes', 'Can kill processes'), ('can_view_process_logs', 'Can view process logs')]}, + ), + ] diff --git a/main/models.py b/main/models.py index 9e2740c..33a74f5 100644 --- a/main/models.py +++ b/main/models.py @@ -13,7 +13,9 @@ class Meta: """Meta class for the User model.""" permissions: ClassVar = [ + ("can_boot_processes", "Can boot processes"), ("can_restart_processes", "Can restart processes"), ("can_flush_processes", "Can flush processes"), ("can_kill_processes", "Can kill processes"), + ("can_view_process_logs", "Can view process logs"), ] diff --git a/main/views.py b/main/views.py index 1b3050d..0155625 100644 --- a/main/views.py +++ b/main/views.py @@ -6,7 +6,7 @@ import django_tables2 from django.contrib.auth.decorators import login_required, permission_required -from django.contrib.auth.mixins import LoginRequiredMixin +from django.contrib.auth.mixins import PermissionRequiredMixin from django.http import HttpRequest, HttpResponse, HttpResponseRedirect from django.shortcuts import render from django.urls import reverse, reverse_lazy @@ -164,6 +164,7 @@ async def _get_process_logs(uuid: str) -> list[DecodedResponse]: @login_required +@permission_required("main.can_view_process_logs", raise_exception=True) def logs(request: HttpRequest, uuid: uuid.UUID) -> HttpResponse: """Display the logs of a process. @@ -191,12 +192,13 @@ async def _boot_process(user: str, data: dict[str, str | int]) -> None: pass -class BootProcessView(LoginRequiredMixin, FormView): # type: ignore [type-arg] +class BootProcessView(PermissionRequiredMixin, FormView): # type: ignore [type-arg] """View for the BootProcess form.""" template_name = "main/boot_process.html" form_class = BootProcessForm success_url = reverse_lazy("main:index") + permission_required = "main.can_boot_processes" def form_valid(self, form: BootProcessForm) -> HttpResponse: """Boot a Process when valid form data has been POSTed. diff --git a/tests/conftest.py b/tests/conftest.py index 9fbbe5e..15c14e4 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -1,13 +1,33 @@ """Configuration for pytest.""" import pytest +from django.contrib.auth.models import Permission from django.test import Client @pytest.fixture def auth_client(django_user_model) -> Client: """Return an authenticated client.""" - user = django_user_model.objects.create(username="testuser") + user = django_user_model.objects.create(username="auth_user") + client = Client() + client.force_login(user) + return client + + +@pytest.fixture +def privileged_client(django_user_model) -> Client: + """Return a privileged authenticated client.""" + user = django_user_model.objects.create(username="privileged_user") + permission = Permission.objects.get(codename="can_boot_processes") + user.user_permissions.add(permission) + permission = Permission.objects.get(codename="can_restart_processes") + user.user_permissions.add(permission) + permission = Permission.objects.get(codename="can_flush_processes") + user.user_permissions.add(permission) + permission = Permission.objects.get(codename="can_kill_processes") + user.user_permissions.add(permission) + permission = Permission.objects.get(codename="can_view_process_logs") + user.user_permissions.add(permission) client = Client() client.force_login(user) return client diff --git a/tests/test_views.py b/tests/test_views.py index 7a8c874..2114134 100644 --- a/tests/test_views.py +++ b/tests/test_views.py @@ -31,7 +31,7 @@ def test_index(client, auth_client, admin_client, mocker): assertContains(response, "Boot") -def test_logs(client, auth_client, mocker): +def test_logs(client, auth_client, privileged_client, mocker): """Test the logs view.""" mock = mocker.patch("main.views._get_process_logs") @@ -42,16 +42,20 @@ def test_logs(client, auth_client, mocker): assert response.status_code == HTTPStatus.FOUND assertRedirects(response, f"/accounts/login/?next=/logs/{uuid}") - # Test with an authenticated client. + # Test with an unprivileged authenticated client. + response = auth_client.get(reverse("main:logs", kwargs=dict(uuid=uuid))) + assert response.status_code == HTTPStatus.FORBIDDEN + + # Test with a privileged authenticated client. with assertTemplateUsed(template_name="main/logs.html"): - response = auth_client.get(reverse("main:logs", kwargs=dict(uuid=uuid))) + response = privileged_client.get(reverse("main:logs", kwargs=dict(uuid=uuid))) assert response.status_code == HTTPStatus.OK mock.assert_called_once_with(str(uuid)) assert "log_text" in response.context -def test_process_flush(client, auth_client, mocker): +def test_process_flush(client, auth_client, privileged_client, mocker): """Test the process_flush view.""" mock = mocker.patch("main.views._process_call") @@ -62,8 +66,12 @@ def test_process_flush(client, auth_client, mocker): assert response.status_code == HTTPStatus.FOUND assertRedirects(response, f"/accounts/login/?next=/flush/{uuid}") - # Test with an authenticated client. + # Test with an unprivileged authenticated client. response = auth_client.get(reverse("main:flush", kwargs=dict(uuid=uuid))) + assert response.status_code == HTTPStatus.FORBIDDEN + + # Test with a privileged authenticated client. + response = privileged_client.get(reverse("main:flush", kwargs=dict(uuid=uuid))) assert response.status_code == HTTPStatus.FOUND assert response.url == reverse("main:index") mock.assert_called_once_with(str(uuid), ProcessAction.FLUSH) @@ -74,10 +82,10 @@ class TestBootProcess: template_name = "main/boot_process.html" - def test_boot_process_get(self, auth_client): + def test_boot_process_get(self, privileged_client): """Test the GET request for the BootProcess view.""" with assertTemplateUsed(template_name=self.template_name): - response = auth_client.get(reverse("main:boot_process")) + response = privileged_client.get(reverse("main:boot_process")) assert response.status_code == HTTPStatus.OK assert "form" in response.context @@ -89,18 +97,25 @@ def test_boot_process_get_anon(self, client): assert response.status_code == HTTPStatus.FOUND assertRedirects(response, "/accounts/login/?next=/boot_process/") - def test_boot_process_post_invalid(self, auth_client): + def test_boot_process_get_unprivileged(self, auth_client): + """Test the GET request for the BootProcess view with a privileged client.""" + response = auth_client.get(reverse("main:boot_process")) + assert response.status_code == HTTPStatus.FORBIDDEN + + def test_boot_process_post_invalid(self, privileged_client): """Test the POST request for the BootProcess view with invalid data.""" with assertTemplateUsed(template_name=self.template_name): - response = auth_client.post(reverse("main:boot_process"), data=dict()) + response = privileged_client.post(reverse("main:boot_process"), data=dict()) assert response.status_code == HTTPStatus.OK assert "form" in response.context - def test_boot_process_post_valid(self, auth_client, mocker, dummy_session_data): + def test_boot_process_post_valid( + self, privileged_client, mocker, dummy_session_data + ): """Test the POST request for the BootProcess view.""" mock = mocker.patch("main.views._boot_process") - response = auth_client.post( + response = privileged_client.post( reverse("main:boot_process"), data=dummy_session_data ) assert response.status_code == HTTPStatus.FOUND From 0ac52a4c6a9bdd936db1d68ff56d915cab9e3763 Mon Sep 17 00:00:00 2001 From: James Paul Turner Date: Fri, 20 Sep 2024 14:54:20 +0100 Subject: [PATCH 6/9] typo. --- tests/test_views.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/test_views.py b/tests/test_views.py index 2114134..0821300 100644 --- a/tests/test_views.py +++ b/tests/test_views.py @@ -98,7 +98,7 @@ def test_boot_process_get_anon(self, client): assertRedirects(response, "/accounts/login/?next=/boot_process/") def test_boot_process_get_unprivileged(self, auth_client): - """Test the GET request for the BootProcess view with a privileged client.""" + """Test the GET request for the BootProcess view with an unprivileged client.""" response = auth_client.get(reverse("main:boot_process")) assert response.status_code == HTTPStatus.FORBIDDEN From 21e36b4a49e8b1986580a5322940bb1e0f635831 Mon Sep 17 00:00:00 2001 From: James Paul Turner Date: Fri, 20 Sep 2024 19:33:01 +0100 Subject: [PATCH 7/9] Merge main into permissions. --- .github/workflows/ci.yml | 9 +- .vscode/settings.json | 5 +- README.md | 9 +- codecov.yml | 12 +++ dune_processes/settings/settings.py | 4 + main/templates/main/boot_process.html | 6 +- poetry.lock | 34 +++++- pyproject.toml | 2 + tests/conftest.py | 9 -- tests/test_views.py | 148 ++++++++++++++------------ 10 files changed, 153 insertions(+), 85 deletions(-) create mode 100644 codecov.yml diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index e45330a..9c4ec72 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -31,4 +31,11 @@ jobs: run: poetry install - name: Run tests - run: poetry run pytest + run: poetry run pytest --cov-report=xml + + - name: Upload coverage to Codecov + uses: codecov/codecov-action@v4 + with: + fail_ci_if_error: true + env: + CODECOV_TOKEN: ${{ secrets.CODECOV_TOKEN }} diff --git a/.vscode/settings.json b/.vscode/settings.json index d4817cc..91dd871 100644 --- a/.vscode/settings.json +++ b/.vscode/settings.json @@ -7,5 +7,8 @@ 88 ], "python.testing.unittestEnabled": false, - "python.testing.pytestEnabled": true + "python.testing.pytestEnabled": true, + "python.testing.pytestArgs": [ + "--no-cov" + ] } diff --git a/README.md b/README.md index 0c4d26a..d072760 100644 --- a/README.md +++ b/README.md @@ -1,6 +1,11 @@ -# dune_processes + +[![GitHub](https://img.shields.io/github/license/ImperialCollegeLondon/dune_processes)](https://raw.githubusercontent.com/ImperialCollegeLondon/dune_processes/main/LICENSE) +[![Test and build](https://github.com/ImperialCollegeLondon/dune_processes/actions/workflows/ci.yml/badge.svg)](https://github.com/ImperialCollegeLondon/dune_processes/actions/workflows/ci.yml) +[![codecov](https://codecov.io/gh/ImperialCollegeLondon/dune_processes/graph/badge.svg?token=PG0WTYF8EY)](https://codecov.io/gh/ImperialCollegeLondon/dune_processes) -This repo defines the web app for the Dune Process Manager web interface. +# DUNE processes web interface + +This repo defines the web interface for the DUNE Process Manager. ## For developers diff --git a/codecov.yml b/codecov.yml new file mode 100644 index 0000000..9474a7b --- /dev/null +++ b/codecov.yml @@ -0,0 +1,12 @@ +# Don't fail CI if coverage drops +coverage: + status: + project: + default: + informational: true + patch: + default: + informational: true + +ignore: + - dune_processes/settings/_production.py diff --git a/dune_processes/settings/settings.py b/dune_processes/settings/settings.py index 169bace..1c47f23 100644 --- a/dune_processes/settings/settings.py +++ b/dune_processes/settings/settings.py @@ -136,3 +136,7 @@ INSTALLED_APPS += ["django_bootstrap5"] DJANGO_TABLES2_TEMPLATE = "django_tables2/bootstrap5.html" + +INSTALLED_APPS += ["crispy_forms", "crispy_bootstrap5"] +CRISPY_ALLOWED_TEMPLATE_PACKS = "bootstrap5" +CRISPY_TEMPLATE_PACK = "bootstrap5" diff --git a/main/templates/main/boot_process.html b/main/templates/main/boot_process.html index c9a527b..8a77372 100644 --- a/main/templates/main/boot_process.html +++ b/main/templates/main/boot_process.html @@ -1,4 +1,6 @@ {% extends "main/base.html" %} +{% load crispy_forms_tags %} +{% load django_bootstrap5 %} {% block title %}Boot Process{% endblock title %} @@ -6,8 +8,8 @@
{% csrf_token %} - {{ form }} - + {{ form|crispy }} + {% bootstrap_button button_type="submit" content="Submit" %}
{% endblock content %} diff --git a/poetry.lock b/poetry.lock index 0a8a6ee..d792e11 100644 --- a/poetry.lock +++ b/poetry.lock @@ -171,6 +171,24 @@ files = [ [package.extras] toml = ["tomli"] +[[package]] +name = "crispy-bootstrap5" +version = "2024.2" +description = "Bootstrap5 template pack for django-crispy-forms" +optional = false +python-versions = ">=3.8" +files = [ + {file = "crispy-bootstrap5-2024.2.tar.gz", hash = "sha256:7d1fa40c6faf472e30e85c72551a3d2c9eedbf0abfff920683315e4e6f670f2b"}, + {file = "crispy_bootstrap5-2024.2-py3-none-any.whl", hash = "sha256:3867e320920a6ef156e94f9e0f06a80344c453e1b3bd96cd9dc0522ae9e9afb8"}, +] + +[package.dependencies] +django = ">=4.2" +django-crispy-forms = ">=2" + +[package.extras] +test = ["pytest", "pytest-django"] + [[package]] name = "distlib" version = "0.3.8" @@ -216,6 +234,20 @@ files = [ [package.dependencies] Django = ">=4.2" +[[package]] +name = "django-crispy-forms" +version = "2.3" +description = "Best way to have Django DRY forms" +optional = false +python-versions = ">=3.8" +files = [ + {file = "django_crispy_forms-2.3-py3-none-any.whl", hash = "sha256:efc4c31e5202bbec6af70d383a35e12fc80ea769d464fb0e7fe21768bb138a20"}, + {file = "django_crispy_forms-2.3.tar.gz", hash = "sha256:2db17ae08527201be1273f0df789e5f92819e23dd28fec69cffba7f3762e1a38"}, +] + +[package.dependencies] +django = ">=4.2" + [[package]] name = "django-stubs" version = "5.0.4" @@ -1089,4 +1121,4 @@ brotli = ["brotli"] [metadata] lock-version = "2.0" python-versions = "^3.11" -content-hash = "fe31c0b3c18f9e2201d7dd942661dc7ad7f44f5a0fcb9aa4d359ff1256f6f2e4" +content-hash = "7f1bab17ac1f71ae00794b7356c8a8102b0ac92309a00d3b886191a70c704aa2" diff --git a/pyproject.toml b/pyproject.toml index 4654d47..45de664 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -16,6 +16,8 @@ druncschema = { git = "https://github.com/DUNE-DAQ/druncschema.git" } django-tables2 = "^2.7.0" django-bootstrap5 = "^24.2" pytest-asyncio = "^0.24.0" +django-crispy-forms = "^2.3" +crispy-bootstrap5 = "^2024.2" [tool.poetry.group.dev.dependencies] pytest = "^8.3" diff --git a/tests/conftest.py b/tests/conftest.py index 15c14e4..4a881f5 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -8,15 +8,6 @@ @pytest.fixture def auth_client(django_user_model) -> Client: """Return an authenticated client.""" - user = django_user_model.objects.create(username="auth_user") - client = Client() - client.force_login(user) - return client - - -@pytest.fixture -def privileged_client(django_user_model) -> Client: - """Return a privileged authenticated client.""" user = django_user_model.objects.create(username="privileged_user") permission = Permission.objects.get(codename="can_boot_processes") user.user_permissions.add(permission) diff --git a/tests/test_views.py b/tests/test_views.py index 0821300..eada1e6 100644 --- a/tests/test_views.py +++ b/tests/test_views.py @@ -8,114 +8,124 @@ from main.views import ProcessAction -def test_index(client, auth_client, admin_client, mocker): - """Test the index view.""" - mocker.patch("main.views.get_session_info") +class LoginRequiredTest: + """Tests for views that require authentication.""" - # Test with an anonymous client. - response = client.get(reverse("main:index")) - assert response.status_code == HTTPStatus.FOUND - assertRedirects(response, "/accounts/login/?next=/") + endpoint: str - # Test with an authenticated client. - with assertTemplateUsed(template_name="main/index.html"): - response = auth_client.get(reverse("main:index")) - assert response.status_code == HTTPStatus.OK + def test_login_redirect(self, client): + """Test that the view redirects to the login page.""" + response = client.get(self.endpoint) + assert response.status_code == HTTPStatus.FOUND - # Test with an admin client. - with assertTemplateUsed(template_name="main/index.html"): - response = admin_client.get(reverse("main:index")) - assert response.status_code == HTTPStatus.OK + assertRedirects(response, reverse("main:login") + f"?next={self.endpoint}") - assert "table" in response.context - assertContains(response, "Boot") +class ProcessActionsTest(LoginRequiredTest): + """Grouping the tests for the process action views.""" -def test_logs(client, auth_client, privileged_client, mocker): - """Test the logs view.""" - mock = mocker.patch("main.views._get_process_logs") + action: ProcessAction - uuid = uuid4() + @classmethod + def setup_class(cls): + """Set up the endpoint for the tests.""" + cls.uuid = uuid4() + cls.endpoint = reverse(f"main:{cls.action.value}", kwargs=dict(uuid=cls.uuid)) - # Test with an anonymous client. - response = client.get(reverse("main:logs", kwargs=dict(uuid=uuid))) - assert response.status_code == HTTPStatus.FOUND - assertRedirects(response, f"/accounts/login/?next=/logs/{uuid}") + def test_process_action_view_authenticated(self, auth_client, mocker): + """Test the process action view for an authenticated user.""" + mock = mocker.patch("main.views._process_call") + response = auth_client.get(self.endpoint) + assert response.status_code == HTTPStatus.FOUND + assert response.url == reverse("main:index") - # Test with an unprivileged authenticated client. - response = auth_client.get(reverse("main:logs", kwargs=dict(uuid=uuid))) - assert response.status_code == HTTPStatus.FORBIDDEN + mock.assert_called_once_with(str(self.uuid), self.action) - # Test with a privileged authenticated client. - with assertTemplateUsed(template_name="main/logs.html"): - response = privileged_client.get(reverse("main:logs", kwargs=dict(uuid=uuid))) - assert response.status_code == HTTPStatus.OK - mock.assert_called_once_with(str(uuid)) - assert "log_text" in response.context +class TestIndexView(LoginRequiredTest): + """Tests for the index view.""" + endpoint = reverse("main:index") -def test_process_flush(client, auth_client, privileged_client, mocker): - """Test the process_flush view.""" - mock = mocker.patch("main.views._process_call") + def test_index_view_authenticated(self, auth_client, mocker): + """Test the index view for an authenticated user.""" + mocker.patch("main.views.get_session_info") + with assertTemplateUsed(template_name="main/index.html"): + response = auth_client.get(self.endpoint) + assert response.status_code == HTTPStatus.OK + + def test_index_view_admin(self, admin_client, mocker): + """Test the index view for an admin user.""" + mocker.patch("main.views.get_session_info") + with assertTemplateUsed(template_name="main/index.html"): + response = admin_client.get(self.endpoint) + assert response.status_code == HTTPStatus.OK + assert "table" in response.context + assertContains(response, "Boot") + + +class TestLogsView(LoginRequiredTest): + """Tests for the logs view.""" uuid = uuid4() + endpoint = reverse("main:logs", kwargs=dict(uuid=uuid)) + + def test_logs_view_authenticated(self, auth_client, mocker): + """Test the logs view for an authenticated user.""" + mock = mocker.patch("main.views._get_process_logs") + with assertTemplateUsed(template_name="main/logs.html"): + response = auth_client.get(self.endpoint) + assert response.status_code == HTTPStatus.OK - # Test with an anonymous client. - response = client.get(reverse("main:flush", kwargs=dict(uuid=uuid))) - assert response.status_code == HTTPStatus.FOUND - assertRedirects(response, f"/accounts/login/?next=/flush/{uuid}") + mock.assert_called_once_with(str(self.uuid)) + assert "log_text" in response.context - # Test with an unprivileged authenticated client. - response = auth_client.get(reverse("main:flush", kwargs=dict(uuid=uuid))) - assert response.status_code == HTTPStatus.FORBIDDEN - # Test with a privileged authenticated client. - response = privileged_client.get(reverse("main:flush", kwargs=dict(uuid=uuid))) - assert response.status_code == HTTPStatus.FOUND - assert response.url == reverse("main:index") - mock.assert_called_once_with(str(uuid), ProcessAction.FLUSH) +class TestProcessFlushView(ProcessActionsTest): + """Tests for the process flush view.""" + action = ProcessAction.FLUSH -class TestBootProcess: + +class TestProcessKillView(ProcessActionsTest): + """Tests for the process kill view.""" + + action = ProcessAction.KILL + + +class TestProcessRestartView(ProcessActionsTest): + """Tests for the process restart view.""" + + action = ProcessAction.RESTART + + +class TestBootProcess(LoginRequiredTest): """Grouping the tests for the BootProcess view.""" template_name = "main/boot_process.html" + endpoint = reverse("main:boot_process") - def test_boot_process_get(self, privileged_client): + def test_boot_process_get(self, auth_client): """Test the GET request for the BootProcess view.""" with assertTemplateUsed(template_name=self.template_name): - response = privileged_client.get(reverse("main:boot_process")) + response = auth_client.get(reverse("main:boot_process")) assert response.status_code == HTTPStatus.OK assert "form" in response.context assertContains(response, f'form action="{reverse("main:boot_process")}"') - def test_boot_process_get_anon(self, client): - """Test the GET request for the BootProcess view with an anonymous client.""" - response = client.get(reverse("main:boot_process")) - assert response.status_code == HTTPStatus.FOUND - assertRedirects(response, "/accounts/login/?next=/boot_process/") - - def test_boot_process_get_unprivileged(self, auth_client): - """Test the GET request for the BootProcess view with an unprivileged client.""" - response = auth_client.get(reverse("main:boot_process")) - assert response.status_code == HTTPStatus.FORBIDDEN - - def test_boot_process_post_invalid(self, privileged_client): + def test_boot_process_post_invalid(self, auth_client): """Test the POST request for the BootProcess view with invalid data.""" with assertTemplateUsed(template_name=self.template_name): - response = privileged_client.post(reverse("main:boot_process"), data=dict()) + response = auth_client.post(reverse("main:boot_process"), data=dict()) assert response.status_code == HTTPStatus.OK assert "form" in response.context - def test_boot_process_post_valid( - self, privileged_client, mocker, dummy_session_data - ): + def test_boot_process_post_valid(self, auth_client, mocker, dummy_session_data): """Test the POST request for the BootProcess view.""" mock = mocker.patch("main.views._boot_process") - response = privileged_client.post( + response = auth_client.post( reverse("main:boot_process"), data=dummy_session_data ) assert response.status_code == HTTPStatus.FOUND From 16d0708c728255c308066114a5f881608eb343ae Mon Sep 17 00:00:00 2001 From: James Paul Turner Date: Mon, 23 Sep 2024 17:13:53 +0100 Subject: [PATCH 8/9] Squished five permissions into two. --- main/migrations/0002_alter_user_options.py | 4 ++-- main/models.py | 5 +---- main/views.py | 8 ++++---- tests/conftest.py | 8 +------- 4 files changed, 8 insertions(+), 17 deletions(-) diff --git a/main/migrations/0002_alter_user_options.py b/main/migrations/0002_alter_user_options.py index 8b4ed6d..49ebbf2 100644 --- a/main/migrations/0002_alter_user_options.py +++ b/main/migrations/0002_alter_user_options.py @@ -1,4 +1,4 @@ -# Generated by Django 5.1.1 on 2024-09-20 13:49 +# Generated by Django 5.1.1 on 2024-09-23 16:11 from django.db import migrations @@ -12,6 +12,6 @@ class Migration(migrations.Migration): operations = [ migrations.AlterModelOptions( name='user', - options={'permissions': [('can_boot_processes', 'Can boot processes'), ('can_restart_processes', 'Can restart processes'), ('can_flush_processes', 'Can flush processes'), ('can_kill_processes', 'Can kill processes'), ('can_view_process_logs', 'Can view process logs')]}, + options={'permissions': [('can_modify_processes', 'Can modify processes'), ('can_view_process_logs', 'Can view process logs')]}, ), ] diff --git a/main/models.py b/main/models.py index 33a74f5..f403c49 100644 --- a/main/models.py +++ b/main/models.py @@ -13,9 +13,6 @@ class Meta: """Meta class for the User model.""" permissions: ClassVar = [ - ("can_boot_processes", "Can boot processes"), - ("can_restart_processes", "Can restart processes"), - ("can_flush_processes", "Can flush processes"), - ("can_kill_processes", "Can kill processes"), + ("can_modify_processes", "Can modify processes"), ("can_view_process_logs", "Can view process logs"), ] diff --git a/main/views.py b/main/views.py index 0155625..2b3bf73 100644 --- a/main/views.py +++ b/main/views.py @@ -100,7 +100,7 @@ async def _process_call(uuid: str, action: ProcessAction) -> None: @login_required -@permission_required("main.can_restart_processes", raise_exception=True) +@permission_required("main.can_modify_processes", raise_exception=True) def restart_process(request: HttpRequest, uuid: uuid.UUID) -> HttpResponse: """Restart the process associated to the given UUID. @@ -117,7 +117,7 @@ def restart_process(request: HttpRequest, uuid: uuid.UUID) -> HttpResponse: @login_required -@permission_required("main.can_kill_processes", raise_exception=True) +@permission_required("main.can_modify_processes", raise_exception=True) def kill_process(request: HttpRequest, uuid: uuid.UUID) -> HttpResponse: """Kill the process associated to the given UUID. @@ -133,7 +133,7 @@ def kill_process(request: HttpRequest, uuid: uuid.UUID) -> HttpResponse: @login_required -@permission_required("main.can_flush_processes", raise_exception=True) +@permission_required("main.can_modify_processes", raise_exception=True) def flush_process(request: HttpRequest, uuid: uuid.UUID) -> HttpResponse: """Flush the process associated to the given UUID. @@ -198,7 +198,7 @@ class BootProcessView(PermissionRequiredMixin, FormView): # type: ignore [type- template_name = "main/boot_process.html" form_class = BootProcessForm success_url = reverse_lazy("main:index") - permission_required = "main.can_boot_processes" + permission_required = "main.can_modify_processes" def form_valid(self, form: BootProcessForm) -> HttpResponse: """Boot a Process when valid form data has been POSTed. diff --git a/tests/conftest.py b/tests/conftest.py index 4a881f5..0acd305 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -9,13 +9,7 @@ def auth_client(django_user_model) -> Client: """Return an authenticated client.""" user = django_user_model.objects.create(username="privileged_user") - permission = Permission.objects.get(codename="can_boot_processes") - user.user_permissions.add(permission) - permission = Permission.objects.get(codename="can_restart_processes") - user.user_permissions.add(permission) - permission = Permission.objects.get(codename="can_flush_processes") - user.user_permissions.add(permission) - permission = Permission.objects.get(codename="can_kill_processes") + permission = Permission.objects.get(codename="can_modify_processes") user.user_permissions.add(permission) permission = Permission.objects.get(codename="can_view_process_logs") user.user_permissions.add(permission) From f3a563cd252639f6cb2e6da0b39a86c4e1fa1d4b Mon Sep 17 00:00:00 2001 From: James Paul Turner Date: Wed, 25 Sep 2024 11:40:41 +0100 Subject: [PATCH 9/9] Add permission tests for views. --- tests/conftest.py | 20 +++++++++++++++++++- tests/test_views.py | 45 ++++++++++++++++++++++++++++++++------------- 2 files changed, 51 insertions(+), 14 deletions(-) diff --git a/tests/conftest.py b/tests/conftest.py index 0acd305..d7a7a7c 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -8,9 +8,27 @@ @pytest.fixture def auth_client(django_user_model) -> Client: """Return an authenticated client.""" - user = django_user_model.objects.create(username="privileged_user") + user = django_user_model.objects.create(username="user") + client = Client() + client.force_login(user) + return client + + +@pytest.fixture +def auth_process_client(django_user_model) -> Client: + """Return a authenticated client with modify process privilege.""" + user = django_user_model.objects.create(username="process_user") permission = Permission.objects.get(codename="can_modify_processes") user.user_permissions.add(permission) + client = Client() + client.force_login(user) + return client + + +@pytest.fixture +def auth_logs_client(django_user_model) -> Client: + """Return a authenticated client with view logs privilege.""" + user = django_user_model.objects.create(username="logs_user") permission = Permission.objects.get(codename="can_view_process_logs") user.user_permissions.add(permission) client = Client() diff --git a/tests/test_views.py b/tests/test_views.py index eada1e6..0310b11 100644 --- a/tests/test_views.py +++ b/tests/test_views.py @@ -32,10 +32,15 @@ def setup_class(cls): cls.uuid = uuid4() cls.endpoint = reverse(f"main:{cls.action.value}", kwargs=dict(uuid=cls.uuid)) - def test_process_action_view_authenticated(self, auth_client, mocker): - """Test the process action view for an authenticated user.""" - mock = mocker.patch("main.views._process_call") + def test_process_action_view_unprivileged(self, auth_client): + """Test the process action view for an unprivileged user.""" response = auth_client.get(self.endpoint) + assert response.status_code == HTTPStatus.FORBIDDEN + + def test_process_action_view_privileged(self, auth_process_client, mocker): + """Test the process action view for a privileged user.""" + mock = mocker.patch("main.views._process_call") + response = auth_process_client.get(self.endpoint) assert response.status_code == HTTPStatus.FOUND assert response.url == reverse("main:index") @@ -70,11 +75,16 @@ class TestLogsView(LoginRequiredTest): uuid = uuid4() endpoint = reverse("main:logs", kwargs=dict(uuid=uuid)) - def test_logs_view_authenticated(self, auth_client, mocker): - """Test the logs view for an authenticated user.""" + def test_logs_view_unprivileged(self, auth_client): + """Test the logs view for an unprivileged user.""" + response = auth_client.get(self.endpoint) + assert response.status_code == HTTPStatus.FORBIDDEN + + def test_logs_view_privileged(self, auth_logs_client, mocker): + """Test the logs view for a privileged user.""" mock = mocker.patch("main.views._get_process_logs") with assertTemplateUsed(template_name="main/logs.html"): - response = auth_client.get(self.endpoint) + response = auth_logs_client.get(self.endpoint) assert response.status_code == HTTPStatus.OK mock.assert_called_once_with(str(self.uuid)) @@ -105,27 +115,36 @@ class TestBootProcess(LoginRequiredTest): template_name = "main/boot_process.html" endpoint = reverse("main:boot_process") - def test_boot_process_get(self, auth_client): - """Test the GET request for the BootProcess view.""" + def test_boot_process_get_unprivileged(self, auth_client): + """Test the GET request for the BootProcess view (unprivileged).""" + response = auth_client.get(reverse("main:boot_process")) + assert response.status_code == HTTPStatus.FORBIDDEN + + def test_boot_process_get_privileged(self, auth_process_client): + """Test the GET request for the BootProcess view (privileged).""" with assertTemplateUsed(template_name=self.template_name): - response = auth_client.get(reverse("main:boot_process")) + response = auth_process_client.get(reverse("main:boot_process")) assert response.status_code == HTTPStatus.OK assert "form" in response.context assertContains(response, f'form action="{reverse("main:boot_process")}"') - def test_boot_process_post_invalid(self, auth_client): + def test_boot_process_post_invalid(self, auth_process_client): """Test the POST request for the BootProcess view with invalid data.""" with assertTemplateUsed(template_name=self.template_name): - response = auth_client.post(reverse("main:boot_process"), data=dict()) + response = auth_process_client.post( + reverse("main:boot_process"), data=dict() + ) assert response.status_code == HTTPStatus.OK assert "form" in response.context - def test_boot_process_post_valid(self, auth_client, mocker, dummy_session_data): + def test_boot_process_post_valid( + self, auth_process_client, mocker, dummy_session_data + ): """Test the POST request for the BootProcess view.""" mock = mocker.patch("main.views._boot_process") - response = auth_client.post( + response = auth_process_client.post( reverse("main:boot_process"), data=dummy_session_data ) assert response.status_code == HTTPStatus.FOUND