From cdefbd010c4e97ec6a27beb0fb3c4c6cc3f2778d Mon Sep 17 00:00:00 2001 From: ixgadmin Date: Thu, 17 Sep 2026 12:44:52 +0000 Subject: [PATCH] Serve document and certificate files through a permission-checked download MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Media was unreachable from either portal, which is why the certificate button on the buyer compliance page returned page-not-found and no document row had anything to click. The cause was structural: the portal and ops hosts proxy location / to the app with a path prefix, so a stored /media/... URL arrives as /portal/media/... and matches no route. Add a download view per portal behind the same role gate as the page that links to it — buyer-visible documents and published certificates for buyers, anything the ops list shows for operations — resolving the stored path safely under MEDIA_ROOT so a traversal or an absolute path is refused. Point the seven templates that list files at the new routes. Also add a portal-wide render sweep: nothing in the suite was fetching the ops or portal pages, so a view could lose its template_name and 500 in front of a user with every test still green. --- apps/buyer/urls.py | 2 + apps/buyer/views.py | 34 ++++- apps/core/tests.py | 59 ++++++++ apps/documents/downloads.py | 60 ++++++++ apps/documents/tests.py | 209 +++++++++++++++++++++++++++ apps/ops/urls.py | 2 + apps/ops/views.py | 28 ++++ templates/ops/batch_detail.html | 2 +- templates/ops/coa_review_detail.html | 6 +- templates/ops/document_list.html | 2 +- templates/portal/batch_detail.html | 2 +- templates/portal/compliance.html | 2 +- templates/portal/dashboard.html | 2 +- templates/portal/document_list.html | 2 +- 14 files changed, 402 insertions(+), 10 deletions(-) create mode 100644 apps/core/tests.py create mode 100644 apps/documents/downloads.py create mode 100644 apps/documents/tests.py diff --git a/apps/buyer/urls.py b/apps/buyer/urls.py index fa2e12c..f2e1f78 100644 --- a/apps/buyer/urls.py +++ b/apps/buyer/urls.py @@ -10,6 +10,8 @@ urlpatterns = [ path('batches/', views.BatchListView.as_view(), name='batch_list'), path('batches//', views.BatchDetailView.as_view(), name='batch_detail'), path('documents/', views.DocumentListView.as_view(), name='document_list'), + path('documents//download/', views.DocumentDownloadView.as_view(), name='document_download'), + path('certificates//download/', views.CoADownloadView.as_view(), name='coa_download'), path('subscription/', views.SubscriptionView.as_view(), name='subscription'), path('compliance/', views.ComplianceView.as_view(), name='compliance'), path('help/', views.GuideIndexView.as_view(), name='guide_index'), diff --git a/apps/buyer/views.py b/apps/buyer/views.py index ed05b77..d650f02 100644 --- a/apps/buyer/views.py +++ b/apps/buyer/views.py @@ -1,10 +1,13 @@ from django.contrib.auth.mixins import LoginRequiredMixin, UserPassesTestMixin from django.db.models import Count +from django.shortcuts import get_object_or_404 +from django.views import View from django.views.generic import TemplateView, ListView, DetailView from apps.batches.models import ProcessingBatch from apps.shipments.models import Shipment -from apps.documents.models import Document +from apps.documents.downloads import file_response +from apps.documents.models import CertificateOfAnalysis, Document from apps.esg.models import ESGFieldReport from apps.subscriptions.models import Subscription from apps.guides.views import GuideDetailView as BaseGuideDetailView @@ -88,6 +91,35 @@ class DocumentListView(BuyerRequiredMixin, ListView): return ctx +class DocumentDownloadView(BuyerRequiredMixin, View): + """Serve the stored file behind a buyer-visible document. + + A buyer may only fetch files from documents marked buyer-visible, so a + guessed document id belonging to an operations-only row is a 404 rather + than a leak. + """ + + def get(self, request, pk): + doc = get_object_or_404(Document, pk=pk, visibility=Document.Visibility.BUYER) + return file_response(doc.file_url, doc.file_name) + + +class CoADownloadView(BuyerRequiredMixin, View): + """Serve the summary PDF of a published certificate of analysis. + + Only a published certificate is downloadable, matching the compliance page + that links to it: a certificate still awaiting release stays internal. + """ + + def get(self, request, pk): + coa = get_object_or_404( + CertificateOfAnalysis, + pk=pk, + status=CertificateOfAnalysis.Status.PUBLISHED, + ) + return file_response(coa.file_url, coa.file_name) + + class SubscriptionView(BuyerRequiredMixin, DetailView): model = Subscription template_name = "portal/subscription.html" diff --git a/apps/core/tests.py b/apps/core/tests.py new file mode 100644 index 0000000..0c0b213 --- /dev/null +++ b/apps/core/tests.py @@ -0,0 +1,59 @@ +"""Every portal page must actually render. + +Nothing in the suite was rendering the ops or portal HTML pages, which is how a +view can lose a line — a `template_name`, say — and 500 in front of a user while +68 tests still pass. These sweeps fetch each read-only page with an empty +database, so they cover the empty-state branches as well as the templates. +""" +from django.contrib.auth import get_user_model +from django.test import Client, TestCase + +User = get_user_model() + +OPS_ROUTES = [ + "/ops/", + "/ops/batches/", + "/ops/qc/", + "/ops/qc/verify/", + "/ops/coa/", + "/ops/shipments/", + "/ops/capa/", + "/ops/documents/", + "/ops/esg/", + "/ops/eudr/", + "/ops/help/", +] + +PORTAL_ROUTES = [ + "/portal/", + "/portal/batches/", + "/portal/documents/", + "/portal/compliance/", + "/portal/subscription/", + "/portal/help/", +] + + +class PortalPagesRenderTests(TestCase): + def make_user(self, email, role): + return User.objects.create_user( + email=email, password="pass1234", role=role, + full_name=email.split("@")[0], is_staff=True, + ) + + def sweep(self, user, routes, host): + client = Client(SERVER_NAME=host) + client.force_login(user) + for route in routes: + with self.subTest(route=route): + response = client.get(route) + body = response.content + self.assertEqual(response.status_code, 200, f"{route} did not render") + for marker in (b"Traceback", b"TemplateDoesNotExist", b"Django Version"): + self.assertNotIn(marker, body, f"{route} rendered an error page") + + def test_ops_pages_render(self): + self.sweep(self.make_user("ops-sweep@ixg.local", "ops"), OPS_ROUTES, "ops.ixgltd.com") + + def test_buyer_pages_render(self): + self.sweep(self.make_user("buyer-sweep@ixg.local", "buyer"), PORTAL_ROUTES, "portal.ixgltd.com") diff --git a/apps/documents/downloads.py b/apps/documents/downloads.py new file mode 100644 index 0000000..628a4ff --- /dev/null +++ b/apps/documents/downloads.py @@ -0,0 +1,60 @@ +"""Permission-checked document downloads. + +Neither portal can serve a stored document today, and the reason is structural +rather than a missing link. The portals sit behind nginx servers that proxy +``location /`` to the Django app **with a path prefix** (``/portal/`` and +``/ops/``), so a stored ``/media/...`` URL arrives at Django as +``/portal/media/...`` — a route that does not exist. Django's own DEBUG media +route is registered at ``/media/``, not under either prefix, so even a file that +is present on disk comes back as a page-not-found. + +Serving media directly from the web server would not be the answer either: +every document row carries a visibility setting (buyer / ops / admin) and every +certificate carries a status, so the file behind a row is not public. The +download has to pass through the same permission gate as the page that links to +it. + +So: these helpers resolve a stored ``file_url`` to a real path under +``MEDIA_ROOT`` — refusing anything that escapes it — and stream the bytes back +to a caller that has already passed a portal permission gate. +""" +from pathlib import Path + +from django.conf import settings +from django.http import FileResponse, Http404 + + +def resolve_media_path(file_url): + """Absolute path for a stored ``/media/...`` file_url, or None if unacceptable. + + None means "do not serve this": an empty value, a path outside MEDIA_URL, or + a path that resolves outside MEDIA_ROOT (``../`` and symlink escapes). + """ + if not file_url: + return None + + media_url = settings.MEDIA_URL.rstrip("/") # "/media" + rel = file_url.strip() + if rel.startswith(media_url + "/"): + rel = rel[len(media_url) + 1:] + elif rel.startswith("/"): + # An absolute filesystem path, rather than a path under MEDIA_URL. + return None + + base = Path(settings.MEDIA_ROOT).resolve() + target = (base / rel).resolve() + if target != base and base not in target.parents: + return None + return target + + +def file_response(file_url, download_name=None): + """Stream a stored file back as an inline response. + + Raises 404 when the value cannot be resolved or the file is not on disk, so a + broken row reads as "not found" rather than as a server error. + """ + path = resolve_media_path(file_url) + if path is None or not path.is_file(): + raise Http404("Document file not found") + return FileResponse(open(path, "rb"), filename=download_name or path.name) diff --git a/apps/documents/tests.py b/apps/documents/tests.py new file mode 100644 index 0000000..2624923 --- /dev/null +++ b/apps/documents/tests.py @@ -0,0 +1,209 @@ +"""Downloads must be gated the same way the page that links to them is. + +Two classes of failure matter here and neither is caught by "the link renders": +a file that cannot be served at all (the state the portals were in), and a file +served to someone who should not have it. Both are covered below, plus the path +handling that turns a stored value into a real file. +""" +import os +import shutil +import tempfile +from decimal import Decimal +from pathlib import Path + +from django.contrib.auth import get_user_model +from django.test import Client, TestCase, override_settings + +from apps.batches.models import ProcessingBatch + +from .downloads import resolve_media_path +from .models import CertificateOfAnalysis, Document + +User = get_user_model() + +PDF_BYTES = b"%PDF-1.4\n% synthetic document for tests\n%%EOF\n" + + +def make_user(email, role): + # Mirrors the Keycloak OIDC backend, which creates all users as is_staff=True. + return User.objects.create_user( + email=email, password="pass1234", role=role, + full_name=email.split("@")[0], is_staff=True, + ) + + +class DownloadTestCase(TestCase): + """Fixtures: a real temp media root, plus one user per role.""" + + def setUp(self): + self.media_root = Path(tempfile.mkdtemp(prefix="ixg-download-test-")) + self.addCleanup(shutil.rmtree, self.media_root, True) + self.settings_override = override_settings(MEDIA_ROOT=self.media_root) + self.settings_override.enable() + self.addCleanup(self.settings_override.disable) + + self.buyer = make_user("buyer@ixg.local", "buyer") + self.ops = make_user("ops@ixg.local", "ops") + self.admin = make_user("admin@ixg.local", "admin") + + self.batch = ProcessingBatch.objects.create( + batch_reference="IXG-BATCH-TEST-1", + grade="A", + weight_kg=Decimal("1000.00"), + origin_facility="ASI - Katsina State, Nigeria", + created_by=self.ops, + ) + + self.buyer_doc = self.make_document("packing-list.pdf", Document.Visibility.BUYER) + self.ops_doc = self.make_document("lab-report.pdf", Document.Visibility.OPS) + self.admin_doc = self.make_document("internal-note.pdf", Document.Visibility.ADMIN) + self.published_coa = self.make_coa("coa-published.pdf", CertificateOfAnalysis.Status.PUBLISHED) + self.pending_coa = self.make_coa("coa-pending.pdf", CertificateOfAnalysis.Status.PENDING_APPROVAL) + + def write_file(self, name): + """Put real bytes on disk under MEDIA_ROOT and return the stored file_url.""" + rel = f"ixg-documents/{self.batch.batch_reference.lower()}/{name}" + target = self.media_root / rel + target.parent.mkdir(parents=True, exist_ok=True) + target.write_bytes(PDF_BYTES) + return f"/media/{rel}" + + def make_document(self, name, visibility): + return Document.objects.create( + batch=self.batch, + doc_type=Document.DocType.OTHER, + file_url=self.write_file(name), + file_name=name, + visibility=visibility, + uploaded_by=self.ops, + ) + + def make_coa(self, name, status): + return CertificateOfAnalysis.objects.create( + batch=self.batch, + file_url=self.write_file(name), + file_name=name, + status=status, + uploaded_by=self.ops, + ) + + def get(self, user, url, host): + c = Client(SERVER_NAME=host) + c.force_login(user) + return c.get(url) + + @staticmethod + def body(response): + if getattr(response, "streaming", False): + return b"".join(response.streaming_content) + return response.content + + +class BuyerDownloadTests(DownloadTestCase): + def test_buyer_downloads_a_buyer_visible_document(self): + r = self.get(self.buyer, f"/portal/documents/{self.buyer_doc.id}/download/", "portal.ixgltd.com") + self.assertEqual(r.status_code, 200) + self.assertEqual(self.body(r), PDF_BYTES) + self.assertIn("packing-list.pdf", r.headers.get("Content-Disposition", "")) + + def test_buyer_cannot_download_an_operations_document(self): + r = self.get(self.buyer, f"/portal/documents/{self.ops_doc.id}/download/", "portal.ixgltd.com") + self.assertEqual(r.status_code, 404) + + def test_buyer_cannot_download_an_admin_document(self): + r = self.get(self.buyer, f"/portal/documents/{self.admin_doc.id}/download/", "portal.ixgltd.com") + self.assertEqual(r.status_code, 404) + + def test_buyer_downloads_a_published_certificate(self): + r = self.get(self.buyer, f"/portal/certificates/{self.published_coa.id}/download/", "portal.ixgltd.com") + self.assertEqual(r.status_code, 200) + self.assertEqual(self.body(r), PDF_BYTES) + + def test_buyer_cannot_download_a_certificate_awaiting_release(self): + r = self.get(self.buyer, f"/portal/certificates/{self.pending_coa.id}/download/", "portal.ixgltd.com") + self.assertEqual(r.status_code, 404) + + def test_anonymous_visitor_is_refused(self): + c = Client(SERVER_NAME="portal.ixgltd.com") + r = c.get(f"/portal/documents/{self.buyer_doc.id}/download/") + self.assertIn(r.status_code, (302, 403)) + + def test_ops_user_is_refused_on_the_buyer_route(self): + r = self.get(self.ops, f"/portal/documents/{self.buyer_doc.id}/download/", "portal.ixgltd.com") + self.assertEqual(r.status_code, 403) + + +class OpsDownloadTests(DownloadTestCase): + def test_ops_downloads_a_buyer_visible_document(self): + r = self.get(self.ops, f"/ops/documents/{self.buyer_doc.id}/download/", "ops.ixgltd.com") + self.assertEqual(r.status_code, 200) + self.assertEqual(self.body(r), PDF_BYTES) + + def test_ops_downloads_an_operations_document(self): + r = self.get(self.ops, f"/ops/documents/{self.ops_doc.id}/download/", "ops.ixgltd.com") + self.assertEqual(r.status_code, 200) + + def test_ops_can_open_a_certificate_awaiting_release(self): + """A reviewer needs the pending certificate most of all.""" + r = self.get(self.ops, f"/ops/certificates/{self.pending_coa.id}/download/", "ops.ixgltd.com") + self.assertEqual(r.status_code, 200) + self.assertEqual(self.body(r), PDF_BYTES) + + def test_buyer_is_refused_on_the_ops_route(self): + r = self.get(self.buyer, f"/ops/documents/{self.ops_doc.id}/download/", "ops.ixgltd.com") + self.assertIn(r.status_code, (302, 403)) + + +class MissingAndHostileFileTests(DownloadTestCase): + def test_missing_file_is_not_found_rather_than_a_server_error(self): + doc = Document.objects.create( + batch=self.batch, + doc_type=Document.DocType.OTHER, + file_url="/media/ixg-documents/nowhere/nothing-here.pdf", + file_name="nothing-here.pdf", + visibility=Document.Visibility.BUYER, + uploaded_by=self.ops, + ) + r = self.get(self.buyer, f"/portal/documents/{doc.id}/download/", "portal.ixgltd.com") + self.assertEqual(r.status_code, 404) + + def test_empty_file_url_is_not_found(self): + doc = Document.objects.create( + batch=self.batch, + doc_type=Document.DocType.OTHER, + file_url="", + file_name="empty.pdf", + visibility=Document.Visibility.BUYER, + uploaded_by=self.ops, + ) + r = self.get(self.buyer, f"/portal/documents/{doc.id}/download/", "portal.ixgltd.com") + self.assertEqual(r.status_code, 404) + + def test_a_traversal_out_of_media_root_is_refused(self): + doc = Document.objects.create( + batch=self.batch, + doc_type=Document.DocType.OTHER, + file_url="/media/../../config/settings.py", + file_name="escaped.pdf", + visibility=Document.Visibility.BUYER, + uploaded_by=self.ops, + ) + r = self.get(self.buyer, f"/portal/documents/{doc.id}/download/", "portal.ixgltd.com") + self.assertEqual(r.status_code, 404) + + +class ResolveMediaPathTests(DownloadTestCase): + def test_resolves_a_normal_media_url(self): + path = resolve_media_path(self.buyer_doc.file_url) + self.assertTrue(path and path.is_file()) + + def test_refuses_paths_outside_media_url(self): + self.assertIsNone(resolve_media_path("/etc/passwd")) + self.assertIsNone(resolve_media_path("/app/config/settings.py")) + + def test_refuses_traversal(self): + self.assertIsNone(resolve_media_path("/media/../config/settings.py")) + + def test_refuses_empty_values(self): + self.assertIsNone(resolve_media_path("")) + self.assertIsNone(resolve_media_path(None)) diff --git a/apps/ops/urls.py b/apps/ops/urls.py index a55deb2..24add22 100644 --- a/apps/ops/urls.py +++ b/apps/ops/urls.py @@ -14,6 +14,8 @@ urlpatterns = [ path('capa/', views.CAPAListView.as_view(), name='capa_list'), path('capa//', views.CAPADetailView.as_view(), name='capa_detail'), path('documents/', views.DocumentListView.as_view(), name='document_list'), + path('documents//download/', views.DocumentDownloadView.as_view(), name='document_download'), + path('certificates//download/', views.CoADownloadView.as_view(), name='coa_download'), path('esg/', views.ESGListView.as_view(), name='esg_list'), path('qc/verify/', views.QCVerifyListView.as_view(), name='qc_verify_list'), path('qc/verify//', views.QCVerifyDetailView.as_view(), name='qc_verify_detail'), diff --git a/apps/ops/views.py b/apps/ops/views.py index 8b19e0f..dc7ad06 100644 --- a/apps/ops/views.py +++ b/apps/ops/views.py @@ -3,12 +3,14 @@ from django.db.models import Count, Q from django.shortcuts import redirect from django.utils import timezone from django.views.generic import TemplateView, ListView, DetailView +from django.views import View from django.shortcuts import get_object_or_404 from apps.batches.models import ProcessingBatch from apps.qc.models import QCResult from apps.shipments.models import Shipment from apps.capa.models import CAPARecord +from apps.documents.downloads import file_response from apps.documents.models import Document, CertificateOfAnalysis from apps.esg.models import ESGFieldReport from apps.notifications.models import Notification @@ -191,6 +193,32 @@ class DocumentListView(OpsRequiredMixin, ListView): return ctx +class DocumentDownloadView(OpsRequiredMixin, View): + """Serve the stored file behind a document row to operations staff. + + Operations may fetch any document the ops document list shows it. Note the + list itself has always shown every row regardless of visibility, so this + matches the list rather than being stricter than it; the buyer-side view is + where visibility does the gatekeeping. + """ + + def get(self, request, pk): + doc = get_object_or_404(Document, pk=pk) + return file_response(doc.file_url, doc.file_name) + + +class CoADownloadView(OpsRequiredMixin, View): + """Serve a certificate of analysis to operations staff, at any status. + + Unlike the buyer's view, a certificate still awaiting release is exactly what + a reviewer needs to open, so the status is not part of the gate here. + """ + + def get(self, request, pk): + coa = get_object_or_404(CertificateOfAnalysis, pk=pk) + return file_response(coa.file_url, coa.file_name) + + class ESGListView(OpsRequiredMixin, ListView): model = ESGFieldReport template_name = 'ops/esg_list.html' diff --git a/templates/ops/batch_detail.html b/templates/ops/batch_detail.html index f06df62..bc15b4c 100644 --- a/templates/ops/batch_detail.html +++ b/templates/ops/batch_detail.html @@ -99,7 +99,7 @@ {% if documents %}
    {% for d in documents %} -
  • {{ d.file_name }} {{ d.get_doc_type_display }}
    • +
  • {{ d.file_name }} {{ d.get_doc_type_display }}
    • {% endfor %}
{% else %}

No documents

{% endif %} diff --git a/templates/ops/coa_review_detail.html b/templates/ops/coa_review_detail.html index 17f2e83..356729b 100644 --- a/templates/ops/coa_review_detail.html +++ b/templates/ops/coa_review_detail.html @@ -27,7 +27,7 @@ Released At{{ coa.released_at|date:'d/m/Y H:i'|default:'-' }} Published At{{ coa.published_at|date:'d/m/Y H:i'|default:'-' }} SHA-256{{ coa.sha256_hash|default:'-' }} - Summary PDF {{ coa.file_name }} + Summary PDF {{ coa.file_name }} @@ -36,14 +36,14 @@
Source Documents

Independent laboratory report (authoritative):

{% if coa.source_document %} - {{ coa.source_document.file_name }} + {{ coa.source_document.file_name }} {% else %}

No lab report linked. Upload the original laboratory PDF for the batch.

{% endif %} {% if lab_documents %}

All lab documents for batch:

- {% for d in lab_documents %}{% endfor %} + {% for d in lab_documents %}{% endfor %} {% endif %}
diff --git a/templates/ops/document_list.html b/templates/ops/document_list.html index db2476f..4210a3d 100644 --- a/templates/ops/document_list.html +++ b/templates/ops/document_list.html @@ -32,7 +32,7 @@ {% for d in documents %} - {{ d.file_name }} + {{ d.file_name }} {{ d.get_doc_type_display }} {{ d.batch.batch_reference|default:'-' }} {{ d.get_visibility_display }} diff --git a/templates/portal/batch_detail.html b/templates/portal/batch_detail.html index be2a6e4..1a4aa26 100644 --- a/templates/portal/batch_detail.html +++ b/templates/portal/batch_detail.html @@ -44,7 +44,7 @@ {% if documents %}
    {% for d in documents %} -
  • {{ d.file_name }} {{ d.get_doc_type_display }}
    • +
  • {{ d.file_name }} {{ d.get_doc_type_display }}
    • {% endfor %}
{% else %}

No documents

{% endif %} diff --git a/templates/portal/compliance.html b/templates/portal/compliance.html index a9882bc..1b278f5 100644 --- a/templates/portal/compliance.html +++ b/templates/portal/compliance.html @@ -24,7 +24,7 @@ {{ c.batch.batch_reference }} Unrefined Shea Butter {{ c.issue_date|date:'d/m/Y'|default:'-' }} - PDF + PDF {% empty %} No approved certificates available diff --git a/templates/portal/dashboard.html b/templates/portal/dashboard.html index 385c564..b78f71d 100644 --- a/templates/portal/dashboard.html +++ b/templates/portal/dashboard.html @@ -54,7 +54,7 @@
Recent Documents
{% for d in documents %}
- {{ d.file_name }}
+ {{ d.file_name }}
{{ d.get_doc_type_display }} · {{ d.uploaded_at|date:'d/m/Y' }}
{% empty %} diff --git a/templates/portal/document_list.html b/templates/portal/document_list.html index 0e78d6b..3df965e 100644 --- a/templates/portal/document_list.html +++ b/templates/portal/document_list.html @@ -30,7 +30,7 @@ {% for d in documents %} - {{ d.file_name }} + {{ d.file_name }} {{ d.get_doc_type_display }} {{ d.batch.batch_reference|default:'-' }} {{ d.uploaded_at|date:'d/m/Y' }}