Fix 500 on legacy signed URLs
A TimestampSigner value is an HMAC over 'payload:timestamp', so a plain Signer's HMAC check accepts it and the embedded timestamp then reached the JSON decoder, raising JSONDecodeError (not BadSignature) and surfacing as a 500. That broke every stored visual-match thumbnail URL minted before the stable scheme, so the J-ID match tiles never loaded on prod. Detect the legacy shape by its extra separator and verify it with TimestampSigner; malformed input returns None instead of raising.
This commit is contained in:
@@ -36,17 +36,24 @@ def sign_payload(payload, salt, now=None):
|
|||||||
def load_payload(signature, salt, legacy_max_age=86400):
|
def load_payload(signature, salt, legacy_max_age=86400):
|
||||||
"""Verify a signed payload; ``None`` when missing, tampered with or expired.
|
"""Verify a signed payload; ``None`` when missing, tampered with or expired.
|
||||||
|
|
||||||
Signatures minted before the stable scheme (``TimestampSigner``) are still
|
Legacy ``TimestampSigner`` values are still accepted for one release.
|
||||||
accepted for one release so pages open across the deploy keep working.
|
Detect them by their extra separator (``payload:timestamp:signature``):
|
||||||
|
a plain ``Signer`` accepts the HMAC a ``TimestampSigner`` computed over
|
||||||
|
``payload:timestamp`` and then chokes on the embedded timestamp while
|
||||||
|
decoding the JSON payload, which used to surface as a 500.
|
||||||
"""
|
"""
|
||||||
try:
|
if not signature:
|
||||||
data = signing.Signer(salt=salt).unsign_object(signature)
|
return None
|
||||||
except signing.BadSignature:
|
if signature.count(":") >= 2:
|
||||||
try:
|
try:
|
||||||
return signing.TimestampSigner(salt=salt).unsign_object(
|
return signing.TimestampSigner(salt=salt).unsign_object(
|
||||||
signature, max_age=legacy_max_age
|
signature, max_age=legacy_max_age
|
||||||
)
|
)
|
||||||
except signing.BadSignature:
|
except (signing.BadSignature, ValueError):
|
||||||
|
return None
|
||||||
|
try:
|
||||||
|
data = signing.Signer(salt=salt).unsign_object(signature)
|
||||||
|
except (signing.BadSignature, ValueError):
|
||||||
return None
|
return None
|
||||||
if not isinstance(data, dict):
|
if not isinstance(data, dict):
|
||||||
return None
|
return None
|
||||||
|
|||||||
@@ -12,8 +12,10 @@ import shutil
|
|||||||
import tempfile
|
import tempfile
|
||||||
import time
|
import time
|
||||||
from pathlib import Path
|
from pathlib import Path
|
||||||
|
from unittest import mock
|
||||||
|
|
||||||
from django.contrib.auth import get_user_model
|
from django.contrib.auth import get_user_model
|
||||||
|
from django.core import signing
|
||||||
from django.core.files.uploadedfile import SimpleUploadedFile
|
from django.core.files.uploadedfile import SimpleUploadedFile
|
||||||
from django.test import Client, TestCase, override_settings
|
from django.test import Client, TestCase, override_settings
|
||||||
|
|
||||||
@@ -23,7 +25,7 @@ from rest_framework.authtoken.models import Token
|
|||||||
|
|
||||||
from apps.library import services
|
from apps.library import services
|
||||||
from apps.library.models import MediaItem, MediaLocation, TempUpload
|
from apps.library.models import MediaItem, MediaLocation, TempUpload
|
||||||
from apps.library.signing_urls import sign_payload
|
from apps.library.signing_urls import load_payload, sign_payload
|
||||||
|
|
||||||
User = get_user_model()
|
User = get_user_model()
|
||||||
|
|
||||||
@@ -137,3 +139,29 @@ class MediaCacheTests(TestCase):
|
|||||||
f"max-age={services.TEMP_CACHE_SECONDS}", response["Cache-Control"]
|
f"max-age={services.TEMP_CACHE_SECONDS}", response["Cache-Control"]
|
||||||
)
|
)
|
||||||
self.assertNotIn("immutable", response["Cache-Control"])
|
self.assertNotIn("immutable", response["Cache-Control"])
|
||||||
|
|
||||||
|
def test_legacy_timestamp_signatures_are_accepted(self):
|
||||||
|
"""URLs minted before the stable scheme must not 500.
|
||||||
|
|
||||||
|
A TimestampSigner HMAC also passes a plain Signer's check, so the
|
||||||
|
embedded timestamp used to reach the JSON decoder and blow up.
|
||||||
|
"""
|
||||||
|
payload = {"item": self.item.id, "user": self.user.id, "action": "raw"}
|
||||||
|
legacy = signing.dumps(payload, salt=services.MEDIA_FILE_SALT)
|
||||||
|
self.assertEqual(
|
||||||
|
load_payload(legacy, services.MEDIA_FILE_SALT)["item"], self.item.id
|
||||||
|
)
|
||||||
|
response = self.client.get(
|
||||||
|
f"/api/files/J-{self.item.id}/raw/", {"sig": legacy}
|
||||||
|
)
|
||||||
|
self.assertEqual(response.status_code, 200)
|
||||||
|
|
||||||
|
def test_expired_and_malformed_signatures_return_none(self):
|
||||||
|
payload = {"item": self.item.id, "user": self.user.id, "action": "raw"}
|
||||||
|
with mock.patch.object(signing, "time") as clock:
|
||||||
|
clock.time.return_value = time.time() - 3 * 86400
|
||||||
|
expired = signing.dumps(payload, salt=services.MEDIA_FILE_SALT)
|
||||||
|
self.assertIsNone(load_payload(expired, services.MEDIA_FILE_SALT))
|
||||||
|
self.assertIsNone(load_payload("bogus", services.MEDIA_FILE_SALT))
|
||||||
|
self.assertIsNone(load_payload("a:b", services.MEDIA_FILE_SALT))
|
||||||
|
self.assertIsNone(load_payload("", services.MEDIA_FILE_SALT))
|
||||||
|
|||||||
Reference in New Issue
Block a user