diff --git a/CHANGELOG.md b/CHANGELOG.md index 24931b29..22919b4e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,6 +11,7 @@ and this project adheres to ### Changed - 🔒️(backend) enhance API input validation to strengthen security #1053 +- 🦺(backend) strengthen API validation for recording options #1063 ### Fixed diff --git a/src/backend/core/api/serializers.py b/src/backend/core/api/serializers.py index ae050399..84092368 100644 --- a/src/backend/core/api/serializers.py +++ b/src/backend/core/api/serializers.py @@ -1,6 +1,7 @@ """Client serializers for the Meet core app.""" # pylint: disable=abstract-method,no-name-in-module +from typing import Literal from django.conf import settings from django.core.exceptions import SuspiciousOperation @@ -204,6 +205,27 @@ class BaseValidationOnlySerializer(serializers.Serializer): raise NotImplementedError(f"{self.__class__.__name__} is validation-only") +class RecordingOptions(BaseModel): + """Configuration options for recording. + + Attributes: + language: ISO 639-1 language code compatible with whisperX. + When `None`, the transcription engine will attempt to + auto-detect the spoken language. + transcribe: Whether to transcribe the recorded audio. + When `None`, falls back to the application default. + original_mode: The original recording mode before any override. + Must be one of the valid RecordingModeChoices values when provided. + + """ + + language: str | None = None + transcribe: bool | None = None + original_mode: Literal["screen_recording", "transcript"] | None = None + + model_config = {"extra": "forbid"} + + class StartRecordingSerializer(BaseValidationOnlySerializer): """Validate start recording requests.""" @@ -216,10 +238,11 @@ class StartRecordingSerializer(BaseValidationOnlySerializer): "screen_recording or transcript.", }, ) - options = serializers.JSONField( + options = SchemaField( + schema=RecordingOptions | None, required=False, allow_null=True, - default=dict, + help_text="Recording options", ) diff --git a/src/backend/core/api/viewsets.py b/src/backend/core/api/viewsets.py index b2f735ea..f9e76659 100644 --- a/src/backend/core/api/viewsets.py +++ b/src/backend/core/api/viewsets.py @@ -296,12 +296,14 @@ class RoomViewSet( ) mode = serializer.validated_data["mode"] - options = serializer.validated_data["options"] + options = serializer.validated_data.get("options") room = self.get_object() # May raise exception if an active or initiated recording already exist for the room recording = models.Recording.objects.create( - room=room, mode=mode, options=options + room=room, + mode=mode, + options=options.model_dump(exclude_none=True) if options else {}, ) models.RecordingAccess.objects.create( diff --git a/src/backend/core/tests/rooms/test_api_rooms_start_recording.py b/src/backend/core/tests/rooms/test_api_rooms_start_recording.py index 9d3b1a73..36900c4c 100644 --- a/src/backend/core/tests/rooms/test_api_rooms_start_recording.py +++ b/src/backend/core/tests/rooms/test_api_rooms_start_recording.py @@ -199,3 +199,308 @@ def test_start_recording_success( access = recording.accesses.first() assert access.user == user assert access.role == "owner" + + +@pytest.mark.parametrize("value", ["fr", "en", "nl", "de"]) +def test_start_recording_options_language_valid( + settings, mock_worker_service_factory, mock_worker_manager, value +): + """Should accept a valid ISO 639-1 language code.""" + settings.RECORDING_ENABLE = True + room = RoomFactory() + user = UserFactory() + room.accesses.create(user=user, role="owner") + client = APIClient() + client.force_login(user) + + response = client.post( + f"/api/v1.0/rooms/{room.id}/start-recording/", + {"mode": "screen_recording", "options": {"language": value}}, + format="json", + ) + + assert response.status_code == 201 + + recording = Recording.objects.get(room=room) + assert recording.options == {"language": value} + + +@pytest.mark.parametrize("value", ["invalid-value", "francais", "123"]) +def test_start_recording_options_language_not_validated( + settings, mock_worker_service_factory, mock_worker_manager, value +): + """Invalid language codes are currently accepted — no format validation yet. + + TODO: tighten this once language validation is introduced. + """ + settings.RECORDING_ENABLE = True + room = RoomFactory() + user = UserFactory() + room.accesses.create(user=user, role="owner") + client = APIClient() + client.force_login(user) + + response = client.post( + f"/api/v1.0/rooms/{room.id}/start-recording/", + {"mode": "screen_recording", "options": {"language": value}}, + format="json", + ) + + assert response.status_code == 201 + + +def test_start_recording_options_language_null( + settings, mock_worker_service_factory, mock_worker_manager +): + """Should accept null language (triggers auto-detection).""" + settings.RECORDING_ENABLE = True + room = RoomFactory() + user = UserFactory() + room.accesses.create(user=user, role="owner") + client = APIClient() + client.force_login(user) + + response = client.post( + f"/api/v1.0/rooms/{room.id}/start-recording/", + {"mode": "screen_recording", "options": {"language": None}}, + format="json", + ) + + assert response.status_code == 201 + recording = Recording.objects.get(room=room) + assert recording.options == {} + + +@pytest.mark.parametrize("value", [True, 1, "y", "on", "true", "yes", "t"]) +def test_start_recording_options_transcribe_valid_true( + settings, mock_worker_service_factory, mock_worker_manager, value +): + """Should accept transcribe with any valid pydantic true values.""" + settings.RECORDING_ENABLE = True + room = RoomFactory() + user = UserFactory() + room.accesses.create(user=user, role="owner") + client = APIClient() + client.force_login(user) + + response = client.post( + f"/api/v1.0/rooms/{room.id}/start-recording/", + {"mode": "screen_recording", "options": {"transcribe": value}}, + format="json", + ) + + assert response.status_code == 201 + recording = Recording.objects.get(room=room) + assert recording.options == {"transcribe": True} + + +@pytest.mark.parametrize("value", [False, 0, "n", "off", "false", "no", "f"]) +def test_start_recording_options_transcribe_valid_false( + settings, mock_worker_service_factory, mock_worker_manager, value +): + """Should accept transcribe with any valid pydantic false values.""" + settings.RECORDING_ENABLE = True + room = RoomFactory() + user = UserFactory() + room.accesses.create(user=user, role="owner") + client = APIClient() + client.force_login(user) + + response = client.post( + f"/api/v1.0/rooms/{room.id}/start-recording/", + {"mode": "screen_recording", "options": {"transcribe": value}}, + format="json", + ) + + assert response.status_code == 201 + recording = Recording.objects.get(room=room) + assert recording.options == {"transcribe": False} + + +def test_start_recording_options_transcribe_null( + settings, mock_worker_service_factory, mock_worker_manager +): + """Should accept transcribe=null (falls back to application default).""" + settings.RECORDING_ENABLE = True + room = RoomFactory() + user = UserFactory() + room.accesses.create(user=user, role="owner") + client = APIClient() + client.force_login(user) + + response = client.post( + f"/api/v1.0/rooms/{room.id}/start-recording/", + {"mode": "screen_recording", "options": {"transcribe": None}}, + format="json", + ) + + assert response.status_code == 201 + recording = Recording.objects.get(room=room) + assert recording.options == {} + + +def test_start_recording_options_null( + settings, mock_worker_service_factory, mock_worker_manager +): + """Should accept options=null.""" + settings.RECORDING_ENABLE = True + room = RoomFactory() + user = UserFactory() + room.accesses.create(user=user, role="owner") + client = APIClient() + client.force_login(user) + + response = client.post( + f"/api/v1.0/rooms/{room.id}/start-recording/", + {"mode": "screen_recording", "options": None}, + format="json", + ) + + assert response.status_code == 201 + recording = Recording.objects.get(room=room) + assert recording.options == {} + + +def test_start_recording_options_omitted( + settings, mock_worker_service_factory, mock_worker_manager +): + """Should accept a request with no options field at all.""" + settings.RECORDING_ENABLE = True + room = RoomFactory() + user = UserFactory() + room.accesses.create(user=user, role="owner") + client = APIClient() + client.force_login(user) + + response = client.post( + f"/api/v1.0/rooms/{room.id}/start-recording/", + {"mode": "screen_recording"}, + format="json", + ) + + assert response.status_code == 201 + recording = Recording.objects.get(room=room) + assert recording.options == {} + + +def test_start_recording_options_unknown_field_rejected(settings): + """Should reject unknown fields in options (extra='forbid').""" + settings.RECORDING_ENABLE = True + room = RoomFactory() + user = UserFactory() + room.accesses.create(user=user, role="owner") + client = APIClient() + client.force_login(user) + + response = client.post( + f"/api/v1.0/rooms/{room.id}/start-recording/", + {"mode": "screen_recording", "options": {"unknown_field": "value"}}, + format="json", + ) + + assert response.status_code == 400 + + +@pytest.mark.parametrize("value", ["foo", 12]) +def test_start_recording_options_invalid_transcribe_type(settings, value): + """Should reject non-boolean transcribe values.""" + settings.RECORDING_ENABLE = True + room = RoomFactory() + user = UserFactory() + room.accesses.create(user=user, role="owner") + client = APIClient() + client.force_login(user) + + response = client.post( + f"/api/v1.0/rooms/{room.id}/start-recording/", + {"mode": "screen_recording", "options": {"transcribe": value}}, + format="json", + ) + + assert response.status_code == 400 + + +@pytest.mark.parametrize("value", ["screen_recording", "transcript"]) +def test_start_recording_options_original_mode_valid( + settings, mock_worker_service_factory, mock_worker_manager, value +): + """Should accept valid recording mode choices for original_mode.""" + settings.RECORDING_ENABLE = True + room = RoomFactory() + user = UserFactory() + room.accesses.create(user=user, role="owner") + client = APIClient() + client.force_login(user) + + response = client.post( + f"/api/v1.0/rooms/{room.id}/start-recording/", + {"mode": "screen_recording", "options": {"original_mode": value}}, + format="json", + ) + + assert response.status_code == 201 + recording = Recording.objects.get(room=room) + assert recording.options == {"original_mode": value} + + +def test_start_recording_options_original_mode_null( + settings, mock_worker_service_factory, mock_worker_manager +): + """Should accept original_mode=null.""" + settings.RECORDING_ENABLE = True + room = RoomFactory() + user = UserFactory() + room.accesses.create(user=user, role="owner") + client = APIClient() + client.force_login(user) + + response = client.post( + f"/api/v1.0/rooms/{room.id}/start-recording/", + {"mode": "screen_recording", "options": {"original_mode": None}}, + format="json", + ) + + assert response.status_code == 201 + recording = Recording.objects.get(room=room) + assert recording.options == {} + + +def test_start_recording_options_original_mode_omitted( + settings, mock_worker_service_factory, mock_worker_manager +): + """Should accept a request with original_mode omitted.""" + settings.RECORDING_ENABLE = True + room = RoomFactory() + user = UserFactory() + room.accesses.create(user=user, role="owner") + client = APIClient() + client.force_login(user) + + response = client.post( + f"/api/v1.0/rooms/{room.id}/start-recording/", + {"mode": "screen_recording", "options": {}}, + format="json", + ) + + assert response.status_code == 201 + recording = Recording.objects.get(room=room) + assert recording.options == {} + + +@pytest.mark.parametrize("value", ["invalid_mode", "foo", 123, "SCREEN_RECORDING"]) +def test_start_recording_options_original_mode_invalid(settings, value): + """Should reject invalid recording mode values for original_mode.""" + settings.RECORDING_ENABLE = True + room = RoomFactory() + user = UserFactory() + room.accesses.create(user=user, role="owner") + client = APIClient() + client.force_login(user) + + response = client.post( + f"/api/v1.0/rooms/{room.id}/start-recording/", + {"mode": "screen_recording", "options": {"original_mode": value}}, + format="json", + ) + + assert response.status_code == 400