diff --git a/CHANGELOG.md b/CHANGELOG.md index 16bf673e7..be3df557b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -13,6 +13,7 @@ and this project adheres to - ✨(frontend) let users pick the recording encoding - ✨(backend) introduce a token exchange endpoint for iframe embeds - ⚡️(backend) send recording notification emails from a Celery task +- ✨(backend) add room soft-deletion to the external API ### Changed diff --git a/UPGRADE.md b/UPGRADE.md index 5099b4965..3d8289020 100644 --- a/UPGRADE.md +++ b/UPGRADE.md @@ -16,6 +16,14 @@ the following command inside your docker container: ## [Unreleased] +### Purging deleted rooms + +Deleting a room now only soft deletes it: it is hidden but keeps its slug and PIN code. The `purge_inactive_rooms` command also permanently deletes soft-deleted rooms. See [the room purge documentation](docs/features/room-purge.md). + +- Inactive soft-deleted rooms are purged along with the other inactive rooms: if `ROOM_INACTIVITY_DELETION_DAYS` is already set, nothing else is needed. +- The new `ROOM_DELETED_RETENTION_DAYS` setting purges soft-deleted rooms that many days after their deletion, whether they are inactive or not. It is unset by default. +- As for inactive rooms, a deleted room holding a saved recording its users may still access is kept until that recording expires. + ### Marketing / Brevo integration now uses `django-lasuite` The in-house marketing service (`core.services.marketing`) has been removed and diff --git a/src/backend/core/admin.py b/src/backend/core/admin.py index 30138e85e..73315a212 100644 --- a/src/backend/core/admin.py +++ b/src/backend/core/admin.py @@ -286,9 +286,16 @@ class RoomAdmin(admin.ModelAdmin): "get_owner", "created_at", "last_started_at", + "deleted_at", + ] + list_filter = ["access_level", "created_at", "last_started_at", "deleted_at"] + readonly_fields = [ + "id", + "created_at", + "updated_at", + "last_started_at", + "deleted_at", ] - list_filter = ["access_level", "created_at", "last_started_at"] - readonly_fields = ["id", "created_at", "updated_at", "last_started_at"] def get_queryset(self, request): """Optimize queries by prefetching related access and user data to avoid N+1 queries.""" diff --git a/src/backend/core/analytics/events.py b/src/backend/core/analytics/events.py index c7c1bc412..fdef4aa77 100644 --- a/src/backend/core/analytics/events.py +++ b/src/backend/core/analytics/events.py @@ -9,6 +9,7 @@ class AnalyticsEvent(StrEnum): # Rooms ROOM_CREATED = "room_created" ROOM_UPDATED = "room_updated" + ROOM_DELETED = "room_deleted" # Roomkit (meeting-room SIP devices) ROOMKIT_JOINED = "roomkit_joined" diff --git a/src/backend/core/api/exceptions.py b/src/backend/core/api/exceptions.py new file mode 100644 index 000000000..76833fc27 --- /dev/null +++ b/src/backend/core/api/exceptions.py @@ -0,0 +1,30 @@ +"""Exceptions and guards shared by the API endpoints.""" + +from django.utils.translation import gettext_lazy as _ + +from rest_framework import exceptions, status + +from core import models + + +class RoomSoftDeleted(exceptions.APIException): + """Raised when the requested room has been soft deleted.""" + + status_code = status.HTTP_410_GONE + default_detail = _("This room has been deleted.") + default_code = "room_deleted" + + +def ensure_room_not_deleted(resource): + """Raise a 410 Gone if the resource is a soft-deleted room. + + Accepts a room or its parent resource, as referenced by accesses. Call it + after permissions are checked, to avoid revealing room. + """ + if isinstance(resource, models.Room): + room = resource + else: + room = getattr(resource, "room", None) + + if room is not None and room.is_deleted: + raise RoomSoftDeleted() diff --git a/src/backend/core/api/serializers.py b/src/backend/core/api/serializers.py index fb52dc9e3..8bbf89b82 100644 --- a/src/backend/core/api/serializers.py +++ b/src/backend/core/api/serializers.py @@ -26,6 +26,7 @@ from rest_framework.exceptions import PermissionDenied from timezone_field.rest_framework import TimeZoneSerializerField from core import models, utils +from core.api.exceptions import ensure_room_not_deleted logger = logging.getLogger(__name__) @@ -114,6 +115,7 @@ class ResourceAccessSerializerMixin: _("You must be administrator or owner of a room to add accesses to it.") ) + ensure_room_not_deleted(resource) return resource diff --git a/src/backend/core/api/viewsets.py b/src/backend/core/api/viewsets.py index b0ec01522..2897b88eb 100644 --- a/src/backend/core/api/viewsets.py +++ b/src/backend/core/api/viewsets.py @@ -42,6 +42,7 @@ from rest_framework.settings import api_settings from core import analytics, enums, models, utils from core.api import throttling +from core.api.exceptions import ensure_room_not_deleted from core.api.filters import ListFileFilter from core.enums import MEDIA_STORAGE_URL_PATTERN from core.recording.enums import FileExtension @@ -77,7 +78,7 @@ from core.services.participants_management import ( ParticipantsManagementException, ) from core.services.room_creation import RoomCreation -from core.services.room_management import RoomManagement +from core.services.room_management import RoomManagement, RoomManagementException from core.services.room_roles import ( RoomRoleError, RoomRoleService, @@ -261,7 +262,7 @@ class UserViewSet( return drf_response.Response(data) -class RoomViewSet( +class RoomViewSet( # pylint: disable=too-many-public-methods mixins.CreateModelMixin, mixins.DestroyModelMixin, mixins.UpdateModelMixin, @@ -287,10 +288,9 @@ class RoomViewSet( filter_kwargs = {"pk": self.kwargs["pk"]} except ValueError: filter_kwargs = {"slug": slugify(self.kwargs["pk"])} - queryset = self.filter_queryset(self.get_queryset()) - obj = get_object_or_404(queryset, **filter_kwargs) - # May raise a permission denied + obj = get_object_or_404(models.Room.all_objects, **filter_kwargs) self.check_object_permissions(self.request, obj) + ensure_room_not_deleted(obj) return obj def retrieve(self, request, *args, **kwargs): @@ -342,6 +342,27 @@ class RoomViewSet( serializer = self.get_serializer(queryset, many=True) return drf_response.Response(serializer.data) + def perform_destroy(self, instance): + """Soft delete the room and close its LiveKit room. + + The room and its recordings are kept in database for traceability. + """ + try: + RoomManagement.soft_delete(instance) + except RoomManagementException as e: + raise drf_exceptions.APIException( + "Could not delete the room, please try again." + ) from e + + analytics.capture( + self.request.user, + analytics.AnalyticsEvent.ROOM_DELETED, + { + "room_id": str(instance.pk), + "access_level": instance.access_level, + }, + ) + def perform_create(self, serializer): """Set the current user as owner of the newly created room. @@ -1039,6 +1060,13 @@ class ResourceAccessViewSet( return queryset + def get_object(self): + """Accesses to a soft-deleted room can be read but no longer modified.""" + access = super().get_object() + if self.request.method not in drf_permissions.SAFE_METHODS: + ensure_room_not_deleted(access.resource) + return access + class RecordingViewSet( mixins.DestroyModelMixin, diff --git a/src/backend/core/external_api/viewsets.py b/src/backend/core/external_api/viewsets.py index aaa5d2956..e528d5538 100644 --- a/src/backend/core/external_api/viewsets.py +++ b/src/backend/core/external_api/viewsets.py @@ -21,11 +21,13 @@ from rest_framework import ( from rest_framework import ( status as drf_status, ) +from rest_framework.generics import get_object_or_404 from core import analytics, api, models +from core.api.exceptions import ensure_room_not_deleted from core.api.feature_flag import FeatureFlag from core.services.jwt_token import JwtTokenService -from core.services.room_management import RoomManagement +from core.services.room_management import RoomManagement, RoomManagementException from core.services.transit_code import TransitCodeService from ..services.provisional_user_service import ( @@ -142,6 +144,7 @@ class ApplicationViewSet(viewsets.ViewSet): class RoomViewSet( mixins.CreateModelMixin, + mixins.DestroyModelMixin, mixins.RetrieveModelMixin, mixins.ListModelMixin, mixins.UpdateModelMixin, @@ -159,9 +162,13 @@ class RoomViewSet( - create: Create a new room owned by the user (requires 'rooms:create' scope) - partial_update: Update a room's access level and configuration, for administrators and owners only (requires 'rooms:update' scope) + - destroy: Soft delete a room and close its LiveKit room, for owners only + (requires 'rooms:delete' scope) + + Detail operations on a soft-deleted room answer 410 Gone. """ - http_method_names = ["get", "post", "patch", "head", "options"] + http_method_names = ["get", "post", "patch", "delete", "head", "options"] authentication_classes = [ authentication.ApplicationJWTAuthentication, @@ -176,6 +183,17 @@ class RoomViewSet( queryset = models.Room.objects.all() serializer_class = serializers.RoomSerializer + def get_object(self): + """Get the room, answer 410 if it has been deleted. + + Permissions are checked first so a deleted room is only revealed to + users who would have been granted access to it. + """ + room = get_object_or_404(models.Room.all_objects, pk=self.kwargs["pk"]) + self.check_object_permissions(self.request, room) + ensure_room_not_deleted(room) + return room + def list(self, request, *args, **kwargs): """Limit listed rooms to the ones related to the authenticated user.""" @@ -239,6 +257,16 @@ class RoomViewSet( self._track_room_event(room, analytics.AnalyticsEvent.ROOM_CREATED) + def perform_destroy(self, instance): + """Soft delete the room, close its LiveKit room, then log and track it.""" + try: + RoomManagement.soft_delete(instance) + except RoomManagementException as e: + raise drf_exceptions.APIException( + "Could not delete the room, please try again." + ) from e + self._track_room_event(instance, analytics.AnalyticsEvent.ROOM_DELETED) + def perform_update(self, serializer: serializers.RoomSerializer): """Persist the room update, sync it to LiveKit, then log and track it.""" diff --git a/src/backend/core/migrations/0027_room_deleted_at.py b/src/backend/core/migrations/0027_room_deleted_at.py new file mode 100644 index 000000000..95e2f38e5 --- /dev/null +++ b/src/backend/core/migrations/0027_room_deleted_at.py @@ -0,0 +1,29 @@ +# Generated by Django 5.2.14 on 2026-09-16 10:00 + +import django.db.models.manager +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ('core', '0026_alter_application_scopes'), + ] + + operations = [ + migrations.AddField( + model_name='room', + name='deleted_at', + field=models.DateTimeField(blank=True, null=True), + ), + migrations.AlterModelOptions( + name='room', + options={'default_manager_name': 'all_objects', 'ordering': ('name',), 'verbose_name': 'Room', 'verbose_name_plural': 'Rooms'}, + ), + migrations.AlterModelManagers( + name='room', + managers=[ + ('all_objects', django.db.models.manager.Manager()), + ], + ), + ] diff --git a/src/backend/core/models.py b/src/backend/core/models.py index bcf6092b7..0ae5c2909 100644 --- a/src/backend/core/models.py +++ b/src/backend/core/models.py @@ -407,6 +407,33 @@ class ResourceAccess(BaseModel): return super().delete(*args, **kwargs) +class RoomQuerySet(models.QuerySet): + """QuerySet exposing the room lifecycle filters.""" + + def active(self): + """Rooms that have not been soft deleted.""" + return self.filter(deleted_at__isnull=True) + + def deleted(self): + """Rooms that have been soft deleted.""" + return self.filter(deleted_at__isnull=False) + + +class RoomManager(models.Manager.from_queryset(RoomQuerySet)): + """Manager hiding soft-deleted rooms, exposed as ``Room.objects``. + + It is deliberately not the model's default manager: Django relies on + ``_default_manager`` for unique validation, which must see deleted rooms as + they still guard their slug and pin code. Other object relations (e.g. + ``recording.room``) go through the base manager and still resolve deleted + rooms, which keeps recordings and their notifications working. + """ + + def get_queryset(self): + """Exclude soft-deleted rooms.""" + return super().get_queryset().active() + + class Room(Resource): """Model for one room""" @@ -430,6 +457,7 @@ class Room(Resource): verbose_name=_("Visio room configuration"), help_text=_("Values for Visio parameters to configure the room."), ) + deleted_at = models.DateTimeField(null=True, blank=True) pin_code = models.CharField( max_length=None, unique=True, @@ -446,8 +474,13 @@ class Room(Resource): editable=False, ) + # Managers + objects = RoomManager() + all_objects = models.Manager.from_queryset(RoomQuerySet)() + class Meta: db_table = "meet_room" + default_manager_name = "all_objects" ordering = ("name",) verbose_name = _("Room") verbose_name_plural = _("Rooms") @@ -470,6 +503,23 @@ class Room(Resource): ) super().save(*args, **kwargs) + @property + def is_deleted(self): + """Whether the room has been soft deleted.""" + return self.deleted_at is not None + + def soft_delete(self): + """Soft delete the room. + + The room is hidden from the default manager making it impossible to + list, join or update. + """ + if self.deleted_at: + raise RuntimeError("This room is already deleted.") + + self.deleted_at = timezone.now() + self.save(update_fields=["deleted_at"]) + def clean_fields(self, exclude=None): """ Automatically generate the slug from the name and make sure it does not look like a UUID. @@ -505,7 +555,7 @@ class Room(Resource): for _ in range(settings.ROOM_TELEPHONY_PIN_MAX_RETRIES): pin_code = str(secrets.randbelow(max_value)).zfill(length) - if not Room.objects.filter(pin_code=pin_code).exists(): + if not Room.all_objects.filter(pin_code=pin_code).exists(): return pin_code # Log a warning as a temporary measure until backend observability is implemented. diff --git a/src/backend/core/services/livekit_events.py b/src/backend/core/services/livekit_events.py index 167b7b4b1..638f1c4f8 100644 --- a/src/backend/core/services/livekit_events.py +++ b/src/backend/core/services/livekit_events.py @@ -271,20 +271,24 @@ class LiveKitEventsService: ) raise ActionFailedError("Failed to process room started event") from e - room_updated_count = models.Room.objects.filter(pk=room_id).update( + try: + room = models.Room.all_objects.get(id=room_id) + except models.Room.DoesNotExist as err: + raise ActionFailedError(f"Room with ID {room_id} does not exist") from err + + # The block below is intended to fix the issue where long-lived livekit + # tokens allow users who already entered a room to re-create it, + # even if it was closed. + if room.is_deleted: + self._close_deleted_room(room_id) + return + + # Update through the queryset to skip the full_clean run by save() + models.Room.all_objects.filter(pk=room.pk).update( last_started_at=timezone.now() ) - if not room_updated_count: - raise ActionFailedError(f"Room with ID {room_id} does not exist") if settings.ROOM_TELEPHONY_ENABLED or settings.ROOMKIT_ENABLED: - try: - room = models.Room.objects.get(pk=room_id) - except models.Room.DoesNotExist as err: - raise ActionFailedError( - f"Room with ID {room_id} does not exist" - ) from err - try: self.sip_management.ensure_dispatch_rule(room) except SIPException as e: @@ -292,6 +296,25 @@ class LiveKitEventsService: f"Failed to create sip dispatch rule for room {room_id}" ) from e + @staticmethod + def _close_deleted_room(room_id): + """Close a LiveKit room recreated after its room was soft deleted. + + LiveKit auto-creates a room on join, so a participant still holding a + valid token can bring a deleted room back to life until the token expires. + """ + + logger.warning( + "LiveKit room %s started for a deleted room, closing it", room_id + ) + + try: + RoomManagement.delete_room(str(room_id)) + except RoomNotFoundException: + logger.info("LiveKit room %s is already closed", room_id) + except RoomManagementException as e: + raise ActionFailedError(f"Failed to close deleted room {room_id}") from e + def _handle_room_finished(self, data): """Handle 'room_finished' event.""" diff --git a/src/backend/core/services/room_management.py b/src/backend/core/services/room_management.py index 6dfe8b035..0ce7043a3 100644 --- a/src/backend/core/services/room_management.py +++ b/src/backend/core/services/room_management.py @@ -2,10 +2,13 @@ # pylint: disable=no-name-in-module +import contextlib import json from logging import getLogger from typing import Dict, Optional +from django.conf import settings + from asgiref.sync import async_to_sync from livekit.api import ( DeleteRoomRequest, @@ -15,6 +18,9 @@ from livekit.api import ( ) from core import utils +from core.models import Room + +from .sip_management import SIPException, SIPManagement logger = getLogger(__name__) @@ -115,6 +121,38 @@ class RoomManagement: finally: await lkapi.aclose() + @classmethod + def soft_delete(cls, room: Room): + """Soft delete a room, remove its SIP routing and close its LiveKit room.""" + + room.soft_delete() + + try: + cls._delete_dispatch_rule(room) + + with contextlib.suppress(RoomNotFoundException): + cls.delete_room(str(room.id)) + except Exception: + Room.all_objects.filter(pk=room.pk, deleted_at=room.deleted_at).update( + deleted_at=None + ) + room.deleted_at = None + raise + + @staticmethod + def _delete_dispatch_rule(room: Room): + """Delete a room's SIP dispatch rule, so its PIN no longer routes calls. + + This cannot be left to the room_finished webhook: a roomkit join creates + the rule before any LiveKit room exists, so no room may ever finish. + """ + + if settings.ROOM_TELEPHONY_ENABLED or settings.ROOMKIT_ENABLED: + try: + SIPManagement().delete_dispatch_rule(room.id) + except SIPException as e: + raise RoomManagementException("Could not delete dispatch rule") from e + @classmethod def sync_room_metadata(cls, room): """Push a room's configuration and access level to its LiveKit room metadata. diff --git a/src/backend/core/services/sip_management.py b/src/backend/core/services/sip_management.py index 0c934b8c1..dde9730f2 100644 --- a/src/backend/core/services/sip_management.py +++ b/src/backend/core/services/sip_management.py @@ -124,7 +124,11 @@ class SIPManagement: @async_to_sync async def delete_dispatch_rule(self, room_id): - """Delete all SIP inbound dispatch rules associated with a specific room.""" + """Delete all SIP inbound dispatch rules associated with a specific room. + + A rule deleted meanwhile (e.g. by both a room deletion and its + room_finished webhook) is not an error. + """ rules_ids = await self._list_dispatch_rules_ids(room_id) @@ -138,9 +142,16 @@ class SIPManagement: lkapi = utils.create_livekit_client() try: for rule_id in rules_ids: - await lkapi.sip.delete_sip_dispatch_rule( - delete=DeleteSIPDispatchRuleRequest(sip_dispatch_rule_id=rule_id) - ) + try: + await lkapi.sip.delete_sip_dispatch_rule( + delete=DeleteSIPDispatchRuleRequest( + sip_dispatch_rule_id=rule_id + ) + ) + except TwirpError as e: + if e.code != TwirpErrorCode.NOT_FOUND: + raise + logger.info("Dispatch rule %s was already deleted", rule_id) return True diff --git a/src/backend/core/tests/rooms/test_api_rooms_create.py b/src/backend/core/tests/rooms/test_api_rooms_create.py index 745a638f0..51ac78d2f 100644 --- a/src/backend/core/tests/rooms/test_api_rooms_create.py +++ b/src/backend/core/tests/rooms/test_api_rooms_create.py @@ -117,6 +117,29 @@ def test_api_rooms_create_authenticated_existing_slug(): assert response.json() == {"slug": ["Room with this Slug already exists."]} +def test_api_rooms_create_authenticated_slug_held_by_soft_deleted_room(): + """ + A deleted room keeps its slug: creating a room with the same name should + fail validation rather than hit the database constraint. + """ + RoomFactory(name="my room").soft_delete() + user = UserFactory() + + client = APIClient() + client.force_login(user) + + response = client.post( + "/api/v1.0/rooms/", + { + "name": "My Room!", + }, + ) + + assert response.status_code == 400 + assert response.json() == {"slug": ["Room with this Slug already exists."]} + assert Room.all_objects.count() == 1 + + def test_api_rooms_create_authenticated_user_default_access_level(): """ The user's default room access level should be applied to the new room diff --git a/src/backend/core/tests/rooms/test_api_rooms_delete.py b/src/backend/core/tests/rooms/test_api_rooms_delete.py index a7bb53d55..aea1ff4aa 100644 --- a/src/backend/core/tests/rooms/test_api_rooms_delete.py +++ b/src/backend/core/tests/rooms/test_api_rooms_delete.py @@ -2,11 +2,16 @@ Test rooms API endpoints in the Meet core app: delete. """ +from unittest import mock + import pytest from rest_framework.test import APIClient +from ...analytics import AnalyticsEvent from ...factories import RoomFactory, UserFactory -from ...models import Room +from ...models import Room, RoomAccessLevel +from ...services.room_management import RoomManagement, RoomNotFoundException +from ...services.sip_management import SIPManagement pytestmark = pytest.mark.django_db @@ -83,12 +88,52 @@ def test_api_rooms_delete_administrators(): assert Room.objects.count() == 1 -def test_api_rooms_delete_owners(): +@mock.patch("core.api.viewsets.analytics.capture") +@mock.patch.object(SIPManagement, "delete_dispatch_rule") +@mock.patch.object(RoomManagement, "delete_room") +def test_api_rooms_delete_owners(mock_delete_room, _, mock_capture): """ Authenticated users should be able to delete a room for which they are directly - owner. + owner. The room is soft deleted, its LiveKit room is closed and a ROOM_DELETED + analytics event is emitted. """ user = UserFactory() + room = RoomFactory(users=[(user, "owner")], access_level=RoomAccessLevel.TRUSTED) + + client = APIClient() + client.force_login(user) + + response = client.delete( + f"/api/v1.0/rooms/{room.id}/", + ) + + assert response.status_code == 204 + mock_delete_room.assert_called_once_with(str(room.id)) + assert Room.objects.exists() is False + assert Room.all_objects.get(id=room.id).deleted_at is not None + + mock_capture.assert_called_once_with( + user, + AnalyticsEvent.ROOM_DELETED, + {"room_id": str(room.pk), "access_level": RoomAccessLevel.TRUSTED}, + ) + + +@mock.patch.object(SIPManagement, "delete_dispatch_rule") +@mock.patch.object( + RoomManagement, + "delete_room", + side_effect=RoomNotFoundException("Room does not exist"), +) +def test_api_rooms_delete_owners_room_not_live( + mock_delete_room, mock_delete_dispatch_rule, settings +): + """ + Deleting a room that is not live in LiveKit should still soft delete it, and + delete its SIP dispatch rule, as no room_finished webhook will. + """ + settings.ROOM_TELEPHONY_ENABLED = True + user = UserFactory() room = RoomFactory(users=[(user, "owner")]) client = APIClient() @@ -99,4 +144,47 @@ def test_api_rooms_delete_owners(): ) assert response.status_code == 204 + mock_delete_room.assert_called_once_with(str(room.id)) + mock_delete_dispatch_rule.assert_called_once_with(room.id) assert Room.objects.exists() is False + assert Room.all_objects.get(id=room.id).deleted_at is not None + + +@mock.patch.object(RoomManagement, "delete_room") +def test_api_rooms_delete_soft_deleted(mock_delete_room): + """Deleting a room that is already soft deleted should return a 410.""" + user = UserFactory() + room = RoomFactory(users=[(user, "owner")]) + room.soft_delete() + + client = APIClient() + client.force_login(user) + + response = client.delete( + f"/api/v1.0/rooms/{room.id}/", + ) + + assert response.status_code == 410 + assert response.json() == {"detail": "This room has been deleted."} + mock_delete_room.assert_not_called() + + +@mock.patch.object(RoomManagement, "delete_room") +def test_api_rooms_delete_soft_deleted_not_owner(mock_delete_room): + """ + Deleting a soft-deleted room as a non-owner should return a 403, + not revealing that the room has been deleted. + """ + user = UserFactory() + room = RoomFactory(users=[(user, "administrator")]) + room.soft_delete() + + client = APIClient() + client.force_login(user) + + response = client.delete( + f"/api/v1.0/rooms/{room.id}/", + ) + + assert response.status_code == 403 + mock_delete_room.assert_not_called() 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 83d4bfba5..03c7ed17d 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 @@ -2,7 +2,8 @@ Test rooms API endpoints in the Meet core app: participants management. """ -# pylint: disable=redefined-outer-name,unused-argument,protected-access,no-name-in-module,too-many-lines +# pylint: disable=redefined-outer-name,unused-argument,protected-access +# pylint: disable=no-name-in-module,too-many-lines import random from unittest import mock 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 f40693303..f7efe5614 100644 --- a/src/backend/core/tests/rooms/test_api_rooms_retrieve.py +++ b/src/backend/core/tests/rooms/test_api_rooms_retrieve.py @@ -193,6 +193,28 @@ def test_api_rooms_retrieve_anonymous_unregistered_not_allowed(): assert response.json() == {"detail": "No Room matches the given query."} +@pytest.mark.parametrize("allow_unregistered_rooms", [True, False]) +@pytest.mark.parametrize("lookup", ["id", "slug"]) +@mock.patch("core.utils.generate_token", return_value="foo") +def test_api_rooms_retrieve_soft_deleted( + mock_token, lookup, allow_unregistered_rooms, settings +): + """ + Retrieving a soft-deleted room should return a 410, and never fall back + to an unregistered room with the same slug. + """ + settings.ALLOW_UNREGISTERED_ROOMS = allow_unregistered_rooms + room = RoomFactory(access_level=RoomAccessLevel.PUBLIC) + room.soft_delete() + + client = APIClient() + response = client.get(f"/api/v1.0/rooms/{getattr(room, lookup)!s}/") + + assert response.status_code == 410 + assert response.json() == {"detail": "This room has been deleted."} + mock_token.assert_not_called() + + @mock.patch("core.utils.generate_token", return_value="foo") @override_settings( LIVEKIT_CONFIGURATION={ diff --git a/src/backend/core/tests/services/test_livekit_events.py b/src/backend/core/tests/services/test_livekit_events.py index c69027fae..a1e618976 100644 --- a/src/backend/core/tests/services/test_livekit_events.py +++ b/src/backend/core/tests/services/test_livekit_events.py @@ -1,7 +1,7 @@ """ Test LiveKitEvents service. """ -# pylint: disable=W0621,W0613, W0212, E0611 +# pylint: disable=W0621,W0613, W0212, E0611, C0302 import logging import uuid @@ -27,7 +27,10 @@ from core.services.livekit_events import ( to_recording_event, ) from core.services.lobby import LobbyService -from core.services.room_management import RoomManagementException +from core.services.room_management import ( + RoomManagementException, + RoomNotFoundException, +) from core.services.sip_management import ( SIPException, SIPManagement, @@ -796,6 +799,61 @@ def test_handle_room_started_raises_error_for_nonexistent_room(service): service._handle_room_started(mock_data) +@mock.patch.object(SIPManagement, "ensure_dispatch_rule") +@mock.patch("core.services.room_management.RoomManagement.delete_room") +def test_handle_room_started_closes_deleted_room( + mock_delete_room, mock_ensure_dispatch_rule, service, settings +): + """Should close a LiveKit room recreated for a soft-deleted room.""" + settings.ROOM_TELEPHONY_ENABLED = True + room = RoomFactory() + room.soft_delete() + mock_data = mock.MagicMock() + mock_data.room.name = str(room.id) + + service._handle_room_started(mock_data) + + mock_delete_room.assert_called_once_with(str(room.id)) + mock_ensure_dispatch_rule.assert_not_called() + + +@mock.patch( + "core.services.room_management.RoomManagement.delete_room", + side_effect=RoomNotFoundException("Room does not exist"), +) +def test_handle_room_started_ignores_already_closed_deleted_room( + mock_delete_room, service +): + """Should proceed silently when the deleted room is already closed in LiveKit.""" + room = RoomFactory() + room.soft_delete() + mock_data = mock.MagicMock() + mock_data.room.name = str(room.id) + + service._handle_room_started(mock_data) + + mock_delete_room.assert_called_once_with(str(room.id)) + + +@mock.patch( + "core.services.room_management.RoomManagement.delete_room", + side_effect=RoomManagementException("Could not delete room"), +) +def test_handle_room_started_raises_error_when_closing_deleted_room_fails( + mock_delete_room, service +): + """Should raise ActionFailedError when the deleted room cannot be closed.""" + room = RoomFactory() + room.soft_delete() + mock_data = mock.MagicMock() + mock_data.room.name = str(room.id) + + expected_error = f"Failed to close deleted room {room.id}" + + with pytest.raises(ActionFailedError, match=expected_error): + service._handle_room_started(mock_data) + + @mock.patch.object( api.WebhookReceiver, "receive", side_effect=Exception("Invalid payload") ) diff --git a/src/backend/core/tests/services/test_room_management.py b/src/backend/core/tests/services/test_room_management.py index 50f4b3926..015ebb1d3 100644 --- a/src/backend/core/tests/services/test_room_management.py +++ b/src/backend/core/tests/services/test_room_management.py @@ -2,16 +2,19 @@ from unittest import mock +from django.db import connection + import pytest from livekit.api import TwirpError from core.factories import RoomFactory -from core.models import RoomAccessLevel +from core.models import Room, RoomAccessLevel from core.services.room_management import ( RoomManagement, RoomManagementException, RoomNotFoundException, ) +from core.services.sip_management import SIPException, SIPManagement @mock.patch("core.services.room_management.utils.create_livekit_client") @@ -62,6 +65,148 @@ def test_delete_room_raises_management_exception(mock_create_livekit_client): mock_api.aclose.assert_awaited_once() +@pytest.mark.django_db(transaction=True) +@mock.patch.object(SIPManagement, "delete_dispatch_rule") +@mock.patch.object(RoomManagement, "delete_room") +def test_soft_delete_commits_before_closing_livekit_room(mock_delete_room, _): + """The deletion is committed before the LiveKit room is closed, so the + room_started webhook of a participant reconnecting right away sees it.""" + room = RoomFactory() + + def assert_deletion_committed(room_name): + assert connection.in_atomic_block is False + assert Room.all_objects.get(id=room_name).is_deleted + + mock_delete_room.side_effect = assert_deletion_committed + + RoomManagement.soft_delete(room) + + mock_delete_room.assert_called_once_with(str(room.id)) + + +@pytest.mark.django_db +@pytest.mark.parametrize( + "error", + [ + RoomManagementException("Could not delete room"), + ConnectionError("LiveKit is unreachable"), + ], +) +@mock.patch.object(SIPManagement, "delete_dispatch_rule") +@mock.patch.object(RoomManagement, "delete_room") +def test_soft_delete_failure_rolls_back_and_can_be_retried(mock_delete_room, _, error): + """A failed soft delete leaves the room untouched, in database and in memory, + so it can be retried.""" + room = RoomFactory() + mock_delete_room.side_effect = error + + with pytest.raises(type(error)): + RoomManagement.soft_delete(room) + + assert room.deleted_at is None + assert Room.objects.filter(id=room.id).exists() + + mock_delete_room.side_effect = None + RoomManagement.soft_delete(room) + + assert mock_delete_room.call_count == 2 + assert room.deleted_at is not None + assert Room.all_objects.get(id=room.id).deleted_at is not None + assert Room.objects.filter(id=room.id).exists() is False + + +@pytest.mark.django_db +@pytest.mark.parametrize("sip_setting", ["ROOM_TELEPHONY_ENABLED", "ROOMKIT_ENABLED"]) +@mock.patch.object(SIPManagement, "delete_dispatch_rule") +@mock.patch.object( + RoomManagement, + "delete_room", + side_effect=RoomNotFoundException("Room does not exist"), +) +def test_soft_delete_room_not_live_deletes_dispatch_rule( + mock_delete_room, mock_delete_dispatch_rule, sip_setting, settings +): + """A roomkit join creates the dispatch rule before any LiveKit room exists, + so no room_finished webhook deletes it: the soft delete must, or the + retained PIN would still route SIP calls to the deleted room.""" + settings.ROOM_TELEPHONY_ENABLED = False + settings.ROOMKIT_ENABLED = False + setattr(settings, sip_setting, True) + room = RoomFactory() + + RoomManagement.soft_delete(room) + + mock_delete_room.assert_called_once_with(str(room.id)) + mock_delete_dispatch_rule.assert_called_once_with(room.id) + assert Room.all_objects.get(id=room.id).is_deleted + + +@pytest.mark.django_db +def test_soft_delete_live_room_deletes_dispatch_rule_before_closing_it(settings): + """The dispatch rule of a live room is deleted before the LiveKit room is + closed, so a dispatch rule failure rolls the deletion back without having + disconnected the participants.""" + settings.ROOM_TELEPHONY_ENABLED = True + room = RoomFactory() + calls = mock.Mock() + + with ( + mock.patch.object(RoomManagement, "delete_room", calls.delete_room), + mock.patch.object( + SIPManagement, "delete_dispatch_rule", calls.delete_dispatch_rule + ), + ): + RoomManagement.soft_delete(room) + + assert calls.mock_calls == [ + mock.call.delete_dispatch_rule(room.id), + mock.call.delete_room(str(room.id)), + ] + + +@pytest.mark.django_db +@mock.patch.object(SIPManagement, "delete_dispatch_rule") +@mock.patch.object(RoomManagement, "delete_room") +def test_soft_delete_sip_disabled_skips_dispatch_rule( + mock_delete_room, mock_delete_dispatch_rule, settings +): + """Without telephony nor roomkit, no dispatch rule is looked up.""" + settings.ROOM_TELEPHONY_ENABLED = False + settings.ROOMKIT_ENABLED = False + room = RoomFactory() + + RoomManagement.soft_delete(room) + + mock_delete_room.assert_called_once_with(str(room.id)) + mock_delete_dispatch_rule.assert_not_called() + assert Room.all_objects.get(id=room.id).is_deleted + + +@pytest.mark.django_db +@mock.patch.object( + SIPManagement, + "delete_dispatch_rule", + side_effect=SIPException("Could not delete dispatch rules"), +) +@mock.patch.object(RoomManagement, "delete_room") +def test_soft_delete_dispatch_rule_failure_rolls_back( + mock_delete_room, mock_delete_dispatch_rule, settings +): + """A dispatch rule that can't be deleted fails the soft delete like a LiveKit + room that can't be closed: it is rolled back so it can be retried, and the + LiveKit room is left open.""" + settings.ROOMKIT_ENABLED = True + room = RoomFactory() + + with pytest.raises(RoomManagementException): + RoomManagement.soft_delete(room) + + mock_delete_room.assert_not_called() + mock_delete_dispatch_rule.assert_called_once_with(room.id) + assert room.deleted_at is None + assert Room.objects.filter(id=room.id).exists() + + @mock.patch.object(RoomManagement, "update_metadata") def test_sync_room_metadata_pushes_configuration_and_access_level(mock_update_metadata): """The room's configuration and access level are forwarded to LiveKit.""" diff --git a/src/backend/core/tests/services/test_sip_management.py b/src/backend/core/tests/services/test_sip_management.py index b12324994..ae71f59b2 100644 --- a/src/backend/core/tests/services/test_sip_management.py +++ b/src/backend/core/tests/services/test_sip_management.py @@ -257,6 +257,31 @@ def test_delete_dispatch_rule_multiple_rules(mock_client_factory, mock_list_rule mock_api.aclose.assert_called_once() +@mock.patch("core.services.sip_management.SIPManagement._list_dispatch_rules_ids") +@mock.patch("core.utils.create_livekit_client") +def test_delete_dispatch_rule_already_deleted(mock_client_factory, mock_list_rules): + """A rule deleted between listing and deletion (e.g. by both a room deletion and + its room_finished webhook) should not fail, nor stop the other deletions.""" + sip_management = SIPManagement() + room = RoomFactory(access_level=RoomAccessLevel.RESTRICTED, pin_code="1234") + + mock_list_rules.return_value = ["rule-1", "rule-2"] + mock_api = create_mock_livekit_client() + mock_api.sip.delete_sip_dispatch_rule = mock.AsyncMock( + side_effect=[ + TwirpError(msg="sip dispatch rule not found", code="not_found", status=404), + None, + ] + ) + mock_client_factory.return_value = mock_api + + result = sip_management.delete_dispatch_rule(room.id) + + assert result is True + assert mock_api.sip.delete_sip_dispatch_rule.call_count == 2 + mock_api.aclose.assert_called_once() + + @mock.patch("core.services.sip_management.SIPManagement._list_dispatch_rules_ids") @mock.patch("core.utils.create_livekit_client") def test_delete_dispatch_rule_partial_failure(mock_client_factory, mock_list_rules): diff --git a/src/backend/core/tests/test_api_resource_accesses.py b/src/backend/core/tests/test_api_resource_accesses.py index 9ac1ad1d9..22cb08cc9 100644 --- a/src/backend/core/tests/test_api_resource_accesses.py +++ b/src/backend/core/tests/test_api_resource_accesses.py @@ -2,6 +2,8 @@ Test resource accesses API endpoints in the Meet core app. """ +# pylint: disable=too-many-lines + import random from unittest import mock from uuid import uuid4 @@ -956,3 +958,99 @@ def test_api_room_user_access_delete_owners_last_owner(): assert response.status_code == 403 assert ResourceAccess.objects.count() == 1 + + +# Soft-deleted rooms + + +def test_api_room_user_accesses_create_soft_deleted_room(): + """ + Owners of a soft-deleted room should not be allowed to add accesses to it. + """ + user = UserFactory() + room = RoomFactory(users=[(user, "owner")]) + room.soft_delete() + + client = APIClient() + client.force_login(user) + + response = client.post( + "/api/v1.0/resource-accesses/", + { + "user": str(UserFactory().id), + "resource": str(room.id), + "role": "member", + }, + ) + + assert response.status_code == 410 + assert response.json() == {"detail": "This room has been deleted."} + assert ResourceAccess.objects.count() == 1 + + +def test_api_room_user_accesses_create_soft_deleted_room_not_administrator(): + """ + Users without privileges on a soft-deleted room should get a 403, + not revealing that the room has been deleted. + """ + user = UserFactory() + room = RoomFactory(users=[(user, "member")]) + room.soft_delete() + + client = APIClient() + client.force_login(user) + + response = client.post( + "/api/v1.0/resource-accesses/", + { + "user": str(UserFactory().id), + "resource": str(room.id), + "role": "member", + }, + ) + + assert response.status_code == 403 + assert ResourceAccess.objects.count() == 1 + + +def test_api_room_user_accesses_update_soft_deleted_room(): + """ + Owners of a soft-deleted room should not be allowed to update its accesses. + """ + user = UserFactory() + room = RoomFactory(users=[(user, "owner")]) + access = UserResourceAccessFactory(resource=room, role="member") + room.soft_delete() + + client = APIClient() + client.force_login(user) + + response = client.patch( + f"/api/v1.0/resource-accesses/{access.id!s}/", + {"role": "administrator"}, + format="json", + ) + + assert response.status_code == 410 + access.refresh_from_db() + assert access.role == "member" + + +def test_api_room_user_access_delete_soft_deleted_room(): + """ + Owners of a soft-deleted room should not be allowed to remove its accesses. + """ + user = UserFactory() + room = RoomFactory(users=[(user, "owner")]) + access = UserResourceAccessFactory(resource=room, role="member") + room.soft_delete() + + client = APIClient() + client.force_login(user) + + response = client.delete( + f"/api/v1.0/resource-accesses/{access.id!s}/", + ) + + assert response.status_code == 410 + assert ResourceAccess.objects.filter(id=access.id).exists() is True diff --git a/src/backend/core/tests/test_external_api_rooms.py b/src/backend/core/tests/test_external_api_rooms.py index 842f9f8e1..91170da04 100644 --- a/src/backend/core/tests/test_external_api_rooms.py +++ b/src/backend/core/tests/test_external_api_rooms.py @@ -26,7 +26,12 @@ from core.models import ( RoomAccessLevel, User, ) -from core.services.room_management import RoomManagement +from core.services.room_management import ( + RoomManagement, + RoomManagementException, + RoomNotFoundException, +) +from core.services.sip_management import SIPManagement pytestmark = pytest.mark.django_db @@ -563,6 +568,53 @@ def test_api_rooms_retrieve_not_found(): assert "no room matches the given query." in str(response.data).lower() +def test_api_rooms_retrieve_invalid_id(): + """Retrieving a room with a malformed id should return a 404.""" + + user = UserFactory() + token = generate_test_token(user, [ApplicationScope.ROOMS_RETRIEVE]) + + client = APIClient() + client.credentials(HTTP_AUTHORIZATION=f"Bearer {token}") + response = client.get("/external-api/v1.0/rooms/not-a-uuid/") + + assert response.status_code == 404 + + +def test_api_rooms_retrieve_soft_deleted(): + """Retrieving a soft-deleted room should return a 410.""" + + user = UserFactory() + room = RoomFactory(users=[(user, RoleChoices.OWNER)]) + room.soft_delete() + + token = generate_test_token(user, [ApplicationScope.ROOMS_RETRIEVE]) + + client = APIClient() + client.credentials(HTTP_AUTHORIZATION=f"Bearer {token}") + response = client.get(f"/external-api/v1.0/rooms/{room.id}/") + + assert response.status_code == 410 + assert response.json() == {"detail": "This room has been deleted."} + + +def test_api_rooms_retrieve_soft_deleted_not_member(): + """Retrieving a soft-deleted room without any role on it should return a 403, + not revealing that the room has been deleted.""" + + user = UserFactory() + room = RoomFactory() + room.soft_delete() + + token = generate_test_token(user, [ApplicationScope.ROOMS_RETRIEVE]) + + client = APIClient() + client.credentials(HTTP_AUTHORIZATION=f"Bearer {token}") + response = client.get(f"/external-api/v1.0/rooms/{room.id}/") + + assert response.status_code == 403 + + def test_api_rooms_create_requires_authentication(): """Creating rooms without authentication should return 401.""" @@ -1392,6 +1444,212 @@ def test_api_rooms_update_tracks_analytics(mock_update_metadata, mock_capture): mock_update_metadata.assert_called_once() +@mock.patch.object(RoomManagement, "delete_room") +def test_api_rooms_delete_requires_authentication(mock_delete_room): + """Deleting a room without authentication should return 401.""" + + room = RoomFactory(users=[(UserFactory(), RoleChoices.OWNER)]) + + client = APIClient() + response = client.delete(f"/external-api/v1.0/rooms/{room.id}/") + + assert response.status_code == 401 + assert Room.objects.filter(id=room.id).exists() is True + mock_delete_room.assert_not_called() + + +@mock.patch.object(RoomManagement, "delete_room") +def test_api_rooms_delete_requires_scope(mock_delete_room): + """Deleting a room requires the ROOMS_DELETE scope.""" + + user = UserFactory() + room = RoomFactory(users=[(user, RoleChoices.OWNER)]) + + # Token without ROOMS_DELETE scope + token = generate_test_token( + user, [ApplicationScope.ROOMS_RETRIEVE, ApplicationScope.ROOMS_UPDATE] + ) + + client = APIClient() + client.credentials(HTTP_AUTHORIZATION=f"Bearer {token}") + response = client.delete(f"/external-api/v1.0/rooms/{room.id}/") + + assert response.status_code == 403 + assert ( + "insufficient permissions. required scope: rooms:delete" + in str(response.data).lower() + ) + assert Room.objects.filter(id=room.id).exists() is True + mock_delete_room.assert_not_called() + + +@pytest.mark.parametrize("role", [RoleChoices.ADMIN, RoleChoices.MEMBER, None]) +@mock.patch.object(RoomManagement, "delete_room") +def test_api_rooms_delete_without_ownership(mock_delete_room, role): + """Only owners should be able to delete a room, administrators included.""" + + user = UserFactory() + users = [(user, role)] if role else [] + room = RoomFactory(users=users) + + token = generate_test_token(user, [ApplicationScope.ROOMS_DELETE]) + + client = APIClient() + client.credentials(HTTP_AUTHORIZATION=f"Bearer {token}") + response = client.delete(f"/external-api/v1.0/rooms/{room.id}/") + + assert response.status_code == 403 + assert Room.objects.filter(id=room.id).exists() is True + mock_delete_room.assert_not_called() + + +@mock.patch.object(RoomManagement, "delete_room") +def test_api_rooms_delete_unknown_room(mock_delete_room): + """Deleting a room that does not exist should return 404.""" + + user = UserFactory() + token = generate_test_token(user, [ApplicationScope.ROOMS_DELETE]) + + client = APIClient() + client.credentials(HTTP_AUTHORIZATION=f"Bearer {token}") + response = client.delete(f"/external-api/v1.0/rooms/{uuid.uuid4()}/") + + assert response.status_code == 404 + mock_delete_room.assert_not_called() + + +@mock.patch("core.external_api.viewsets.analytics.capture") +@mock.patch.object(SIPManagement, "delete_dispatch_rule") +@mock.patch.object(RoomManagement, "delete_room") +def test_api_rooms_delete_success(mock_delete_room, _, mock_capture): + """Owners should be able to delete a room: it is soft deleted, its LiveKit + room is closed and a ROOM_DELETED analytics event is emitted.""" + + user = UserFactory() + room = RoomFactory( + users=[(user, RoleChoices.OWNER)], access_level=RoomAccessLevel.TRUSTED + ) + + token = generate_test_token(user, [ApplicationScope.ROOMS_DELETE]) + application = Application.objects.get() + + client = APIClient() + client.credentials(HTTP_AUTHORIZATION=f"Bearer {token}") + response = client.delete(f"/external-api/v1.0/rooms/{room.id}/") + + assert response.status_code == 204 + mock_delete_room.assert_called_once_with(str(room.id)) + assert Room.objects.filter(id=room.id).exists() is False + assert Room.all_objects.get(id=room.id).deleted_at is not None + + mock_capture.assert_called_once() + captured_user, event, properties = mock_capture.call_args[0] + + assert captured_user == user + assert event == AnalyticsEvent.ROOM_DELETED + assert properties == { + "room_id": str(room.pk), + "access_level": RoomAccessLevel.TRUSTED, + "client_id": str(application.client_id), + "external_api": True, + "auth_method": "ApplicationJWTAuthentication", + "$set": {"email": user.email}, + } + + +@mock.patch("core.external_api.viewsets.analytics.capture") +@mock.patch.object(SIPManagement, "delete_dispatch_rule") +@mock.patch.object( + RoomManagement, + "delete_room", + side_effect=RoomManagementException("Could not delete room"), +) +def test_api_rooms_delete_livekit_failure(mock_delete_room, _, mock_capture): + """When the LiveKit room can't be closed, the deletion should be rolled back + and no analytics event emitted.""" + + user = UserFactory() + room = RoomFactory(users=[(user, RoleChoices.OWNER)]) + + token = generate_test_token(user, [ApplicationScope.ROOMS_DELETE]) + + client = APIClient() + client.credentials(HTTP_AUTHORIZATION=f"Bearer {token}") + response = client.delete(f"/external-api/v1.0/rooms/{room.id}/") + + assert response.status_code == 500 + assert response.json() == {"detail": "Could not delete the room, please try again."} + mock_delete_room.assert_called_once_with(str(room.id)) + assert Room.objects.get(id=room.id).deleted_at is None + mock_capture.assert_not_called() + + +@mock.patch.object(SIPManagement, "delete_dispatch_rule") +@mock.patch.object( + RoomManagement, + "delete_room", + side_effect=RoomNotFoundException("Room does not exist"), +) +def test_api_rooms_delete_room_not_live( + mock_delete_room, mock_delete_dispatch_rule, settings +): + """Deleting a room that is not live in LiveKit should still soft delete it, + and delete its SIP dispatch rule, as no room_finished webhook will.""" + + settings.ROOMKIT_ENABLED = True + user = UserFactory() + room = RoomFactory(users=[(user, RoleChoices.OWNER)]) + + token = generate_test_token(user, [ApplicationScope.ROOMS_DELETE]) + + client = APIClient() + client.credentials(HTTP_AUTHORIZATION=f"Bearer {token}") + response = client.delete(f"/external-api/v1.0/rooms/{room.id}/") + + assert response.status_code == 204 + mock_delete_room.assert_called_once_with(str(room.id)) + mock_delete_dispatch_rule.assert_called_once_with(room.id) + assert Room.objects.filter(id=room.id).exists() is False + assert Room.all_objects.get(id=room.id).deleted_at is not None + + +@mock.patch.object(RoomManagement, "delete_room") +def test_api_rooms_delete_soft_deleted(mock_delete_room): + """Deleting a room that is already soft deleted should return a 410.""" + + user = UserFactory() + room = RoomFactory(users=[(user, RoleChoices.OWNER)]) + room.soft_delete() + + token = generate_test_token(user, [ApplicationScope.ROOMS_DELETE]) + + client = APIClient() + client.credentials(HTTP_AUTHORIZATION=f"Bearer {token}") + response = client.delete(f"/external-api/v1.0/rooms/{room.id}/") + + assert response.status_code == 410 + mock_delete_room.assert_not_called() + + +@mock.patch.object(RoomManagement, "delete_room") +def test_api_rooms_delete_soft_deleted_not_owner(mock_delete_room): + """Deleting a soft-deleted room as a non-owner should return a 403, + not revealing that the room has been deleted.""" + + user = UserFactory() + room = RoomFactory(users=[(user, RoleChoices.ADMIN)]) + room.soft_delete() + + token = generate_test_token(user, [ApplicationScope.ROOMS_DELETE]) + + client = APIClient() + client.credentials(HTTP_AUTHORIZATION=f"Bearer {token}") + response = client.delete(f"/external-api/v1.0/rooms/{room.id}/") + + assert response.status_code == 403 + mock_delete_room.assert_not_called() + + def test_api_rooms_response_no_url(settings): """Response should not include url field when APPLICATION_BASE_URL is None.""" settings.APPLICATION_BASE_URL = None diff --git a/src/frontend/src/features/rooms/api/requestEntry.ts b/src/frontend/src/features/rooms/api/requestEntry.ts index fcbba3ae5..a10e58018 100644 --- a/src/frontend/src/features/rooms/api/requestEntry.ts +++ b/src/frontend/src/features/rooms/api/requestEntry.ts @@ -13,6 +13,8 @@ export enum ApiLobbyStatus { DENIED = 'denied', TIMEOUT = 'timeout', ACCEPTED = 'accepted', + // Client-side only: the room was deleted while waiting + DELETED = 'deleted', } export interface ApiRequestEntry { diff --git a/src/frontend/src/features/rooms/components/Conference.tsx b/src/frontend/src/features/rooms/components/Conference.tsx index 6901adf9d..5389bf0bd 100644 --- a/src/frontend/src/features/rooms/components/Conference.tsx +++ b/src/frontend/src/features/rooms/components/Conference.tsx @@ -86,6 +86,7 @@ export const Conference = ({ const { status: fetchStatus, isError: isFetchError, + error: fetchError, data, } = useQuery({ queryKey: fetchKey, @@ -99,6 +100,8 @@ export const Conference = ({ if (error.statusCode == '404') { createRoom({ slug: roomId, username }) } + // A deleted room can't be recreated, surface the error instead + if (error.statusCode == '410') throw error }), retry: false, }) @@ -199,6 +202,15 @@ export const Conference = ({ }, [apiConfig?.livekit]) const { t } = useTranslation('rooms') + if (fetchError?.statusCode == 410) { + return ( + + ) + } + if (isCreateError) { // this error screen should be replaced by a proper waiting room for anonymous user. return ( @@ -283,6 +295,7 @@ export const Conference = ({ return case DisconnectReason.DUPLICATE_IDENTITY: case DisconnectReason.PARTICIPANT_REMOVED: + case DisconnectReason.ROOM_DELETED: navigateTo( 'feedback', {}, diff --git a/src/frontend/src/features/rooms/components/Lobby.tsx b/src/frontend/src/features/rooms/components/Lobby.tsx index 34ceba21c..9a26c1b2b 100644 --- a/src/frontend/src/features/rooms/components/Lobby.tsx +++ b/src/frontend/src/features/rooms/components/Lobby.tsx @@ -74,7 +74,9 @@ export const Lobby = ({ const { openLoginHint } = useLoginHint() const handleSubmit = async () => { - const { data } = await refetchRoom() + const { data, error } = await refetchRoom() + + if (error?.statusCode == 410) return if (!data?.livekit) { // Display a message to inform the user that by logging in, they won't have to wait for room entry approval. @@ -88,7 +90,22 @@ export const Lobby = ({ enterRoom() } - switch (status) { + const isRoomDeleted = isError && error?.statusCode == 410 + const lobbyStatus = isRoomDeleted ? ApiLobbyStatus.DELETED : status + + switch (lobbyStatus) { + case ApiLobbyStatus.DELETED: + return ( + + + {t('deleted.title')} + + + {t('deleted.body')} + + + ) + case ApiLobbyStatus.TIMEOUT: return ( diff --git a/src/frontend/src/features/rooms/hooks/useLobby.ts b/src/frontend/src/features/rooms/hooks/useLobby.ts index 2ccf79e8d..ba4d0a3d8 100644 --- a/src/frontend/src/features/rooms/hooks/useLobby.ts +++ b/src/frontend/src/features/rooms/hooks/useLobby.ts @@ -1,6 +1,7 @@ import { useCallback, useEffect, useRef, useState } from 'react' import { useQuery } from '@tanstack/react-query' import { keys } from '@/api/queryKeys' +import { ApiError } from '@/api/ApiError' import { requestEntry, ApiLobbyStatus, @@ -40,24 +41,33 @@ export const useLobby = ({ const { data: waitingData } = useQuery({ queryKey: [keys.requestEntry, roomId], queryFn: async () => { - const response = await requestEntry({ - roomId, - username, - }) + try { + const response = await requestEntry({ + roomId, + username, + }) - if (response.id) { - setLobbyParticipantId(roomId, response.id) + if (response.id) { + setLobbyParticipantId(roomId, response.id) + } + if (response.status === ApiLobbyStatus.ACCEPTED) { + clearWaitingTimeout() + setStatus(ApiLobbyStatus.ACCEPTED) + onAccepted(response) + } else if (response.status === ApiLobbyStatus.DENIED) { + clearWaitingTimeout() + setStatus(ApiLobbyStatus.DENIED) + } + return response + } catch (error) { + // The room was deleted while waiting, stop polling + if (error instanceof ApiError && error.statusCode === 410) { + clearWaitingTimeout() + setStatus(ApiLobbyStatus.DELETED) + return { status: ApiLobbyStatus.DELETED } + } + throw error } - - if (response.status === ApiLobbyStatus.ACCEPTED) { - clearWaitingTimeout() - setStatus(ApiLobbyStatus.ACCEPTED) - onAccepted(response) - } else if (response.status === ApiLobbyStatus.DENIED) { - clearWaitingTimeout() - setStatus(ApiLobbyStatus.DENIED) - } - return response }, refetchInterval: POLL_INTERVAL_MS, refetchOnWindowFocus: false, diff --git a/src/frontend/src/features/rooms/routes/Feedback.tsx b/src/frontend/src/features/rooms/routes/Feedback.tsx index 7e93ed694..1107694a2 100644 --- a/src/frontend/src/features/rooms/routes/Feedback.tsx +++ b/src/frontend/src/features/rooms/routes/Feedback.tsx @@ -31,6 +31,7 @@ const buttonClass = css({ enum DisconnectReasonKey { DuplicateIdentity = 'duplicateIdentity', ParticipantRemoved = 'participantRemoved', + RoomDeleted = 'roomDeleted', } const FeedbackRoute = () => { @@ -46,6 +47,8 @@ const FeedbackRoute = () => { return DisconnectReasonKey.DuplicateIdentity case DisconnectReason.PARTICIPANT_REMOVED: return DisconnectReasonKey.ParticipantRemoved + case DisconnectReason.ROOM_DELETED: + return DisconnectReasonKey.RoomDeleted } }, []) @@ -56,7 +59,10 @@ const FeedbackRoute = () => { } }, []) - const showBackButton = reasonKey !== DisconnectReasonKey.ParticipantRemoved + // Rejoining is not possible once removed or once the room is deleted + const showBackButton = + reasonKey !== DisconnectReasonKey.ParticipantRemoved && + reasonKey !== DisconnectReasonKey.RoomDeleted return ( diff --git a/src/frontend/src/locales/de/rooms.json b/src/frontend/src/locales/de/rooms.json index 36d45bc94..168da30f7 100644 --- a/src/frontend/src/locales/de/rooms.json +++ b/src/frontend/src/locales/de/rooms.json @@ -3,7 +3,8 @@ "heading": { "normal": "Du hast das Meeting verlassen", "duplicateIdentity": "Du bist dem Meeting von einem anderen Gerät aus beigetreten", - "participantRemoved": "Du wurdest vom Host aus dem Meeting entfernt" + "participantRemoved": "Du wurdest vom Host aus dem Meeting entfernt", + "roomDeleted": "Dieses Meeting wurde gelöscht" }, "home": "Zur Startseite zurückkehren", "back": "Dem Meeting erneut beitreten" @@ -94,6 +95,10 @@ "timeoutInvite": { "title": "Du kannst diesem Meeting nicht beitreten", "body": "Niemand hat auf deine Anfrage reagiert" + }, + "deleted": { + "title": "Du kannst diesem Meeting nicht beitreten", + "body": "Dieses Meeting wurde gelöscht." } }, "leaveRoomPrompt": "Hiermit verlässt du das Meeting.", @@ -215,6 +220,10 @@ "heading": "Authentifizierung erforderlich", "body": "Dieser Raum wurde noch nicht erstellt. Bitte authentifiziere dich, um ihn zu erstellen, oder warte, bis eine authentifizierte Person dies tut." }, + "deletedRoom": { + "heading": "Meeting gelöscht", + "body": "Dieses Meeting wurde gelöscht und kann nicht mehr betreten werden." + }, "screenShare": { "title": "Bildschirmfreigabe nicht möglich", "ariaLabel": "Bildschirmfreigabe nicht möglich", diff --git a/src/frontend/src/locales/en/rooms.json b/src/frontend/src/locales/en/rooms.json index 0d2486729..40a2b9312 100644 --- a/src/frontend/src/locales/en/rooms.json +++ b/src/frontend/src/locales/en/rooms.json @@ -3,7 +3,8 @@ "heading": { "normal": "You have left the meeting", "duplicateIdentity": "You have joined the meeting from another device", - "participantRemoved": "You have been removed from the meeting by a host" + "participantRemoved": "You have been removed from the meeting by a host", + "roomDeleted": "This meeting has been deleted" }, "home": "Return to home", "back": "Rejoin the meeting" @@ -94,6 +95,10 @@ "timeoutInvite": { "title": "You cannot join this call", "body": "No one responded to your request" + }, + "deleted": { + "title": "You cannot join this call", + "body": "This meeting has been deleted." } }, "leaveRoomPrompt": "This will make you leave the meeting.", @@ -215,6 +220,10 @@ "heading": "Authentication Required", "body": "This room has not been created yet. Please authenticate to create it or wait for an authenticated user to do so." }, + "deletedRoom": { + "heading": "Meeting deleted", + "body": "This meeting has been deleted and can no longer be joined." + }, "screenShare": { "title": "Unable to share your screen", "ariaLabel": "Unable to share your screen", diff --git a/src/frontend/src/locales/es/rooms.json b/src/frontend/src/locales/es/rooms.json index 242a2b258..4b12087f5 100644 --- a/src/frontend/src/locales/es/rooms.json +++ b/src/frontend/src/locales/es/rooms.json @@ -3,7 +3,8 @@ "heading": { "normal": "Has salido de la reunión", "duplicateIdentity": "Te has unido a la reunión desde otro dispositivo", - "participantRemoved": "Un administrador te ha expulsado de la llamada" + "participantRemoved": "Un administrador te ha expulsado de la llamada", + "roomDeleted": "Esta reunión ha sido eliminada" }, "home": "Volver al inicio", "back": "Volver a la reunión" @@ -94,6 +95,10 @@ "timeoutInvite": { "title": "No puedes participar en esta llamada", "body": "Nadie ha respondido a tu solicitud de participación en la llamada" + }, + "deleted": { + "title": "No puedes participar en esta llamada", + "body": "Esta reunión ha sido eliminada." } }, "leaveRoomPrompt": "Volver al inicio hará que salgas de la reunión.", @@ -215,6 +220,10 @@ "heading": "Autenticación necesaria", "body": "Esta reunión todavía no se ha creado. Autentícate para crearla o espera a que lo haga un usuario autenticado." }, + "deletedRoom": { + "heading": "Reunión eliminada", + "body": "Esta reunión ha sido eliminada y ya no es posible unirse a ella." + }, "screenShare": { "title": "No se puede compartir tu pantalla", "ariaLabel": "No se puede compartir tu pantalla", diff --git a/src/frontend/src/locales/fr/rooms.json b/src/frontend/src/locales/fr/rooms.json index 73dec24ff..070dba84a 100644 --- a/src/frontend/src/locales/fr/rooms.json +++ b/src/frontend/src/locales/fr/rooms.json @@ -3,7 +3,8 @@ "heading": { "normal": "Vous avez quitté la réunion", "duplicateIdentity": "Vous avez rejoint la réunion depuis un autre appareil", - "participantRemoved": "Vous avez été exclu de l'appel par un administrateur" + "participantRemoved": "Vous avez été exclu de l'appel par un administrateur", + "roomDeleted": "Cette réunion a été supprimée" }, "home": "Retourner à l'accueil", "back": "Réintégrer la réunion" @@ -94,6 +95,10 @@ "timeoutInvite": { "title": "Vous ne pouvez pas participer à cet appel", "body": "Personne n'a répondu à votre demande de participation à l'appel" + }, + "deleted": { + "title": "Vous ne pouvez pas participer à cet appel", + "body": "Cette réunion a été supprimée." } }, "leaveRoomPrompt": "Revenir à l'accueil vous fera quitter la réunion.", @@ -215,6 +220,10 @@ "heading": "Authentification requise", "body": "Cette réunion n'a pas encore été créée. Veuillez vous authentifier pour la créer ou attendre qu'un utilisateur authentifié le fasse." }, + "deletedRoom": { + "heading": "Réunion supprimée", + "body": "Cette réunion a été supprimée et n'est plus accessible." + }, "screenShare": { "title": "Impossible de partager votre écran", "ariaLabel": "Impossible de partager votre écran", diff --git a/src/frontend/src/locales/nl/rooms.json b/src/frontend/src/locales/nl/rooms.json index cc110aee5..0832bccf7 100644 --- a/src/frontend/src/locales/nl/rooms.json +++ b/src/frontend/src/locales/nl/rooms.json @@ -3,7 +3,8 @@ "heading": { "normal": "Je hebt de vergadering verlaten", "duplicateIdentity": "U heeft de vergadering via een ander apparaat geopend", - "participantRemoved": "U bent door een beheerder uit het gesprek verwijderd" + "participantRemoved": "U bent door een beheerder uit het gesprek verwijderd", + "roomDeleted": "Deze vergadering is verwijderd" }, "home": "Keer terug naar het hoofdscherm", "back": "Sluit weer bij de vergadering aan" @@ -94,6 +95,10 @@ "timeoutInvite": { "title": "U kunt niet deelnemen aan dit gesprek", "body": "Niemand heeft gereageerd op uw verzoek om deel te nemen aan het gesprek" + }, + "deleted": { + "title": "U kunt niet deelnemen aan dit gesprek", + "body": "Deze vergadering is verwijderd." } }, "leaveRoomPrompt": "Dat zal u de vergadering doen verlaten.", @@ -215,6 +220,10 @@ "heading": "Verificatie vereist", "body": "Deze ruimte is nog niet gemaakt. Logt u alstublieft in om hem aan te maken, of wacht tot een ingelogde gebruiker dat doet." }, + "deletedRoom": { + "heading": "Vergadering verwijderd", + "body": "Deze vergadering is verwijderd en u kunt er niet meer aan deelnemen." + }, "screenShare": { "title": "Kan uw scherm niet delen", "ariaLabel": "Kan uw scherm niet delen",