From 3ae42393a646a594b2e70145a69023dfc2363a21 Mon Sep 17 00:00:00 2001 From: Daniele Procida Date: Mon, 20 Apr 2026 05:05:01 +0300 Subject: [PATCH 1/6] Updated psycopg2 --- dashboard/requirements.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dashboard/requirements.txt b/dashboard/requirements.txt index ebc23e5..1ccb8b6 100644 --- a/dashboard/requirements.txt +++ b/dashboard/requirements.txt @@ -2,7 +2,7 @@ Django==5.2.13 PyYAML==6.0.2 docutils==0.21.2 whitenoise==6.9.0 -psycopg2-binary==2.9.10 +psycopg2-binary==2.9.11 tzdata==2025.1 django-browser-reload==1.18.0 django-tinymce==4.1.0 From bfc38a171b79d802786a9eedb3eca6da753815cf Mon Sep 17 00:00:00 2001 From: Daniele Procida Date: Mon, 20 Apr 2026 05:47:48 +0300 Subject: [PATCH 2/6] Added a testing shim for OIDC. This allows tests in environments that haven't loaded OIDC to continue running. See https://docs.pytest.org/en/latest/reference/fixtures.html#conftest-py-sharing-fixtures-across-multiple-files for notes on shared fixtures. --- dashboard/conftest.py | 28 ++++++++++++++++++++++++++++ 1 file changed, 28 insertions(+) create mode 100644 dashboard/conftest.py diff --git a/dashboard/conftest.py b/dashboard/conftest.py new file mode 100644 index 0000000..67d462a --- /dev/null +++ b/dashboard/conftest.py @@ -0,0 +1,28 @@ +import sys +import types + +from django.http import HttpResponse + + +# Creat a "fake" mozilla_django_oidc.views so that tests will run, +# even if mozilla_django_oidc is not available at import time for +# tests. + +if "mozilla_django_oidc" not in sys.modules: + oidc_module = types.ModuleType("mozilla_django_oidc") + oidc_views_module = types.ModuleType("mozilla_django_oidc.views") + + class _DummyOIDCView: + @classmethod + def as_view(cls): + def _view(request, *args, **kwargs): + return HttpResponse("") + + return _view + + oidc_views_module.OIDCAuthenticationRequestView = _DummyOIDCView + oidc_views_module.OIDCAuthenticationCallbackView = _DummyOIDCView + oidc_views_module.OIDCLogoutView = _DummyOIDCView + oidc_module.views = oidc_views_module + sys.modules["mozilla_django_oidc"] = oidc_module + sys.modules["mozilla_django_oidc.views"] = oidc_views_module \ No newline at end of file From 3a8a10d0949647981555bbfcc8b879fc96290f6d Mon Sep 17 00:00:00 2001 From: Daniele Procida Date: Mon, 20 Apr 2026 06:26:24 +0300 Subject: [PATCH 3/6] Added a series of tests for view functions Updated: test_toggle_condition_url_patterns (Simplified, and now only checks the reversed route, rather than the string) New checks for user permissions: * test_action_toggle_commitment_denies_user_without_permission * test_action_toggle_condition_denies_user_without_permission * test_action_select_reason_denies_user_without_permission New checks for HTTP methods: * test_action_toggle_commitment_rejects_non_put_method * test_action_toggle_condition_rejects_non_put_method * test_action_select_reason_rejects_non_put_method New checks for expected results: new: test_action_toggle_commitment_allows_authorized_put_and_updates_commitment new: test_action_toggle_condition_allows_authorized_put_and_updates_status new: test_action_select_reason_allows_authorized_put_and_sets_reason I used Copilot to help create these tests. --- dashboard/projects/test_views.py | 255 ++++++++++++++++++++++++++++++- 1 file changed, 251 insertions(+), 4 deletions(-) diff --git a/dashboard/projects/test_views.py b/dashboard/projects/test_views.py index b70c423..9c3c873 100644 --- a/dashboard/projects/test_views.py +++ b/dashboard/projects/test_views.py @@ -1,10 +1,257 @@ import pytest +from urllib.parse import parse_qs, urlparse from django.urls import reverse +from django.contrib.auth.models import Permission, User + +from framework.models import Condition, Level, Objective, ObjectiveGroup, Reason, WorkCycle +from projects.models import Commitment, Project, ProjectObjective, ProjectObjectiveCondition def test_toggle_condition_url_patterns(): - assert reverse( + url = reverse("projects:action_toggle_condition", args=[1]) + assert url == "/action_toggle_condition/1" + + +@pytest.fixture +def user_without_permissions(client): + user = User.objects.create_user(username="no_perm", password="password") + client.login(username="no_perm", password="password") + return user + + +@pytest.fixture +def user_can_change_commitment(client): + user = User.objects.create_user(username="change_commitment", password="password") + permission = Permission.objects.get( + codename="change_commitment", + content_type__app_label="projects", + ) + user.user_permissions.add(permission) + client.login(username="change_commitment", password="password") + return user + + +@pytest.fixture +def user_can_change_projectobjectivecondition(client): + user = User.objects.create_user( + username="change_projectobjectivecondition", password="password" + ) + permission = Permission.objects.get( + codename="change_projectobjectivecondition", + content_type__app_label="projects", + ) + user.user_permissions.add(permission) + client.login(username="change_projectobjectivecondition", password="password") + return user + + +@pytest.fixture +def user_can_change_projectobjective(client): + user = User.objects.create_user( + username="change_projectobjective", password="password" + ) + permission = Permission.objects.get( + codename="change_projectobjective", + content_type__app_label="projects", + ) + user.user_permissions.add(permission) + client.login(username="change_projectobjective", password="password") + return user + + +@pytest.fixture +def objective_group(): + return ObjectiveGroup.objects.create(name="group") + + +@pytest.fixture +def objective(objective_group): + return Objective.objects.create(name="objective", group=objective_group, weight=1) + + +@pytest.fixture +def level(): + return Level.objects.create(name="level", value=1) + + +@pytest.fixture +def work_cycle(): + return WorkCycle.objects.create(name="wc", timestamp="2026-01-01", is_current=True) + + +@pytest.fixture +def project(objective, level, work_cycle): + return Project.objects.create(name="project") + + +@pytest.fixture +def condition(objective, level): + return Condition.objects.create(name="condition", objective=objective, level=level) + + +@pytest.fixture +def project_objective(project, objective): + return ProjectObjective.objects.get(project=project, objective=objective) + + +@pytest.fixture +def project_objective_condition(project, objective, condition): + return ProjectObjectiveCondition.objects.get( + project=project, + objective=objective, + condition=condition, + ) + + +@pytest.fixture +def commitment(project, objective, level, work_cycle): + return Commitment.objects.get( + project=project, + objective=objective, + level=level, + work_cycle=work_cycle, + ) + + +@pytest.fixture +def reason(): + return Reason.objects.create(name="not-started", value=1) + + +@pytest.mark.django_db +def test_action_toggle_commitment_denies_user_without_permission( + client, user_without_permissions, commitment +): + url = reverse("projects:action_toggle_commitment", args=[commitment.id]) + response = client.put(url) + + assert response.status_code == 302 + expected_redirect = f"{reverse('login')}?next={url}" + assert response.url == expected_redirect + + +@pytest.mark.django_db +def test_action_toggle_condition_denies_user_without_permission( + client, user_without_permissions, project_objective_condition +): + url = ( + reverse( + "projects:action_toggle_condition", + args=[project_objective_condition.id], + ) + + "?status=&target=done" + ) + response = client.put(url) + + assert response.status_code == 302 + parsed = urlparse(response.url) + assert parsed.path == reverse("login") + assert parse_qs(parsed.query)["next"][0] == url + + +@pytest.mark.django_db +def test_action_select_reason_denies_user_without_permission( + client, user_without_permissions, project_objective, reason +): + url = reverse("projects:action_select_reason", args=[project_objective.id]) + response = client.generic( + "PUT", + url, + data=f"ifnotstarted={reason.id}", + content_type="application/x-www-form-urlencoded", + ) + + assert response.status_code == 302 + expected_redirect = f"{reverse('login')}?next={url}" + assert response.url == expected_redirect + + +@pytest.mark.django_db +def test_action_toggle_commitment_rejects_non_put_method( + client, user_can_change_commitment, commitment +): + url = reverse("projects:action_toggle_commitment", args=[commitment.id]) + response = client.get(url) + + assert response.status_code == 405 + + +@pytest.mark.django_db +def test_action_toggle_commitment_allows_authorized_put_and_updates_commitment( + client, user_can_change_commitment, commitment +): + assert commitment.committed is False + + url = reverse("projects:action_toggle_commitment", args=[commitment.id]) + response = client.put(url) + + commitment.refresh_from_db() + assert response.status_code == 200 + assert commitment.committed is True + assert response["HX-Trigger-After-Swap"] == "updateCommitment" + + +@pytest.mark.django_db +def test_action_toggle_condition_rejects_non_put_method( + client, user_can_change_projectobjectivecondition, project_objective_condition +): + url = ( + reverse( + "projects:action_toggle_condition", + args=[project_objective_condition.id], + ) + + "?status=&target=done" + ) + response = client.get(url) + + assert response.status_code == 405 + + +@pytest.mark.django_db +def test_action_toggle_condition_allows_authorized_put_and_updates_status( + client, user_can_change_projectobjectivecondition, project_objective_condition +): + assert project_objective_condition.status == "" + + url = ( + reverse( "projects:action_toggle_condition", - args=[1], - query={"status": "DO", "target": "done"} - ) == "/action_toggle_condition/1?status=DO&target=done" + args=[project_objective_condition.id], + ) + + "?status=&target=done" + ) + response = client.put(url) + + project_objective_condition.refresh_from_db() + assert response.status_code == 200 + assert project_objective_condition.status == "DO" + assert "HX-Trigger-After-Swap" in response + + +@pytest.mark.django_db +def test_action_select_reason_rejects_non_put_method( + client, user_can_change_projectobjective, project_objective +): + url = reverse("projects:action_select_reason", args=[project_objective.id]) + response = client.get(url) + + assert response.status_code == 405 + + +@pytest.mark.django_db +def test_action_select_reason_allows_authorized_put_and_sets_reason( + client, user_can_change_projectobjective, project_objective, reason +): + assert project_objective.unstarted_reason is None + + url = reverse("projects:action_select_reason", args=[project_objective.id]) + response = client.generic( + "PUT", + url, + data=f"ifnotstarted={reason.id}", + content_type="application/x-www-form-urlencoded", + ) + + project_objective.refresh_from_db() + assert response.status_code == 200 + assert project_objective.unstarted_reason_id == reason.id From 77815810c6c4790be974256c0d10dfbde4e2f4f1 Mon Sep 17 00:00:00 2001 From: Daniele Procida Date: Mon, 20 Apr 2026 07:01:19 +0300 Subject: [PATCH 4/6] Added a permissions constraint for project_basic_form_save This fixes an auth issue. It was possible to POST a change to a project, without even being authenticated. The only safeguard was the user interface, that didn't offer make it possible. This adds @permission_required("projects.change_project") to the view, and some tests. * added a fixture for a user with permission to change a project * added a test: do we reject changes from unauthenticated users? * added a test: do we reject changes from unauthorised users? * added a test: do we allow and save changes from users with the right permissions? I used Copilot to uncover the issue and help create the tests. --- dashboard/projects/test_views.py | 84 ++++++++++++++++++++++++++++++++ dashboard/projects/views.py | 1 + 2 files changed, 85 insertions(+) diff --git a/dashboard/projects/test_views.py b/dashboard/projects/test_views.py index 9c3c873..468ae65 100644 --- a/dashboard/projects/test_views.py +++ b/dashboard/projects/test_views.py @@ -59,6 +59,18 @@ def user_can_change_projectobjective(client): return user +@pytest.fixture +def user_can_change_project(client): + user = User.objects.create_user(username="change_project", password="password") + permission = Permission.objects.get( + codename="change_project", + content_type__app_label="projects", + ) + user.user_permissions.add(permission) + client.login(username="change_project", password="password") + return user + + @pytest.fixture def objective_group(): return ObjectiveGroup.objects.create(name="group") @@ -118,6 +130,78 @@ def reason(): return Reason.objects.create(name="not-started", value=1) +@pytest.mark.django_db +def test_project_basic_form_save_denies_unauthenticated_user(client, project): + original_owner = project.owner + url = reverse("projects:project_basic_form_save", args=[project.id]) + response = client.post( + url, + data={ + "name": project.name, + "url": project.url, + "group": "", + "owner": "changed owner", + "driver": project.driver or "", + "agreement_status": "", + "last_review": "", + "last_review_status": "", + }, + ) + + project.refresh_from_db() + assert response.status_code == 302 + assert response.url == f"{reverse('login')}?next={url}" + assert project.owner == original_owner + + +@pytest.mark.django_db +def test_project_basic_form_save_denies_user_without_permission( + client, user_without_permissions, project +): + original_owner = project.owner + url = reverse("projects:project_basic_form_save", args=[project.id]) + response = client.post( + url, + data={ + "name": project.name, + "url": project.url, + "group": "", + "owner": "changed owner", + "driver": project.driver or "", + "agreement_status": "", + "last_review": "", + "last_review_status": "", + }, + ) + + project.refresh_from_db() + assert response.status_code == 302 + assert response.url == f"{reverse('login')}?next={url}" + assert project.owner == original_owner + + +@pytest.mark.django_db +def test_project_basic_form_save_allows_user_with_permission(client, user_can_change_project, project): + url = reverse("projects:project_basic_form_save", args=[project.id]) + response = client.post( + url, + data={ + "name": project.name, + "url": project.url, + "group": "", + "owner": "changed owner", + "driver": project.driver or "", + "agreement_status": "", + "last_review": "", + "last_review_status": "", + }, + ) + + project.refresh_from_db() + assert response.status_code == 200 + assert project.owner == "changed owner" + + @pytest.mark.django_db def test_action_toggle_commitment_denies_user_without_permission( client, user_without_permissions, commitment diff --git a/dashboard/projects/views.py b/dashboard/projects/views.py index 4e88c26..73e7a89 100644 --- a/dashboard/projects/views.py +++ b/dashboard/projects/views.py @@ -234,6 +234,7 @@ def action_select_reason(request, projectobjective_id): # form methods +@permission_required("projects.change_project") @require_http_methods(["POST"]) def project_basic_form_save(request, project_id): instance = Project.objects.get(id=project_id) From d2239f56401f8551d058a0b15b94a7d952323ddf Mon Sep 17 00:00:00 2001 From: David Wilding Date: Sun, 26 Apr 2026 08:48:14 +0800 Subject: [PATCH 5/6] remove duplicate imports --- dashboard/projects/test_views.py | 15 --------------- 1 file changed, 15 deletions(-) diff --git a/dashboard/projects/test_views.py b/dashboard/projects/test_views.py index 3422bcf..2e7b0c6 100644 --- a/dashboard/projects/test_views.py +++ b/dashboard/projects/test_views.py @@ -20,21 +20,6 @@ ProjectObjectiveCondition, ) -from framework.models import ( - Condition, - Level, - Objective, - ObjectiveGroup, - Reason, - WorkCycle, -) -from projects.models import ( - Commitment, - Project, - ProjectObjective, - ProjectObjectiveCondition, -) - def test_toggle_condition_url_patterns(): url = reverse("projects:action_toggle_condition", args=[1]) From 2b1deed0e33f4facdd9aef60ef61aa14f311b43f Mon Sep 17 00:00:00 2001 From: David Wilding Date: Sun, 26 Apr 2026 08:49:59 +0800 Subject: [PATCH 6/6] move user_can_change_project to conftest.py --- dashboard/conftest.py | 12 ++++++++++++ dashboard/projects/test_views.py | 13 ------------- 2 files changed, 12 insertions(+), 13 deletions(-) diff --git a/dashboard/conftest.py b/dashboard/conftest.py index 25a9e35..3ef9017 100644 --- a/dashboard/conftest.py +++ b/dashboard/conftest.py @@ -50,6 +50,18 @@ def user_can_change_projectobjective(client): return user +@pytest.fixture +def user_can_change_project(client): + user = User.objects.create_user(username="change_project", password="password") + permission = Permission.objects.get( + codename="change_project", + content_type__app_label="projects", + ) + user.user_permissions.add(permission) + client.login(username="change_project", password="password") + return user + + @pytest.fixture def user_can_change_workcycle(client): user = User.objects.create_user(username="change_workcycle", password="password") diff --git a/dashboard/projects/test_views.py b/dashboard/projects/test_views.py index 2e7b0c6..371ebd1 100644 --- a/dashboard/projects/test_views.py +++ b/dashboard/projects/test_views.py @@ -3,7 +3,6 @@ from django.test import override_settings from django.urls import reverse -from django.contrib.auth.models import Permission, User from framework.models import ( Condition, @@ -26,18 +25,6 @@ def test_toggle_condition_url_patterns(): assert url == "/action_toggle_condition/1" -@pytest.fixture -def user_can_change_project(client): - user = User.objects.create_user(username="change_project", password="password") - permission = Permission.objects.get( - codename="change_project", - content_type__app_label="projects", - ) - user.user_permissions.add(permission) - client.login(username="change_project", password="password") - return user - - @pytest.fixture def objective_group(): return ObjectiveGroup.objects.create(name="group")