diff --git a/tests/browser/test_admin_project_access.py b/tests/browser/test_admin_project_access.py index c608e082e..7e3138797 100644 --- a/tests/browser/test_admin_project_access.py +++ b/tests/browser/test_admin_project_access.py @@ -286,7 +286,8 @@ def test_audit_log_created_on_staff_user_access( # Login as staff user self.perform_login(driver, "admin", TEST_PASSWORD, wait) - # Navigate to owner's project + # Navigate to owner's project; the pk URL redirects to the canonical + # full_id URL, which serves the page and creates the single audit log driver.get(f"{self.live_server_url}/projects/{project.pk}/") # Verify audit log created @@ -298,7 +299,7 @@ def test_audit_log_created_on_staff_user_access( log = logs.first() assert log.action == ProjectAccessLog.Action.VIEW - assert log.view_name == "ProjectDetailView" + assert log.view_name == "ProjectDetailByFullIdView" def test_regular_user_denied_access_not_logged(self, driver, owner, project, wait): """Test that regular user denied access is NOT logged. diff --git a/wafer_space/notifications/views.py b/wafer_space/notifications/views.py index 4698a18dd..9af5b0d2a 100644 --- a/wafer_space/notifications/views.py +++ b/wafer_space/notifications/views.py @@ -74,10 +74,8 @@ def mark_notification_read(request, notification_id): with contextlib.suppress(Exception): # For ProjectFile, redirect to the project detail page if hasattr(notification.content_object, "project"): - return redirect( - "projects:detail", - pk=notification.content_object.project.pk, - ) + project = notification.content_object.project + return redirect(project.get_absolute_url()) # Default: redirect back to notifications list return redirect("notifications:list") diff --git a/wafer_space/projects/forms.py b/wafer_space/projects/forms.py index 2b5e75847..b9f35a1e4 100644 --- a/wafer_space/projects/forms.py +++ b/wafer_space/projects/forms.py @@ -452,7 +452,9 @@ def clean_project_id(self): msg = "Project ID is required" raise ValidationError(msg) - if not project_id.isalnum(): + # ASCII check matters: .upper() leaves "àbcd" as "ÀBCD", which passes + # .isalnum() but has no place in the ASCII-only canonical project URL. + if not (project_id.isascii() and project_id.isalnum()): msg = "Project ID must be alphanumeric (A-Z, 0-9)" raise ValidationError(msg) diff --git a/wafer_space/projects/models.py b/wafer_space/projects/models.py index da0eb0f8a..b188bac82 100644 --- a/wafer_space/projects/models.py +++ b/wafer_space/projects/models.py @@ -13,6 +13,7 @@ from django.core.validators import FileExtensionValidator from django.db import models from django.db import transaction +from django.urls import reverse from django.utils import timezone from django.utils.formats import date_format from django.utils.functional import cached_property @@ -50,11 +51,18 @@ class CheckExecutionContext: def validate_project_id(value: str) -> None: - """Validate project ID is 4 alphanumeric uppercase characters.""" + """Validate project ID is 4 alphanumeric uppercase ASCII characters. + + Uses an explicit ASCII check rather than ``str.isalnum()`` alone, which + also accepts non-ASCII alphanumerics (e.g. "ÀBCD", which is unchanged by + ``str.upper()`` and so passes the uppercase check too). Those cannot be + reversed into the ASCII-only canonical project URL, so accepting one here + would store a project whose every link raises NoReverseMatch. + """ if len(value) != PROJECT_ID_LENGTH: msg = "Project ID must be exactly 4 characters" raise ValidationError(msg) - if not value.isalnum(): + if not (value.isascii() and value.isalnum()): msg = "Project ID must be alphanumeric (A-Z, 0-9)" raise ValidationError(msg) # Check that any letters present are uppercase (digits are okay) @@ -349,6 +357,22 @@ def save(self, *args, **kwargs): "proprietary_terms_url": self.proprietary_terms_url, } + def get_absolute_url(self) -> str: + """Get URL for the project's detail view. + + Prefers the canonical manufacturing-ID URL, falling back to the pk + URL for projects not yet on a shuttle (which have no full_id). + + Returns: + str: URL for project detail. + + """ + if self.full_id: + return reverse( + "projects:detail_by_full_id", kwargs={"full_id": self.full_id} + ) + return reverse("projects:detail", kwargs={"pk": self.pk}) + def clean(self): """Validate model, including core field immutability. diff --git a/wafer_space/projects/tests/test_shuttle_integration.py b/wafer_space/projects/tests/test_shuttle_integration.py index a1b8a9c26..268376d0b 100644 --- a/wafer_space/projects/tests/test_shuttle_integration.py +++ b/wafer_space/projects/tests/test_shuttle_integration.py @@ -55,6 +55,21 @@ def test_full_id_without_both_returns_empty(self): project = Project.objects.create(user=self.user, name="Test Project") assert project.full_id == "" + def test_get_absolute_url_uses_full_id_when_on_a_shuttle(self): + """The canonical URL is the manufacturing-ID one once assigned.""" + project = Project.objects.create( + user=self.user, name="Test Project", shuttle=self.shuttle, project_id="ABCD" + ) + assert project.get_absolute_url() == "/projects/G891ABCD/" + + def test_get_absolute_url_falls_back_to_pk_without_full_id(self): + """Projects with no shuttle have no full_id, so the pk URL stands in.""" + project = Project.objects.create( + user=self.user, name="Test Project", project_id="ABCD" + ) + assert project.full_id == "" + assert project.get_absolute_url() == f"/projects/{project.pk}/" + def test_shuttle_run_display_with_shuttle(self): """Test shuttle_run_display property returns formatted string.""" project = Project.objects.create( diff --git a/wafer_space/projects/tests/test_validators.py b/wafer_space/projects/tests/test_validators.py index 60a588ff4..595dd1cc6 100644 --- a/wafer_space/projects/tests/test_validators.py +++ b/wafer_space/projects/tests/test_validators.py @@ -47,6 +47,18 @@ def test_invalid_project_id_special_characters(self): validate_project_id("AB-D") assert "alphanumeric" in str(exc_info.value) + def test_invalid_project_id_non_ascii_alphanumeric(self): + """Test that non-ASCII alphanumerics are invalid. + + "ÀBCD" satisfies str.isalnum() and is left unchanged by str.upper(), + so it passed both the alphanumeric and uppercase checks. It has no + place in the ASCII-only canonical project URL: storing one made every + page that links the project raise NoReverseMatch. + """ + with pytest.raises(ValidationError) as exc_info: + validate_project_id("ÀBCD") + assert "alphanumeric" in str(exc_info.value) + def test_invalid_project_id_spaces(self): """Test that spaces are invalid.""" with pytest.raises(ValidationError) as exc_info: diff --git a/wafer_space/projects/tests/test_views.py b/wafer_space/projects/tests/test_views.py index 4b8bc5769..831702c5e 100644 --- a/wafer_space/projects/tests/test_views.py +++ b/wafer_space/projects/tests/test_views.py @@ -1,5 +1,6 @@ """Tests for project views.""" +import uuid from datetime import timedelta from unittest.mock import Mock from unittest.mock import patch @@ -7,9 +8,11 @@ import pytest from django.contrib.messages import get_messages from django.core.cache import cache +from django.db import connection from django.template.loader import render_to_string from django.test import Client from django.test import TestCase +from django.test.utils import CaptureQueriesContext from django.urls import reverse from django.utils import timezone @@ -18,6 +21,7 @@ from wafer_space.projects.models import ManufacturabilityCheck from wafer_space.projects.models import PrecheckImageRevision from wafer_space.projects.models import Project +from wafer_space.projects.models import ProjectAccessLog from wafer_space.projects.models import ProjectFile from wafer_space.projects.security import SecurityValidationError from wafer_space.projects.tests.factories import ManufacturabilityCheckFactory @@ -410,6 +414,232 @@ def test_detail_shows_cob_change_badge_in_check_history(self): assert "CoB Change" in response.content.decode() +@pytest.mark.django_db +class TestProjectListQueryCount: + """The project list must not run a query per project.""" + + def test_query_count_does_not_grow_with_project_count(self, client): + """Guard against an N+1 in the list. + + Every row links via get_absolute_url(), which reads full_id and so + touches the shuttle FK. Without select_related("shuttle") that costs + one extra query per project. Asserting the count is unchanged by + adding rows catches that without pinning a brittle absolute number. + """ + user = UserFactory() + shuttle = Shuttle.objects.create(name="G881", description="List run") + ProjectFactory(user=user, shuttle=shuttle) + client.force_login(user) + url = reverse("projects:list") + + with CaptureQueriesContext(connection) as one_project: + client.get(url) + + ProjectFactory.create_batch(5, user=user, shuttle=shuttle) + + with CaptureQueriesContext(connection) as six_projects: + client.get(url) + + assert len(six_projects.captured_queries) == len(one_project.captured_queries) + + +@pytest.mark.django_db +class TestProjectDetailPkRedirect: + """The pk URL redirects to the canonical full_id URL.""" + + def test_pk_redirects_to_full_id_for_owner(self, client): + """Owner hitting the pk URL is sent to the canonical full_id URL.""" + user = UserFactory() + shuttle = Shuttle.objects.create(name="G871", description="Redirect run") + project = ProjectFactory(user=user, shuttle=shuttle) + client.force_login(user) + + response = client.get(reverse("projects:detail", kwargs={"pk": project.pk})) + + assert response.status_code == HTTP_FOUND + assert response["Location"] == reverse( + "projects:detail_by_full_id", kwargs={"full_id": project.full_id} + ) + + def test_redirect_preserves_query_string(self, client): + """The canonical URL answers the request the pk URL was asked. + + Dropping the query string would silently discard the caller's + parameters on every redirected link. + """ + user = UserFactory() + shuttle = Shuttle.objects.create(name="G880", description="Query run") + project = ProjectFactory(user=user, shuttle=shuttle) + client.force_login(user) + + response = client.get( + reverse("projects:detail", kwargs={"pk": project.pk}), + {"tab": "files", "page": "2"}, + ) + + assert response.status_code == HTTP_FOUND + canonical = reverse( + "projects:detail_by_full_id", kwargs={"full_id": project.full_id} + ) + assert response["Location"] == f"{canonical}?tab=files&page=2" + + def test_pk_serves_page_when_project_has_no_shuttle(self, client): + """Without a shuttle there is no full_id, so the pk URL serves the page.""" + user = UserFactory() + project = ProjectFactory(user=user, shuttle=None) + client.force_login(user) + + response = client.get(reverse("projects:detail", kwargs={"pk": project.pk})) + + assert project.full_id == "" + assert response.status_code == HTTP_OK + + def test_anonymous_is_sent_to_login_and_learns_no_full_id(self, client): + """Anonymous users must not learn a project's manufacturing ID. + + Regression test: redirecting before the login check ran turned this + view into an existence oracle that disclosed the full_id to anyone + holding a pk. + """ + shuttle = Shuttle.objects.create(name="G872", description="Anon run") + project = ProjectFactory(shuttle=shuttle) + + response = client.get(reverse("projects:detail", kwargs={"pk": project.pk})) + + assert response.status_code == HTTP_FOUND + assert "/accounts/login/" in response["Location"] + assert project.full_id not in response["Location"] + + def test_anonymous_gets_login_redirect_for_unknown_pk(self, client): + """An unknown pk must not 404 for anonymous users, which leaks existence.""" + known = ProjectFactory( + shuttle=Shuttle.objects.create(name="G873", description="Oracle run") + ) + missing_pk = uuid.uuid4() + + known_response = client.get(reverse("projects:detail", kwargs={"pk": known.pk})) + missing_response = client.get( + reverse("projects:detail", kwargs={"pk": missing_pk}) + ) + + # Both must be indistinguishable to an anonymous caller: same status, + # and both sent to login rather than one revealing the project. + assert known_response.status_code == HTTP_FOUND + assert missing_response.status_code == HTTP_FOUND + assert "/accounts/login/" in known_response["Location"] + assert "/accounts/login/" in missing_response["Location"] + assert known.full_id not in known_response["Location"] + + def test_non_owner_is_forbidden_and_learns_no_full_id(self, client): + """A non-owner gets 403 rather than a redirect disclosing the full_id.""" + shuttle = Shuttle.objects.create(name="G874", description="Denied run") + project = ProjectFactory(shuttle=shuttle) + client.force_login(UserFactory()) + + response = client.get(reverse("projects:detail", kwargs={"pk": project.pk})) + + assert response.status_code == HTTP_FORBIDDEN + assert project.full_id not in response.content.decode() + + def test_staff_pk_access_logs_once_against_canonical_view(self, client): + """Staff pk access creates exactly one log, named for the canonical view. + + The redirect leg must not log: the canonical request it redirects to + creates the audit entry, so logging both would record one access twice. + """ + shuttle = Shuttle.objects.create(name="G875", description="Audit run") + project = ProjectFactory(shuttle=shuttle) + client.force_login(UserFactory(is_staff=True)) + + response = client.get( + reverse("projects:detail", kwargs={"pk": project.pk}), follow=True + ) + + assert response.status_code == HTTP_OK + logs = ProjectAccessLog.objects.filter(project=project) + assert logs.count() == 1 + assert logs.get().view_name == "ProjectDetailByFullIdView" + + +@pytest.mark.django_db +class TestProjectDetailByFullIdView: + """The canonical /projects// URL.""" + + def test_owner_can_view_by_full_id(self, client): + """Owner can load the project through its manufacturing ID.""" + user = UserFactory() + shuttle = Shuttle.objects.create(name="G876", description="Canonical run") + project = ProjectFactory(user=user, shuttle=shuttle) + client.force_login(user) + + response = client.get( + reverse("projects:detail_by_full_id", kwargs={"full_id": project.full_id}) + ) + + assert response.status_code == HTTP_OK + assert response.context["project"] == project + + def test_staff_can_view_by_full_id(self, client): + """Staff can load another user's project through its manufacturing ID.""" + shuttle = Shuttle.objects.create(name="G877", description="Staff run") + project = ProjectFactory(shuttle=shuttle) + client.force_login(UserFactory(is_staff=True)) + + response = client.get( + reverse("projects:detail_by_full_id", kwargs={"full_id": project.full_id}) + ) + + assert response.status_code == HTTP_OK + + def test_non_owner_cannot_view_by_full_id(self, client): + """A non-owner is denied on the canonical URL too.""" + shuttle = Shuttle.objects.create(name="G878", description="Denied run") + project = ProjectFactory(shuttle=shuttle) + client.force_login(UserFactory()) + + response = client.get( + reverse("projects:detail_by_full_id", kwargs={"full_id": project.full_id}) + ) + + assert response.status_code == HTTP_FORBIDDEN + + def test_requires_login(self, client): + """The canonical URL requires login.""" + shuttle = Shuttle.objects.create(name="G879", description="Anon run") + project = ProjectFactory(shuttle=shuttle) + + response = client.get( + reverse("projects:detail_by_full_id", kwargs={"full_id": project.full_id}) + ) + + assert response.status_code == HTTP_FOUND + assert "/accounts/login/" in response["Location"] + + def test_unknown_full_id_returns_404(self, client): + """A well-formed but unused manufacturing ID is a 404.""" + client.force_login(UserFactory()) + + response = client.get("/projects/G999ZZZZ/") + + assert response.status_code == HTTP_NOT_FOUND + + @pytest.mark.parametrize( + "bad_id", + [ + "g871abcd", # lowercase + "ABCDEFG", # too short + "ABCDEFGHI", # too long + ], + ) + def test_malformed_full_id_does_not_resolve(self, client, bad_id): + """Only 8 upper-case alphanumerics route to the canonical view.""" + client.force_login(UserFactory()) + + response = client.get(f"/projects/{bad_id}/") + + assert response.status_code == HTTP_NOT_FOUND + + @pytest.mark.django_db class TestProjectCreateView(TestCase): """Test ProjectCreateView.""" @@ -2212,7 +2442,7 @@ def test_staff_sees_slot_assignments(self, client): slot2.save() url = reverse("projects:detail", kwargs={"pk": project.pk}) - response = client.get(url) + response = client.get(url, follow=True) # follow redirect to full_id URL assert response.status_code == HTTP_OK content = response.content.decode() @@ -2243,7 +2473,7 @@ def test_regular_user_does_not_see_slots(self, client): slot.save() url = reverse("projects:detail", kwargs={"pk": project.pk}) - response = client.get(url) + response = client.get(url, follow=True) # follow redirect to full_id URL assert response.status_code == HTTP_OK content = response.content.decode() diff --git a/wafer_space/projects/urls.py b/wafer_space/projects/urls.py index e75e73379..33f3aee3e 100644 --- a/wafer_space/projects/urls.py +++ b/wafer_space/projects/urls.py @@ -1,6 +1,7 @@ """URL configuration for projects app.""" from django.urls import path +from django.urls import re_path from . import views from .views import ProjectAdminSummaryView @@ -19,6 +20,12 @@ # Project CRUD path("", views.ProjectListView.as_view(), name="list"), path("/", views.ProjectDetailView.as_view(), name="detail"), + # Canonical URL using 8-character manufacturing ID (e.g., G801ABCD) + re_path( + r"^(?P[A-Z0-9]{8})/$", + views.ProjectDetailByFullIdView.as_view(), + name="detail_by_full_id", + ), path("create/", views.ProjectCreateView.as_view(), name="create"), path("/update/", views.ProjectUpdateView.as_view(), name="update"), path("/delete/", views.ProjectDeleteView.as_view(), name="delete"), diff --git a/wafer_space/projects/views.py b/wafer_space/projects/views.py index fd6168d82..5a4aaa2be 100644 --- a/wafer_space/projects/views.py +++ b/wafer_space/projects/views.py @@ -18,6 +18,7 @@ from django.db.models import OuterRef from django.db.models import Prefetch from django.db.models import Subquery +from django.http import Http404 from django.http import JsonResponse from django.shortcuts import get_object_or_404 from django.shortcuts import redirect @@ -32,6 +33,7 @@ from django.views.generic import View from wafer_space.core.enums import SlotSize +from wafer_space.shuttles.models import SHUTTLE_ID_LENGTH from wafer_space.shuttles.models import Shuttle from .exceptions import InvalidStateTransitionError @@ -66,25 +68,62 @@ def get_queryset(self): - Regular users: only their own projects - Staff users: all projects from all users + + Selects the shuttle: every row links via get_absolute_url(), which + reads full_id and so touches the shuttle, one query per project. """ # Cast user since LoginRequiredMixin ensures authentication user = cast("User", self.request.user) if user.is_staff: # Staff users see all projects - return Project.objects.all().select_related("user").order_by("-created_at") + return ( + Project.objects.all() + .select_related("user", "shuttle") + .order_by("-created_at") + ) # Regular users see only their own projects - return Project.objects.filter(user=user).order_by("-created_at") + return ( + Project.objects.filter(user=user) + .select_related("user", "shuttle") + .order_by("-created_at") + ) class ProjectDetailView(LoginRequiredMixin, ProjectOwnerOrStaffMixin, DetailView): - """View a single project with its files.""" + """View a single project with its files. + + When accessed by UUID pk, redirects to the canonical full_id URL + if the project has been assigned to a shuttle. + """ model = Project template_name = "projects/project_detail.html" context_object_name = "project" + def dispatch(self, request, *args, **kwargs): + """Redirect to the canonical full_id URL when accessed by pk. + + Only authenticated, authorised users are redirected. Everyone else + falls through to ``super()`` and gets the response they would have + got without this view: anonymous users the login redirect, and + unauthorised users a 403. Redirecting them instead would disclose + the project's existence and its manufacturing ID to someone who is + not allowed to see it, which the permission mixin exists to prevent. + """ + if "pk" in kwargs and request.user.is_authenticated: + project = self.get_object() + if project.full_id and self.test_func(): + # Carry the query string over: the canonical URL should answer + # the same request the pk URL was asked, not a truncated one. + url = project.get_absolute_url() + query_string = request.META.get("QUERY_STRING", "") + if query_string: + url = f"{url}?{query_string}" + return redirect(url) + return super().dispatch(request, *args, **kwargs) + def get_context_data(self, **kwargs): """Add project files and status to context.""" context = super().get_context_data(**kwargs) @@ -186,6 +225,41 @@ def get_context_data(self, **kwargs): return context +class ProjectDetailByFullIdView(ProjectDetailView): + """View a project by its 8-character manufacturing ID (e.g., G801ABCD). + + This is the canonical URL for projects that have been assigned to a shuttle. + Inherits all functionality from ProjectDetailView, including login, + permission checks, and audit logging. The parent's pk-redirect logic + does not trigger here because the URL provides ``full_id``, not ``pk``. + """ + + def get_object(self, queryset=None): + """Look up project by full_id from URL.""" + if queryset is None: + queryset = self.get_queryset() + + full_id = self.kwargs.get("full_id") + if not full_id: + msg = "No project ID provided" + raise Http404(msg) + + # Parse full_id into shuttle name and project_id + expected_length = SHUTTLE_ID_LENGTH + PROJECT_ID_LENGTH + if len(full_id) != expected_length: + msg = f"Invalid project ID format: expected {expected_length} characters" + raise Http404(msg) + + shuttle_name = full_id[:SHUTTLE_ID_LENGTH] + project_id = full_id[SHUTTLE_ID_LENGTH:] + + return get_object_or_404( + queryset, + shuttle__name=shuttle_name, + project_id=project_id, + ) + + class ProjectCreateView(LoginRequiredMixin, CreateView): """Create a new project.""" @@ -218,7 +292,7 @@ def get_success_url(self): """Redirect to project detail page.""" # self.object is set after form_valid succeeds assert self.object is not None - return reverse_lazy("projects:detail", kwargs={"pk": self.object.pk}) + return self.object.get_absolute_url() class ProjectUpdateView(LoginRequiredMixin, ProjectOwnerOrStaffMixin, UpdateView): @@ -295,7 +369,7 @@ def get_success_url(self): """Redirect to project detail page.""" # self.object is set after form_valid succeeds assert self.object is not None - return reverse_lazy("projects:detail", kwargs={"pk": self.object.pk}) + return self.object.get_absolute_url() class ProjectDeleteView(LoginRequiredMixin, ProjectOwnerOrStaffMixin, DeleteView): @@ -365,7 +439,7 @@ def post(self, request, pk): msg += f" (URL rewritten: {metadata['rewrite_reason']})" messages.success(request, msg) - return redirect("projects:detail", pk=pk) + return redirect(project.get_absolute_url()) except SecurityValidationError as e: messages.error(request, f"Security validation failed: {e}") @@ -568,7 +642,7 @@ def post(self, request, pk, check_id): msg = "Check could not be cancelled (already finished or in error state)." messages.warning(request, msg) - return redirect("projects:detail", pk=pk) + return redirect(project.get_absolute_url()) class ManufacturabilityCheckAdminStatusView( @@ -685,7 +759,7 @@ def post(self, request, pk): ) # Always redirect back to project detail page - return redirect("projects:detail", pk=pk) + return redirect(project.get_absolute_url()) class ProjectIDCheckView(LoginRequiredMixin, View): @@ -1021,4 +1095,4 @@ def check_drc_update_requeue(request, check_id): except ValueError as e: messages.error(request, str(e)) - return redirect("projects:detail", pk=check.project.pk) + return redirect(check.project.get_absolute_url()) diff --git a/wafer_space/projects/views_compliance.py b/wafer_space/projects/views_compliance.py index c8457dd6c..2522fe616 100644 --- a/wafer_space/projects/views_compliance.py +++ b/wafer_space/projects/views_compliance.py @@ -74,13 +74,13 @@ def compliance_certification_create(request: HttpRequest, pk: UUID) -> HttpRespo request, "This project has not been checked for manufacturability yet.", ) - return redirect("projects:detail", pk=project.pk) + return redirect(project.get_absolute_url()) if project.latest_file_manufacturable is not True: messages.error( request, "This project must pass manufacturability checks before certification.", ) - return redirect("projects:detail", pk=project.pk) + return redirect(project.get_absolute_url()) # Check if already certified for context try: @@ -116,7 +116,7 @@ def compliance_certification_create(request: HttpRequest, pk: UUID) -> HttpRespo request, "Compliance certification completed successfully.", ) - return redirect("projects:detail", pk=project.pk) + return redirect(project.get_absolute_url()) messages.error(request, "Please correct the errors below.") # Pre-populate form if already certified diff --git a/wafer_space/shuttles/models.py b/wafer_space/shuttles/models.py index b18c3ef9b..3c936d419 100644 --- a/wafer_space/shuttles/models.py +++ b/wafer_space/shuttles/models.py @@ -35,9 +35,12 @@ def validate_shuttle_id(value: str) -> None: msg = "Shuttle ID must start with 'G8'" raise ValidationError(msg) - # Check last two characters are digits + # Check last two characters are digits. The ASCII check matters: fullwidth + # and Arabic-Indic digits satisfy str.isdigit() and are read by int(), but + # cannot be reversed into the ASCII-only canonical project URL built from + # the shuttle name. suffix = value[2:] - if not suffix.isdigit(): + if not (suffix.isascii() and suffix.isdigit()): msg = ( f"Shuttle ID must end with two digits " f"({SHUTTLE_ID_MIN_NUMBER:02d}-{SHUTTLE_ID_MAX_NUMBER:02d})" diff --git a/wafer_space/shuttles/templates/shuttles/assignment_dashboard.html b/wafer_space/shuttles/templates/shuttles/assignment_dashboard.html index 734a6eb5b..1edd1448d 100644 --- a/wafer_space/shuttles/templates/shuttles/assignment_dashboard.html +++ b/wafer_space/shuttles/templates/shuttles/assignment_dashboard.html @@ -171,7 +171,7 @@
Project Assignment
{% endif %} - {{ project.name }} + {{ project.name }} {{ project.user.username|default:"—" }} diff --git a/wafer_space/shuttles/tests/test_validators.py b/wafer_space/shuttles/tests/test_validators.py index 132d87cbb..f1b6c074a 100644 --- a/wafer_space/shuttles/tests/test_validators.py +++ b/wafer_space/shuttles/tests/test_validators.py @@ -65,6 +65,20 @@ def test_invalid_shuttle_id_non_numeric_suffix(self): validate_shuttle_id("G8AB") assert "two digits" in str(exc_info.value) + def test_invalid_shuttle_id_non_ascii_digit_suffix(self): + """Test that non-ASCII digits are invalid. + + Fullwidth digits satisfy str.isdigit() and int() reads them as 88, so + they passed both the digit and range checks, but they cannot appear in + the ASCII-only canonical project URL built from the shuttle name. + + The suffix is written as escapes: a literal fullwidth digit here would + be indistinguishable from an ASCII 8 to anyone reading the source. + """ + with pytest.raises(ValidationError) as exc_info: + validate_shuttle_id("G8\uff18\uff18") + assert "two digits" in str(exc_info.value) + def test_invalid_shuttle_id_one_digit_suffix(self): """Test that single digit suffix is invalid.""" with pytest.raises(ValidationError) as exc_info: diff --git a/wafer_space/templates/projects/admin_summary.html b/wafer_space/templates/projects/admin_summary.html index 2152ab528..b38b6bc64 100644 --- a/wafer_space/templates/projects/admin_summary.html +++ b/wafer_space/templates/projects/admin_summary.html @@ -446,7 +446,7 @@

Project Summary

{% endif %} - {{ project.name }} + {{ project.name }} {{ project.user.username }} {{ project.user.email }} diff --git a/wafer_space/templates/projects/compliance_certification_form.html b/wafer_space/templates/projects/compliance_certification_form.html index 34f675b31..563cc1778 100644 --- a/wafer_space/templates/projects/compliance_certification_form.html +++ b/wafer_space/templates/projects/compliance_certification_form.html @@ -13,7 +13,7 @@ Projects @@ -81,8 +81,7 @@
Certification Form
{{ form.fields.end_use_statement.help_text }}
- Cancel + Cancel Cancel diff --git a/wafer_space/templates/projects/project_file_submit_url.html b/wafer_space/templates/projects/project_file_submit_url.html index e6ec63f2a..81cc5aa0c 100644 --- a/wafer_space/templates/projects/project_file_submit_url.html +++ b/wafer_space/templates/projects/project_file_submit_url.html @@ -134,7 +134,7 @@
Submit for Download Cancel diff --git a/wafer_space/templates/projects/project_form.html b/wafer_space/templates/projects/project_form.html index a87b3934b..253654349 100644 --- a/wafer_space/templates/projects/project_form.html +++ b/wafer_space/templates/projects/project_form.html @@ -106,7 +106,7 @@
Project Details
{% endif %} Cancel diff --git a/wafer_space/templates/projects/project_list.html b/wafer_space/templates/projects/project_list.html index 9eb07b0d2..c27aa25c1 100644 --- a/wafer_space/templates/projects/project_list.html +++ b/wafer_space/templates/projects/project_list.html @@ -35,7 +35,7 @@

My Projects

- {{ project.name }} {% if project.user == request.user %} Your Project