diff --git a/aincrad/tests.py b/aincrad/tests.py index 020f3656..1d9f6a38 100644 --- a/aincrad/tests.py +++ b/aincrad/tests.py @@ -1,6 +1,7 @@ from hashlib import sha256 import pytest +from django.core.files.uploadedfile import SimpleUploadedFile from django.test.utils import override_settings from arch.factories import HintFactory, ProblemFactory @@ -10,7 +11,8 @@ from dashboard.models import Announcement from hanabi.factories import HanabiContestFactory, HanabiPlayerFactory from hanabi.models import HanabiParticipation, HanabiReplay -from opal.factories import OpalPuzzleFactory +from opal.factories import OpalHuntFactory, OpalPuzzleFactory +from opal.models import OpalPuzzle from payments.factories import PaymentLogFactory from roster.factories import ( InvoiceFactory, @@ -25,6 +27,10 @@ TARGET_HASH = sha256(EXAMPLE_PASSWORD.encode("ascii")).hexdigest() +def opal_pdf(body: bytes) -> SimpleUploadedFile: + return SimpleUploadedFile("tetrogram.pdf", b"%PDF-1.4 " + body) + + @pytest.fixture def aincrad_setup(db): active_semester = SemesterFactory.create(name="New") @@ -748,9 +754,8 @@ def test_announcement(otis): @pytest.mark.django_db @override_settings(API_TARGET_HASH=TARGET_HASH) def test_opal_handler(otis): - OpalPuzzleFactory.create( - hunt__slug="teammate", slug="tetrogram", is_metapuzzle=True - ) + hunt = OpalHuntFactory.create(slug="teammate") + OpalPuzzleFactory.create(hunt=hunt, slug="tetrogram", is_metapuzzle=True) resp = otis.post_20x( "api", json={ @@ -763,6 +768,27 @@ def test_opal_handler(otis): assert puzzle_json["hunt__slug"] == "teammate" assert puzzle_json["slug"] == "tetrogram" assert puzzle_json["is_metapuzzle"] is True + # A puzzle with no PDF yet reports no fingerprint, which is what tells a sync + # to upload rather than skip. + assert puzzle_json["content"] == "" + assert puzzle_json["content_hash"] == "" + + +@pytest.mark.django_db +@override_settings(API_TARGET_HASH=TARGET_HASH) +def test_opal_handler_reports_content_hash(otis): + puzzle = OpalPuzzleFactory.create(hunt__slug="teammate", slug="tetrogram") + puzzle.content.save("tetrogram.pdf", opal_pdf(b"first"), save=False) + puzzle.content_hash = "a" * 64 + puzzle.save() + + resp = otis.post_20x( + "api", + json={"action": "opal_list", "token": EXAMPLE_PASSWORD}, + ) + puzzle_json = resp.json()["puzzles"][0] + assert puzzle_json["content_hash"] == "a" * 64 + assert puzzle_json["content"].endswith("tetrogram.pdf") @pytest.mark.django_db @@ -783,3 +809,151 @@ def test_apply_uuid_handler(otis): ).count() == 1 ) + + +@pytest.mark.django_db +@override_settings(API_TARGET_HASH=TARGET_HASH) +def test_opal_pdf_upload(otis): + puzzle = OpalPuzzleFactory.create(hunt__slug="teammate", slug="tetrogram") + assert puzzle.content_hash == "" + + resp = otis.post_20x( + "opal-pdf-upload", + data={ + "token": EXAMPLE_PASSWORD, + "pk": puzzle.pk, + "content": opal_pdf(b"first draft"), + }, + ) + assert resp.json()["status"] == "created" + puzzle.refresh_from_db() + assert puzzle.content.read() == b"%PDF-1.4 first draft" + assert puzzle.content.name.startswith("opals/teammate/") + assert puzzle.content.name.endswith("/tetrogram.pdf") + assert puzzle.content_hash == sha256(b"%PDF-1.4 first draft").hexdigest() + assert resp.json()["content_hash"] == puzzle.content_hash + + first_name = puzzle.content.name + + # Same bytes again: the server should recognize them and leave storage alone. + resp = otis.post_20x( + "opal-pdf-upload", + data={ + "token": EXAMPLE_PASSWORD, + "pk": puzzle.pk, + "content": opal_pdf(b"first draft"), + }, + ) + assert resp.json()["status"] == "unchanged" + puzzle.refresh_from_db() + assert puzzle.content.name == first_name + + # Different bytes: rewritten in place, with no suffixed duplicate left behind. + resp = otis.post_20x( + "opal-pdf-upload", + data={ + "token": EXAMPLE_PASSWORD, + "pk": puzzle.pk, + "content": opal_pdf(b"second draft"), + }, + ) + assert resp.json()["status"] == "updated" + puzzle.refresh_from_db() + assert puzzle.content.name == first_name + assert puzzle.content.read() == b"%PDF-1.4 second draft" + assert puzzle.content_hash == sha256(b"%PDF-1.4 second draft").hexdigest() + + +@pytest.mark.django_db +@override_settings(API_TARGET_HASH=TARGET_HASH) +def test_opal_pdf_upload_renamed_file_leaves_no_orphan(otis): + puzzle = OpalPuzzleFactory.create(hunt__slug="teammate", slug="tetrogram") + otis.post_20x( + "opal-pdf-upload", + data={ + "token": EXAMPLE_PASSWORD, + "pk": puzzle.pk, + "content": SimpleUploadedFile("old_name.pdf", b"%PDF-1.4 draft"), + }, + ) + puzzle.refresh_from_db() + old_name = puzzle.content.name + assert puzzle.content.storage.exists(old_name) + + otis.post_20x( + "opal-pdf-upload", + data={ + "token": EXAMPLE_PASSWORD, + "pk": puzzle.pk, + "content": SimpleUploadedFile("tetrogram.pdf", b"%PDF-1.4 draft"), + }, + ) + puzzle.refresh_from_db() + assert puzzle.content.name.endswith("tetrogram.pdf") + assert not puzzle.content.storage.exists(old_name) + + +@pytest.mark.django_db +@override_settings(API_TARGET_HASH=TARGET_HASH) +@pytest.mark.parametrize( + "upload", + ( + SimpleUploadedFile("tetrogram.tex", b"%PDF-1.4 not really"), + SimpleUploadedFile("tetrogram.pdf", b"\\documentclass{article}"), + ), + ids=("wrong extension", "wrong bytes"), +) +def test_opal_pdf_upload_rejects_non_pdf(otis, upload: SimpleUploadedFile): + puzzle = OpalPuzzleFactory.create(hunt__slug="teammate", slug="tetrogram") + otis.post_40x( + "opal-pdf-upload", + data={"token": EXAMPLE_PASSWORD, "pk": puzzle.pk, "content": upload}, + ) + puzzle.refresh_from_db() + assert not puzzle.content + assert puzzle.content_hash == "" + + +@pytest.mark.django_db +@override_settings(API_TARGET_HASH=TARGET_HASH) +def test_opal_pdf_upload_failed_auth(otis): + puzzle = OpalPuzzleFactory.create(hunt__slug="teammate", slug="tetrogram") + resp = otis.post_40x( + "opal-pdf-upload", + data={ + "token": "this wrong password is not a puzzle", + "pk": puzzle.pk, + "content": opal_pdf(b"first draft"), + }, + ) + assert resp.status_code == 418 + puzzle.refresh_from_db() + assert not puzzle.content + + +@pytest.mark.django_db +@override_settings(API_TARGET_HASH=TARGET_HASH) +def test_opal_pdf_upload_unknown_puzzle(otis): + otis.post_not_found( + "opal-pdf-upload", + data={ + "token": EXAMPLE_PASSWORD, + "pk": 1729, + "content": opal_pdf(b"first draft"), + }, + ) + assert not OpalPuzzle.objects.exists() + + +@pytest.mark.django_db +@override_settings(API_TARGET_HASH=TARGET_HASH) +@pytest.mark.parametrize("pk", ("", "tetrogram"), ids=("empty", "not a number")) +def test_opal_pdf_upload_malformed_pk(otis, pk: str): + otis.post_40x( + "opal-pdf-upload", + data={ + "token": EXAMPLE_PASSWORD, + "pk": pk, + "content": opal_pdf(b"first draft"), + }, + ) diff --git a/aincrad/urls.py b/aincrad/urls.py index 857c004a..55869d4a 100644 --- a/aincrad/urls.py +++ b/aincrad/urls.py @@ -2,4 +2,7 @@ from . import views -urlpatterns = [path(r"api/", views.api, name="api")] +urlpatterns = [ + path(r"api/", views.api, name="api"), + path(r"api/opal-pdf/", views.opal_pdf_upload, name="opal-pdf-upload"), +] diff --git a/aincrad/views.py b/aincrad/views.py index 630d22d8..906ae5b3 100644 --- a/aincrad/views.py +++ b/aincrad/views.py @@ -1,5 +1,6 @@ import json import logging +import os import string from datetime import timedelta from decimal import Decimal @@ -10,6 +11,7 @@ from allauth.socialaccount.models import SocialAccount from django.conf import settings from django.core.exceptions import PermissionDenied, SuspiciousOperation +from django.core.files.uploadhandler import TemporaryFileUploadHandler from django.db.models.aggregates import Sum from django.db.models.query import QuerySet, prefetch_related_objects from django.db.models.query_utils import Q @@ -24,7 +26,7 @@ from arch.models import Hint, Problem from dashboard.models import Announcement, PSet from hanabi.models import HanabiContest, HanabiParticipation, HanabiPlayer, HanabiReplay -from opal.models import OpalPuzzle +from opal.models import OpalPuzzle, sha256_of from payments.models import Job from roster.models import ( ApplyUUID, @@ -666,6 +668,7 @@ def opal_handler(action: str, data: JSONData) -> JsonResponse: "order", "num_to_unlock", "content", + "content_hash", "is_metapuzzle", "answer", "partial_answers", @@ -677,6 +680,90 @@ def opal_handler(action: str, data: JSONData) -> JsonResponse: ) +@csrf_exempt +def opal_pdf_upload(request: HttpRequest) -> JsonResponse: + """Replace one OPAL puzzle's PDF, skipping the write if the bytes are unchanged. + + Separate from `api` because that view is JSON-only (`json.loads(request.body)`), + and one puzzle per request because that keeps a partial sync legible: the client + gets a per-file verdict instead of one opaque result for the whole batch. + """ + if request.method != "POST": + raise PermissionDenied("Must use POST") + + # Swap the upload handler before anything reads request.POST, which is what + # populates request.FILES. The project configures memory-only uploads capped at + # FILE_UPLOAD_MAX_MEMORY_SIZE, and that handler measures the whole request body: + # a PDF over the cap would be discarded with no error at all, leaving + # request.FILES empty while the sync script saw a 200. Streaming to disk instead + # takes the size limit out of the picture. Safe under @csrf_exempt, which stops + # CsrfViewMiddleware from touching request.POST first. + request.upload_handlers = [TemporaryFileUploadHandler(request)] + + if (bad_token := reject_bad_token(request.POST.get("token"))) is not None: + return bad_token + + # isdigit() rather than leaving it to the ORM, which raises ValueError (a 500) + # on a pk that is not a number at all. + pk = request.POST.get("pk", "") + if not pk.isdigit(): + raise SuspiciousOperation("No valid puzzle pk provided") + puzzle = get_object_or_404(OpalPuzzle, pk=int(pk)) + + upload = request.FILES.get("content") + if upload is None: + raise SuspiciousOperation("No file provided") + + filename = os.path.basename(upload.name or "") + if not filename.lower().endswith(".pdf"): + return JsonResponse( + {"error": f"{filename} is not named like a PDF"}, status=400 + ) + # Cheap guard against a sync pointed at the wrong directory, or at a PDF that + # never finished being written. The model only validates the extension. + header = upload.read(5) + upload.seek(0) + if header != b"%PDF-": + return JsonResponse({"error": f"{filename} is not a PDF"}, status=400) + + digest = sha256_of(upload) + # Same bytes under a new name still has to be written, since the storage key + # ends in the basename and the old one would otherwise stay the live file. + stored_name = os.path.basename(puzzle.content.name or "") if puzzle.content else "" + if digest == puzzle.content_hash and stored_name == filename: + return JsonResponse( + { + "status": "unchanged", + "pk": puzzle.pk, + "hunt": puzzle.hunt.slug, + "slug": puzzle.slug, + "content": puzzle.content.name, + "content_hash": digest, + } + ) + + status = "updated" if puzzle.content else "created" + if puzzle.content: + # Delete the old blob before writing. The storage key ends in the file's + # basename, so a renamed PDF would strand the old one; and locally + # FileSystemStorage would suffix the new name rather than overwrite it. + puzzle.content.delete(save=False) + puzzle.content.save(filename, upload, save=False) + puzzle.content_hash = digest + puzzle.save() + + return JsonResponse( + { + "status": status, + "pk": puzzle.pk, + "hunt": puzzle.hunt.slug, + "slug": puzzle.slug, + "content": puzzle.content.name, + "content_hash": digest, + } + ) + + def announcement_handler(action: str, data: JSONData) -> JsonResponse: del action announcement, is_new = Announcement.objects.update_or_create( @@ -698,6 +785,17 @@ def apply_handler(action: str, data: JSONData) -> JsonResponse: return JsonResponse({"pk": au.pk}) +def reject_bad_token(token: str | None) -> JsonResponse | None: + """Return the response to send if `token` is no good, or None if it checks out.""" + if token is None: + raise SuspiciousOperation("No token provided") + elif settings.API_TARGET_HASH is None: + return JsonResponse({"error": "Not accepting tokens right now"}, status=503) + elif sha256(token.encode("ascii")).hexdigest() != settings.API_TARGET_HASH: + return JsonResponse({"error": "🧋"}, status=418) + return None + + @csrf_exempt def api(request: HttpRequest) -> JsonResponse: if not request.method == "POST": @@ -711,12 +809,8 @@ def api(request: HttpRequest) -> JsonResponse: raise SuspiciousOperation("You need to provide an action, silly") action = data["action"] - if "token" not in data: - raise SuspiciousOperation("No token provided") - elif settings.API_TARGET_HASH is None: - return JsonResponse({"error": "Not accepting tokens right now"}, status=503) - elif sha256(data["token"].encode("ascii")).hexdigest() != settings.API_TARGET_HASH: - return JsonResponse({"error": "🧋"}, status=418) + if (bad_token := reject_bad_token(data.get("token"))) is not None: + return bad_token if action in ( "grade_problem_set", diff --git a/opal/admin.py b/opal/admin.py index be96801f..cce9e667 100644 --- a/opal/admin.py +++ b/opal/admin.py @@ -4,7 +4,7 @@ from django.forms import ModelForm from django.http import HttpRequest -from opal.models import OpalAttempt, OpalHunt, OpalPuzzle +from opal.models import OpalAttempt, OpalHunt, OpalPuzzle, sha256_of @admin.register(OpalHunt) @@ -47,6 +47,7 @@ class OpalPuzzleAdmin(admin.ModelAdmin): ("achievement", admin.EmptyFieldListFilter), ) search_fields = ("hunt__name", "slug", "title") + readonly_fields = ("content_hash",) def save_model( self, @@ -55,15 +56,19 @@ def save_model( form: ModelForm[OpalPuzzle], change: bool, ) -> None: - if "content" in form.changed_data and obj.content: - uploaded_name = os.path.basename(obj.content.name or "") - if uploaded_name != obj.slug + ".pdf": - messages.warning( - request, - f"The file {uploaded_name} does not match the slug {obj.slug}; " - "it was saved under that name anyway, but check you uploaded " - "the right file.", - ) + if "content" in form.changed_data: + # Keep the fingerprint in step with the bytes, or the bulk-sync API would + # compare a new PDF against a stale hash and decide nothing had changed. + obj.content_hash = sha256_of(obj.content) if obj.content else "" + if obj.content: + uploaded_name = os.path.basename(obj.content.name or "") + if uploaded_name != obj.slug + ".pdf": + messages.warning( + request, + f"The file {uploaded_name} does not match the slug {obj.slug}; " + "it was saved under that name anyway, but check you uploaded " + "the right file.", + ) super().save_model(request, obj, form, change) diff --git a/opal/migrations/0019_opalpuzzle_content_hash.py b/opal/migrations/0019_opalpuzzle_content_hash.py new file mode 100644 index 00000000..f9f03261 --- /dev/null +++ b/opal/migrations/0019_opalpuzzle_content_hash.py @@ -0,0 +1,22 @@ +# Generated by Django 6.0.8 on 2026-08-23 04:51 + +from django.db import migrations, models + + +class Migration(migrations.Migration): + dependencies = [ + ("opal", "0018_opalhunt_artwork_slug"), + ] + + operations = [ + migrations.AddField( + model_name="opalpuzzle", + name="content_hash", + field=models.CharField( + blank=True, + default="", + help_text="SHA-256 of the puzzle file, so a bulk sync can skip PDFs whose bytes did not change. Blank means unknown, which makes the sync re-upload.", + max_length=64, + ), + ), + ] diff --git a/opal/models.py b/opal/models.py index 45342851..2321004a 100644 --- a/opal/models.py +++ b/opal/models.py @@ -1,10 +1,11 @@ import os import string -from hashlib import pbkdf2_hmac +from hashlib import pbkdf2_hmac, sha256 from typing import Any from django.conf import settings from django.contrib.auth.models import User +from django.core.files.base import File from django.core.validators import FileExtensionValidator from django.db import models from django.db.models.query import QuerySet @@ -25,6 +26,20 @@ def answerize(s: str) -> str: return "".join(c for c in s.upper() if c in ALLOWED_ANSWER_CHARACTERS) +def sha256_of(f: File[Any]) -> str: + """Fingerprint a file's bytes, so a sync can tell which PDFs actually changed. + + Reads in chunks rather than all at once, since these are PDFs and the caller + may be holding a handle to remote storage. ``File.chunks()`` rewinds first and + Django rewinds again before writing, so this is safe to call on a file that is + about to be saved. + """ + digest = sha256() + for chunk in f.chunks(): + digest.update(chunk) + return digest.hexdigest() + + class LiveOpalHuntManager(models.Manager): def get_queryset(self) -> QuerySet["OpalHunt"]: now = timezone.now() @@ -120,6 +135,13 @@ class OpalPuzzle(models.Model): null=True, blank=True, ) + content_hash = models.CharField( + max_length=64, + blank=True, + default="", + help_text="SHA-256 of the puzzle file, so a bulk sync can skip PDFs whose " + "bytes did not change. Blank means unknown, which makes the sync re-upload.", + ) guess_limit = models.PositiveSmallIntegerField( default=20, help_text="Maximum number of guesses to allow" diff --git a/otisweb/test_view_guards.py b/otisweb/test_view_guards.py index bd83a324..cb321952 100644 --- a/otisweb/test_view_guards.py +++ b/otisweb/test_view_guards.py @@ -72,6 +72,7 @@ # (url name, why) pair; adding to this list should be a conscious decision. PUBLIC_VIEWS: dict[str | None, str] = { "api": "aincrad API; authenticated by a hashed token in the POST body", + "opal-pdf-upload": "aincrad API; authenticated by a hashed token in the POST body", "certify": "capability URL; the checksum is verified before anything renders", "hanabi-contests": "public list of Hanabi contests", "hanabi-replays": "public results, gated on the contest being over",