diff --git a/src/backend/core/api/serializers.py b/src/backend/core/api/serializers.py index 6eb375493..61b34d4e4 100644 --- a/src/backend/core/api/serializers.py +++ b/src/backend/core/api/serializers.py @@ -356,6 +356,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 bce886f39..34af49238 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.""" @@ -69,13 +69,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 @@ -85,10 +86,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 dd09fc771..8568142dd 100644 --- a/src/backend/core/api/viewsets.py +++ b/src/backend/core/api/viewsets.py @@ -538,13 +538,10 @@ class RoomViewSet( participant, livekit = lobby_service.request_entry( room=room, - request=request, + user=request.user, **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 a361abcf6..5b10228bb 100644 --- a/src/backend/core/services/lobby.py +++ b/src/backend/core/services/lobby.py @@ -131,23 +131,6 @@ class LobbyService: if participant_ids: self._redis().srem(self._get_index_key(room_id), *participant_ids) - @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. @@ -178,8 +161,9 @@ class LobbyService: def request_entry( self, room: models.Room, - request, + user, username: str, + participant_id: Optional[uuid.UUID] = None, ) -> Tuple[LobbyParticipant, Optional[Dict]]: """Request entry to a room for a participant. @@ -194,52 +178,51 @@ 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(username) room_id = str(room.id) - user_role = room.get_role(request.user) + user_role = room.get_role(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), - entered_at=timezone.now().isoformat(), - ) - else: - participant.status = LobbyParticipantStatus.ACCEPTED + if self.can_bypass_lobby(room=room, user=user, role=user_role): + if not is_new_participant: + self.clear_participant_cache(room.id, participant.id) + participant.status = LobbyParticipantStatus.ACCEPTED livekit_config = utils.generate_livekit_config( room_id=room_id, - user=request.user, + user=user, 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._save_participant(room.id, 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 livekit_config = utils.generate_livekit_config( room_id=room_id, - user=request.user, + user=user, username=username, color=participant.color, configuration=room.configuration, - participant_id=participant_id, + participant_id=participant.id, role=user_role, ) @@ -257,24 +240,35 @@ class LobbyService: ) self._index_touch(room_id) - def enter( - self, room_id: UUID, participant_id: str, username: str - ) -> LobbyParticipant: - """Add participant to waiting lobby.""" - - color = utils.generate_color(participant_id) + def _create_participant(self, username: str) -> LobbyParticipant: + """Create a new waiting participant without persisting it. + Participant identifiers are minted here, server-side, exclusively. + """ + participant_id = str(uuid.uuid4()) participant = LobbyParticipant( status=LobbyParticipantStatus.WAITING, username=username, id=participant_id, - color=color, entered_at=timezone.now().isoformat(), + color=utils.generate_color(participant_id), ) + 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, + ) + self._index_add(room_id, participant.id) + + def _notify_entry_request(self, 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, }, @@ -283,16 +277,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, - ) - self._index_add(room_id, participant_id) - - return participant - def _get_participant( self, room_id: UUID, participant_id: str ) -> Optional[LobbyParticipant]: @@ -353,7 +337,7 @@ class LobbyService: room_id: UUID, participant_id: str, allow_entry: bool, - ) -> None: + ) -> LobbyParticipant: """Handle decision on participant entry. Updates participant status based on allow_entry: @@ -371,7 +355,7 @@ class LobbyService: "timeout": settings.LOBBY_DENIED_TIMEOUT, } - self._update_participant_status(room_id, participant_id, **decision) + return self._update_participant_status(room_id, participant_id, **decision) def _update_participant_status( self, @@ -379,7 +363,7 @@ class LobbyService: participant_id: str, status: LobbyParticipantStatus, timeout: int, - ) -> None: + ) -> LobbyParticipant: """Update participant status with appropriate timeout.""" cache_key = self._get_cache_key(room_id, participant_id) @@ -402,6 +386,8 @@ class LobbyService: cache.set(cache_key, participant.to_dict(), timeout=timeout) self._index_touch(room_id) + return participant + def clear_room_cache(self, room_id: UUID) -> None: """Clear all participant entries from the cache for a specific room.""" 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 04df0df68..2fba4de13 100644 --- a/src/backend/core/tests/rooms/test_api_rooms_lobby.py +++ b/src/backend/core/tests/rooms/test_api_rooms_lobby.py @@ -15,9 +15,7 @@ from rest_framework.test import APIClient from ... import utils from ...factories import RoomFactory, UserFactory from ...models import RoomAccessLevel -from ...services.lobby import ( - LobbyService, -) +from ...services.lobby import LobbyService pytestmark = pytest.mark.django_db @@ -31,7 +29,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 @@ -49,11 +46,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() == { @@ -82,7 +78,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 @@ -100,11 +95,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() == { @@ -133,7 +127,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 @@ -176,11 +169,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() == { @@ -207,7 +199,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 @@ -216,9 +207,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"} ), @@ -231,11 +220,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", @@ -246,9 +230,8 @@ def test_request_entry_public_room(settings): "livekit": {"token": "test-token"}, } - # Verify lobby cache is still empty after the request - lobby_keys = cache.keys(f"mocked-cache-prefix_{room.id}_*") - assert not lobby_keys + assert not cache.keys(f"mocked-cache-prefix_{room.id}_*") + assert not LobbyService()._index_members(room.id) @freeze_time("2025-01-01 10:00:00") @@ -259,7 +242,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 @@ -268,9 +250,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( @@ -285,11 +266,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", @@ -300,9 +276,8 @@ def test_request_entry_authenticated_user_public_room(settings): "livekit": {"token": "test-token"}, } - # Verify lobby cache is still empty after the request - lobby_keys = cache.keys(f"mocked-cache-prefix_{room.id}_*") - assert not lobby_keys + assert not cache.keys(f"mocked-cache-prefix_{room.id}_*") + assert not LobbyService()._index_members(room.id) @freeze_time("2025-01-01 10:00:00") @@ -311,7 +286,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 @@ -326,9 +300,9 @@ 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"}) + LobbyService()._index_add(room.id, "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( @@ -337,16 +311,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", @@ -357,9 +329,8 @@ def test_request_entry_waiting_participant_public_room(settings): "livekit": {"token": "test-token"}, } - # Verify participant remains in the lobby cache after acceptance - lobby_keys = cache.keys(f"mocked-cache-prefix_{room.id}_*") - assert len(lobby_keys) == 1 + assert not cache.keys(f"mocked-cache-prefix_{room.id}_*") + assert not LobbyService()._index_members(room.id) def test_request_entry_invalid_data(): @@ -662,15 +633,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( @@ -679,9 +649,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/", @@ -695,34 +662,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 @@ -741,7 +706,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( @@ -762,3 +726,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 1d10e89b7..d95691c15 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 @@ -28,10 +28,8 @@ from core.factories import ( UserFactory, UserResourceAccessFactory, ) -from core.services.lobby import ( - LobbyService, -) from core.models import ApplicationScope +from core.services.lobby import LobbyParticipant, LobbyParticipantStatus, LobbyService pytestmark = pytest.mark.django_db @@ -860,7 +858,16 @@ 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", + entered_at="2025-01-01T10:00:00+00:00", + ), + ) # Accept participant LobbyService().handle_participant_entry(room.id, participant_identity, True) diff --git a/src/backend/core/tests/rooms/test_api_rooms_subtitle.py b/src/backend/core/tests/rooms/test_api_rooms_subtitle.py index 8b4143870..52bd1aa3c 100644 --- a/src/backend/core/tests/rooms/test_api_rooms_subtitle.py +++ b/src/backend/core/tests/rooms/test_api_rooms_subtitle.py @@ -232,6 +232,7 @@ def test_start_subtitle_wrong_signature(settings, mock_livekit_token): "detail": "Invalid LiveKit token: Signature verification failed" } + @pytest.fixture def user_access_token(): """Generate a valid user access JWT, sent with the "Bearer" scheme.""" diff --git a/src/backend/core/tests/services/test_lobby.py b/src/backend/core/tests/services/test_lobby.py index f2bb695b6..172540ab4 100644 --- a/src/backend/core/tests/services/test_lobby.py +++ b/src/backend/core/tests/services/test_lobby.py @@ -2,20 +2,20 @@ Test lobby service. """ -# pylint: disable=W0621,W0613, W0212, R0913, C0302 +# pylint: disable=W0621,W0613, W0212, R0913, C0302, R0917 # ruff: noqa: PLR0913, PLR0917 import uuid from unittest import mock -from django.conf import settings +from django.conf import settings as django_settings from django.contrib.auth.models import AnonymousUser from django.core.cache import cache -from django.http import HttpResponse import pytest from freezegun import freeze_time +from core import utils from core.factories import RoomFactory, UserFactory, UserResourceAccessFactory from core.models import RoleChoices, RoomAccessLevel from core.services.lobby import ( @@ -151,63 +151,10 @@ def test_get_cache_key(lobby_service, participant_id): room = RoomFactory(access_level=RoomAccessLevel.RESTRICTED) cache_key = lobby_service._get_cache_key(room.id, participant_id) - expected_key = f"{settings.LOBBY_KEY_PREFIX}_{room.id!s}_{participant_id}" + expected_key = f"{django_settings.LOBBY_KEY_PREFIX}_{room.id!s}_{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) @@ -271,94 +218,99 @@ def test_can_bypass_lobby_private_room_with_any_role(role, lobby_service): @mock.patch("core.utils.generate_livekit_config") def test_request_entry_public_room( - mock_generate_config, lobby_service, participant_id, username + mock_generate_config, lobby_service, participant_id, username, settings ): """Test requesting entry to a public room.""" - request = mock.Mock() - request.user = AnonymousUser() + settings.LOBBY_KEY_PREFIX = "mocked-cache-prefix" + + user = AnonymousUser() room = RoomFactory(access_level=RoomAccessLevel.PUBLIC) - mocked_participant = LobbyParticipant( - status=LobbyParticipantStatus.UNKNOWN, - username=username, - id=participant_id, - color="#123456", - entered_at="2025-01-01T10:00:00+00:00", + cache.set( + f"mocked-cache-prefix_{room.id}_{participant_id}", + { + "id": participant_id, + "username": username, + "status": "waiting", + "color": "#123456", + "entered_at": "2025-01-01T10:00:00+00:00", + }, ) - 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, user, username, participant_id=participant_id + ) assert participant.status == LobbyParticipantStatus.ACCEPTED assert livekit_config == {"token": "test-token"} mock_generate_config.assert_called_once_with( room_id=str(room.id), - user=request.user, + user=user, username=username, color=participant.color, configuration=room.configuration, - participant_id="test-participant-id", + participant_id=participant_id, role=None, ) - lobby_service._get_participant.assert_called_once_with(room.id, participant_id) - @mock.patch("core.utils.generate_livekit_config") def test_request_entry_trusted_room( - mock_generate_config, lobby_service, participant_id, username + mock_generate_config, lobby_service, participant_id, username, settings ): """Test requesting entry to a trusted room when the user is authenticated.""" - request = mock.Mock() - request.user = UserFactory() + settings.LOBBY_KEY_PREFIX = "mocked-cache-prefix" + + user = UserFactory() room = RoomFactory(access_level=RoomAccessLevel.TRUSTED) - mocked_participant = LobbyParticipant( - status=LobbyParticipantStatus.UNKNOWN, - username=username, - id=participant_id, - color="#123456", - entered_at="2025-01-01T10:00:00+00:00", + cache.set( + f"mocked-cache-prefix_{room.id}_{participant_id}", + { + "id": participant_id, + "username": username, + "status": "waiting", + "color": "#123456", + "entered_at": "2025-01-01T10:00:00+00:00", + }, ) - 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, user, username, participant_id=participant_id + ) assert participant.status == LobbyParticipantStatus.ACCEPTED assert livekit_config == {"token": "test-token"} mock_generate_config.assert_called_once_with( room_id=str(room.id), - user=request.user, + user=user, username=username, color=participant.color, configuration=room.configuration, - participant_id="test-participant-id", + participant_id=participant_id, role=None, ) - 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.""" - request = mock.Mock() - request.COOKIES = {settings.LOBBY_COOKIE_NAME: participant_id} - request.user = AnonymousUser() + """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.""" + + 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( @@ -368,14 +320,20 @@ def test_request_entry_new_participant( color="#123456", entered_at="2025-01-01T10:00:00+00:00", ) - 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, user, 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(username) + mock_notify.assert_called_once_with(str(room.id)) @mock.patch("core.services.lobby.LobbyService.refresh_waiting_status") @@ -383,9 +341,7 @@ def test_request_entry_waiting_participant( mock_refresh, lobby_service, participant_id, username ): """Test requesting entry for a waiting participant.""" - request = mock.Mock() - request.COOKIES = {settings.LOBBY_COOKIE_NAME: participant_id} - request.user = AnonymousUser() + user = AnonymousUser() room = RoomFactory(access_level=RoomAccessLevel.RESTRICTED) @@ -396,10 +352,11 @@ def test_request_entry_waiting_participant( color="#123456", entered_at="2025-01-01T10:00:00+00:00", ) - 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, user, username, participant_id=participant_id + ) assert participant.status == LobbyParticipantStatus.WAITING assert livekit_config is None @@ -409,82 +366,122 @@ def test_request_entry_waiting_participant( @mock.patch("core.utils.generate_livekit_config") def test_request_entry_accepted_participant( - mock_generate_config, lobby_service, participant_id, username + mock_generate_config, lobby_service, participant_id, username, settings ): """Test requesting entry for an accepted participant.""" - request = mock.Mock() - request.user = AnonymousUser() - request.COOKIES = {settings.LOBBY_COOKIE_NAME: participant_id} + settings.LOBBY_KEY_PREFIX = "mocked-cache-prefix" + user = AnonymousUser() room = RoomFactory(access_level=RoomAccessLevel.RESTRICTED) - mocked_participant = LobbyParticipant( - status=LobbyParticipantStatus.ACCEPTED, - username=username, - id=participant_id, - color="#123456", - entered_at="2025-01-01T10:00:00+00:00", + cache.set( + f"mocked-cache-prefix_{room.id}_{participant_id}", + { + "id": participant_id, + "username": username, + "status": "accepted", + "color": "#123456", + "entered_at": "2025-01-01T10:00:00+00:00", + }, ) - 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, user, username, participant_id=participant_id + ) assert participant.status == LobbyParticipantStatus.ACCEPTED assert livekit_config == {"token": "test-token"} mock_generate_config.assert_called_once_with( room_id=str(room.id), - user=request.user, + user=user, username=username, color="#123456", configuration=room.configuration, participant_id="test-participant-id", role=None, ) - lobby_service._get_participant.assert_called_once_with(room.id, participant_id) + + +@mock.patch("core.utils.generate_livekit_config") +def test_request_entry_accepted_participant_username_is_bound( + mock_generate_config, lobby_service, participant_id, settings +): + """An accepted identifier must join under the username the host accepted. + + The participant identifier is a bearer value: a stolen or replayed + identifier must not be able to enter the room under a different + display name than the one the acceptance decision was made on. + """ + settings.LOBBY_KEY_PREFIX = "mocked-cache-prefix" + user = AnonymousUser() + + room = RoomFactory(access_level=RoomAccessLevel.RESTRICTED) + + lobby_service._save_participant( + room.id, + LobbyParticipant( + id=participant_id, + username="accepted-name", + status=LobbyParticipantStatus.ACCEPTED, + color="#123456", + entered_at="2025-01-01T10:00:00+00:00", + ), + ) + + mock_generate_config.return_value = {"token": "test-token"} + + participant, livekit_config = lobby_service.request_entry( + room, user, "spoofed-name", participant_id=participant_id + ) + + assert participant.status == LobbyParticipantStatus.ACCEPTED + assert livekit_config == {"token": "test-token"} + assert mock_generate_config.call_args.kwargs["username"] == "accepted-name" @mock.patch("core.utils.generate_livekit_config") def test_request_entry_participant_with_role( - mock_generate_config, lobby_service, participant_id, username + mock_generate_config, lobby_service, participant_id, username, settings ): """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} + settings.LOBBY_KEY_PREFIX = "mocked-cache-prefix" + + user = UserFactory() room = RoomFactory(access_level=RoomAccessLevel.RESTRICTED) - UserResourceAccessFactory(resource=room, user=request.user, role="administrator") + UserResourceAccessFactory(resource=room, user=user, role="administrator") - mocked_participant = LobbyParticipant( - status=LobbyParticipantStatus.ACCEPTED, - username=username, - id=participant_id, - color="#123456", - entered_at="2025-01-01T10:00:00+00:00", + cache.set( + f"mocked-cache-prefix_{room.id}_{participant_id}", + { + "id": participant_id, + "username": username, + "status": "accepted", + "color": "#123456", + "entered_at": "2025-01-01T10:00:00+00:00", + }, ) - 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, user, username, participant_id=participant_id + ) assert participant.status == LobbyParticipantStatus.ACCEPTED assert livekit_config == {"token": "test-token"} mock_generate_config.assert_called_once_with( room_id=str(room.id), - user=request.user, + user=user, username=username, color="#123456", configuration=room.configuration, participant_id="test-participant-id", role="administrator", ) - lobby_service._get_participant.assert_called_once_with(room.id, participant_id) @mock.patch("core.services.lobby.cache") @@ -495,87 +492,70 @@ def test_refresh_waiting_status(mock_cache, lobby_service, participant_id): room = RoomFactory(access_level=RoomAccessLevel.RESTRICTED) lobby_service.refresh_waiting_status(room.id, participant_id) mock_cache.touch.assert_called_once_with( - "mocked_cache_key", settings.LOBBY_WAITING_TIMEOUT + "mocked_cache_key", django_settings.LOBBY_WAITING_TIMEOUT ) lobby_service._index_touch.assert_called_once_with(room.id) -# pylint: disable=R0917 @mock.patch("core.services.lobby.cache") @mock.patch("core.utils.generate_color") @mock.patch("core.utils.notify_participants") @mock.patch("core.services.lobby.LobbyService._index_add") @freeze_time("2025-01-01 10:00:00") -def test_enter_success( +def test_create_participant_not_persisted_until_saved( mock_index_add, mock_notify, mock_generate_color, mock_cache, lobby_service, - participant_id, username, ): - """Test successful participant entry.""" + """Creation is in-memory; explicitly saving persists and indexes the participant.""" 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(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" assert participant.entered_at == "2025-01-01T10:00:00+00:00" - lobby_service._get_cache_key.assert_called_once_with(room.id, participant_id) + mock_cache.set.assert_not_called() + mock_index_add.assert_not_called() + lobby_service._save_participant(room.id, participant) + + 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, + timeout=django_settings.LOBBY_WAITING_TIMEOUT, ) - mock_notify.assert_called_once_with( - room_name=str(room.pk), notification_data={"type": "participantWaiting"} - ) - mock_index_add.assert_called_once_with(room.id, participant_id) + mock_index_add.assert_called_once_with(room.id, participant.id) -# pylint: disable=R0917 -@mock.patch("core.services.lobby.cache") -@mock.patch("core.utils.generate_color") @mock.patch("core.utils.notify_participants") @mock.patch("core.services.lobby.LobbyService._index_add") def test_enter_with_notification_error( mock_index_add, mock_notify, - mock_generate_color, - mock_cache, lobby_service, participant_id, username, ): - """Test participant entry with notification error.""" - mock_generate_color.return_value = "#123456" + """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"} ) - mock_index_add.assert_called_once_with(room.id, participant_id) @mock.patch("core.services.lobby.cache") @@ -627,7 +607,9 @@ def test_list_waiting_participants_empty(mock_cache, lobby_service): @mock.patch("core.services.lobby.cache") -def test_list_waiting_participants(mock_cache, lobby_service, participant_dict): +def test_list_waiting_participants( + mock_cache, lobby_service, participant_dict, settings +): """Test listing waiting participants with valid data.""" room = RoomFactory(access_level=RoomAccessLevel.RESTRICTED) cache_key = f"{settings.LOBBY_KEY_PREFIX}_{room.id!s}_participant1" @@ -649,8 +631,8 @@ def test_list_waiting_participants(mock_cache, lobby_service, participant_dict): def test_list_waiting_participants_multiple(mock_cache, lobby_service): """Test listing multiple waiting participants with valid data.""" room = RoomFactory(access_level=RoomAccessLevel.RESTRICTED) - cache_key1 = f"{settings.LOBBY_KEY_PREFIX}_{room.id!s}_participant1" - cache_key2 = f"{settings.LOBBY_KEY_PREFIX}_{room.id!s}_participant2" + cache_key1 = f"{django_settings.LOBBY_KEY_PREFIX}_{room.id!s}_participant1" + cache_key2 = f"{django_settings.LOBBY_KEY_PREFIX}_{room.id!s}_participant2" participant1 = { "status": "waiting", @@ -694,7 +676,7 @@ def test_list_waiting_participants_multiple(mock_cache, lobby_service): @mock.patch("core.services.lobby.cache") -def test_list_waiting_participants_corrupted_data(mock_cache, lobby_service): +def test_list_waiting_participants_corrupted_data(mock_cache, lobby_service, settings): """Test listing waiting participants with corrupted data.""" room = RoomFactory(access_level=RoomAccessLevel.RESTRICTED) cache_key = f"{settings.LOBBY_KEY_PREFIX}_{room.id!s}_participant1" @@ -712,8 +694,8 @@ def test_list_waiting_participants_corrupted_data(mock_cache, lobby_service): def test_list_waiting_participants_partially_corrupted(mock_cache, lobby_service): """Test listing waiting participants with one valid and one corrupted entry.""" room = RoomFactory(access_level=RoomAccessLevel.RESTRICTED) - cache_key1 = f"{settings.LOBBY_KEY_PREFIX}_{room.id!s}_participant1" - cache_key2 = f"{settings.LOBBY_KEY_PREFIX}_{room.id!s}_participant2" + cache_key1 = f"{django_settings.LOBBY_KEY_PREFIX}_{room.id!s}_participant1" + cache_key2 = f"{django_settings.LOBBY_KEY_PREFIX}_{room.id!s}_participant2" valid_participant = { "status": "waiting", @@ -753,8 +735,8 @@ def test_list_waiting_participants_partially_corrupted(mock_cache, lobby_service def test_list_waiting_participants_non_waiting(mock_cache, lobby_service): """Test listing only waiting participants (not accepted/denied).""" room = RoomFactory(access_level=RoomAccessLevel.RESTRICTED) - cache_key1 = f"{settings.LOBBY_KEY_PREFIX}_{room.id!s}_participant1" - cache_key2 = f"{settings.LOBBY_KEY_PREFIX}_{room.id!s}_participant2" + cache_key1 = f"{django_settings.LOBBY_KEY_PREFIX}_{room.id!s}_participant1" + cache_key2 = f"{django_settings.LOBBY_KEY_PREFIX}_{room.id!s}_participant2" participant1 = { "status": "waiting", @@ -797,7 +779,7 @@ def test_handle_participant_entry_allow(mock_update, lobby_service, participant_ room.id, participant_id, status=LobbyParticipantStatus.ACCEPTED, - timeout=settings.LOBBY_ACCEPTED_TIMEOUT, + timeout=django_settings.LOBBY_ACCEPTED_TIMEOUT, ) @@ -811,7 +793,7 @@ def test_handle_participant_entry_deny(mock_update, lobby_service, participant_i room.id, participant_id, status=LobbyParticipantStatus.DENIED, - timeout=settings.LOBBY_DENIED_TIMEOUT, + timeout=django_settings.LOBBY_DENIED_TIMEOUT, ) @@ -958,12 +940,12 @@ def test_clear_room_empty(settings, lobby_service): assert cache.keys(f"test-lobby_{room_id!s}_*") == [] -def test_clear_participant_cache(lobby_service): +def test_clear_participant_cache(lobby_service, settings): """Test clearing a specific participant entry from cache.""" room_id = uuid.uuid4() participant_id = "test-participant-id" - cache_key = f"{settings.LOBBY_KEY_PREFIX}_{room_id!s}_{participant_id}" + cache_key = f"{django_settings.LOBBY_KEY_PREFIX}_{room_id!s}_{participant_id}" participant_data = { "status": "waiting", "username": "test-username", @@ -986,7 +968,7 @@ def test_clear_participant_cache_nonexistent(lobby_service): room_id = uuid.uuid4() participant_id = "nonexistent-participant" - cache_key = f"{settings.LOBBY_KEY_PREFIX}_{room_id!s}_{participant_id}" + cache_key = f"{django_settings.LOBBY_KEY_PREFIX}_{room_id!s}_{participant_id}" assert cache.get(cache_key) is None lobby_service.clear_participant_cache(room_id, participant_id) @@ -994,7 +976,7 @@ def test_clear_participant_cache_nonexistent(lobby_service): assert cache.get(cache_key) is None -def test_index_add_members_remove_roundtrip(lobby_service): +def test_index_add_members_remove_roundtrip(lobby_service, settings): """The room index records, lists and forgets participant ids.""" room_id = uuid.uuid4() @@ -1023,9 +1005,10 @@ def test_enter_registers_participant_in_room_index( """Entering the lobby must index the participant id for the room.""" room_id = uuid.uuid4() - lobby_service.enter(room_id, participant_id, username) + participant = lobby_service._create_participant(username) + lobby_service._save_participant(room_id, participant) - assert lobby_service._index_members(room_id) == frozenset([participant_id]) + assert lobby_service._index_members(room_id) == frozenset([participant.id]) def test_list_waiting_participants_prunes_stale_index_ids(settings, lobby_service): @@ -1074,3 +1057,70 @@ def test_refresh_waiting_status_rearms_room_index_ttl(lobby_service, participant assert redis_client.ttl(index_key) > 10 assert lobby_service._index_members(room_id) == frozenset([participant_id]) + + +@pytest.mark.parametrize( + "cached_status", + [None, LobbyParticipantStatus.WAITING, LobbyParticipantStatus.ACCEPTED], +) +def test_bypass_clears_lobby_admission_before_room_becomes_restricted( + lobby_service, cached_status +): + """Bypass writes no admission; later restricted entry requires approval.""" + room = RoomFactory(access_level=RoomAccessLevel.PUBLIC) + user = AnonymousUser() + participant_id = None + if cached_status is not None: + participant_id = str(uuid.uuid4()) + participant = LobbyParticipant( + status=cached_status, + username="Guest", + id=participant_id, + color="#123456", + entered_at="2025-01-01T10:00:00+00:00", + ) + lobby_service._save_participant(room.id, participant) + + with ( + mock.patch.object( + utils, "generate_livekit_config", return_value={"token": "test"} + ) as generate_config, + mock.patch.object(lobby_service, "_notify_entry_request") as notify, + mock.patch.object( + lobby_service, "_save_participant", wraps=lobby_service._save_participant + ) as save, + mock.patch.object( + lobby_service, + "clear_participant_cache", + wraps=lobby_service.clear_participant_cache, + ) as clear, + ): + admitted, config = lobby_service.request_entry( + room, user, "Guest", participant_id=participant_id + ) + assert admitted.status == LobbyParticipantStatus.ACCEPTED + assert config == {"token": "test"} + assert lobby_service._get_participant(room.id, admitted.id) is None + assert admitted.id not in lobby_service._index_members(room.id) + notify.assert_not_called() + save.assert_not_called() + if cached_status is None: + clear.assert_not_called() + else: + clear.assert_called_once_with(room.id, participant_id) + + room.access_level = RoomAccessLevel.RESTRICTED + generate_config.reset_mock() + waiting, config = lobby_service.request_entry( + room, user, "Guest", participant_id=admitted.id + ) + + assert waiting.status == LobbyParticipantStatus.WAITING + assert config is None + generate_config.assert_not_called() + notify.assert_called_once_with(str(room.id)) + save.assert_called_once_with(room.id, waiting) + cached = lobby_service._get_participant(room.id, waiting.id) + assert cached is not None + assert cached.status == LobbyParticipantStatus.WAITING + assert waiting.id in lobby_service._index_members(room.id) diff --git a/src/backend/meet/settings.py b/src/backend/meet/settings.py index 1c081a779..ea285c5d2 100755 --- a/src/backend/meet/settings.py +++ b/src/backend/meet/settings.py @@ -986,11 +986,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 25a801972..fcbba3ae5 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 3e5f14bc8..2ccf79e8d 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 = 3_000 @@ -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 000000000..f21f4a9b0 --- /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]