From 0aa4f6389b6387569b1f9a43ab07cef8aea879e8 Mon Sep 17 00:00:00 2001 From: lebaudantoine Date: Tue, 4 Mar 2025 10:52:24 +0100 Subject: [PATCH] =?UTF-8?q?=E2=9C=A8(backend)=20add=20trusted=20user=20acc?= =?UTF-8?q?ess=20level=20for=20rooms?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Introduce new intermediate access level between public and restricted that allows authenticated users to join rooms without admin approval. Not making this the default level yet as current 12hr sessions would create painful user experience for accessing rooms. Will reconsider default settings after improving session management. This access level definition may evolve to become stricter in the future, potentially limiting access to authenticated users who share the same organization as the room admin. --- src/backend/core/api/serializers.py | 11 ++- .../0012_alter_room_access_level.py | 18 ++++ src/backend/core/models.py | 1 + src/backend/core/services/lobby_service.py | 22 ++++- .../core/tests/rooms/test_api_rooms_list.py | 2 + .../tests/rooms/test_api_rooms_retrieve.py | 65 +++++++++++++- .../core/tests/services/test_lobby_service.py | 84 +++++++++++++++++++ 7 files changed, 199 insertions(+), 4 deletions(-) create mode 100644 src/backend/core/migrations/0012_alter_room_access_level.py diff --git a/src/backend/core/api/serializers.py b/src/backend/core/api/serializers.py index b09079ba..d340c3ad 100644 --- a/src/backend/core/api/serializers.py +++ b/src/backend/core/api/serializers.py @@ -128,7 +128,16 @@ class RoomSerializer(serializers.ModelSerializer): if not is_admin: del output["configuration"] - if role is not None or instance.is_public: + should_access_room = ( + ( + instance.access_level == models.RoomAccessLevel.TRUSTED + and request.user.is_authenticated + ) + or role is not None + or instance.is_public + ) + + if should_access_room: room_id = f"{instance.id!s}" username = request.query_params.get("username", None) output["livekit"] = utils.generate_livekit_config( diff --git a/src/backend/core/migrations/0012_alter_room_access_level.py b/src/backend/core/migrations/0012_alter_room_access_level.py new file mode 100644 index 00000000..9fc04cfc --- /dev/null +++ b/src/backend/core/migrations/0012_alter_room_access_level.py @@ -0,0 +1,18 @@ +# Generated by Django 5.1.6 on 2025-03-04 09:51 + +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ('core', '0011_remove_resource_is_public_room_access_level'), + ] + + operations = [ + migrations.AlterField( + model_name='room', + name='access_level', + field=models.CharField(choices=[('public', 'Public Access'), ('trusted', 'Trusted Access'), ('restricted', 'Restricted Access')], default='public', max_length=50), + ) + ] diff --git a/src/backend/core/models.py b/src/backend/core/models.py index 17de9756..11baa28a 100644 --- a/src/backend/core/models.py +++ b/src/backend/core/models.py @@ -84,6 +84,7 @@ class RoomAccessLevel(models.TextChoices): """Room access level choices.""" PUBLIC = "public", _("Public Access") + TRUSTED = "trusted", _("Trusted Access") RESTRICTED = "restricted", _("Restricted Access") diff --git a/src/backend/core/services/lobby_service.py b/src/backend/core/services/lobby_service.py index 5d9f7470..47eabd49 100644 --- a/src/backend/core/services/lobby_service.py +++ b/src/backend/core/services/lobby_service.py @@ -14,7 +14,7 @@ from django.core.cache import cache from asgiref.sync import async_to_sync from livekit.api import LiveKitAPI, SendDataRequest, TwirpError # pylint: disable=E0611 -from core import utils +from core import models, utils logger = logging.getLogger(__name__) @@ -111,6 +111,24 @@ class LobbyService: samesite="Lax", ) + @staticmethod + def can_bypass_lobby(room, user) -> bool: + """Determines if a user can bypass the waiting lobby and join a room directly. + + A user can bypass the lobby if: + 1. The room is public (open to everyone) + 2. The room has TRUSTED access level and the user is authenticated + + Note: Room access levels can change while participants are waiting in the lobby. + This function only checks the current state and should be called each time + a participant requests entry to ensure consistent access control, even for + participants who have already begun waiting. + """ + return room.is_public or ( + room.access_level == models.RoomAccessLevel.TRUSTED + and user.is_authenticated + ) + def request_entry( self, room, @@ -133,7 +151,7 @@ class LobbyService: participant_id = self._get_or_create_participant_id(request) participant = self._get_participant(room.id, participant_id) - if room.is_public: + if self.can_bypass_lobby(room=room, user=request.user): if participant is None: participant = LobbyParticipant( status=LobbyParticipantStatus.ACCEPTED, diff --git a/src/backend/core/tests/rooms/test_api_rooms_list.py b/src/backend/core/tests/rooms/test_api_rooms_list.py index 6d600a30..2cd4c01f 100644 --- a/src/backend/core/tests/rooms/test_api_rooms_list.py +++ b/src/backend/core/tests/rooms/test_api_rooms_list.py @@ -17,6 +17,7 @@ pytestmark = pytest.mark.django_db def test_api_rooms_list_anonymous(): """Anonymous users should not be able to list rooms.""" RoomFactory(access_level=RoomAccessLevel.PUBLIC) + RoomFactory(access_level=RoomAccessLevel.TRUSTED) RoomFactory(access_level=RoomAccessLevel.RESTRICTED) client = APIClient() @@ -40,6 +41,7 @@ def test_api_rooms_list_authenticated(): other_user = UserFactory() RoomFactory(access_level=RoomAccessLevel.PUBLIC) + RoomFactory(access_level=RoomAccessLevel.TRUSTED) RoomFactory(access_level=RoomAccessLevel.RESTRICTED) room_user_accesses = RoomFactory( access_level=RoomAccessLevel.RESTRICTED, users=[user] diff --git a/src/backend/core/tests/rooms/test_api_rooms_retrieve.py b/src/backend/core/tests/rooms/test_api_rooms_retrieve.py index 7d3a8c28..308e65c2 100644 --- a/src/backend/core/tests/rooms/test_api_rooms_retrieve.py +++ b/src/backend/core/tests/rooms/test_api_rooms_retrieve.py @@ -36,6 +36,25 @@ def test_api_rooms_retrieve_anonymous_private_pk(): } +def test_api_rooms_retrieve_anonymous_trusted_pk(): + """ + Anonymous users should be allowed to retrieve a room that has a trusted access_level, + but should not be given any token. + """ + room = RoomFactory(access_level=RoomAccessLevel.TRUSTED) + client = APIClient() + response = client.get(f"/api/v1.0/rooms/{room.id!s}/") + + assert response.status_code == 200 + assert response.json() == { + "access_level": "trusted", + "id": str(room.id), + "is_administrable": False, + "name": room.name, + "slug": room.slug, + } + + def test_api_rooms_retrieve_anonymous_private_pk_no_dashes(): """It should be possible to get a room by its id stripped of its dashes.""" room = RoomFactory(access_level=RoomAccessLevel.RESTRICTED) @@ -172,7 +191,7 @@ def test_api_rooms_retrieve_anonymous_unregistered_not_allowed(): ) def test_api_rooms_retrieve_anonymous_public(mock_token): """ - Anonymous users should be able to retrieve a room with a token provided it is public. + Anonymous users should be able to retrieve a room with a token provided, if the room is public. """ room = RoomFactory(access_level=RoomAccessLevel.PUBLIC) client = APIClient() @@ -240,6 +259,50 @@ def test_api_rooms_retrieve_authenticated_public(mock_token): ) +@mock.patch("core.utils.generate_token", return_value="foo") +@override_settings( + LIVEKIT_CONFIGURATION={ + "api_key": "key", + "api_secret": "secret", + "url": "test_url_value", + } +) +def test_api_rooms_retrieve_authenticated_trusted(mock_token): + """ + Authenticated users should be allowed to retrieve a room and get a token for a room to + which they are not related, provided the room has a trusted access_level. + They should not see related users. + """ + room = RoomFactory(access_level=RoomAccessLevel.TRUSTED) + + user = UserFactory() + client = APIClient() + client.force_login(user) + + response = client.get( + f"/api/v1.0/rooms/{room.id!s}/", + ) + assert response.status_code == 200 + + expected_name = f"{room.id!s}" + assert response.json() == { + "access_level": str(room.access_level), + "id": str(room.id), + "is_administrable": False, + "livekit": { + "url": "test_url_value", + "room": expected_name, + "token": "foo", + }, + "name": room.name, + "slug": room.slug, + } + + mock_token.assert_called_once_with( + room=expected_name, user=user, username=None, color=None + ) + + def test_api_rooms_retrieve_authenticated(): """ Authenticated users should be allowed to retrieve a private room to which they diff --git a/src/backend/core/tests/services/test_lobby_service.py b/src/backend/core/tests/services/test_lobby_service.py index 1207c59a..6249e0ae 100644 --- a/src/backend/core/tests/services/test_lobby_service.py +++ b/src/backend/core/tests/services/test_lobby_service.py @@ -188,6 +188,54 @@ def test_prepare_response_new_cookie(lobby_service, participant_id): assert not cookie["max-age"] +def test_can_bypass_lobby_public_room(lobby_service): + """Should return True for public rooms regardless of user auth.""" + room = RoomFactory(access_level=RoomAccessLevel.PUBLIC) + + # Anonymous user + user = mock.Mock() + user.is_authenticated = False + assert lobby_service.can_bypass_lobby(room, user) is True + + # Authenticated user + user.is_authenticated = True + assert lobby_service.can_bypass_lobby(room, user) is True + + +def test_can_bypass_lobby_trusted_room_authenticated(lobby_service): + """Should return True for trusted rooms with authenticated users.""" + room = RoomFactory(access_level=RoomAccessLevel.TRUSTED) + + # Authenticated user + user = mock.Mock() + user.is_authenticated = True + assert lobby_service.can_bypass_lobby(room, user) is True + + +def test_can_bypass_lobby_trusted_room_anonymous(lobby_service): + """Should return False for trusted rooms with anonymous users.""" + room = RoomFactory(access_level=RoomAccessLevel.TRUSTED) + + # Anonymous user + user = mock.Mock() + user.is_authenticated = False + assert lobby_service.can_bypass_lobby(room, user) is False + + +def test_can_bypass_lobby_private_room(lobby_service): + """Should return False for private rooms regardless of user auth.""" + room = RoomFactory(access_level=RoomAccessLevel.RESTRICTED) + + # Anonymous user + user = mock.Mock() + user.is_authenticated = False + assert lobby_service.can_bypass_lobby(room, user) is False + + # Authenticated user + user.is_authenticated = True + assert lobby_service.can_bypass_lobby(room, user) is False + + @mock.patch("core.utils.generate_livekit_config") def test_request_entry_public_room( mock_generate_config, lobby_service, participant_id, username @@ -223,6 +271,42 @@ def test_request_entry_public_room( 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 +): + """Test requesting entry to a trusted room when the user is authenticated.""" + request = mock.Mock() + request.user = mock.Mock() + request.user.is_authenticated = True + + room = RoomFactory(access_level=RoomAccessLevel.TRUSTED) + + mocked_participant = LobbyParticipant( + status=LobbyParticipantStatus.UNKNOWN, + username=username, + 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) + + 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, + username=username, + color=participant.color, + ) + + lobby_service._get_participant.assert_called_once_with(room.id, participant_id) + + @mock.patch("core.services.lobby_service.LobbyService.enter") def test_request_entry_new_participant( mock_enter, lobby_service, participant_id, username