From 7afa165013f096ab31a383c151ce3a177d9ffb89 Mon Sep 17 00:00:00 2001 From: lebaudantoine Date: Fri, 8 Nov 2024 17:01:25 +0100 Subject: [PATCH] =?UTF-8?q?=E2=9C=A8(backend)=20offer=20an=20endpoint=20to?= =?UTF-8?q?=20save=20recording?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit I've protected this endpoint with a feature flag, and an authentication class, as it will be exposed on the public internet. I've tried to keep the viewset logic as minimal as possible, I've to ship smth and will continue iterating on this piece of code. At some point, abstracting webhook endpoint and authentication class might be beneficial for the project. YAGNI as of today. --- src/backend/core/api/permissions.py | 10 + src/backend/core/api/viewsets.py | 51 +++++ .../core/recording/event/authentication.py | 3 + .../test_api_recordings_storage_hook.py | 203 ++++++++++++++++++ src/backend/meet/settings.py | 6 + 5 files changed, 273 insertions(+) create mode 100644 src/backend/core/tests/recording/test_api_recordings_storage_hook.py diff --git a/src/backend/core/api/permissions.py b/src/backend/core/api/permissions.py index 45984909..8165e25c 100644 --- a/src/backend/core/api/permissions.py +++ b/src/backend/core/api/permissions.py @@ -109,3 +109,13 @@ class IsRecordingEnabled(permissions.BasePermission): def has_permission(self, request, view): """Determine if access is allowed based on settings.""" return settings.RECORDING_ENABLE + + +class IsStorageEventEnabled(permissions.BasePermission): + """Check if the storage event feature is enabled.""" + + message = "Access denied, storage event is disabled." + + def has_permission(self, request, view): + """Determine if access is allowed based on settings.""" + return settings.RECORDING_STORAGE_EVENT_ENABLE diff --git a/src/backend/core/api/viewsets.py b/src/backend/core/api/viewsets.py index 95d7a72a..2ae6f5c1 100644 --- a/src/backend/core/api/viewsets.py +++ b/src/backend/core/api/viewsets.py @@ -26,6 +26,13 @@ from rest_framework import ( ) from core import models, utils +from core.recording.event.authentication import StorageEventAuthentication +from core.recording.event.exceptions import ( + InvalidBucketError, + InvalidFileTypeError, + ParsingEventDataError, +) +from core.recording.event.parsers import get_parser from core.recording.worker.exceptions import ( RecordingStartError, RecordingStopError, @@ -403,3 +410,47 @@ class RecordingViewSet( .get_queryset() .filter(Q(accesses__user=user) | Q(accesses__team__in=user.get_teams())) ) + + @decorators.action( + detail=False, + methods=["post"], + url_path="storage-hook", + authentication_classes=[StorageEventAuthentication], + permission_classes=[permissions.IsStorageEventEnabled], + ) + def on_storage_event_received(self, request, pk=None): # pylint: disable=unused-argument + """Handle incoming storage hook events for recordings.""" + + parser = get_parser() + + try: + recording_id = parser.get_recording_id(request.data) + + except ParsingEventDataError as e: + raise drf_exceptions.PermissionDenied(f"Invalid request data: {e}") from e + + except InvalidBucketError as e: + raise drf_exceptions.PermissionDenied("Invalid bucket specified") from e + + except InvalidFileTypeError as e: + return drf_response.Response( + {"message": f"Ignore this file type, {e}"}, + ) + + try: + recording = models.Recording.objects.get(id=recording_id) + except models.Recording.DoesNotExist as e: + raise drf_exceptions.NotFound("No recording found for this event.") from e + + if not recording.is_savable(): + raise drf_exceptions.PermissionDenied( + f"Recording with ID {recording_id} cannot be saved because it is either," + " in an error state or has already been saved." + ) + + recording.status = models.RecordingStatusChoices.SAVED + recording.save() + + return drf_response.Response( + {"message": "Event processed."}, + ) diff --git a/src/backend/core/recording/event/authentication.py b/src/backend/core/recording/event/authentication.py index 158a247f..768c4f42 100644 --- a/src/backend/core/recording/event/authentication.py +++ b/src/backend/core/recording/event/authentication.py @@ -50,6 +50,9 @@ class StorageEventAuthentication(BaseAuthentication): if not settings.RECORDING_ENABLE_STORAGE_EVENT_AUTH: return MachineUser(), None + if not settings.RECORDING_ENABLE_STORAGE_EVENT_AUTH: + return MachineUser(), None + required_token = settings.RECORDING_STORAGE_EVENT_TOKEN if not required_token: if settings.RECORDING_ENABLE_STORAGE_EVENT_AUTH: diff --git a/src/backend/core/tests/recording/test_api_recordings_storage_hook.py b/src/backend/core/tests/recording/test_api_recordings_storage_hook.py new file mode 100644 index 00000000..73372044 --- /dev/null +++ b/src/backend/core/tests/recording/test_api_recordings_storage_hook.py @@ -0,0 +1,203 @@ +""" +Test recordings API endpoints in the Meet core app: save recording. +""" + +# pylint: disable=W0621,W0613 + +import uuid +from unittest import mock + +import pytest +from rest_framework.test import APIClient + +from ...factories import RecordingFactory +from ...models import Recording, RecordingStatusChoices +from ...recording.event.exceptions import ( + InvalidBucketError, + InvalidFileTypeError, + ParsingEventDataError, +) + +pytestmark = pytest.mark.django_db + + +@pytest.fixture +def recording_settings(settings): + """Configure recording-related and storage event Django settings.""" + settings.RECORDING_STORAGE_EVENT_TOKEN = "testAuthToken" + settings.RECORDING_STORAGE_EVENT_ENABLE = True + return settings + + +@pytest.fixture +def mock_get_parser(): + """Mock 'get_parser' factory function.""" + with mock.patch("core.api.viewsets.get_parser") as mock_parser: + yield mock_parser + + +def test_save_recording_anonymous(settings, client): + """Anonymous users should not be allowed to save room recordings.""" + settings.RECORDING_STORAGE_EVENT_TOKEN = "testAuthToken" + + RecordingFactory(status="active") + + response = client.post( + "/api/v1.0/recordings/storage-hook/", + {"recording_data": "valid-data"}, + ) + + assert response.status_code == 401 + assert Recording.objects.count() == 1 + + +def test_save_recording_wrong_bearer(settings, client): + """Requests with incorrect bearer token should be rejected when auth is required.""" + + settings.RECORDING_STORAGE_EVENT_TOKEN = "testAuthToken" + + response = client.post( + "/api/v1.0/recordings/storage-hook/", + {"recording_data": "valid-data"}, + HTTP_AUTHORIZATION="Bearer wrongAuthToken", + ) + + assert response.status_code == 401 + + +def test_save_recording_permission_needed(settings, client): + """Recordings should not be saved when feature is disabled.""" + + settings.RECORDING_STORAGE_EVENT_TOKEN = "testAuthToken" + settings.RECORDING_STORAGE_EVENT_ENABLE = False + + response = client.post( + "/api/v1.0/recordings/storage-hook/", + {"recording_data": "valid-data"}, + HTTP_AUTHORIZATION="Bearer testAuthToken", + ) + + assert response.status_code == 403 + + +def test_save_recording_parsing_error(recording_settings, mock_get_parser, client): + """Test handling of parsing errors in recording event data.""" + mock_parser = mock.Mock() + mock_parser.get_recording_id.side_effect = ParsingEventDataError("Error message") + mock_get_parser.return_value = mock_parser + + response = client.post( + "/api/v1.0/recordings/storage-hook/", + {"recording_data": "valid-data"}, + HTTP_AUTHORIZATION="Bearer testAuthToken", + ) + + assert response.status_code == 403 + assert response.json() == {"detail": "Invalid request data: Error message"} + + +def test_save_recording_bucket_error(recording_settings, mock_get_parser, client): + """Test handling of invalid storage bucket errors in recording event data.""" + + mock_parser = mock.Mock() + mock_parser.get_recording_id.side_effect = InvalidBucketError("Error message") + mock_get_parser.return_value = mock_parser + + response = client.post( + "/api/v1.0/recordings/storage-hook/", + {"recording_data": "valid-data"}, + HTTP_AUTHORIZATION="Bearer testAuthToken", + ) + + assert response.status_code == 403 + assert response.json() == {"detail": "Invalid bucket specified"} + + +def test_save_recording_filetype_error(recording_settings, mock_get_parser): + """Test handling of unsupported file types in recording event data.""" + + mock_parser = mock.Mock() + mock_parser.get_recording_id.side_effect = InvalidFileTypeError( + "unsupported '.json'" + ) + mock_get_parser.return_value = mock_parser + + client = APIClient() + + response = client.post( + "/api/v1.0/recordings/storage-hook/", + {"recording_data": "valid-data"}, + HTTP_AUTHORIZATION="Bearer testAuthToken", + ) + + assert response.status_code == 200 + assert response.json() == {"message": "Ignore this file type, unsupported '.json'"} + + +def test_save_recording_unknown_recording(recording_settings, mock_get_parser, client): + """Test handling of events for non-existent recordings.""" + + RecordingFactory(status="active") + + mock_parser = mock.Mock() + mock_parser.get_recording_id.return_value = uuid.uuid4() + mock_get_parser.return_value = mock_parser + + response = client.post( + "/api/v1.0/recordings/storage-hook/", + {"recording_data": "valid-data"}, + HTTP_AUTHORIZATION="Bearer testAuthToken", + ) + + assert response.status_code == 404 + assert response.json() == {"detail": "No recording found for this event."} + + +@pytest.mark.parametrize( + "status", ["failed_to_start", "aborted", "failed_to_stop", "saved", "initiated"] +) +def test_save_recording_non_savable_recording( + recording_settings, mock_get_parser, client, status +): + """Test that recordings in non-savable states cannot be saved.""" + + recording = RecordingFactory(status=status) + + mock_parser = mock.Mock() + mock_parser.get_recording_id.return_value = recording.id + mock_get_parser.return_value = mock_parser + + response = client.post( + "/api/v1.0/recordings/storage-hook/", + {"recording_data": "valid-data"}, + HTTP_AUTHORIZATION="Bearer testAuthToken", + ) + + assert response.status_code == 403 + assert response.json() == { + "detail": f"Recording with ID {recording.id} cannot be saved because it is either," + " in an error state or has already been saved." + } + + +@pytest.mark.parametrize("status", ["active", "stopped"]) +def test_save_recording_success(recording_settings, mock_get_parser, client, status): + """Test successful saving of recordings in valid states.""" + + recording = RecordingFactory(status=status) + + mock_parser = mock.Mock() + mock_parser.get_recording_id.return_value = recording.id + mock_get_parser.return_value = mock_parser + + response = client.post( + "/api/v1.0/recordings/storage-hook/", + {"recording_data": "valid-data"}, + HTTP_AUTHORIZATION="Bearer testAuthToken", + ) + + assert response.status_code == 200 + assert response.json() == {"message": "Event processed."} + + recording.refresh_from_db() + assert recording.status == RecordingStatusChoices.SAVED diff --git a/src/backend/meet/settings.py b/src/backend/meet/settings.py index fef21a63..e04095d5 100755 --- a/src/backend/meet/settings.py +++ b/src/backend/meet/settings.py @@ -432,6 +432,12 @@ class Base(Configuration): RECORDING_ENABLE_STORAGE_EVENT_AUTH = values.BooleanValue( True, environ_name="RECORDING_ENABLE_STORAGE_EVENT_AUTH", environ_prefix=None ) + RECORDING_STORAGE_EVENT_ENABLE = values.BooleanValue( + False, environ_name="RECORDING_STORAGE_EVENT_ENABLE", environ_prefix=None + ) + RECORDING_STORAGE_EVENT_TOKEN = values.Value( + None, environ_name="RECORDING_STORAGE_HOOK_TOKEN", environ_prefix=None + ) # pylint: disable=invalid-name @property