diff --git a/src/backend/core/api/serializers.py b/src/backend/core/api/serializers.py index 0e1b5323..d9420ee4 100644 --- a/src/backend/core/api/serializers.py +++ b/src/backend/core/api/serializers.py @@ -183,7 +183,7 @@ class RoomSerializer(serializers.ModelSerializer): user=request.user, username=username, configuration=output["configuration"], - is_admin_or_owner=is_admin_or_owner, + role=role, ) else: del output["pin_code"] diff --git a/src/backend/core/services/lobby.py b/src/backend/core/services/lobby.py index 90ee4512..8e61b97d 100644 --- a/src/backend/core/services/lobby.py +++ b/src/backend/core/services/lobby.py @@ -104,21 +104,30 @@ class LobbyService: ) @staticmethod - def can_bypass_lobby(room, user) -> bool: + def can_bypass_lobby(room, user, role) -> 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 + 2. The room has RESTRICTED access level and the user has any role 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 + return ( + room.is_public + or ( + room.access_level == models.RoomAccessLevel.TRUSTED + and user.is_authenticated + ) + or ( + room.access_level == models.RoomAccessLevel.RESTRICTED + and user.is_authenticated + and role is not None + ) ) def request_entry( @@ -144,8 +153,9 @@ class LobbyService: participant = self._get_participant(room.id, participant_id) room_id = str(room.id) + user_role = room.get_role(request.user) - if self.can_bypass_lobby(room=room, user=request.user): + if self.can_bypass_lobby(room=room, user=request.user, role=user_role): if participant is None: participant = LobbyParticipant( status=LobbyParticipantStatus.ACCEPTED, @@ -162,8 +172,8 @@ class LobbyService: username=username, color=participant.color, configuration=room.configuration, - is_admin_or_owner=False, participant_id=participant_id, + role=user_role, ) return participant, livekit_config @@ -183,8 +193,8 @@ class LobbyService: username=username, color=participant.color, configuration=room.configuration, - is_admin_or_owner=False, participant_id=participant_id, + role=user_role, ) return participant, livekit_config diff --git a/src/backend/core/services/room_roles.py b/src/backend/core/services/room_roles.py index f7d7a0fa..0cb10363 100644 --- a/src/backend/core/services/room_roles.py +++ b/src/backend/core/services/room_roles.py @@ -4,7 +4,7 @@ Single entry point for changing a user's role on a room, used by: - the in-meeting endpoint (promote/demote a connected participant) - (more to come soon) -`ResourceAccess` is the source of truth. The LiveKit `room_admin` +`ResourceAccess` is the source of truth. The LiveKit `room_role` participant attribute is only a projection of it, synced best-effort. """ @@ -138,7 +138,7 @@ class RoomRoleService: livekit_synced = self._sync_livekit_role( room_name=room_name, participant_identity=str(participant_identity), - is_admin=role == models.RoleChoices.ADMIN, + role=str(role), ) return { @@ -147,7 +147,7 @@ class RoomRoleService: } @staticmethod - def _sync_livekit_role(room_name: str, participant_identity: str, is_admin: bool): + def _sync_livekit_role(room_name: str, participant_identity: str, role: str): """Mirror the role to the participant's LiveKit attributes. Best-effort: returns False on failure instead of raising, so callers @@ -157,7 +157,7 @@ class RoomRoleService: ParticipantsManagement().update( room_name=room_name, identity=participant_identity, - attributes={"room_admin": "true" if is_admin else "false"}, + attributes={"room_role": role}, ) except ParticipantNotFoundException: # The participant left between the presence check and the update: 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 afd85d2f..1aa0a74f 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 @@ -80,7 +80,7 @@ def test_mute_participant_with_livekit_token_for_this_room(mock_livekit_client): room = RoomFactory() user = AnonymousUser() - token = utils.generate_token(str(room.id), user, is_admin_or_owner=False) + token = utils.generate_token(str(room.id), user) url = reverse("rooms-mute-participant", kwargs={"pk": room.id}) response = client.post( @@ -106,7 +106,7 @@ def test_mute_participant_with_livekit_token_for_another_room_forbidden( other_room = RoomFactory() user = AnonymousUser() - token = utils.generate_token(str(other_room.id), user, is_admin_or_owner=False) + token = utils.generate_token(str(other_room.id), user) url = reverse("rooms-mute-participant", kwargs={"pk": target_room.id}) response = client.post( @@ -146,7 +146,7 @@ def test_mute_participant_everyone_can_mute_disabled_blocks_non_admin( room = RoomFactory(configuration={"everyone_can_mute": False}) user = AnonymousUser() - token = utils.generate_token(str(room.id), user, is_admin_or_owner=False) + token = utils.generate_token(str(room.id), user) url = reverse("rooms-mute-participant", kwargs={"pk": room.id}) response = client.post( @@ -293,7 +293,7 @@ def test_mute_participant_admin_with_token_for_this_room(mock_livekit_client): ) # Token identity matches the admin user so LiveKitTokenAuthentication # resolves request.user back to the admin. - token = utils.generate_token(str(room.id), user, is_admin_or_owner=True) + token = utils.generate_token(str(room.id), user) url = reverse("rooms-mute-participant", kwargs={"pk": room.id}) response = client.post( @@ -323,7 +323,7 @@ def test_mute_participant_admin_with_token_for_another_room(mock_livekit_client) # Token is scoped to a DIFFERENT room, and admin status must only be # honored when established via session, never via a LiveKit # token, which can be replayed off-host. - token = utils.generate_token(str(other_room.id), user, is_admin_or_owner=True) + token = utils.generate_token(str(other_room.id), user) url = reverse("rooms-mute-participant", kwargs={"pk": target_room.id}) response = client.post( @@ -354,7 +354,7 @@ def test_mute_participant_admin_token_replayed_does_not_grant_admin( role=random.choice(["administrator", "owner"]), ) # The token is the only credential. - token = utils.generate_token(str(room.id), admin_user, is_admin_or_owner=True) + token = utils.generate_token(str(room.id), admin_user) url = reverse("rooms-mute-participant", kwargs={"pk": room.id}) response = client.post( @@ -374,7 +374,7 @@ def test_mute_participant_livekit_token_triggers_presence_check(mock_livekit_cli room = RoomFactory() user = AnonymousUser() - token = utils.generate_token(str(room.id), user, is_admin_or_owner=False) + token = utils.generate_token(str(room.id), user) url = reverse("rooms-mute-participant", kwargs={"pk": room.id}) response = client.post( @@ -405,7 +405,7 @@ def test_mute_participant_livekit_token_presence_check_returns_participant( ) user = AnonymousUser() - token = utils.generate_token(str(room.id), user, is_admin_or_owner=False) + token = utils.generate_token(str(room.id), user) url = reverse("rooms-mute-participant", kwargs={"pk": room.id}) response = client.post( @@ -433,7 +433,7 @@ def test_mute_participant_livekit_token_presence_check_participant_not_found( ) user = AnonymousUser() - token = utils.generate_token(str(room.id), user, is_admin_or_owner=False) + token = utils.generate_token(str(room.id), user) url = reverse("rooms-mute-participant", kwargs={"pk": room.id}) response = client.post( @@ -462,7 +462,7 @@ def test_mute_participant_livekit_token_presence_check_twirp_error_forbidden( ) user = AnonymousUser() - token = utils.generate_token(str(room.id), user, is_admin_or_owner=False) + token = utils.generate_token(str(room.id), user) url = reverse("rooms-mute-participant", kwargs={"pk": room.id}) response = client.post( 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 e8c3bfe0..d27b13bf 100644 --- a/src/backend/core/tests/rooms/test_api_rooms_retrieve.py +++ b/src/backend/core/tests/rooms/test_api_rooms_retrieve.py @@ -12,7 +12,7 @@ import pytest from rest_framework.test import APIClient from ...factories import RoomFactory, UserFactory, UserResourceAccessFactory -from ...models import RoomAccessLevel +from ...models import RoomAccessLevel, RoleChoices pytestmark = pytest.mark.django_db @@ -278,7 +278,7 @@ def test_api_rooms_retrieve_authenticated_public(mock_token): username=None, color=None, sources=["camera"], - is_admin_or_owner=False, + role=None, participant_id=None, ) @@ -330,7 +330,7 @@ def test_api_rooms_retrieve_authenticated_trusted(mock_token): username=None, color=None, sources=None, - is_admin_or_owner=False, + role=None, participant_id=None, ) @@ -418,7 +418,7 @@ def test_api_rooms_retrieve_members(mock_token, django_assert_num_queries, setti username=None, color=None, sources=["camera"], - is_admin_or_owner=False, + role=str(RoleChoices.MEMBER), participant_id=None, ) @@ -511,6 +511,6 @@ def test_api_rooms_retrieve_administrators( username=None, color=None, sources=None, - is_admin_or_owner=True, + role=str(user_access.role), participant_id=None, ) diff --git a/src/backend/core/tests/rooms/test_api_rooms_update_participant_role.py b/src/backend/core/tests/rooms/test_api_rooms_update_participant_role.py index 956fd5f3..6fd9e739 100644 --- a/src/backend/core/tests/rooms/test_api_rooms_update_participant_role.py +++ b/src/backend/core/tests/rooms/test_api_rooms_update_participant_role.py @@ -120,7 +120,7 @@ def test_update_participant_role_promotes_authenticated_target( mock_sync.assert_called_once_with( room_name=str(room.pk), participant_identity=str(target.sub), - is_admin=True, + role="administrator", ) diff --git a/src/backend/core/tests/services/test_lobby.py b/src/backend/core/tests/services/test_lobby.py index ce7746c7..8fc9728c 100644 --- a/src/backend/core/tests/services/test_lobby.py +++ b/src/backend/core/tests/services/test_lobby.py @@ -9,13 +9,14 @@ import uuid 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 -from core.factories import RoomFactory -from core.models import RoomAccessLevel +from core.factories import RoomFactory, UserFactory, UserResourceAccessFactory +from core.models import RoleChoices, RoomAccessLevel from core.services.lobby import ( LobbyParticipant, LobbyParticipantNotFound, @@ -188,17 +189,17 @@ def test_prepare_response_new_cookie(lobby_service, participant_id): def test_can_bypass_lobby_public_room(lobby_service): - """Should return True for public rooms regardless of user auth.""" + """Should return True for public rooms regardless of user auth and role.""" 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 + assert lobby_service.can_bypass_lobby(room, user, role=None) is True # Authenticated user user.is_authenticated = True - assert lobby_service.can_bypass_lobby(room, user) is True + assert lobby_service.can_bypass_lobby(room, user, role=None) is True def test_can_bypass_lobby_trusted_room_authenticated(lobby_service): @@ -208,7 +209,7 @@ def test_can_bypass_lobby_trusted_room_authenticated(lobby_service): # Authenticated user user = mock.Mock() user.is_authenticated = True - assert lobby_service.can_bypass_lobby(room, user) is True + assert lobby_service.can_bypass_lobby(room, user, role=None) is True def test_can_bypass_lobby_trusted_room_anonymous(lobby_service): @@ -218,21 +219,34 @@ def test_can_bypass_lobby_trusted_room_anonymous(lobby_service): # Anonymous user user = mock.Mock() user.is_authenticated = False - assert lobby_service.can_bypass_lobby(room, user) is False + assert lobby_service.can_bypass_lobby(room, user, role=None) is False def test_can_bypass_lobby_private_room(lobby_service): - """Should return False for private rooms regardless of user auth.""" + """Should return False for private rooms regardless of user auth if role is not.""" 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 + assert lobby_service.can_bypass_lobby(room, user, role=None) is False # Authenticated user user.is_authenticated = True - assert lobby_service.can_bypass_lobby(room, user) is False + assert lobby_service.can_bypass_lobby(room, user, role=None) is False + + +@pytest.mark.parametrize( + "role", + [RoleChoices.MEMBER, RoleChoices.ADMIN, RoleChoices.OWNER], +) +def test_can_bypass_lobby_private_room_with_any_role(role, lobby_service): + """Should return True for private rooms if the user is authenticated and has any role.""" + room = RoomFactory(access_level=RoomAccessLevel.RESTRICTED) + + user = mock.Mock() + user.is_authenticated = True + assert lobby_service.can_bypass_lobby(room, user, role=role) is True @mock.patch("core.utils.generate_livekit_config") @@ -241,7 +255,7 @@ def test_request_entry_public_room( ): """Test requesting entry to a public room.""" request = mock.Mock() - request.user = mock.Mock() + request.user = AnonymousUser() room = RoomFactory(access_level=RoomAccessLevel.PUBLIC) @@ -266,8 +280,8 @@ def test_request_entry_public_room( username=username, color=participant.color, configuration=room.configuration, - is_admin_or_owner=False, participant_id="test-participant-id", + role=None, ) lobby_service._get_participant.assert_called_once_with(room.id, participant_id) @@ -279,8 +293,7 @@ def test_request_entry_trusted_room( ): """Test requesting entry to a trusted room when the user is authenticated.""" request = mock.Mock() - request.user = mock.Mock() - request.user.is_authenticated = True + request.user = UserFactory() room = RoomFactory(access_level=RoomAccessLevel.TRUSTED) @@ -305,8 +318,8 @@ def test_request_entry_trusted_room( username=username, color=participant.color, configuration=room.configuration, - is_admin_or_owner=False, participant_id="test-participant-id", + role=None, ) lobby_service._get_participant.assert_called_once_with(room.id, participant_id) @@ -319,6 +332,7 @@ def test_request_entry_new_participant( """Test requesting entry for a new participant.""" request = mock.Mock() request.COOKIES = {settings.LOBBY_COOKIE_NAME: participant_id} + request.user = AnonymousUser() room = RoomFactory(access_level=RoomAccessLevel.RESTRICTED) @@ -348,6 +362,7 @@ 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) @@ -374,7 +389,7 @@ def test_request_entry_accepted_participant( ): """Test requesting entry for an accepted participant.""" request = mock.Mock() - request.user = mock.Mock() + request.user = AnonymousUser() request.COOKIES = {settings.LOBBY_COOKIE_NAME: participant_id} room = RoomFactory(access_level=RoomAccessLevel.RESTRICTED) @@ -400,8 +415,48 @@ def test_request_entry_accepted_participant( username=username, color="#123456", configuration=room.configuration, - is_admin_or_owner=False, 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_participant_with_role( + mock_generate_config, lobby_service, participant_id, username +): + """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) + + UserResourceAccessFactory(resource=room, user=request.user, role="administrator") + + mocked_participant = LobbyParticipant( + status=LobbyParticipantStatus.ACCEPTED, + 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="#123456", + configuration=room.configuration, + participant_id="test-participant-id", + role="administrator", ) lobby_service._get_participant.assert_called_once_with(room.id, participant_id) diff --git a/src/backend/core/utils.py b/src/backend/core/utils.py index a9a7525c..a73e86f5 100644 --- a/src/backend/core/utils.py +++ b/src/backend/core/utils.py @@ -65,7 +65,7 @@ def generate_token( username: Optional[str] = None, color: Optional[str] = None, sources: Optional[List[str]] = None, - is_admin_or_owner: bool = False, + role: Optional[str] = None, participant_id: Optional[str] = None, ) -> str: """Generate a LiveKit access token for a user in a specific room. @@ -79,7 +79,7 @@ def generate_token( If none, a value will be generated sources: (Optional[List[str]]): List of media sources the user can publish If none, defaults to LIVEKIT_DEFAULT_SOURCES. - is_admin_or_owner (bool): Whether user has admin privileges + role (Optional[str]): Room's access role if any participant_id (Optional[str]): Stable identifier for anonymous users; used as identity when user.is_anonymous. @@ -87,6 +87,7 @@ def generate_token( str: The LiveKit JWT access token. """ + is_admin_or_owner = role in ("owner", "administrator") if is_admin_or_owner: sources = settings.LIVEKIT_DEFAULT_SOURCES @@ -127,7 +128,7 @@ def generate_token( .with_identity(identity) .with_name(display_name) .with_attributes( - {"color": color, "room_admin": "true" if is_admin_or_owner else "false"} + {"color": color, "room_role": role } ) ) @@ -138,7 +139,7 @@ def generate_livekit_config( room_id: str, user, username: str, - is_admin_or_owner: bool, + role: Optional[str] = None, color: Optional[str] = None, configuration: Optional[dict] = None, participant_id: Optional[str] = None, @@ -149,7 +150,7 @@ def generate_livekit_config( room_id: Room identifier user: User instance requesting access username: Display name in room - is_admin_or_owner (bool): Whether the user has admin/owner privileges for this room. + role (str): Room's access role if any color (Optional[str]): Optional color to associate with the participant. configuration (Optional[dict]): Room configuration dict that can override default settings. participant_id (Optional[str]): Stable identifier for anonymous users; @@ -172,7 +173,7 @@ def generate_livekit_config( username=username, color=color, sources=sources, - is_admin_or_owner=is_admin_or_owner, + role=role, participant_id=participant_id, ), } diff --git a/src/frontend/src/features/participants/components/ParticipantRow.tsx b/src/frontend/src/features/participants/components/ParticipantRow.tsx index 78233a47..a5ddd2a5 100644 --- a/src/frontend/src/features/participants/components/ParticipantRow.tsx +++ b/src/frontend/src/features/participants/components/ParticipantRow.tsx @@ -5,7 +5,7 @@ import { Text } from '@/primitives/Text' import { useTranslation } from 'react-i18next' import { Avatar } from '@/components/Avatar' import { getParticipantColor } from '@/features/rooms/utils/getParticipantColor' -import { getParticipantIsRoomAdmin } from '@/features/rooms/utils/getParticipantIsRoomAdmin' +import { getParticipantIsRoomOwner } from '@/features/rooms/utils/getParticipantIsRoomAdminOrOwner' import { type LocalParticipant, type Participant, Track } from 'livekit-client' import { isLocal } from '@/utils/livekit' import { @@ -146,7 +146,7 @@ export const ParticipantRow = ({ participant }: ParticipantListItemProps) => { )} - {getParticipantIsRoomAdmin(participant) && ( + {getParticipantIsRoomOwner(participant) && ( {t('participants.host')} )} diff --git a/src/frontend/src/features/rooms/api/ApiRoom.ts b/src/frontend/src/features/rooms/api/ApiRoom.ts index 5fb201a2..4e00bc3b 100644 --- a/src/frontend/src/features/rooms/api/ApiRoom.ts +++ b/src/frontend/src/features/rooms/api/ApiRoom.ts @@ -28,3 +28,5 @@ export type ApiRoom = { livekit?: ApiLiveKit configuration?: RoomConfiguration } + +export type ParticipantRole = 'member' | 'administrator' | 'owner' diff --git a/src/frontend/src/features/rooms/livekit/hooks/usePublishSourcesManager.ts b/src/frontend/src/features/rooms/livekit/hooks/usePublishSourcesManager.ts index b9b26e85..f5bd1666 100644 --- a/src/frontend/src/features/rooms/livekit/hooks/usePublishSourcesManager.ts +++ b/src/frontend/src/features/rooms/livekit/hooks/usePublishSourcesManager.ts @@ -8,7 +8,7 @@ import { useRemoteParticipants } from '@livekit/components-react' import { useUpdateParticipantsPermissions } from '@/features/rooms/api/updateParticipantsPermissions' import { useRoomData } from '@/features/rooms/livekit/hooks/useRoomData' import { isSubsetOf } from '@/features/rooms/utils/isSubsetOf' -import { getParticipantIsRoomAdmin } from '@/features/rooms/utils/getParticipantIsRoomAdmin' +import { getParticipantIsRoomAdminOrOwner } from '@/features/rooms/utils/getParticipantIsRoomAdminOrOwner' import Source = Track.Source import { NotificationType, @@ -48,7 +48,7 @@ export const usePublishSourcesManager = () => { }) const unprivilegedRemoteParticipants = remoteParticipants.filter( - (participant) => !getParticipantIsRoomAdmin(participant) + (participant) => !getParticipantIsRoomAdminOrOwner(participant) ) const currentSources = useMemo(() => { diff --git a/src/frontend/src/features/rooms/utils/getParticipantIsRoomAdmin.ts b/src/frontend/src/features/rooms/utils/getParticipantIsRoomAdmin.ts deleted file mode 100644 index e4afae46..00000000 --- a/src/frontend/src/features/rooms/utils/getParticipantIsRoomAdmin.ts +++ /dev/null @@ -1,12 +0,0 @@ -import type { Participant } from 'livekit-client' - -export const getParticipantIsRoomAdmin = ( - participant: Participant -): boolean => { - const attributes = participant.attributes - - if (!attributes) { - return false - } - return attributes?.room_admin === 'true' -} diff --git a/src/frontend/src/features/rooms/utils/getParticipantIsRoomAdminOrOwner.ts b/src/frontend/src/features/rooms/utils/getParticipantIsRoomAdminOrOwner.ts new file mode 100644 index 00000000..e843a4ae --- /dev/null +++ b/src/frontend/src/features/rooms/utils/getParticipantIsRoomAdminOrOwner.ts @@ -0,0 +1,20 @@ +import type { Participant } from 'livekit-client' +import { ParticipantRole } from '@/features/rooms/api/ApiRoom' + +const participantHasRoomRole = ( + participant: Participant, + roles: ParticipantRole[] +): boolean => { + const role = participant.attributes?.room_role + return role !== undefined && roles.includes(role as ParticipantRole) +} + +export const getParticipantIsRoomAdmin = (participant: Participant): boolean => + participantHasRoomRole(participant, ['administrator']) + +export const getParticipantIsRoomOwner = (participant: Participant): boolean => + participantHasRoomRole(participant, ['owner']) + +export const getParticipantIsRoomAdminOrOwner = ( + participant: Participant +): boolean => participantHasRoomRole(participant, ['administrator', 'owner'])