diff --git a/CHANGELOG.md b/CHANGELOG.md index 5490f8cb..775ab14d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,10 @@ and this project adheres to ## [Unreleased] +### Added + +- ✨(backend) add OpenShift-compatible recording download endpoint + ### Changed - ✨(backend) accept form-urlencoded on the user token endpoint diff --git a/src/backend/core/api/viewsets.py b/src/backend/core/api/viewsets.py index bbaa1da4..8a33e511 100644 --- a/src/backend/core/api/viewsets.py +++ b/src/backend/core/api/viewsets.py @@ -12,12 +12,13 @@ from django.core.exceptions import ValidationError as DjangoValidationError from django.core.files.storage import default_storage from django.db import IntegrityError, transaction from django.db.models import Q -from django.http import Http404 +from django.http import Http404, StreamingHttpResponse from django.shortcuts import get_object_or_404 from django.utils import timezone from django.utils.text import slugify from django.utils.translation import gettext_lazy as _ +from botocore.exceptions import BotoCoreError, ClientError from django_filters import rest_framework as django_filters from rest_framework import ( decorators, @@ -113,6 +114,14 @@ from .feature_flag import FeatureFlag logger = getLogger(__name__) +class StorageUnavailable(drf_exceptions.APIException): + """Exception raised when recording storage cannot be reached.""" + + status_code = drf_status.HTTP_503_SERVICE_UNAVAILABLE + default_detail = "The recording storage is temporarily unavailable." + default_code = "storage_unavailable" + + class NestedGenericViewSet(viewsets.GenericViewSet): """ A generic Viewset aims to be used in a nested route context. @@ -1220,6 +1229,87 @@ class RecordingViewSet( return drf_response.Response("authorized", headers=request.headers, status=200) + @decorators.action(detail=False, methods=["get"], url_path="download") + def media_download(self, request, *args, **kwargs): + """ + Stream a recording file directly from S3 to the browser. + + This endpoint is registered at /media/recordings/. in core/urls.py + and handles the download flow on OpenShift where the Nginx auth_request + mechanism is not available. + + Django acts as a streaming proxy: bytes flow from S3 (internal cluster URL) + directly to the client response. This avoids any page navigation or redirect, + so the browser triggers a native download without leaving the current page. + + Access control mirrors media_auth: the user must have the "retrieve" ability + on the recording and the recording must be in a saved state. + """ + recording_id = self.kwargs.get("recording_id") + extension = self.kwargs.get("extension") + + if extension not in [file.value for file in FileExtension]: + raise drf_exceptions.ValidationError({"detail": "Unsupported extension."}) + + try: + recording = models.Recording.objects.get(id=recording_id) + except models.Recording.DoesNotExist as e: + raise drf_exceptions.NotFound("No recording found for this id.") from e + + if extension != recording.extension: + raise drf_exceptions.NotFound("No recording found with this extension.") + + abilities = recording.get_abilities(request.user) + + if not abilities["retrieve"]: + logger.debug( + "User '%s' lacks permission for recording download", request.user.id + ) + raise drf_exceptions.PermissionDenied() + + if not recording.is_saved: + logger.debug("Recording '%s' has not been saved", recording) + raise drf_exceptions.PermissionDenied() + + s3_client = default_storage.connection.meta.client + try: + s3_object = s3_client.get_object( + Bucket=default_storage.bucket_name, + Key=recording.key, + ) + except ClientError as error: + error_code = error.response.get("Error", {}).get("Code") + if error_code in {"404", "NoSuchKey", "NoSuchObject"}: + raise drf_exceptions.NotFound( + "No recording file found in storage." + ) from error + logger.exception("Unable to retrieve recording from storage") + raise StorageUnavailable() from error + except BotoCoreError as error: + logger.exception("Unable to retrieve recording from storage") + raise StorageUnavailable() from error + + content_type = s3_object.get("ContentType") or "video/mp4" + content_length = s3_object.get("ContentLength") + filename = f"{recording_id}.{extension}" + body = s3_object["Body"] + + def stream(): + try: + yield from body.iter_chunks(chunk_size=65536) + finally: + body.close() + + streaming_response = StreamingHttpResponse( + stream(), + content_type=content_type, + ) + streaming_response["Content-Disposition"] = f'attachment; filename="{filename}"' + if content_length: + streaming_response["Content-Length"] = content_length + + return streaming_response + # pylint: disable=too-many-public-methods class FileViewSet( diff --git a/src/backend/core/tests/recording/test_api_recordings_retrieve.py b/src/backend/core/tests/recording/test_api_recordings_retrieve.py index 827bdd09..3255a3e8 100644 --- a/src/backend/core/tests/recording/test_api_recordings_retrieve.py +++ b/src/backend/core/tests/recording/test_api_recordings_retrieve.py @@ -3,8 +3,12 @@ Test recordings API endpoints in the Meet core app: retrieve. """ import random +from unittest import mock + +from django.core.files.storage import default_storage import pytest +from botocore.exceptions import ClientError, EndpointConnectionError from freezegun import freeze_time from rest_framework.test import APIClient @@ -14,6 +18,11 @@ from ...models import RecordingStatusChoices pytestmark = pytest.mark.django_db +def media_recording_url(recording): + """Return the direct media URL for a recording.""" + return f"/media/recordings/{recording.id!s}.{recording.extension}" + + def test_api_recording_retrieve_anonymous(): """Anonymous users should not be able to retrieve recordings.""" recording = RecordingFactory() @@ -25,6 +34,9 @@ def test_api_recording_retrieve_anonymous(): "detail": "Authentication credentials were not provided." } + response = client.get(media_recording_url(recording)) + assert response.status_code == 401 + def test_api_recording_retrieve_authenticated(): """Authenticated users without access receive 404 when requesting recordings. @@ -44,6 +56,9 @@ def test_api_recording_retrieve_authenticated(): assert response.status_code == 404 assert response.json() == {"detail": "No Recording matches the given query."} + response = client.get(media_recording_url(recording)) + assert response.status_code == 403 + def test_api_recording_retrieve_members(): """ @@ -63,6 +78,11 @@ def test_api_recording_retrieve_members(): "detail": "You do not have permission to perform this action." } + recording.status = RecordingStatusChoices.SAVED + recording.save(update_fields=["status"]) + response = client.get(media_recording_url(recording)) + assert response.status_code == 403 + def test_api_recording_retrieve_administrators(settings): """A user who is an administrator of a recording should be able to retrieve it.""" @@ -137,6 +157,78 @@ def test_api_recording_retrieve_owners(settings): } +@pytest.mark.parametrize("role", ["owner", "administrator"]) +def test_api_recording_media_download_authorized(role): + """Owners and administrators can download a saved recording.""" + user = UserFactory() + recording = RecordingFactory(status=RecordingStatusChoices.SAVED) + UserRecordingAccessFactory(recording=recording, user=user, role=role) + + client = APIClient() + client.force_login(user) + body = mock.Mock(iter_chunks=mock.Mock(return_value=[b"recording content"])) + s3_object = { + "Body": body, + "ContentType": "video/mp4", + "ContentLength": len(b"recording content"), + } + with mock.patch.object( + default_storage.connection.meta.client, + "get_object", + return_value=s3_object, + ) as get_object: + response = client.get(media_recording_url(recording)) + + assert response.status_code == 200 + get_object.assert_called_once_with( + Bucket=default_storage.bucket_name, + Key=recording.key, + ) + assert response["Content-Type"] == "video/mp4" + assert response["Content-Disposition"] == ( + f'attachment; filename="{recording.id}.{recording.extension}"' + ) + assert b"".join(response.streaming_content) == b"recording content" + body.close.assert_called_once_with() + + +def test_api_recording_media_download_missing_object(): + """A missing recording object should return 404.""" + user = UserFactory() + recording = RecordingFactory(status=RecordingStatusChoices.SAVED) + UserRecordingAccessFactory(recording=recording, user=user, role="owner") + + client = APIClient() + client.force_login(user) + error = ClientError( + {"Error": {"Code": "NoSuchKey", "Message": "The object does not exist."}}, + "GetObject", + ) + with mock.patch.object( + default_storage.connection.meta.client, "get_object", side_effect=error + ): + response = client.get(media_recording_url(recording)) + + assert response.status_code == 404 + + +def test_api_recording_media_download_storage_unavailable(): + """A storage connection failure should return 503.""" + user = UserFactory() + recording = RecordingFactory(status=RecordingStatusChoices.SAVED) + UserRecordingAccessFactory(recording=recording, user=user, role="owner") + + client = APIClient() + client.force_login(user) + error = EndpointConnectionError(endpoint_url="https://storage.test") + with mock.patch.object( + default_storage.connection.meta.client, "get_object", side_effect=error + ): + response = client.get(media_recording_url(recording)) + + assert response.status_code == 503 + + @freeze_time("2023-01-15 12:00:00") def test_api_recording_retrieve_compute_expiration_date_correctly(settings): """Test that the API returns the correct expiration date for a non-expired recording.""" diff --git a/src/backend/core/urls.py b/src/backend/core/urls.py index 6209216f..f013e6c7 100644 --- a/src/backend/core/urls.py +++ b/src/backend/core/urls.py @@ -60,6 +60,13 @@ urlpatterns = [ ] ), ), + # The request is handled by RecordingViewSet.media_download which validates + # access rights and streams the recording file directly from MinIO to the browser. + path( + "media/recordings/.", + viewsets.RecordingViewSet.as_view({"get": "media_download"}), + name="recording_media_download", + ), ] if settings.EXTERNAL_API_ENABLED: