diff --git a/src/backend/core/api/serializers.py b/src/backend/core/api/serializers.py index e6b0acac..21932195 100644 --- a/src/backend/core/api/serializers.py +++ b/src/backend/core/api/serializers.py @@ -273,6 +273,11 @@ class RequestEntrySerializer(BaseValidationOnlySerializer): """Validate request entry data.""" username = serializers.CharField(required=True) + participant_id = serializers.UUIDField(required=False, allow_null=True) + + def validate_participant_id(self, value): + """The id is a bearer credential: never trusted, only looked up.""" + return str(value) if value else None class ParticipantEntrySerializer(BaseValidationOnlySerializer): diff --git a/src/backend/core/api/throttling.py b/src/backend/core/api/throttling.py index 96b033a1..10cdbfb0 100644 --- a/src/backend/core/api/throttling.py +++ b/src/backend/core/api/throttling.py @@ -1,11 +1,11 @@ """Throttling modules for the API.""" -from django.conf import settings - from lasuite.drf.throttling import MonitoredThrottleMixin from rest_framework.throttling import AnonRateThrottle, UserRateThrottle from sentry_sdk import capture_message +from . import serializers + def sentry_monitoring_throttle_failure(message): """Log when a failure occurs to detect rate limiting issues.""" @@ -42,13 +42,14 @@ class RequestEntryAnonRateThrottle(MonitoredAnonRateThrottle): def get_cache_key(self, request, view): """Use the lobby participant cookie ID as the throttle cache key. - Only throttle if a cookie is already set. If no cookie exists yet, - return None to skip throttling — the cookie will be set on the first - response, and throttling will apply from the second request onward. + Only throttle requests carrying a participant identifier. The + identifier is returned by the first request-entry response and + echoed back by the client from the second request onward, which is + when throttling starts applying. - Keying on the cookie rather than the IP address prevents penalising - multiple users behind the same NAT/proxy, and is consistent with how - LobbyService identifies participants. + Keying on the identifier rather than the IP address prevents + penalising multiple users behind the same NAT/proxy, and is + consistent with how the lobby identifies participants. Note: as per DRF documentation, application-level throttling is not a security measure against brute-force or DoS attacks. This throttle exists @@ -58,10 +59,14 @@ class RequestEntryAnonRateThrottle(MonitoredAnonRateThrottle): if request.user and request.user.is_authenticated: return None # Only throttle unauthenticated requests. - participant_id = request.COOKIES.get(settings.LOBBY_COOKIE_NAME) + serializer = serializers.RequestEntrySerializer(data=request.data) + if not serializer.is_valid(): + return None - if participant_id is None: - return None # No throttling for cookieless requests + participant_id = serializer.validated_data.get("participant_id") + + if not participant_id: + return None # No throttling for unidentified requests return self.cache_format % { "scope": self.scope, diff --git a/src/backend/core/api/viewsets.py b/src/backend/core/api/viewsets.py index 7f1ba6d7..034557e1 100644 --- a/src/backend/core/api/viewsets.py +++ b/src/backend/core/api/viewsets.py @@ -571,10 +571,7 @@ class RoomViewSet( request=request, **serializer.validated_data, ) - response = drf_response.Response({**participant.to_dict(), "livekit": livekit}) - lobby_service.prepare_response(response, participant.id) - - return response + return drf_response.Response({**participant.to_dict(), "livekit": livekit}) @decorators.action( detail=True, diff --git a/src/backend/core/services/lobby.py b/src/backend/core/services/lobby.py index 8e61b97d..8c4b5ae9 100644 --- a/src/backend/core/services/lobby.py +++ b/src/backend/core/services/lobby.py @@ -86,23 +86,6 @@ class LobbyService: """Generate cache key for participant(s) data.""" return f"{settings.LOBBY_KEY_PREFIX}_{room_id!s}_{participant_id}" - @staticmethod - def _get_or_create_participant_id(request) -> str: - """Extract unique participant identifier from the request.""" - return request.COOKIES.get(settings.LOBBY_COOKIE_NAME, str(uuid.uuid4())) - - @staticmethod - def prepare_response(response, participant_id): - """Set participant cookie if needed.""" - if not response.cookies.get(settings.LOBBY_COOKIE_NAME): - response.set_cookie( - key=settings.LOBBY_COOKIE_NAME, - value=participant_id, - httponly=True, - secure=True, - samesite="Lax", - ) - @staticmethod def can_bypass_lobby(room, user, role) -> bool: """Determines if a user can bypass the waiting lobby and join a room directly. @@ -135,6 +118,7 @@ class LobbyService: room: models.Room, request, username: str, + participant_id: Optional[uuid.UUID] = None, ) -> Tuple[LobbyParticipant, Optional[Dict]]: """Request entry to a room for a participant. @@ -149,22 +133,20 @@ class LobbyService: 5. If denied, do nothing. """ - participant_id = self._get_or_create_participant_id(request) - participant = self._get_participant(room.id, participant_id) + participant = None + if participant_id: + participant = self._get_participant(room.id, participant_id) + + is_new_participant = participant is None + if is_new_participant: + participant = self._create_participant(room.id, username) room_id = str(room.id) user_role = room.get_role(request.user) if self.can_bypass_lobby(room=room, user=request.user, role=user_role): - if participant is None: - participant = LobbyParticipant( - status=LobbyParticipantStatus.ACCEPTED, - username=username, - id=participant_id, - color=utils.generate_color(participant_id), - ) - else: - participant.status = LobbyParticipantStatus.ACCEPTED + participant.status = LobbyParticipantStatus.ACCEPTED + self._save_participant(room.id, participant) livekit_config = utils.generate_livekit_config( room_id=room_id, @@ -172,18 +154,18 @@ class LobbyService: username=username, color=participant.color, configuration=room.configuration, - participant_id=participant_id, + participant_id=participant.id, role=user_role, ) return participant, livekit_config livekit_config = None - if participant is None: - participant = self.enter(room.id, participant_id, username) + if is_new_participant: + self._notify_entry_request(room_id) elif participant.status == LobbyParticipantStatus.WAITING: - self.refresh_waiting_status(room.id, participant_id) + self.refresh_waiting_status(room.id, participant.id) elif participant.status == LobbyParticipantStatus.ACCEPTED: # wrongly named, contains access token to join a room @@ -193,7 +175,7 @@ class LobbyService: username=username, color=participant.color, configuration=room.configuration, - participant_id=participant_id, + participant_id=participant.id, role=user_role, ) @@ -210,27 +192,36 @@ class LobbyService: self._get_cache_key(room_id, participant_id), settings.LOBBY_WAITING_TIMEOUT ) - def enter( - self, room_id: UUID, participant_id: str, username: str - ) -> LobbyParticipant: - """Add participant to waiting lobby. + def _create_participant(self, room_id: UUID, username: str) -> LobbyParticipant: + """Create and persist a new waiting participant. - Create a new participant entry in waiting status and notify room - participants of the new entry request. + Participant identifiers are minted here, server-side, exclusively. """ - - color = utils.generate_color(participant_id) - + participant_id = str(uuid.uuid4()) participant = LobbyParticipant( status=LobbyParticipantStatus.WAITING, username=username, id=participant_id, - color=color, + color=utils.generate_color(participant_id), + ) + self._save_participant(room_id, participant) + + return participant + + def _save_participant(self, room_id: UUID, participant: LobbyParticipant): + """Persist a participant in the room's lobby.""" + cache.set( + self._get_cache_key(room_id, participant.id), + participant.to_dict(), + timeout=settings.LOBBY_WAITING_TIMEOUT, ) + @staticmethod + def _notify_entry_request(room_id: str): + """Notify room participants of a new entry request.""" try: utils.notify_participants( - room_name=str(room_id), + room_name=room_id, notification_data={ "type": settings.LOBBY_NOTIFICATION_TYPE, }, @@ -239,15 +230,6 @@ class LobbyService: # If room not created yet, there is no participants to notify logger.exception("Failed to notify room participants") - cache_key = self._get_cache_key(room_id, participant_id) - cache.set( - cache_key, - participant.to_dict(), - timeout=settings.LOBBY_WAITING_TIMEOUT, - ) - - return participant - def _get_participant( self, room_id: UUID, participant_id: str ) -> Optional[LobbyParticipant]: diff --git a/src/backend/core/tests/rooms/test_api_rooms_lobby.py b/src/backend/core/tests/rooms/test_api_rooms_lobby.py index 37ca2124..765ea33b 100644 --- a/src/backend/core/tests/rooms/test_api_rooms_lobby.py +++ b/src/backend/core/tests/rooms/test_api_rooms_lobby.py @@ -14,9 +14,6 @@ from rest_framework.test import APIClient from ... import utils from ...factories import RoomFactory, UserFactory from ...models import RoomAccessLevel -from ...services.lobby import ( - LobbyService, -) pytestmark = pytest.mark.django_db @@ -29,7 +26,6 @@ def test_request_entry_anonymous(settings): room = RoomFactory(access_level=RoomAccessLevel.RESTRICTED) client = APIClient() - settings.LOBBY_COOKIE_NAME = "mocked-cookie" settings.LOBBY_KEY_PREFIX = "mocked-cache-prefix" # Lobby cache should be empty before the request @@ -47,11 +43,10 @@ def test_request_entry_anonymous(settings): assert response.status_code == 200 - # Verify the lobby cookie was properly set - cookie = response.cookies.get("mocked-cookie") - assert cookie is not None - - participant_id = cookie.value + # The participant identifier is returned in the response body; no + # cookie is involved anymore + assert not response.cookies + participant_id = response.json()["id"] # Verify response content matches expected structure and values assert response.json() == { @@ -78,7 +73,6 @@ def test_request_entry_authenticated_user(settings): client = APIClient() client.force_login(user) - settings.LOBBY_COOKIE_NAME = "mocked-cookie" settings.LOBBY_KEY_PREFIX = "mocked-cache-prefix" # Lobby cache should be empty before the request @@ -96,11 +90,10 @@ def test_request_entry_authenticated_user(settings): assert response.status_code == 200 - # Verify the lobby cookie was properly set - cookie = response.cookies.get("mocked-cookie") - assert cookie is not None - - participant_id = cookie.value + # The participant identifier is returned in the response body; no + # cookie is involved anymore + assert not response.cookies + participant_id = response.json()["id"] # Verify response content matches expected structure and values assert response.json() == { @@ -127,7 +120,6 @@ def test_request_entry_with_existing_participants(settings): client = APIClient() # Configure test settings for cookies and cache - settings.LOBBY_COOKIE_NAME = "mocked-cookie" settings.LOBBY_KEY_PREFIX = "mocked-cache-prefix" # Add two participants already waiting in the lobby @@ -168,11 +160,10 @@ def test_request_entry_with_existing_participants(settings): # Verify successful response assert response.status_code == 200 - # Verify the lobby cookie was properly set for the new participant - cookie = response.cookies.get("mocked-cookie") - assert cookie is not None - - participant_id = cookie.value + # The participant identifier is returned in the response body; no + # cookie is involved anymore + assert not response.cookies + participant_id = response.json()["id"] # Verify response content matches expected structure and values assert response.json() == { @@ -197,7 +188,6 @@ def test_request_entry_public_room(settings): room = RoomFactory(access_level=RoomAccessLevel.PUBLIC) client = APIClient() - settings.LOBBY_COOKIE_NAME = "mocked-cookie" settings.LOBBY_KEY_PREFIX = "mocked-cache-prefix" # Lobby cache should be empty before the request @@ -206,9 +196,7 @@ def test_request_entry_public_room(settings): with ( mock.patch.object(utils, "notify_participants", return_value=None), - mock.patch.object( - LobbyService, "_get_or_create_participant_id", return_value="123" - ), + mock.patch("core.services.lobby.uuid.uuid4", return_value="123"), mock.patch.object( utils, "generate_livekit_config", return_value={"token": "test-token"} ), @@ -221,11 +209,6 @@ def test_request_entry_public_room(settings): assert response.status_code == 200 - # Verify the lobby cookie was set - cookie = response.cookies.get("mocked-cookie") - assert cookie is not None - assert cookie.value == "123" - # Verify response content matches expected structure and values assert response.json() == { "id": "123", @@ -235,9 +218,10 @@ def test_request_entry_public_room(settings): "livekit": {"token": "test-token"}, } - # Verify lobby cache is still empty after the request + # The accepted participant is persisted, out of the waiting list lobby_keys = cache.keys(f"mocked-cache-prefix_{room.id}_*") - assert not lobby_keys + assert len(lobby_keys) == 1 + assert cache.get(lobby_keys[0])["status"] == "accepted" def test_request_entry_authenticated_user_public_room(settings): @@ -247,7 +231,6 @@ def test_request_entry_authenticated_user_public_room(settings): client = APIClient() client.force_login(user) - settings.LOBBY_COOKIE_NAME = "mocked-cookie" settings.LOBBY_KEY_PREFIX = "mocked-cache-prefix" # Lobby cache should be empty before the request @@ -256,9 +239,8 @@ def test_request_entry_authenticated_user_public_room(settings): with ( mock.patch.object(utils, "notify_participants", return_value=None), - mock.patch.object( - LobbyService, - "_get_or_create_participant_id", + mock.patch( + "core.services.lobby.uuid.uuid4", return_value="2f7f162f-e7d1-421b-90e7-02bfbfbf8def", ), mock.patch.object( @@ -273,11 +255,6 @@ def test_request_entry_authenticated_user_public_room(settings): assert response.status_code == 200 - # Verify the lobby cookie was set - cookie = response.cookies.get("mocked-cookie") - assert cookie is not None - assert cookie.value == "2f7f162f-e7d1-421b-90e7-02bfbfbf8def" - # Verify response content matches expected structure and values assert response.json() == { "id": "2f7f162f-e7d1-421b-90e7-02bfbfbf8def", @@ -287,9 +264,10 @@ def test_request_entry_authenticated_user_public_room(settings): "livekit": {"token": "test-token"}, } - # Verify lobby cache is still empty after the request + # The accepted participant is persisted, out of the waiting list lobby_keys = cache.keys(f"mocked-cache-prefix_{room.id}_*") - assert not lobby_keys + assert len(lobby_keys) == 1 + assert cache.get(lobby_keys[0])["status"] == "accepted" def test_request_entry_waiting_participant_public_room(settings): @@ -297,7 +275,6 @@ def test_request_entry_waiting_participant_public_room(settings): room = RoomFactory(access_level=RoomAccessLevel.PUBLIC) client = APIClient() - settings.LOBBY_COOKIE_NAME = "mocked-cookie" settings.LOBBY_KEY_PREFIX = "mocked-cache-prefix" # Add a waiting participant to the room's lobby cache @@ -311,9 +288,7 @@ def test_request_entry_waiting_participant_public_room(settings): }, ) - # Simulate a browser with existing participant cookie - client.cookies.load({"mocked-cookie": "2f7f162f-e7d1-421b-90e7-02bfbfbf8def"}) - + # Simulate a returning participant echoing its identifier with ( mock.patch.object(utils, "notify_participants", return_value=None), mock.patch.object( @@ -322,16 +297,14 @@ def test_request_entry_waiting_participant_public_room(settings): ): response = client.post( f"/api/v1.0/rooms/{room.id}/request-entry/", - {"username": "user1"}, + { + "username": "user1", + "participant_id": "2f7f162f-e7d1-421b-90e7-02bfbfbf8def", + }, ) assert response.status_code == 200 - # Verify the lobby cookie was set - cookie = response.cookies.get("mocked-cookie") - assert cookie is not None - assert cookie.value == "2f7f162f-e7d1-421b-90e7-02bfbfbf8def" - # Verify response content matches expected structure and values assert response.json() == { "id": "2f7f162f-e7d1-421b-90e7-02bfbfbf8def", @@ -637,15 +610,14 @@ def test_list_waiting_participants_empty(settings): @mock.patch.object( utils, "generate_livekit_config", return_value={"token": "test-token"} ) -def test_request_entry_throttling_anonymous_without_cookie( +def test_request_entry_throttling_anonymous_unidentified( mock_notify_participants, mock_generate_livekit_config, settings ): - """Anonymous users without a cookie should not be throttled.""" + """Requests without a participant identifier should not be throttled.""" room = RoomFactory(access_level=RoomAccessLevel.RESTRICTED) client = APIClient() - settings.LOBBY_COOKIE_NAME = "mocked-cookie" settings.REST_FRAMEWORK["DEFAULT_THROTTLE_RATES"]["request_entry"] = "1/minute" response = client.post( @@ -654,9 +626,6 @@ def test_request_entry_throttling_anonymous_without_cookie( ) assert response.status_code == 200 - assert response.cookies.get("mocked-cookie") is not None - - client.cookies.clear() # Simulate a new cookieless request response = client.post( f"/api/v1.0/rooms/{room.id}/request-entry/", @@ -670,34 +639,32 @@ def test_request_entry_throttling_anonymous_without_cookie( @mock.patch.object( utils, "generate_livekit_config", return_value={"token": "test-token"} ) -def test_request_entry_throttling_anonymous_with_cookie( +def test_request_entry_throttling_anonymous_identified( mock_notify_participants, mock_generate_livekit_config, settings ): - """Anonymous users with a cookie should be throttled after exceeding the rate limit.""" + """Identified requests should be throttled after exceeding the rate limit.""" room = RoomFactory(access_level=RoomAccessLevel.RESTRICTED) client = APIClient() - settings.LOBBY_COOKIE_NAME = "mocked-cookie" settings.REST_FRAMEWORK["DEFAULT_THROTTLE_RATES"]["request_entry"] = "2/minute" participant_id = str(uuid.uuid4()) - client.cookies.load({"mocked-cookie": participant_id}) response = client.post( f"/api/v1.0/rooms/{room.id}/request-entry/", - {"username": "test_user"}, + {"username": "test_user", "participant_id": participant_id}, ) assert response.status_code == 200 response = client.post( f"/api/v1.0/rooms/{room.id}/request-entry/", - {"username": "test_user"}, + {"username": "test_user", "participant_id": participant_id}, ) assert response.status_code == 200 response = client.post( f"/api/v1.0/rooms/{room.id}/request-entry/", - {"username": "test_user"}, + {"username": "test_user", "participant_id": participant_id}, ) assert response.status_code == 429 @@ -716,7 +683,6 @@ def test_request_entry_throttling_authenticated_user( client = APIClient() client.force_login(user) - settings.LOBBY_COOKIE_NAME = "mocked-cookie" settings.REST_FRAMEWORK["DEFAULT_THROTTLE_RATES"]["request_entry"] = "2/minute" response = client.post( @@ -737,3 +703,124 @@ def test_request_entry_throttling_authenticated_user( ) assert response.status_code == 429 + + +def test_request_entry_with_participant_id(settings): + """Echoing the previously issued identifier preserves the lobby identity across requests.""" + room = RoomFactory(access_level=RoomAccessLevel.RESTRICTED) + client = APIClient() + + settings.LOBBY_KEY_PREFIX = "mocked-cache-prefix" + + with ( + mock.patch.object(utils, "notify_participants", return_value=None), + mock.patch.object(utils, "generate_color", return_value="mocked-color"), + ): + response = client.post( + f"/api/v1.0/rooms/{room.id}/request-entry/", + {"username": "test_user"}, + ) + + assert response.status_code == 200 + participant_id = response.json()["id"] + + # Echoing the identifier must be recognized as the same + # participant: no duplicate in the lobby + response = client.post( + f"/api/v1.0/rooms/{room.id}/request-entry/", + {"username": "test_user", "participant_id": participant_id}, + ) + + assert response.status_code == 200 + assert response.json()["id"] == participant_id + assert response.json()["status"] == "waiting" + + lobby_keys = cache.keys(f"mocked-cache-prefix_{room.id}_*") + assert len(lobby_keys) == 1 + + +def test_request_entry_unknown_participant_id_not_seeded(settings): + """An identifier unknown to the room's lobby must not be honored.""" + room = RoomFactory(access_level=RoomAccessLevel.RESTRICTED) + client = APIClient() + + settings.LOBBY_KEY_PREFIX = "mocked-cache-prefix" + + forged_id = str(uuid.uuid4()) + + with ( + mock.patch.object(utils, "notify_participants", return_value=None), + mock.patch.object(utils, "generate_color", return_value="mocked-color"), + ): + response = client.post( + f"/api/v1.0/rooms/{room.id}/request-entry/", + {"username": "test_user", "participant_id": forged_id}, + ) + + assert response.status_code == 200 + assert response.json()["id"] != forged_id + + # Nothing was stored under the forged identifier + assert cache.get(f"mocked-cache-prefix_{room.id}_{forged_id}") is None + + +def test_request_entry_participant_id_bound_to_room(settings): + """An identifier minted for one room must not be honored in another.""" + room = RoomFactory(access_level=RoomAccessLevel.RESTRICTED) + other_room = RoomFactory(access_level=RoomAccessLevel.RESTRICTED) + client = APIClient() + + with ( + mock.patch.object(utils, "notify_participants", return_value=None), + mock.patch.object(utils, "generate_color", return_value="mocked-color"), + ): + response = client.post( + f"/api/v1.0/rooms/{room.id}/request-entry/", + {"username": "test_user"}, + ) + participant_id = response.json()["id"] + + response = client.post( + f"/api/v1.0/rooms/{other_room.id}/request-entry/", + {"username": "test_user", "participant_id": participant_id}, + ) + + assert response.status_code == 200 + assert response.json()["id"] != participant_id + + +def test_request_entry_legacy_cookie_ignored(): + """The retired cookie channel must not be honored anymore.""" + room = RoomFactory(access_level=RoomAccessLevel.RESTRICTED) + client = APIClient() + + legacy_participant_id = str(uuid.uuid4()) + client.cookies["lobbyParticipantId"] = legacy_participant_id + + with ( + mock.patch.object(utils, "notify_participants", return_value=None), + mock.patch.object(utils, "generate_color", return_value="mocked-color"), + ): + response = client.post( + f"/api/v1.0/rooms/{room.id}/request-entry/", + {"username": "test_user"}, + ) + + assert response.status_code == 200 + returned_id = response.json()["id"] + assert returned_id != legacy_participant_id + uuid.UUID(returned_id) + + +def test_request_entry_malformed_participant_id(settings): + """A non-UUID identifier is rejected by the serializer with a 400.""" + room = RoomFactory(access_level=RoomAccessLevel.RESTRICTED) + client = APIClient() + + response = client.post( + f"/api/v1.0/rooms/{room.id}/request-entry/", + {"username": "test_user", "participant_id": "../../../evil-key"}, + ) + + assert response.status_code == 400 + assert "participant_id" in response.json() 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 795e6c7c..558780b3 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 @@ -20,7 +20,11 @@ from rest_framework.test import APIClient from core import utils from core.factories import RoomFactory, UserFactory, UserResourceAccessFactory -from core.services.lobby import LobbyService +from core.services.lobby import ( + LobbyParticipant, + LobbyParticipantStatus, + LobbyService, +) pytestmark = pytest.mark.django_db @@ -849,7 +853,15 @@ def test_remove_participant_success_lobby_cache(mock_livekit_client): participant_identity = str(uuid4()) # Create participant in lobby cache first - LobbyService().enter(room.id, participant_identity, "John doe") + LobbyService()._save_participant( + room.id, + LobbyParticipant( + id=participant_identity, + username="John doe", + status=LobbyParticipantStatus.WAITING, + color="#123456", + ), + ) # Accept participant LobbyService().handle_participant_entry(room.id, participant_identity, True) diff --git a/src/backend/core/tests/services/test_lobby.py b/src/backend/core/tests/services/test_lobby.py index 8fc9728c..86b565b0 100644 --- a/src/backend/core/tests/services/test_lobby.py +++ b/src/backend/core/tests/services/test_lobby.py @@ -3,7 +3,6 @@ Test lobby service. """ # pylint: disable=W0621,W0613, W0212, R0913 -# ruff: noqa: PLR0913 import uuid from unittest import mock @@ -11,7 +10,6 @@ from unittest import mock from django.conf import settings from django.contrib.auth.models import AnonymousUser from django.core.cache import cache -from django.http import HttpResponse import pytest @@ -135,59 +133,6 @@ def test_get_cache_key(lobby_service, participant_id): assert cache_key == expected_key -def test_get_or_create_participant_id_from_cookie(lobby_service): - """Test extracting participant ID from cookie.""" - request = mock.Mock() - request.COOKIES = {settings.LOBBY_COOKIE_NAME: "existing-id"} - - participant_id = lobby_service._get_or_create_participant_id(request) - - assert participant_id == "existing-id" - - -@mock.patch.object(uuid, "uuid4", return_value="generated-id") -def test_get_or_create_participant_id_new(mock_uuid4, lobby_service): - """Test creating new participant ID when cookie is missing.""" - request = mock.Mock() - request.COOKIES = {} - - participant_id = lobby_service._get_or_create_participant_id(request) - - assert participant_id == "generated-id" - mock_uuid4.assert_called_once() - - -def test_prepare_response_existing_cookie(lobby_service, participant_id): - """Test response preparation with existing cookie.""" - response = HttpResponse() - response.cookies[settings.LOBBY_COOKIE_NAME] = "existing-cookie" - - lobby_service.prepare_response(response, participant_id) - - # Verify cookie wasn't set again - cookie = response.cookies.get(settings.LOBBY_COOKIE_NAME) - assert cookie.value == "existing-cookie" - assert cookie.value != participant_id - - -def test_prepare_response_new_cookie(lobby_service, participant_id): - """Test response preparation with new cookie.""" - response = HttpResponse() - - lobby_service.prepare_response(response, participant_id) - - # Verify cookie was set - cookie = response.cookies.get(settings.LOBBY_COOKIE_NAME) - assert cookie is not None - assert cookie.value == participant_id - assert cookie["httponly"] is True - assert cookie["secure"] is True - assert cookie["samesite"] == "Lax" - - # It's a session cookies (no max_age specified): - assert not cookie["max-age"] - - def test_can_bypass_lobby_public_room(lobby_service): """Should return True for public rooms regardless of user auth and role.""" room = RoomFactory(access_level=RoomAccessLevel.PUBLIC) @@ -266,11 +211,12 @@ def test_request_entry_public_room( color="#123456", ) - lobby_service._get_or_create_participant_id = mock.Mock(return_value=participant_id) lobby_service._get_participant = mock.Mock(return_value=mocked_participant) mock_generate_config.return_value = {"token": "test-token"} - participant, livekit_config = lobby_service.request_entry(room, request, username) + participant, livekit_config = lobby_service.request_entry( + room, request, username, participant_id=participant_id + ) assert participant.status == LobbyParticipantStatus.ACCEPTED assert livekit_config == {"token": "test-token"} @@ -304,11 +250,12 @@ def test_request_entry_trusted_room( color="#123456", ) - lobby_service._get_or_create_participant_id = mock.Mock(return_value=participant_id) lobby_service._get_participant = mock.Mock(return_value=mocked_participant) mock_generate_config.return_value = {"token": "test-token"} - participant, livekit_config = lobby_service.request_entry(room, request, username) + participant, livekit_config = lobby_service.request_entry( + room, request, username, participant_id=participant_id + ) assert participant.status == LobbyParticipantStatus.ACCEPTED assert livekit_config == {"token": "test-token"} @@ -325,18 +272,19 @@ def test_request_entry_trusted_room( lobby_service._get_participant.assert_called_once_with(room.id, participant_id) -@mock.patch("core.services.lobby.LobbyService.enter") +@mock.patch("core.services.lobby.LobbyService._notify_entry_request") +@mock.patch("core.services.lobby.LobbyService._create_participant") def test_request_entry_new_participant( - mock_enter, lobby_service, participant_id, username + mock_create, mock_notify, lobby_service, participant_id, username ): - """Test requesting entry for a new participant.""" + """A new participant gets a server-minted identifier - any provided + one is unknown to the lobby and therefore discarded - and the room is + notified of the entry request.""" request = mock.Mock() - request.COOKIES = {settings.LOBBY_COOKIE_NAME: participant_id} request.user = AnonymousUser() room = RoomFactory(access_level=RoomAccessLevel.RESTRICTED) - lobby_service._get_or_create_participant_id = mock.Mock(return_value=participant_id) lobby_service._get_participant = mock.Mock(return_value=None) participant_data = LobbyParticipant( @@ -345,14 +293,20 @@ def test_request_entry_new_participant( id=participant_id, color="#123456", ) - mock_enter.return_value = participant_data + mock_create.return_value = participant_data - participant, livekit_config = lobby_service.request_entry(room, request, username) + forged_id = str(uuid.uuid4()) + participant, livekit_config = lobby_service.request_entry( + room, request, username, participant_id=forged_id + ) assert participant == participant_data assert livekit_config is None - mock_enter.assert_called_once_with(room.id, participant_id, username) - lobby_service._get_participant.assert_called_once_with(room.id, participant_id) + # The provided identifier was looked up, found unknown, and replaced + # by a freshly minted participant + lobby_service._get_participant.assert_called_once_with(room.id, forged_id) + mock_create.assert_called_once_with(room.id, username) + mock_notify.assert_called_once_with(str(room.id)) @mock.patch("core.services.lobby.LobbyService.refresh_waiting_status") @@ -361,7 +315,6 @@ def test_request_entry_waiting_participant( ): """Test requesting entry for a waiting participant.""" request = mock.Mock() - request.COOKIES = {settings.LOBBY_COOKIE_NAME: participant_id} request.user = AnonymousUser() room = RoomFactory(access_level=RoomAccessLevel.RESTRICTED) @@ -372,10 +325,11 @@ def test_request_entry_waiting_participant( id=participant_id, color="#123456", ) - lobby_service._get_or_create_participant_id = mock.Mock(return_value=participant_id) lobby_service._get_participant = mock.Mock(return_value=mocked_participant) - participant, livekit_config = lobby_service.request_entry(room, request, username) + participant, livekit_config = lobby_service.request_entry( + room, request, username, participant_id=participant_id + ) assert participant.status == LobbyParticipantStatus.WAITING assert livekit_config is None @@ -390,7 +344,6 @@ def test_request_entry_accepted_participant( """Test requesting entry for an accepted participant.""" request = mock.Mock() request.user = AnonymousUser() - request.COOKIES = {settings.LOBBY_COOKIE_NAME: participant_id} room = RoomFactory(access_level=RoomAccessLevel.RESTRICTED) @@ -400,12 +353,13 @@ def test_request_entry_accepted_participant( id=participant_id, color="#123456", ) - lobby_service._get_or_create_participant_id = mock.Mock(return_value=participant_id) lobby_service._get_participant = mock.Mock(return_value=mocked_participant) mock_generate_config.return_value = {"token": "test-token"} - participant, livekit_config = lobby_service.request_entry(room, request, username) + participant, livekit_config = lobby_service.request_entry( + room, request, username, participant_id=participant_id + ) assert participant.status == LobbyParticipantStatus.ACCEPTED assert livekit_config == {"token": "test-token"} @@ -428,7 +382,6 @@ def test_request_entry_participant_with_role( """Test requesting entry for a participant with a role on the room.""" request = mock.Mock() request.user = UserFactory() - request.COOKIES = {settings.LOBBY_COOKIE_NAME: participant_id} room = RoomFactory(access_level=RoomAccessLevel.RESTRICTED) @@ -440,12 +393,13 @@ def test_request_entry_participant_with_role( id=participant_id, color="#123456", ) - lobby_service._get_or_create_participant_id = mock.Mock(return_value=participant_id) lobby_service._get_participant = mock.Mock(return_value=mocked_participant) mock_generate_config.return_value = {"token": "test-token"} - participant, livekit_config = lobby_service.request_entry(room, request, username) + participant, livekit_config = lobby_service.request_entry( + room, request, username, participant_id=participant_id + ) assert participant.status == LobbyParticipantStatus.ACCEPTED assert livekit_config == {"token": "test-token"} @@ -472,73 +426,47 @@ def test_refresh_waiting_status(mock_cache, lobby_service, participant_id): ) -# pylint: disable=R0917 @mock.patch("core.services.lobby.cache") @mock.patch("core.utils.generate_color") -@mock.patch("core.utils.notify_participants") -def test_enter_success( - mock_notify, +def test_create_participant( mock_generate_color, mock_cache, lobby_service, - participant_id, username, + settings, ): - """Test successful participant entry.""" + """A created participant is waiting, colored, and persisted.""" mock_generate_color.return_value = "#123456" lobby_service._get_cache_key = mock.Mock(return_value="mocked_cache_key") room = RoomFactory(access_level=RoomAccessLevel.RESTRICTED) - participant = lobby_service.enter(room.id, participant_id, username) + participant = lobby_service._create_participant(room.id, username) - mock_generate_color.assert_called_once_with(participant_id) + # The identifier is minted server-side + uuid.UUID(participant.id) + mock_generate_color.assert_called_once_with(participant.id) assert participant.status == LobbyParticipantStatus.WAITING assert participant.username == username - assert participant.id == participant_id assert participant.color == "#123456" - lobby_service._get_cache_key.assert_called_once_with(room.id, participant_id) + lobby_service._get_cache_key.assert_called_once_with(room.id, participant.id) mock_cache.set.assert_called_once_with( "mocked_cache_key", participant.to_dict(), timeout=settings.LOBBY_WAITING_TIMEOUT, ) - mock_notify.assert_called_once_with( - room_name=str(room.pk), notification_data={"type": "participantWaiting"} - ) -# pylint: disable=R0917 -@mock.patch("core.services.lobby.cache") -@mock.patch("core.utils.generate_color") @mock.patch("core.utils.notify_participants") -def test_enter_with_notification_error( - mock_notify, - mock_generate_color, - mock_cache, - lobby_service, - participant_id, - username, -): - """Test participant entry with notification error.""" - mock_generate_color.return_value = "#123456" +def test_notify_entry_request_with_notification_error(mock_notify, lobby_service): + """A notification error must not break the entry request flow.""" mock_notify.side_effect = NotificationError("Error notifying") - lobby_service._get_cache_key = mock.Mock(return_value="mocked_cache_key") - room = RoomFactory(access_level=RoomAccessLevel.RESTRICTED) - participant = lobby_service.enter(room.id, participant_id, username) + lobby_service._notify_entry_request("room-id") - mock_generate_color.assert_called_once_with(participant_id) - assert participant.status == LobbyParticipantStatus.WAITING - assert participant.username == username - - lobby_service._get_cache_key.assert_called_once_with(room.id, participant_id) - - mock_cache.set.assert_called_once_with( - "mocked_cache_key", - participant.to_dict(), - timeout=settings.LOBBY_WAITING_TIMEOUT, + mock_notify.assert_called_once_with( + room_name="room-id", notification_data={"type": "participantWaiting"} ) diff --git a/src/backend/meet/settings.py b/src/backend/meet/settings.py index 331f7829..cdd171e5 100755 --- a/src/backend/meet/settings.py +++ b/src/backend/meet/settings.py @@ -847,11 +847,6 @@ class Base(Configuration): environ_name="LOBBY_NOTIFICATION_TYPE", environ_prefix=None, ) - LOBBY_COOKIE_NAME = values.Value( - "lobbyParticipantId", - environ_name="LOBBY_COOKIE_NAME", - environ_prefix=None, - ) # Calendar integrations ROOM_CREATION_CALLBACK_CACHE_TIMEOUT = values.PositiveIntegerValue( diff --git a/src/frontend/src/features/rooms/api/requestEntry.ts b/src/frontend/src/features/rooms/api/requestEntry.ts index 25a80197..fcbba3ae 100644 --- a/src/frontend/src/features/rooms/api/requestEntry.ts +++ b/src/frontend/src/features/rooms/api/requestEntry.ts @@ -1,5 +1,6 @@ import { fetchApi } from '@/api/fetchApi' import type { ApiLiveKit } from '@/features/rooms/api/ApiRoom' +import { getLobbyParticipantId } from '@/stores/lobby' export interface RequestEntryParams { roomId: string @@ -15,6 +16,7 @@ export enum ApiLobbyStatus { } export interface ApiRequestEntry { + id?: string status: ApiLobbyStatus livekit?: ApiLiveKit } @@ -23,10 +25,12 @@ export const requestEntry = async ({ roomId, username = '', }: RequestEntryParams) => { + const participantId = getLobbyParticipantId(roomId) return fetchApi(`/rooms/${roomId}/request-entry/`, { method: 'POST', body: JSON.stringify({ username, + ...(participantId && { participant_id: participantId }), }), }) } diff --git a/src/frontend/src/features/rooms/hooks/useLobby.ts b/src/frontend/src/features/rooms/hooks/useLobby.ts index c454d31f..74fe60e7 100644 --- a/src/frontend/src/features/rooms/hooks/useLobby.ts +++ b/src/frontend/src/features/rooms/hooks/useLobby.ts @@ -6,6 +6,7 @@ import { ApiLobbyStatus, type ApiRequestEntry, } from '../api/requestEntry' +import { setLobbyParticipantId } from '@/stores/lobby' export const WAIT_TIMEOUT_MS = 600000 // 10 minutes export const POLL_INTERVAL_MS = 1000 @@ -43,6 +44,11 @@ export const useLobby = ({ roomId, username, }) + + if (response.id) { + setLobbyParticipantId(roomId, response.id) + } + if (response.status === ApiLobbyStatus.ACCEPTED) { clearWaitingTimeout() setStatus(ApiLobbyStatus.ACCEPTED) diff --git a/src/frontend/src/stores/lobby.ts b/src/frontend/src/stores/lobby.ts new file mode 100644 index 00000000..f21f4a9b --- /dev/null +++ b/src/frontend/src/stores/lobby.ts @@ -0,0 +1,23 @@ +import { proxy } from 'valtio' + +type State = { + participantIds: Record +} + +export const layoutStore = proxy({ + participantIds: {}, +}) + +export const setLobbyParticipantId = ( + roomId: string, + participantId: string +) => { + layoutStore.participantIds[roomId] = participantId +} + +export const clearParticipantId = (roomId: string) => { + delete layoutStore.participantIds[roomId] +} + +export const getLobbyParticipantId = (roomId: string) => + layoutStore.participantIds[roomId]