diff --git a/.github/workflows/checks.yaml b/.github/workflows/checks.yaml index 66fd3cc..11200f7 100644 --- a/.github/workflows/checks.yaml +++ b/.github/workflows/checks.yaml @@ -27,6 +27,18 @@ jobs: run: | cd dashboard make test-browser + - name: Run framework tests with OIDC configured + run: | + cd dashboard + make oidc-test-framework + - name: Run projects tests with OIDC configured + run: | + cd dashboard + make oidc-test-projects + - name: Run browser tests with OIDC configured + run: | + cd dashboard + make oidc-test-browser - name: Check static files run: | cd dashboard diff --git a/dashboard/Makefile b/dashboard/Makefile index aef1ed0..982f839 100644 --- a/dashboard/Makefile +++ b/dashboard/Makefile @@ -25,9 +25,6 @@ install: venv $(PIP) install --upgrade pip $(PIP) install -r requirements.txt -install-dev: install - $(PIP) install -r requirements-dev.txt - migrate: install $(MANAGE) migrate @@ -40,11 +37,19 @@ run: init collectstatic: install $(MANAGE) collectstatic --no-input -# For use in CI -collectstatic-clear: install - $(MANAGE) collectstatic --clear --no-input +makemigrations: install + $(MANAGE) makemigrations --no-input + +test: test-framework test-projects test-browser + +clean: + rm -rf $(VENV) + find . -type d -name "__pycache__" -exec rm -r {} + + find . -type f -name "*.pyc" -delete + + +# Extra targets for development and CI. -# For use in CI collectstatic-check: collectstatic-clear @changes=$$(git status --porcelain staticfiles | wc -l); \ if [ $$changes -gt 0 ]; then \ @@ -52,10 +57,9 @@ collectstatic-check: collectstatic-clear exit 1; \ fi -makemigrations: install - $(MANAGE) makemigrations --no-input +collectstatic-clear: install + $(MANAGE) collectstatic --clear --no-input -# For use in CI makemigrations-check: makemigrations @changes=$$(git status --porcelain */migrations | wc -l); \ if [ $$changes -gt 0 ]; then \ @@ -63,22 +67,43 @@ makemigrations-check: makemigrations exit 1; \ fi -test: test-framework test-projects test-browser +install-dev: install + $(PIP) install -r requirements-dev.txt -# For use in CI test-framework: install-dev $(PYTEST) framework -# For use in CI test-projects: install-dev $(PYTEST) projects -# For use in CI -test-browser: install-dev +]test-browser: install-dev $(VENV)/bin/python -m playwright install $(PYTEST) test_browser.py -clean: - rm -rf $(VENV) - find . -type d -name "__pycache__" -exec rm -r {} + - find . -type f -name "*.pyc" -delete +oidc-test-framework: install-dev + DJANGO_OIDC_CLIENT_ID=fake_client_id \ + DJANGO_OIDC_CLIENT_SECRET=fake_client_secret \ + DJANGO_OIDC_AUTHORIZE_URL=https://example.com/oauth2/auth \ + DJANGO_OIDC_ACCESS_TOKEN_URL=https://example.com/oauth2/token \ + DJANGO_OIDC_USER_URL=https://example.com/userinfo \ + DJANGO_OIDC_JWKS_URL=https://example.com/.well-known/jwks.json \ + $(PYTEST) framework + +oidc-test-projects: install-dev + DJANGO_OIDC_CLIENT_ID=fake_client_id \ + DJANGO_OIDC_CLIENT_SECRET=fake_client_secret \ + DJANGO_OIDC_AUTHORIZE_URL=https://example.com/oauth2/auth \ + DJANGO_OIDC_ACCESS_TOKEN_URL=https://example.com/oauth2/token \ + DJANGO_OIDC_USER_URL=https://example.com/userinfo \ + DJANGO_OIDC_JWKS_URL=https://example.com/.well-known/jwks.json \ + $(PYTEST) projects + +oidc-test-browser: install-dev + $(VENV)/bin/python -m playwright install + DJANGO_OIDC_CLIENT_ID=fake_client_id \ + DJANGO_OIDC_CLIENT_SECRET=fake_client_secret \ + DJANGO_OIDC_AUTHORIZE_URL=https://example.com/oauth2/auth \ + DJANGO_OIDC_ACCESS_TOKEN_URL=https://example.com/oauth2/token \ + DJANGO_OIDC_USER_URL=https://example.com/userinfo \ + DJANGO_OIDC_JWKS_URL=https://example.com/.well-known/jwks.json \ + $(PYTEST) test_browser.py diff --git a/dashboard/conftest.py b/dashboard/conftest.py new file mode 100644 index 0000000..027279b --- /dev/null +++ b/dashboard/conftest.py @@ -0,0 +1,111 @@ +import sys +import types + +import pytest + +from django.contrib.auth.models import Permission, User +from django.http import HttpResponse + + +@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 user_can_change_workcycle(client): + user = User.objects.create_user(username="change_workcycle", password="password") + permission = Permission.objects.get( + codename="change_workcycle", + content_type__app_label="framework", + ) + user.user_permissions.add(permission) + client.login(username="change_workcycle", password="password") + return user + + +@pytest.fixture +def user_can_view_workcycle(client): + user = User.objects.create_user(username="view_workcycle", password="password") + permission = Permission.objects.get( + codename="view_workcycle", + content_type__app_label="framework", + ) + user.user_permissions.add(permission) + client.login(username="view_workcycle", password="password") + return user + + +@pytest.fixture +def user_is_staff(client): + user = User.objects.create_user( + username="staffmember", password="password", is_staff=True + ) + client.login(username="staffmember", password="password") + return user + + +# 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 diff --git a/dashboard/framework/test_views.py b/dashboard/framework/test_views.py index cd30723..276b884 100644 --- a/dashboard/framework/test_views.py +++ b/dashboard/framework/test_views.py @@ -2,37 +2,12 @@ import pytest from django.urls import reverse -from django.contrib.auth.models import User, Permission from django.contrib.messages import get_messages from framework.models import WorkCycle, ObjectiveGroup, Objective, Level from projects.models import Project, QI, ProjectObjective -@pytest.fixture -def user_can_change(client): - user = User.objects.create_user(username="user", password="password") - permission = Permission.objects.get( - codename="change_workcycle", - content_type__app_label="framework", - ) - user.user_permissions.add(permission) - client.login(username="user", password="password") - return user - - -@pytest.fixture -def user_can_view(client): - user = User.objects.create_user(username="user", password="password") - permission = Permission.objects.get( - codename="view_workcycle", - content_type__app_label="framework", - ) - user.user_permissions.add(permission) - client.login(username="user", password="password") - return user - - @pytest.fixture def work_cycle(): return WorkCycle.objects.create( @@ -67,7 +42,7 @@ def project(objective): @pytest.mark.django_db def test_admin_apply_qis( - client, user_can_change, work_cycle, project, objective, level + client, user_can_change_workcycle, work_cycle, project, objective, level ): """Test that admin_apply_qis copies current QI values to workcycle QIs.""" @@ -109,7 +84,7 @@ def test_admin_apply_qis( @pytest.mark.django_db def test_admin_apply_qis_user_disallowed( - client, user_can_view, work_cycle, project, objective, level + client, user_can_view_workcycle, work_cycle, project, objective, level ): """Test that a user with framework.view_workcycle permission (only) can't copy QI values.""" @@ -140,7 +115,7 @@ def test_admin_apply_qis_user_disallowed( @pytest.mark.django_db def test_admin_apply_qis_with_multiple_projects( - client, user_can_change, work_cycle, objective, level + client, user_can_change_workcycle, work_cycle, objective, level ): """Test that admin_apply_qis updates QIs for multiple projects.""" @@ -182,7 +157,9 @@ def test_admin_apply_qis_with_multiple_projects( @pytest.mark.django_db -def test_admin_apply_qis_shows_message(client, user_can_change, work_cycle, project): +def test_admin_apply_qis_shows_message( + client, user_can_change_workcycle, work_cycle, project +): """Test that admin_apply_qis displays an info message.""" url = reverse("framework:admin_apply_qis", args=[work_cycle.id]) @@ -195,7 +172,9 @@ def test_admin_apply_qis_shows_message(client, user_can_change, work_cycle, proj @pytest.mark.django_db -def test_admin_apply_qis_with_no_projects(client, user_can_change, work_cycle): +def test_admin_apply_qis_with_no_projects( + client, user_can_change_workcycle, work_cycle +): """Test that admin_apply_qis works even when no projects exist.""" # Call the view with no projects diff --git a/dashboard/projects/test_models.py b/dashboard/projects/test_models.py index 6d16f22..70b0b90 100644 --- a/dashboard/projects/test_models.py +++ b/dashboard/projects/test_models.py @@ -1,6 +1,6 @@ import pytest from django.urls import reverse -from django.contrib.auth.models import User +from django.contrib.auth.models import User, Permission from framework.models import ObjectiveGroup, Objective, Level, Condition from projects.models import ( @@ -10,13 +10,6 @@ ) -@pytest.fixture -def user_is_staff(client): - user = User.objects.create_user(username="staffmember", password="password", is_staff=True) - client.login(username="staffmember", password="password") - return user - - @pytest.fixture def objective_group(): return ObjectiveGroup.objects.create(name="test_objective_group") @@ -66,7 +59,9 @@ def condition2(level2, objective): @pytest.mark.django_db -def test_admin_recalculate_all_levels(client, user_is_staff, project, objective, level1, level2, condition1, condition2): +def test_admin_recalculate_all_levels( + client, user_is_staff, project, objective, level1, level2, condition1, condition2 +): """Test that admin_recalculate_all_levels recalculates ProjectObjective.level_achieved.""" po = ProjectObjective.objects.get(project=project, objective=objective) @@ -140,9 +135,9 @@ def test_project_objective_achieved_level(project, objective, level1): # Mark the second one as not applicable ProjectObjectiveCondition.objects.filter( - project=project, - objective=objective, - condition=condition2, + project=project, + objective=objective, + condition=condition2, ).update(status="NA") po = ProjectObjective.objects.get(project=project, objective=objective) @@ -171,9 +166,9 @@ def test_quality_indicator_single_objective(project, objective, level1): # level1.value = 1 (from fixture) # Expected: 1 * 1 = 1 - ProjectObjective.objects.filter( - project=project, objective=objective - ).update(level_achieved=level1) + ProjectObjective.objects.filter(project=project, objective=objective).update( + level_achieved=level1 + ) assert project.quality_indicator == 1 @@ -187,13 +182,13 @@ def test_quality_indicator_multiple_objectives(project, objective_group): level_a = Level.objects.create(name="level_a", value=2) level_b = Level.objects.create(name="level_b", value=4) - ProjectObjective.objects.filter( - project=project, objective=obj1 - ).update(level_achieved=level_a) + ProjectObjective.objects.filter(project=project, objective=obj1).update( + level_achieved=level_a + ) - ProjectObjective.objects.filter( - project=project, objective=obj2 - ).update(level_achieved=level_b) + ProjectObjective.objects.filter(project=project, objective=obj2).update( + level_achieved=level_b + ) # Expected: (10 * 2) + (5 * 4) = 20 + 20 = 40 assert project.quality_indicator == 40 @@ -207,9 +202,9 @@ def test_quality_indicator_mixed_achieved_and_none(project, objective_group): level_a = Level.objects.create(name="level_a", value=3) - ProjectObjective.objects.filter( - project=project, objective=obj1 - ).update(level_achieved=level_a) + ProjectObjective.objects.filter(project=project, objective=obj1).update( + level_achieved=level_a + ) po2 = ProjectObjective.objects.get(project=project, objective=obj2) assert po2.level_achieved is None @@ -219,12 +214,14 @@ def test_quality_indicator_mixed_achieved_and_none(project, objective_group): @pytest.mark.django_db -def test_project_detail_anchor_navigation(client, project, objective): +def test_project_detail_anchor_navigation( + client, user_without_permissions, project, objective +): """Integration test: verify clicking anchor in list navigates to correct section in detail.""" from django.utils.text import slugify # Get the detail page - url = reverse('projects:project', kwargs={'id': project.id}) + url = reverse("projects:project", kwargs={"id": project.id}) response = client.get(url) assert response.status_code == 200 diff --git a/dashboard/projects/test_views.py b/dashboard/projects/test_views.py index b70c423..cd3ab7b 100644 --- a/dashboard/projects/test_views.py +++ b/dashboard/projects/test_views.py @@ -1,10 +1,279 @@ import pytest +from urllib.parse import parse_qs, urlparse +from django.test import override_settings from django.urls import reverse +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( - "projects:action_toggle_condition", - args=[1], - query={"status": "DO", "target": "done"} - ) == "/action_toggle_condition/1?status=DO&target=done" + url = reverse("projects:action_toggle_condition", args=[1]) + assert url == "/action_toggle_condition/1" + + +@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=[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 + + +# Check that the project list and project detail pages are correctly public/private, +# depending on whether OIDC is configured. + + +@pytest.mark.django_db +@override_settings(OIDC_RP_CLIENT_ID=None) +def test_project_list_no_login(client): + url = reverse("projects:project_list") + response = client.get(url) + assert response.status_code == 200 + + +@pytest.mark.django_db +@override_settings(OIDC_RP_CLIENT_ID="test_client_id") +def test_project_list_oidc_needs_login(client): + url = reverse("projects:project_list") + response = client.get(url) + assert response.status_code == 302 + expected_redirect = f"{reverse('login')}?next={url}" + assert response.url == expected_redirect + + +@pytest.mark.django_db +@override_settings(OIDC_RP_CLIENT_ID="test_client_id") +def test_project_list_oidc_logged_in(client, user_without_permissions): + url = reverse("projects:project_list") + response = client.get(url) + assert response.status_code == 200 + + +@pytest.mark.django_db +@override_settings(OIDC_RP_CLIENT_ID=None) +def test_project_detail_no_login(client, project): + url = reverse("projects:project", kwargs={"id": project.id}) + response = client.get(url) + assert response.status_code == 200 + + +@pytest.mark.django_db +@override_settings(OIDC_RP_CLIENT_ID="test_client_id") +def test_project_detail_oidc_needs_login(client, project): + url = reverse("projects:project", kwargs={"id": project.id}) + response = client.get(url) + assert response.status_code == 302 + expected_redirect = f"{reverse('login')}?next={url}" + assert response.url == expected_redirect + + +@pytest.mark.django_db +@override_settings(OIDC_RP_CLIENT_ID="test_client_id") +def test_project_detail_oidc_logged_in(client, user_without_permissions, project): + url = reverse("projects:project", kwargs={"id": project.id}) + response = client.get(url) + assert response.status_code == 200