diff --git a/CHANGELOG.md b/CHANGELOG.md index 20b6c37d..3f9cddec 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,10 @@ and this project adheres to ## [Unreleased] +### Changed + +- 🔒️(backend) enhance API input validation to strengthen security #1053 + ## [1.9.0] - 2026-03-02 ### Added diff --git a/src/backend/core/api/serializers.py b/src/backend/core/api/serializers.py index d4ce5905..5405b1cb 100644 --- a/src/backend/core/api/serializers.py +++ b/src/backend/core/api/serializers.py @@ -2,9 +2,11 @@ # pylint: disable=abstract-method,no-name-in-module +from django.core.exceptions import SuspiciousOperation from django.utils.translation import gettext_lazy as _ -from livekit.api import ParticipantPermission +from django_pydantic_field.rest_framework import SchemaField +from pydantic import BaseModel, Field from rest_framework import serializers from rest_framework.exceptions import PermissionDenied from timezone_field.rest_framework import TimeZoneSerializerField @@ -261,6 +263,28 @@ class MuteParticipantSerializer(BaseParticipantsManagementSerializer): ) +class ParticipantPermission(BaseModel): + """Mirror the LiveKit ParticipantPermission protobuf. + + Control what a participant is allowed to publish, subscribe, and do within a room. + Unknown fields are rejected. + """ + + can_subscribe: bool | None = None + can_publish: bool | None = None + can_publish_data: bool | None = None + can_publish_sources: list[int] = Field( + default_factory=list + ) # TrackSource enum values + hidden: bool | None = None + recorder: bool | None = None + can_update_metadata: bool | None = None + agent: bool | None = None + can_subscribe_metrics: bool | None = None + + model_config = {"extra": "forbid"} + + class UpdateParticipantSerializer(BaseParticipantsManagementSerializer): """Validate participant update data.""" @@ -272,10 +296,11 @@ class UpdateParticipantSerializer(BaseParticipantsManagementSerializer): allow_null=True, help_text="Participant attributes as JSON object", ) - permission = serializers.DictField( + permission = SchemaField( + schema=ParticipantPermission | None, required=False, allow_null=True, - help_text="Participant permission as JSON object", + help_text="Participant permissions", ) name = serializers.CharField( max_length=255, @@ -285,6 +310,33 @@ class UpdateParticipantSerializer(BaseParticipantsManagementSerializer): help_text="Display name for the participant", ) + def validate_permission(self, permission): + """Validate that the given permission does not include forbidden or unimplemented fields.""" + + if permission is None: + return None + + suspicious_fields = [ + field + for field in ("hidden", "recorder", "agent") + if getattr(permission, field) is not None + ] + if suspicious_fields: + raise SuspiciousOperation( + f"Setting the following participant permissions is not allowed: " + f"{', '.join(suspicious_fields)}." + ) + if permission.can_subscribe_metrics is not None: + raise serializers.ValidationError( + { + "permission": { + "can_subscribe_metrics": "This permission is not implemented." + } + } + ) + + return permission + def validate(self, attrs): """Ensure at least one update field is provided.""" update_fields = ["metadata", "attributes", "permission", "name"] @@ -300,12 +352,4 @@ class UpdateParticipantSerializer(BaseParticipantsManagementSerializer): f"{', '.join(update_fields)}." ) - if "permission" in attrs: - try: - ParticipantPermission(**attrs["permission"]) - except ValueError as e: - raise serializers.ValidationError( - {"permission": f"Invalid permission: {str(e)}"} - ) from e - return attrs diff --git a/src/backend/core/api/viewsets.py b/src/backend/core/api/viewsets.py index 15159770..b2f735ea 100644 --- a/src/backend/core/api/viewsets.py +++ b/src/backend/core/api/viewsets.py @@ -607,13 +607,15 @@ class RoomViewSet( serializer = serializers.UpdateParticipantSerializer(data=request.data) serializer.is_valid(raise_exception=True) + permission = serializer.validated_data.get("permission") + try: ParticipantsManagement().update( room_name=str(room.pk), identity=str(serializer.validated_data["participant_identity"]), metadata=serializer.validated_data.get("metadata"), attributes=serializer.validated_data.get("attributes"), - permission=serializer.validated_data.get("permission"), + permission=permission.model_dump() if permission else None, name=serializer.validated_data.get("name"), ) except ParticipantsManagementException: diff --git a/src/backend/core/tests/rooms/test_api_rooms_participants_management.py b/src/backend/core/tests/rooms/test_api_rooms_participants_management.py index 647fe007..6d51f17d 100644 --- a/src/backend/core/tests/rooms/test_api_rooms_participants_management.py +++ b/src/backend/core/tests/rooms/test_api_rooms_participants_management.py @@ -8,6 +8,7 @@ import random from unittest import mock from uuid import uuid4 +from django.core.exceptions import SuspiciousOperation from django.urls import reverse import pytest @@ -132,11 +133,7 @@ def test_update_participant_success(mock_livekit_client): 1, 2, ], # [TrackSource.CAMERA, TrackSource.MICROPHONE] - "hidden": False, - "recorder": False, "can_update_metadata": True, - "agent": False, - "can_subscribe_metrics": False, }, "name": "John Doe", } @@ -151,6 +148,151 @@ def test_update_participant_success(mock_livekit_client): mock_livekit_client.aclose.assert_called_once() +@pytest.mark.parametrize( + "permission_payload", + [ + {}, # empty dict is valid + {"can_subscribe": True}, + {"can_publish": True}, + {"can_publish_data": True}, + {"can_publish_sources": [1, 2]}, + {"can_update_metadata": True}, + ], +) +def test_update_participant_permission_fields_are_optional( + mock_livekit_client, permission_payload +): + """Test that each required permission field can be passed individually.""" + client = APIClient() + room = RoomFactory() + user = UserFactory() + UserResourceAccessFactory( + resource=room, user=user, role=random.choice(["administrator", "owner"]) + ) + client.force_authenticate(user=user) + + payload = { + "participant_identity": str(uuid4()), + "permission": permission_payload, + } + + url = reverse("rooms-update-participant", kwargs={"pk": room.id}) + response = client.post(url, payload, format="json") + + assert response.status_code == status.HTTP_200_OK + assert response.data == {"status": "success"} + + mock_livekit_client.room.update_participant.assert_called_once() + mock_livekit_client.aclose.assert_called_once() + + +@pytest.mark.parametrize( + "value,permission_key", + [ + (False, "hidden"), + (True, "hidden"), + (False, "recorder"), + (True, "recorder"), + (False, "agent"), + (True, "agent"), + ], +) +@mock.patch("core.api.serializers.SuspiciousOperation", side_effect=SuspiciousOperation) +def test_update_participant_suspicious_permission( + mock_suspicious, value, permission_key +): + """Test update participant raises 400 when a restricted permission is set.""" + client = APIClient() + room = RoomFactory() + user = UserFactory() + UserResourceAccessFactory( + resource=room, user=user, role=random.choice(["administrator", "owner"]) + ) + client.force_authenticate(user=user) + + payload = { + "participant_identity": str(uuid4()), + "permission": { + "can_subscribe": True, + "can_publish": True, + "can_publish_data": True, + "can_update_metadata": False, + permission_key: value, + }, + } + + url = reverse("rooms-update-participant", kwargs={"pk": room.id}) + response = client.post(url, payload, format="json") + + assert response.status_code == status.HTTP_400_BAD_REQUEST + mock_suspicious.assert_called_once_with( + f"Setting the following participant permissions is not allowed: {permission_key}." + ) + + +@mock.patch("core.api.serializers.SuspiciousOperation", side_effect=SuspiciousOperation) +def test_update_participant_suspicious_permission_multiple(mock_suspicious): + """Test update participant raises 400 when multiple suspicious permissions are set.""" + client = APIClient() + room = RoomFactory() + user = UserFactory() + UserResourceAccessFactory( + resource=room, user=user, role=random.choice(["administrator", "owner"]) + ) + client.force_authenticate(user=user) + + payload = { + "participant_identity": str(uuid4()), + "permission": { + "can_subscribe": True, + "can_publish": True, + "can_publish_data": True, + "hidden": True, + "recorder": False, + "can_update_metadata": False, + "agent": True, + "can_subscribe_metrics": False, + }, + } + + url = reverse("rooms-update-participant", kwargs={"pk": room.id}) + response = client.post(url, payload, format="json") + + assert response.status_code == status.HTTP_400_BAD_REQUEST + mock_suspicious.assert_called_once_with( + "Setting the following participant permissions is not allowed: hidden, recorder, agent." + ) + + +@pytest.mark.parametrize("value", (False, True)) +def test_update_participant_unimplemented_can_subscribe_metrics(value): + """Test update participant raises 400 when can_subscribe_metrics is set.""" + client = APIClient() + room = RoomFactory() + user = UserFactory() + UserResourceAccessFactory( + resource=room, user=user, role=random.choice(["administrator", "owner"]) + ) + client.force_authenticate(user=user) + + payload = { + "participant_identity": str(uuid4()), + "permission": { + "can_subscribe": True, + "can_publish": True, + "can_publish_data": True, + "can_update_metadata": False, + "can_subscribe_metrics": value, + }, + } + + url = reverse("rooms-update-participant", kwargs={"pk": room.id}) + response = client.post(url, payload, format="json") + + assert response.status_code == status.HTTP_400_BAD_REQUEST + assert "can_subscribe_metrics" in str(response.data) + + def test_update_participant_forbidden_without_access(): """Test update participant returns 403 when user lacks room privileges.""" client = APIClient() @@ -226,7 +368,17 @@ def test_update_participant_invalid_permission(): response = client.post(url, payload, format="json") assert response.status_code == status.HTTP_400_BAD_REQUEST - assert "Invalid permission" in str(response.data) + assert response.json() == { + "permission": [ + { + "type": "extra_forbidden", + "loc": ["invalid-attributes"], + "msg": "Extra inputs are not permitted", + "input": "True", + "url": "https://errors.pydantic.dev/2.12/v/extra_forbidden", + }, + ] + } def test_update_participant_wrong_metadata_attributes():