From d46127066dbb6defc8373daf60970d2baa640cc1 Mon Sep 17 00:00:00 2001 From: leo <260626284+cameledev@users.noreply.github.com> Date: Wed, 30 Sep 2026 18:02:24 +0200 Subject: [PATCH] =?UTF-8?q?=E2=9C=A8(rooms)=20soft=20delete=20rooms=20from?= =?UTF-8?q?=20internal=20and=20external=20API?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Before this commit: 1. Rooms could be hard-deleted from the internal API (unused by the frontend). 2. The external API had no way to delete a room. 3. The admin could hard-delete rooms. What this commit does: 1. Add a `deleted_at` field to Room. `Room.objects` hides soft-deleted rooms; `all_objects` stays the default manager so deleted rooms keep reserving their slug and PIN code. 2. Soft delete rooms from the internal API (replacing hard delete) and the external API (new DELETE, owner only, `rooms:delete` scope). The LiveKit room is closed and its participants disconnected; if closing fails, the soft delete is rolled back and an error is returned. 3. Answer 410 Gone on detail operations on a deleted room (after permission checks). Its can still be read but not modified. 4. Close LiveKit rooms recreated after deletion by participants still holding a valid token (room_started webhook). 5. Frontend: participants in the call land on the feedback page without the rejoin button, lobby participants see a "meeting deleted" screen, and opening the link shows an error instead of recreating the room. Untouched by this commit: - The admin still hard-deletes rooms, without closing the LiveKit room, and lists soft-deleted rooms alongside active ones. - Purging soft-deleted rooms (hard deletion) is not handled. - Scheduled deletion is not handled. --- CHANGELOG.md | 1 + UPGRADE.md | 8 + src/backend/core/admin.py | 11 +- src/backend/core/analytics/events.py | 1 + src/backend/core/api/exceptions.py | 30 ++ src/backend/core/api/serializers.py | 2 + src/backend/core/api/viewsets.py | 38 ++- src/backend/core/external_api/viewsets.py | 32 ++- .../core/migrations/0027_room_deleted_at.py | 29 ++ src/backend/core/models.py | 52 +++- src/backend/core/services/livekit_events.py | 43 ++- src/backend/core/services/room_management.py | 38 +++ src/backend/core/services/sip_management.py | 19 +- .../core/tests/rooms/test_api_rooms_create.py | 23 ++ .../core/tests/rooms/test_api_rooms_delete.py | 94 ++++++- .../test_api_rooms_participants_management.py | 3 +- .../tests/rooms/test_api_rooms_retrieve.py | 22 ++ .../tests/services/test_livekit_events.py | 62 ++++- .../tests/services/test_room_management.py | 147 +++++++++- .../tests/services/test_sip_management.py | 25 ++ .../core/tests/test_api_resource_accesses.py | 98 +++++++ .../core/tests/test_external_api_rooms.py | 260 +++++++++++++++++- .../src/features/rooms/api/requestEntry.ts | 2 + .../features/rooms/components/Conference.tsx | 13 + .../src/features/rooms/components/Lobby.tsx | 21 +- .../src/features/rooms/hooks/useLobby.ts | 42 +-- .../src/features/rooms/routes/Feedback.tsx | 8 +- src/frontend/src/locales/de/rooms.json | 11 +- src/frontend/src/locales/en/rooms.json | 11 +- src/frontend/src/locales/es/rooms.json | 11 +- src/frontend/src/locales/fr/rooms.json | 11 +- src/frontend/src/locales/nl/rooms.json | 11 +- 32 files changed, 1123 insertions(+), 56 deletions(-) create mode 100644 src/backend/core/api/exceptions.py create mode 100644 src/backend/core/migrations/0027_room_deleted_at.py 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",