mirror of
https://github.com/suitenumerique/docs.git
synced 2026-08-17 21:25:43 +02:00
🐛(backend) handle object storage metadata keys case-insensitively
Object storage metadata keys are case insensitive per the S3 specification, but implementations don't agree on the case they give back. When head_object returns a capitalized "Status", updating the attachment status added a second, lowercase entry instead of replacing it, and the copy request ended up carrying two x-amz-meta-status headers. Ceph RadosGW loses one of them behind a proxy, which invalidates the request signature. The same assumption was made when reading the status back in media-auth and media-check, where an attachment stored on such a backend stayed in "processing" forever. Metadata read from the storage is now normalized to lowercase keys before being consumed or copied over. Signed-off-by: risk-alt <aldu6974@gmail.com>
This commit is contained in:
@@ -24,6 +24,7 @@ and this project adheres to
|
||||
- 🐛(backend) ignore CSPs for API docs in development
|
||||
- 🐛(frontend) export images embedded with a relative url #2573
|
||||
- 🐛(y-provider) fix sentry init #2579
|
||||
- 🐛(backend) handle object storage metadata keys case-insensitively #2576
|
||||
- 🐛(keycloak) fix database env variables in the self-hosting example #2572
|
||||
- 🐛(helm) show the database error while jobs wait for it to be ready #2578
|
||||
|
||||
|
||||
@@ -71,6 +71,7 @@ from core.services.search_indexers import (
|
||||
from core.tasks.access import reset_service_connections_in_cascade
|
||||
from core.tasks.mail import send_ask_for_access_mail
|
||||
from core.utils.analytics import PosthogEventName, posthog_capture
|
||||
from core.utils.dicts import lowercase_keys
|
||||
from core.utils.paths import filter_descendants
|
||||
from core.utils.s3_response_stream import content_stream
|
||||
from core.utils.treebeard import create_tree_node_with_retry
|
||||
@@ -1884,7 +1885,7 @@ class DocumentViewSet(
|
||||
extra_args = {
|
||||
"Metadata": {
|
||||
"owner": str(request.user.id),
|
||||
"status": enums.DocumentAttachmentStatus.PROCESSING,
|
||||
"status": enums.DocumentAttachmentStatus.PROCESSING.value,
|
||||
},
|
||||
"ContentType": serializer.validated_data["content_type"],
|
||||
}
|
||||
@@ -2034,7 +2035,7 @@ class DocumentViewSet(
|
||||
head_resp = s3_client.head_object(Bucket=bucket_name, Key=key)
|
||||
except ClientError as err:
|
||||
raise drf.exceptions.PermissionDenied() from err
|
||||
metadata = head_resp.get("Metadata", {})
|
||||
metadata = lowercase_keys(head_resp.get("Metadata", {}))
|
||||
# In order to be compatible with existing upload without `status` metadata,
|
||||
# we consider them as ready.
|
||||
if (
|
||||
@@ -2238,7 +2239,7 @@ class DocumentViewSet(
|
||||
{"detail": "Media not found"},
|
||||
status=drf.status.HTTP_404_NOT_FOUND,
|
||||
)
|
||||
metadata = head_resp.get("Metadata", {})
|
||||
metadata = lowercase_keys(head_resp.get("Metadata", {}))
|
||||
|
||||
body = {
|
||||
"status": metadata.get("status", enums.DocumentAttachmentStatus.PROCESSING),
|
||||
|
||||
@@ -8,6 +8,7 @@ from lasuite.malware_detection.enums import ReportStatus
|
||||
|
||||
from core.enums import DocumentAttachmentStatus
|
||||
from core.models import Document
|
||||
from core.utils.dicts import lowercase_keys
|
||||
|
||||
logger = logging.getLogger(__name__)
|
||||
security_logger = logging.getLogger("docs.security")
|
||||
@@ -22,8 +23,8 @@ def malware_detection_callback(file_path, status, error_info, **kwargs):
|
||||
s3_client = default_storage.connection.meta.client
|
||||
bucket_name = default_storage.bucket_name
|
||||
head_resp = s3_client.head_object(Bucket=bucket_name, Key=file_path)
|
||||
metadata = head_resp.get("Metadata", {})
|
||||
metadata.update({"status": DocumentAttachmentStatus.READY})
|
||||
metadata = lowercase_keys(head_resp.get("Metadata", {}))
|
||||
metadata.update({"status": DocumentAttachmentStatus.READY.value})
|
||||
# Update status in metadata
|
||||
s3_client.copy_object(
|
||||
Bucket=bucket_name,
|
||||
|
||||
@@ -6,6 +6,7 @@ from django.core.management.base import BaseCommand
|
||||
import magic
|
||||
|
||||
from core.models import Document
|
||||
from core.utils.dicts import lowercase_keys
|
||||
|
||||
# pylint: disable=too-many-locals, broad-exception-caught
|
||||
|
||||
@@ -74,7 +75,7 @@ class Command(BaseCommand):
|
||||
CopySource={"Bucket": bucket_name, "Key": key},
|
||||
Key=key,
|
||||
ContentType=magic_mime_type,
|
||||
Metadata=head_resp.get("Metadata", {}),
|
||||
Metadata=lowercase_keys(head_resp.get("Metadata", {})),
|
||||
MetadataDirective="REPLACE",
|
||||
)
|
||||
total_updated += 1
|
||||
|
||||
@@ -3,6 +3,7 @@ Test media-auth authorization API endpoint in docs core app.
|
||||
"""
|
||||
|
||||
from io import BytesIO
|
||||
from unittest.mock import patch
|
||||
from urllib.parse import urlparse
|
||||
from uuid import uuid4
|
||||
|
||||
@@ -369,6 +370,34 @@ def test_api_documents_media_auth_not_ready_status():
|
||||
assert response.status_code == 403
|
||||
|
||||
|
||||
def test_api_documents_media_auth_uppercase_status_metadata():
|
||||
"""
|
||||
Object storage metadata keys are case insensitive, and some S3 implementations give them
|
||||
back capitalized. A ready attachment should still be served in that case.
|
||||
"""
|
||||
document_id = uuid4()
|
||||
filename = f"{uuid4()!s}.jpg"
|
||||
key = f"{document_id!s}/attachments/{filename:s}"
|
||||
|
||||
factories.DocumentFactory(id=document_id, link_reach="public", attachments=[key])
|
||||
|
||||
head_resp = {
|
||||
"ContentType": "text/plain",
|
||||
"Metadata": {"Status": DocumentAttachmentStatus.READY.value},
|
||||
}
|
||||
|
||||
original_url = f"http://localhost/media/{key:s}"
|
||||
with patch.object(
|
||||
default_storage.connection.meta.client, "head_object", return_value=head_resp
|
||||
):
|
||||
response = APIClient().get(
|
||||
"/api/v1.0/documents/media-auth/", HTTP_X_ORIGINAL_URL=original_url
|
||||
)
|
||||
|
||||
assert response.status_code == 200
|
||||
assert "AWS4-HMAC-SHA256 Credential=" in response["Authorization"]
|
||||
|
||||
|
||||
def test_api_documents_media_auth_missing_status_metadata(settings):
|
||||
"""Attachments without status metadata should be considered as ready"""
|
||||
document_id = uuid4()
|
||||
|
||||
@@ -1,6 +1,7 @@
|
||||
"""Test the "media_check" endpoint."""
|
||||
|
||||
from io import BytesIO
|
||||
from unittest.mock import patch
|
||||
from uuid import uuid4
|
||||
|
||||
from django.core.files.storage import default_storage
|
||||
@@ -116,6 +117,39 @@ def test_api_documents_media_check_anonymous_public_document_ready():
|
||||
}
|
||||
|
||||
|
||||
def test_api_documents_media_check_uppercase_status_metadata():
|
||||
"""
|
||||
Object storage metadata keys are case insensitive, and some S3 implementations give them
|
||||
back capitalized. The "media_check" endpoint should still report the attachment as ready.
|
||||
"""
|
||||
document = factories.DocumentFactory(link_reach="public")
|
||||
|
||||
filename = f"{uuid4()!s}.jpg"
|
||||
key = f"{document.id!s}/attachments/{filename:s}"
|
||||
document.attachments = [key]
|
||||
document.save(update_fields=["attachments"])
|
||||
|
||||
head_resp = {
|
||||
"ContentType": "text/plain",
|
||||
"Metadata": {"Status": DocumentAttachmentStatus.READY.value},
|
||||
}
|
||||
|
||||
client = APIClient()
|
||||
|
||||
with patch.object(
|
||||
default_storage.connection.meta.client, "head_object", return_value=head_resp
|
||||
):
|
||||
response = client.get(
|
||||
f"/api/v1.0/documents/{document.id!s}/media-check/", {"key": key}
|
||||
)
|
||||
|
||||
assert response.status_code == 200
|
||||
assert response.json() == {
|
||||
"status": DocumentAttachmentStatus.READY,
|
||||
"file": f"/media/{key:s}",
|
||||
}
|
||||
|
||||
|
||||
@pytest.mark.parametrize("link_reach", ["restricted", "authenticated"])
|
||||
def test_api_documents_media_check_anonymous_non_public_document(link_reach):
|
||||
"""
|
||||
|
||||
@@ -1,6 +1,7 @@
|
||||
"""Test malware detection callback."""
|
||||
|
||||
import random
|
||||
from unittest.mock import patch
|
||||
|
||||
from django.core.files.base import ContentFile
|
||||
from django.core.files.storage import default_storage
|
||||
@@ -56,6 +57,41 @@ def test_malware_detection_callback_safe_status(safe_file):
|
||||
assert metadata["status"] == DocumentAttachmentStatus.READY
|
||||
|
||||
|
||||
def test_malware_detection_callback_safe_status_uppercase_metadata(safe_file):
|
||||
"""
|
||||
Object storage metadata keys are case insensitive, and some S3 implementations give them
|
||||
back capitalized. The callback should update the existing "status" entry instead of adding
|
||||
a second one, which would end up as two "x-amz-meta-status" headers on the copy request.
|
||||
"""
|
||||
document = DocumentFactory(attachments=[safe_file])
|
||||
|
||||
s3_client = default_storage.connection.meta.client
|
||||
head_resp = {
|
||||
"ContentType": "text/plain",
|
||||
"Metadata": {
|
||||
"Owner": str(document.id),
|
||||
"Status": DocumentAttachmentStatus.PROCESSING.value,
|
||||
},
|
||||
}
|
||||
|
||||
with (
|
||||
patch.object(s3_client, "head_object", return_value=head_resp),
|
||||
patch.object(s3_client, "copy_object") as mock_copy_object,
|
||||
):
|
||||
malware_detection_callback(
|
||||
safe_file,
|
||||
ReportStatus.SAFE,
|
||||
error_info={},
|
||||
document_id=document.id,
|
||||
)
|
||||
|
||||
metadata = mock_copy_object.call_args.kwargs["Metadata"]
|
||||
assert metadata == {
|
||||
"owner": str(document.id),
|
||||
"status": DocumentAttachmentStatus.READY.value,
|
||||
}
|
||||
|
||||
|
||||
def test_malware_detection_callback_unsafe_status(unsafe_file):
|
||||
"""Test malware detection callback with unsafe status."""
|
||||
|
||||
|
||||
@@ -9,7 +9,7 @@ import pycrdt
|
||||
import pytest
|
||||
|
||||
from core import factories
|
||||
from core.utils.dicts import get_value_by_pattern
|
||||
from core.utils.dicts import get_value_by_pattern, lowercase_keys
|
||||
from core.utils.paths import get_ancestor_to_descendants_map
|
||||
from core.utils.users import (
|
||||
get_users_sharing_documents_with_cache_key,
|
||||
@@ -251,3 +251,35 @@ def test_utils_get_value_by_pattern_no_match():
|
||||
result = get_value_by_pattern(data, r"^title\.")
|
||||
|
||||
assert result == []
|
||||
|
||||
|
||||
def test_utils_lowercase_keys_empty():
|
||||
"""Test that an empty dictionary is returned untouched."""
|
||||
assert lowercase_keys({}) == {}
|
||||
|
||||
|
||||
def test_utils_lowercase_keys_mixed_case():
|
||||
"""Test that keys are lowercased while values are left untouched."""
|
||||
data = {"Owner": "42", "STATUS": "Ready", "is_unsafe": "true"}
|
||||
|
||||
assert lowercase_keys(data) == {
|
||||
"owner": "42",
|
||||
"status": "Ready",
|
||||
"is_unsafe": "true",
|
||||
}
|
||||
|
||||
|
||||
def test_utils_lowercase_keys_collision():
|
||||
"""Test that keys differing only by their case are collapsed into a single one."""
|
||||
data = {"Status": "processing", "status": "ready"}
|
||||
result = lowercase_keys(data)
|
||||
|
||||
assert result == {"status": "ready"}
|
||||
|
||||
|
||||
def test_utils_lowercase_keys_does_not_mutate_source():
|
||||
"""Test that the source dictionary is left untouched."""
|
||||
data = {"Status": "processing"}
|
||||
lowercase_keys(data)
|
||||
|
||||
assert data == {"Status": "processing"}
|
||||
|
||||
@@ -22,3 +22,25 @@ def get_value_by_pattern(data, pattern):
|
||||
"""
|
||||
regex = re.compile(pattern)
|
||||
return [value for key, value in data.items() if regex.match(key)]
|
||||
|
||||
|
||||
def lowercase_keys(data):
|
||||
"""
|
||||
Get a copy of a dictionary with all its keys lowercased.
|
||||
|
||||
Useful for object storage metadata: keys are case insensitive per the S3 specification,
|
||||
but implementations don't agree on the case they give back, so reading or updating a
|
||||
metadata entry by its lowercase name is only reliable after this normalization.
|
||||
|
||||
Args:
|
||||
data (dict): Source dictionary
|
||||
|
||||
Returns:
|
||||
dict: New dictionary with lowercased keys. When several keys only differ by their
|
||||
case, the last one encountered wins.
|
||||
|
||||
Example:
|
||||
>>> lowercase_keys({"Owner": "42", "Status": "ready"})
|
||||
{"owner": "42", "status": "ready"}
|
||||
"""
|
||||
return {key.lower(): value for key, value in data.items()}
|
||||
|
||||
Reference in New Issue
Block a user