From 01f4d05d6bc86472d067e1c8bdee801673f681e8 Mon Sep 17 00:00:00 2001 From: lebaudantoine Date: Sat, 15 Feb 2025 14:08:40 +0100 Subject: [PATCH] =?UTF-8?q?=E2=99=BB=EF=B8=8F(backend)=20replace=20is=5Fpu?= =?UTF-8?q?blic=20with=20access=5Flevel=20field?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Replace unused is_public boolean field with access_level to allow for more granular control. Initially maintains public/restricted functionality while enabling future addition of "trusted" access level. --- src/backend/core/api/serializers.py | 6 +-- src/backend/core/factories.py | 3 +- ...ve_resource_is_public_room_access_level.py | 22 +++++++++++ src/backend/core/models.py | 14 ++++++- .../recording/test_api_recordings_list.py | 2 +- .../core/tests/rooms/test_api_rooms_list.py | 17 +++++---- .../tests/rooms/test_api_rooms_retrieve.py | 33 ++++++++-------- .../core/tests/rooms/test_api_rooms_update.py | 20 ++++++---- .../core/tests/test_api_resource_accesses.py | 38 ++++++++++--------- src/backend/core/tests/test_models_rooms.py | 6 +-- src/backend/meet/settings.py | 4 +- .../src/features/rooms/api/ApiRoom.ts | 2 +- 12 files changed, 105 insertions(+), 62 deletions(-) create mode 100644 src/backend/core/migrations/0011_remove_resource_is_public_room_access_level.py diff --git a/src/backend/core/api/serializers.py b/src/backend/core/api/serializers.py index 51b37fbb..252f8556 100644 --- a/src/backend/core/api/serializers.py +++ b/src/backend/core/api/serializers.py @@ -92,7 +92,7 @@ class ListRoomSerializer(serializers.ModelSerializer): class Meta: model = models.Room - fields = ["id", "name", "slug", "is_public"] + fields = ["id", "name", "slug", "access_level"] read_only_fields = ["id", "slug"] @@ -101,7 +101,7 @@ class RoomSerializer(serializers.ModelSerializer): class Meta: model = models.Room - fields = ["id", "name", "slug", "configuration", "is_public"] + fields = ["id", "name", "slug", "configuration", "access_level"] read_only_fields = ["id", "slug"] def to_representation(self, instance): @@ -129,7 +129,7 @@ class RoomSerializer(serializers.ModelSerializer): if not is_admin: del output["configuration"] - if role is not None or instance.is_public: + if role is not None or instance.access_level == models.RoomAccessLevel.PUBLIC: slug = f"{instance.id!s}" username = request.query_params.get("username", None) diff --git a/src/backend/core/factories.py b/src/backend/core/factories.py index 74cfcf29..f075dde7 100644 --- a/src/backend/core/factories.py +++ b/src/backend/core/factories.py @@ -36,8 +36,6 @@ class ResourceFactory(factory.django.DjangoModelFactory): model = models.Resource skip_postgeneration_save = True - is_public = factory.Faker("boolean", chance_of_getting_true=50) - @factory.post_generation def users(self, create, extracted, **kwargs): """Add users to resource from a given list of users.""" @@ -70,6 +68,7 @@ class RoomFactory(ResourceFactory): name = factory.Faker("catch_phrase") slug = factory.LazyAttribute(lambda o: slugify(o.name)) + access_level = factory.fuzzy.FuzzyChoice(models.RoomAccessLevel) class RecordingFactory(factory.django.DjangoModelFactory): diff --git a/src/backend/core/migrations/0011_remove_resource_is_public_room_access_level.py b/src/backend/core/migrations/0011_remove_resource_is_public_room_access_level.py new file mode 100644 index 00000000..2319c1fb --- /dev/null +++ b/src/backend/core/migrations/0011_remove_resource_is_public_room_access_level.py @@ -0,0 +1,22 @@ +# Generated by Django 5.1.5 on 2025-02-16 11:42 + +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ('core', '0010_alter_resourceaccess_options_alter_user_options'), + ] + + operations = [ + migrations.RemoveField( + model_name='resource', + name='is_public', + ), + migrations.AddField( + model_name='room', + name='access_level', + field=models.CharField(choices=[('public', 'Public Access'), ('restricted', 'Restricted Access')], default='public', max_length=50), + ), + ] diff --git a/src/backend/core/models.py b/src/backend/core/models.py index 53653143..d6add13d 100644 --- a/src/backend/core/models.py +++ b/src/backend/core/models.py @@ -80,6 +80,13 @@ class RecordingModeChoices(models.TextChoices): TRANSCRIPT = "transcript", _("TRANSCRIPT") +class RoomAccessLevel(models.TextChoices): + """Room access level choices.""" + + PUBLIC = "public", _("Public Access") + RESTRICTED = "restricted", _("Restricted Access") + + class BaseModel(models.Model): """ Serves as an abstract base model for other models, ensuring that records are validated @@ -241,7 +248,6 @@ def get_resource_roles(resource: models.Model, user: User) -> List[str]: class Resource(BaseModel): """Model to define access control""" - is_public = models.BooleanField(default=settings.RESOURCE_DEFAULT_IS_PUBLIC) users = models.ManyToManyField( User, through="ResourceAccess", @@ -361,7 +367,11 @@ class Room(Resource): primary_key=True, ) slug = models.SlugField(max_length=100, blank=True, null=True, unique=True) - + access_level = models.CharField( + max_length=50, + choices=RoomAccessLevel.choices, + default=settings.RESOURCE_DEFAULT_ACCESS_LEVEL, + ) configuration = models.JSONField( blank=True, default=dict, diff --git a/src/backend/core/tests/recording/test_api_recordings_list.py b/src/backend/core/tests/recording/test_api_recordings_list.py index 90318727..08e71451 100644 --- a/src/backend/core/tests/recording/test_api_recordings_list.py +++ b/src/backend/core/tests/recording/test_api_recordings_list.py @@ -59,8 +59,8 @@ def test_api_recordings_list_authenticated_direct(role): "id": str(recording.id), "created_at": recording.created_at.isoformat().replace("+00:00", "Z"), "room": { + "access_level": str(room.access_level), "id": str(room.id), - "is_public": room.is_public, "name": room.name, "slug": room.slug, }, diff --git a/src/backend/core/tests/rooms/test_api_rooms_list.py b/src/backend/core/tests/rooms/test_api_rooms_list.py index 29a84ddb..6d600a30 100644 --- a/src/backend/core/tests/rooms/test_api_rooms_list.py +++ b/src/backend/core/tests/rooms/test_api_rooms_list.py @@ -9,14 +9,15 @@ from rest_framework.pagination import PageNumberPagination from rest_framework.test import APIClient from ...factories import RoomFactory, UserFactory +from ...models import RoomAccessLevel pytestmark = pytest.mark.django_db def test_api_rooms_list_anonymous(): """Anonymous users should not be able to list rooms.""" - RoomFactory(is_public=False) - RoomFactory(is_public=True) + RoomFactory(access_level=RoomAccessLevel.PUBLIC) + RoomFactory(access_level=RoomAccessLevel.RESTRICTED) client = APIClient() @@ -38,10 +39,12 @@ def test_api_rooms_list_authenticated(): other_user = UserFactory() - RoomFactory(is_public=False) - RoomFactory(is_public=True) - room_user_accesses = RoomFactory(is_public=False, users=[user]) - RoomFactory(is_public=False, users=[other_user]) + RoomFactory(access_level=RoomAccessLevel.PUBLIC) + RoomFactory(access_level=RoomAccessLevel.RESTRICTED) + room_user_accesses = RoomFactory( + access_level=RoomAccessLevel.RESTRICTED, users=[user] + ) + RoomFactory(access_level=RoomAccessLevel.RESTRICTED, users=[other_user]) response = client.get( "/api/v1.0/rooms/", @@ -105,7 +108,7 @@ def test_api_rooms_list_authenticated_distinct(): client = APIClient() client.force_login(user) - room = RoomFactory(is_public=True, users=[user, other_user]) + room = RoomFactory(access_level=RoomAccessLevel.PUBLIC, users=[user, other_user]) response = client.get( "/api/v1.0/rooms/", 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 6fd09000..28aa9a61 100644 --- a/src/backend/core/tests/rooms/test_api_rooms_retrieve.py +++ b/src/backend/core/tests/rooms/test_api_rooms_retrieve.py @@ -12,6 +12,7 @@ import pytest from rest_framework.test import APIClient from ...factories import RoomFactory, UserFactory, UserResourceAccessFactory +from ...models import RoomAccessLevel pytestmark = pytest.mark.django_db @@ -21,15 +22,15 @@ def test_api_rooms_retrieve_anonymous_private_pk(): Anonymous users should be allowed to retrieve a private room but should not be given any token. """ - room = RoomFactory(is_public=False) + room = RoomFactory(access_level=RoomAccessLevel.RESTRICTED) client = APIClient() response = client.get(f"/api/v1.0/rooms/{room.id!s}/") assert response.status_code == 200 assert response.json() == { + "access_level": "restricted", "id": str(room.id), "is_administrable": False, - "is_public": False, "name": room.name, "slug": room.slug, } @@ -37,7 +38,7 @@ def test_api_rooms_retrieve_anonymous_private_pk(): def test_api_rooms_retrieve_anonymous_private_pk_no_dashes(): """It should be possible to get a room by its id stripped of its dashes.""" - room = RoomFactory(is_public=False) + room = RoomFactory(access_level=RoomAccessLevel.RESTRICTED) id_no_dashes = str(room.id) client = APIClient() @@ -45,9 +46,9 @@ def test_api_rooms_retrieve_anonymous_private_pk_no_dashes(): assert response.status_code == 200 assert response.json() == { + "access_level": "restricted", "id": str(room.id), "is_administrable": False, - "is_public": False, "name": room.name, "slug": room.slug, } @@ -55,15 +56,15 @@ def test_api_rooms_retrieve_anonymous_private_pk_no_dashes(): def test_api_rooms_retrieve_anonymous_private_slug(): """It should be possible to get a room by its slug.""" - room = RoomFactory(is_public=False) + room = RoomFactory(access_level=RoomAccessLevel.RESTRICTED) client = APIClient() response = client.get(f"/api/v1.0/rooms/{room.slug!s}/") assert response.status_code == 200 assert response.json() == { + "access_level": "restricted", "id": str(room.id), "is_administrable": False, - "is_public": False, "name": room.name, "slug": room.slug, } @@ -71,15 +72,15 @@ def test_api_rooms_retrieve_anonymous_private_slug(): def test_api_rooms_retrieve_anonymous_private_slug_not_normalized(): """Getting a room by a slug that is not normalized should work.""" - room = RoomFactory(name="Réunion", is_public=False) + room = RoomFactory(name="Réunion", access_level=RoomAccessLevel.RESTRICTED) client = APIClient() response = client.get("/api/v1.0/rooms/Réunion/") assert response.status_code == 200 assert response.json() == { + "access_level": "restricted", "id": str(room.id), "is_administrable": False, - "is_public": False, "name": room.name, "slug": room.slug, } @@ -173,16 +174,16 @@ def test_api_rooms_retrieve_anonymous_public(mock_token): """ Anonymous users should be able to retrieve a room with a token provided it is public. """ - room = RoomFactory(is_public=True) + room = RoomFactory(access_level=RoomAccessLevel.PUBLIC) client = APIClient() response = client.get(f"/api/v1.0/rooms/{room.id!s}/") assert response.status_code == 200 expected_name = f"{room.id!s}" assert response.json() == { + "access_level": str(room.access_level), "id": str(room.id), "is_administrable": False, - "is_public": True, "livekit": { "url": "test_url_value", "room": expected_name, @@ -209,7 +210,7 @@ def test_api_rooms_retrieve_authenticated_public(mock_token): which they are not related, provided the room is public. They should not see related users. """ - room = RoomFactory(is_public=True) + room = RoomFactory(access_level=RoomAccessLevel.PUBLIC) user = UserFactory() client = APIClient() @@ -222,9 +223,9 @@ def test_api_rooms_retrieve_authenticated_public(mock_token): expected_name = f"{room.id!s}" assert response.json() == { + "access_level": str(room.access_level), "id": str(room.id), "is_administrable": False, - "is_public": True, "livekit": { "url": "test_url_value", "room": expected_name, @@ -242,7 +243,7 @@ def test_api_rooms_retrieve_authenticated(): Authenticated users should be allowed to retrieve a private room to which they are not related but should not be given any token. """ - room = RoomFactory(is_public=False) + room = RoomFactory(access_level=RoomAccessLevel.RESTRICTED) user = UserFactory() client = APIClient() @@ -254,9 +255,9 @@ def test_api_rooms_retrieve_authenticated(): assert response.status_code == 200 assert response.json() == { + "access_level": "restricted", "id": str(room.id), "is_administrable": False, - "is_public": False, "name": room.name, "slug": room.slug, } @@ -324,9 +325,9 @@ def test_api_rooms_retrieve_members(mock_token, django_assert_num_queries): expected_name = str(room.id) assert content_dict == { + "access_level": str(room.access_level), "id": str(room.id), "is_administrable": False, - "is_public": room.is_public, "livekit": { "url": "test_url_value", "room": expected_name, @@ -400,9 +401,9 @@ def test_api_rooms_retrieve_administrators(mock_token, django_assert_num_queries ) expected_name = str(room.id) assert content_dict == { + "access_level": str(room.access_level), "id": str(room.id), "is_administrable": True, - "is_public": room.is_public, "configuration": {}, "livekit": { "url": "test_url_value", diff --git a/src/backend/core/tests/rooms/test_api_rooms_update.py b/src/backend/core/tests/rooms/test_api_rooms_update.py index fb682781..da3020a1 100644 --- a/src/backend/core/tests/rooms/test_api_rooms_update.py +++ b/src/backend/core/tests/rooms/test_api_rooms_update.py @@ -8,6 +8,7 @@ import pytest from rest_framework.test import APIClient from ...factories import RoomFactory, UserFactory +from ...models import RoomAccessLevel pytestmark = pytest.mark.django_db @@ -54,17 +55,18 @@ def test_api_rooms_update_members(): not be allowed to update it. """ user = UserFactory() - room = RoomFactory(name="Old name", users=[(user, "member")]) + room = RoomFactory( + access_level=RoomAccessLevel.PUBLIC, name="Old name", users=[(user, "member")] + ) client = APIClient() client.force_login(user) - new_is_public = not room.is_public response = client.put( f"/api/v1.0/rooms/{room.id!s}/", { "name": "New name", "slug": "should-be-ignored", - "is_public": new_is_public, + "access_level": RoomAccessLevel.RESTRICTED, "configuration": {"the_key": "the_value"}, }, format="json", @@ -73,24 +75,26 @@ def test_api_rooms_update_members(): room.refresh_from_db() assert room.name == "Old name" assert room.slug == "old-name" - assert room.is_public != new_is_public + assert room.access_level != RoomAccessLevel.RESTRICTED assert room.configuration == {} def test_api_rooms_update_administrators(): """Administrators or owners of a room should be allowed to update it.""" user = UserFactory() - room = RoomFactory(users=[(user, random.choice(["administrator", "owner"]))]) + room = RoomFactory( + access_level=RoomAccessLevel.RESTRICTED, + users=[(user, random.choice(["administrator", "owner"]))], + ) client = APIClient() client.force_login(user) - new_is_public = not room.is_public response = client.put( f"/api/v1.0/rooms/{room.id!s}/", { "name": "New name", "slug": "should-be-ignored", - "is_public": new_is_public, + "access_level": RoomAccessLevel.PUBLIC, "configuration": {"the_key": "the_value"}, }, format="json", @@ -99,7 +103,7 @@ def test_api_rooms_update_administrators(): room.refresh_from_db() assert room.name == "New name" assert room.slug == "new-name" - assert room.is_public == new_is_public + assert room.access_level == RoomAccessLevel.PUBLIC assert room.configuration == {"the_key": "the_value"} diff --git a/src/backend/core/tests/test_api_resource_accesses.py b/src/backend/core/tests/test_api_resource_accesses.py index 3d7b0ec2..9ac1ad1d 100644 --- a/src/backend/core/tests/test_api_resource_accesses.py +++ b/src/backend/core/tests/test_api_resource_accesses.py @@ -16,7 +16,7 @@ from ..factories import ( UserFactory, UserResourceAccessFactory, ) -from ..models import ResourceAccess, RoleChoices +from ..models import ResourceAccess, RoleChoices, RoomAccessLevel pytestmark = pytest.mark.django_db @@ -44,13 +44,13 @@ def test_api_room_user_accesses_list_authenticated_not_related(): client = APIClient() client.force_login(user) - public_room = RoomFactory(is_public=True) + public_room = RoomFactory(access_level=RoomAccessLevel.PUBLIC) UserResourceAccessFactory(resource=public_room) UserResourceAccessFactory(resource=public_room, role="member") UserResourceAccessFactory(resource=public_room, role="administrator") UserResourceAccessFactory(resource=public_room, role="owner") - private_room = RoomFactory(is_public=False) + private_room = RoomFactory(access_level=RoomAccessLevel.RESTRICTED) UserResourceAccessFactory(resource=private_room) UserResourceAccessFactory(resource=private_room, role="member") UserResourceAccessFactory(resource=private_room, role="administrator") @@ -73,13 +73,17 @@ def test_api_room_user_accesses_list_authenticated_member(): client = APIClient() client.force_login(user) - public_room = RoomFactory(is_public=True, users=[(user, "member")]) + public_room = RoomFactory( + access_level=RoomAccessLevel.PUBLIC, users=[(user, "member")] + ) UserResourceAccessFactory(resource=public_room) UserResourceAccessFactory(resource=public_room, role="member") UserResourceAccessFactory(resource=public_room, role="administrator") UserResourceAccessFactory(resource=public_room, role="owner") - private_room = RoomFactory(is_public=False, users=[(user, "member")]) + private_room = RoomFactory( + access_level=RoomAccessLevel.RESTRICTED, users=[(user, "member")] + ) UserResourceAccessFactory(resource=private_room) UserResourceAccessFactory(resource=private_room, role="member") UserResourceAccessFactory(resource=private_room, role="administrator") @@ -102,7 +106,7 @@ def test_api_room_user_accesses_list_authenticated_administrator(): client = APIClient() client.force_login(user) - public_room = RoomFactory(is_public=True) + public_room = RoomFactory(access_level=RoomAccessLevel.PUBLIC) public_room_accesses = ( # Access for the logged-in user UserResourceAccessFactory( @@ -115,7 +119,7 @@ def test_api_room_user_accesses_list_authenticated_administrator(): UserResourceAccessFactory(resource=public_room, role="owner"), ) - private_room = RoomFactory(is_public=False) + private_room = RoomFactory(access_level=RoomAccessLevel.RESTRICTED) private_room_accesses = ( # Access for the logged-in user UserResourceAccessFactory( @@ -148,7 +152,7 @@ def test_api_room_user_accesses_list_authenticated_owner(): client = APIClient() client.force_login(user) - public_room = RoomFactory(is_public=True) + public_room = RoomFactory(access_level=RoomAccessLevel.PUBLIC) public_room_accesses = ( # Access for the logged-in user UserResourceAccessFactory(resource=public_room, user=user, role="owner"), @@ -158,7 +162,7 @@ def test_api_room_user_accesses_list_authenticated_owner(): UserResourceAccessFactory(resource=public_room, role="administrator"), UserResourceAccessFactory(resource=public_room, role="owner"), ) - private_room = RoomFactory(is_public=False) + private_room = RoomFactory(access_level=RoomAccessLevel.RESTRICTED) private_room_accesses = ( # Access for the logged-in user UserResourceAccessFactory(resource=private_room, user=user, role="owner"), @@ -252,8 +256,8 @@ def test_api_room_user_accesses_retrieve_authenticated_not_related(): client = APIClient() client.force_login(user) - for is_public in [True, False]: - room = RoomFactory(is_public=is_public) + for access_level in [RoomAccessLevel.PUBLIC, RoomAccessLevel.RESTRICTED]: + room = RoomFactory(access_level=access_level) assert len(RoleChoices.choices) == 3 for role, _name in RoleChoices.choices: @@ -277,9 +281,9 @@ def test_api_room_user_accesses_retrieve_authenticated_member(): client = APIClient() client.force_login(user) - for is_public in [True, False]: + for access_level in [RoomAccessLevel.PUBLIC, RoomAccessLevel.RESTRICTED]: room = RoomFactory( - is_public=is_public, + access_level=access_level, users=[(user, "member")], ) assert len(RoleChoices.choices) == 3 @@ -305,8 +309,8 @@ def test_api_room_user_accesses_retrieve_authenticated_administrator(): client = APIClient() client.force_login(user) - for is_public in [True, False]: - room = RoomFactory(is_public=is_public, users=[(user, "administrator")]) + for access_level in [RoomAccessLevel.PUBLIC, RoomAccessLevel.RESTRICTED]: + room = RoomFactory(access_level=access_level, users=[(user, "administrator")]) assert len(RoleChoices.choices) == 3 for role, _name in RoleChoices.choices: @@ -334,8 +338,8 @@ def test_api_room_user_accesses_retrieve_authenticated_owner(): client = APIClient() client.force_login(user) - for is_public in [True, False]: - room = RoomFactory(is_public=is_public, users=[(user, "owner")]) + for access_level in [RoomAccessLevel.PUBLIC, RoomAccessLevel.RESTRICTED]: + room = RoomFactory(access_level=access_level, users=[(user, "owner")]) assert len(RoleChoices.choices) == 3 for role, _name in RoleChoices.choices: diff --git a/src/backend/core/tests/test_models_rooms.py b/src/backend/core/tests/test_models_rooms.py index 8879bfca..ad736eae 100644 --- a/src/backend/core/tests/test_models_rooms.py +++ b/src/backend/core/tests/test_models_rooms.py @@ -8,7 +8,7 @@ from django.core.exceptions import ValidationError import pytest from core.factories import RoomFactory, UserFactory -from core.models import Room +from core.models import Room, RoomAccessLevel pytestmark = pytest.mark.django_db @@ -80,10 +80,10 @@ def test_models_rooms_users(): assert list(room.users.all()) == [user] -def test_models_rooms_is_public_default(): +def test_models_rooms_access_level_default(): """A room should be public by default.""" room = Room.objects.create(name="room") - assert room.is_public is True + assert room.access_level == RoomAccessLevel.PUBLIC # Access rights methods diff --git a/src/backend/meet/settings.py b/src/backend/meet/settings.py index a39af6ce..fbadeba2 100755 --- a/src/backend/meet/settings.py +++ b/src/backend/meet/settings.py @@ -411,8 +411,8 @@ class Base(Configuration): ), "url": values.Value(environ_name="LIVEKIT_API_URL", environ_prefix=None), } - RESOURCE_DEFAULT_IS_PUBLIC = values.BooleanValue( - True, environ_name="RESOURCE_DEFAULT_IS_PUBLIC", environ_prefix=None + RESOURCE_DEFAULT_ACCESS_LEVEL = values.Value( + "public", environ_name="RESOURCE_DEFAULT_ACCESS_LEVEL", environ_prefix=None ) ALLOW_UNREGISTERED_ROOMS = values.BooleanValue( True, environ_name="ALLOW_UNREGISTERED_ROOMS", environ_prefix=None diff --git a/src/frontend/src/features/rooms/api/ApiRoom.ts b/src/frontend/src/features/rooms/api/ApiRoom.ts index 7e5404d0..5676cc99 100644 --- a/src/frontend/src/features/rooms/api/ApiRoom.ts +++ b/src/frontend/src/features/rooms/api/ApiRoom.ts @@ -2,8 +2,8 @@ export type ApiRoom = { id: string name: string slug: string - is_public: boolean is_administrable: boolean + access_level: string livekit?: { url: string room: string