From 8044e3d6d8bca80ba97fe8561b00b45b2ec6a437 Mon Sep 17 00:00:00 2001 From: lebaudantoine Date: Mon, 8 Sep 2025 13:17:03 +0200 Subject: [PATCH] =?UTF-8?q?=E2=99=BB=EF=B8=8F(backend)=20replace=20Django?= =?UTF-8?q?=20permissions=20with=20feature=20flag=20decorator?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Refactor feature flag mechanism from Django permission classes to custom decorator that returns 404 Not Found when features are disabled instead of exposing API structure through permission errors. Improves security by preventing information disclosure about disabled features and provides more appropriate response semantics. Custom decorator approach is better suited for feature toggling than Django's permission system which is designed for authorization. --- src/backend/core/api/feature_flag.py | 45 +++++++++++++++++++ src/backend/core/api/permissions.py | 32 ------------- src/backend/core/api/viewsets.py | 9 ++-- .../test_api_recordings_storage_hook.py | 3 +- .../rooms/test_api_rooms_start_recording.py | 7 +-- .../rooms/test_api_rooms_stop_recording.py | 7 +-- .../tests/rooms/test_api_rooms_subtitle.py | 4 +- 7 files changed, 62 insertions(+), 45 deletions(-) create mode 100644 src/backend/core/api/feature_flag.py diff --git a/src/backend/core/api/feature_flag.py b/src/backend/core/api/feature_flag.py new file mode 100644 index 00000000..b137a7ea --- /dev/null +++ b/src/backend/core/api/feature_flag.py @@ -0,0 +1,45 @@ +"""Feature flag handler for the Meet core app.""" + +from functools import wraps + +from django.conf import settings +from django.http import Http404 + + +class FeatureFlag: + """Check if features are enabled and return error responses.""" + + FLAGS = { + "recording": "RECORDING_ENABLE", + "storage_event": "RECORDING_STORAGE_EVENT_ENABLE", + "subtitle": "ROOM_SUBTITLE_ENABLED", + } + + @classmethod + def flag_is_active(cls, flag_name): + """Check if a feature flag is active.""" + + setting_name = cls.FLAGS.get(flag_name) + + if setting_name is None: + return False + + return getattr(settings, setting_name, False) + + @classmethod + def require(cls, flag_name): + """Decorator to check feature at the beginning of endpoint methods.""" + + if flag_name not in cls.FLAGS: + raise ValueError(f"Unknown feature flag: {flag_name}") + + def decorator(view_func): + @wraps(view_func) + def wrapper(self, request, *args, **kwargs): + if not cls.flag_is_active(flag_name): + raise Http404 + return view_func(self, request, *args, **kwargs) + + return wrapper + + return decorator diff --git a/src/backend/core/api/permissions.py b/src/backend/core/api/permissions.py index 68dff17e..ce1f1934 100644 --- a/src/backend/core/api/permissions.py +++ b/src/backend/core/api/permissions.py @@ -1,7 +1,5 @@ """Permission handlers for the Meet core app.""" -from django.conf import settings - from rest_framework import permissions from ..models import RoleChoices @@ -101,36 +99,6 @@ class HasPrivilegesOnRoom(IsAuthenticated): return obj.is_administrator_or_owner(request.user) -class IsRecordingEnabled(permissions.BasePermission): - """Check if the recording feature is enabled.""" - - message = "Access denied, recording is disabled." - - def has_permission(self, request, view): - """Determine if access is allowed based on settings.""" - return settings.RECORDING_ENABLE - - -class IsStorageEventEnabled(permissions.BasePermission): - """Check if the storage event feature is enabled.""" - - message = "Access denied, storage event is disabled." - - def has_permission(self, request, view): - """Determine if access is allowed based on settings.""" - return settings.RECORDING_STORAGE_EVENT_ENABLE - - -class IsSubtitleEnabled(permissions.BasePermission): - """Check if the subtitle feature is enabled.""" - - message = "Access denied, subtitles are disabled." - - def has_permission(self, request, view): - """Determine if access is allowed based on settings.""" - return settings.ROOM_SUBTITLE_ENABLED - - class HasLiveKitRoomAccess(permissions.BasePermission): """Check if authenticated user's LiveKit token is for the specific room.""" diff --git a/src/backend/core/api/viewsets.py b/src/backend/core/api/viewsets.py index b41ce739..55578e9e 100644 --- a/src/backend/core/api/viewsets.py +++ b/src/backend/core/api/viewsets.py @@ -59,6 +59,7 @@ from core.services.subtitle import SubtitleException, SubtitleService from ..authentication.livekit import LiveKitTokenAuthentication from . import permissions, serializers +from .feature_flag import FeatureFlag # pylint: disable=too-many-ancestors @@ -293,9 +294,9 @@ class RoomViewSet( url_path="start-recording", permission_classes=[ permissions.HasPrivilegesOnRoom, - permissions.IsRecordingEnabled, ], ) + @FeatureFlag.require("recording") def start_room_recording(self, request, pk=None): # pylint: disable=unused-argument """Start recording a room.""" @@ -338,9 +339,9 @@ class RoomViewSet( url_path="stop-recording", permission_classes=[ permissions.HasPrivilegesOnRoom, - permissions.IsRecordingEnabled, ], ) + @FeatureFlag.require("recording") def stop_room_recording(self, request, pk=None): # pylint: disable=unused-argument """Stop room recording.""" @@ -542,11 +543,11 @@ class RoomViewSet( methods=["post"], url_path="start-subtitle", permission_classes=[ - permissions.IsSubtitleEnabled, permissions.HasLiveKitRoomAccess, ], authentication_classes=[LiveKitTokenAuthentication], ) + @FeatureFlag.require("subtitle") def start_subtitle(self, request, pk=None): # pylint: disable=unused-argument """Start realtime transcription for the room. @@ -732,8 +733,8 @@ class RecordingViewSet( methods=["post"], url_path="storage-hook", authentication_classes=[StorageEventAuthentication], - permission_classes=[permissions.IsStorageEventEnabled], ) + @FeatureFlag.require("storage_event") def on_storage_event_received(self, request, pk=None): # pylint: disable=unused-argument """Handle incoming storage hook events for recordings.""" diff --git a/src/backend/core/tests/recording/test_api_recordings_storage_hook.py b/src/backend/core/tests/recording/test_api_recordings_storage_hook.py index 5d923406..f822a70c 100644 --- a/src/backend/core/tests/recording/test_api_recordings_storage_hook.py +++ b/src/backend/core/tests/recording/test_api_recordings_storage_hook.py @@ -77,7 +77,8 @@ def test_save_recording_permission_needed(settings, client): HTTP_AUTHORIZATION="Bearer testAuthToken", ) - assert response.status_code == 403 + assert response.status_code == 404 + assert response.json() == {"detail": "Not found."} def test_save_recording_parsing_error(recording_settings, mock_get_parser, client): diff --git a/src/backend/core/tests/rooms/test_api_rooms_start_recording.py b/src/backend/core/tests/rooms/test_api_rooms_start_recording.py index 7b211317..9d3b1a73 100644 --- a/src/backend/core/tests/rooms/test_api_rooms_start_recording.py +++ b/src/backend/core/tests/rooms/test_api_rooms_start_recording.py @@ -55,8 +55,9 @@ def test_start_recording_anonymous(): assert Recording.objects.count() == 0 -def test_start_recording_non_owner_and_non_administrator(): +def test_start_recording_non_owner_and_non_administrator(settings): """Non-owner and Non-Administrator users should not be allowed to start room recordings.""" + settings.RECORDING_ENABLE = True room = RoomFactory() user = UserFactory() client = APIClient() @@ -88,8 +89,8 @@ def test_start_recording_recording_disabled(settings): {"mode": "screen_recording"}, ) - assert response.status_code == 403 - assert response.json() == {"detail": "Access denied, recording is disabled."} + assert response.status_code == 404 + assert response.json() == {"detail": "Not found."} assert Recording.objects.count() == 0 diff --git a/src/backend/core/tests/rooms/test_api_rooms_stop_recording.py b/src/backend/core/tests/rooms/test_api_rooms_stop_recording.py index 89dc9b93..b003e49e 100644 --- a/src/backend/core/tests/rooms/test_api_rooms_stop_recording.py +++ b/src/backend/core/tests/rooms/test_api_rooms_stop_recording.py @@ -54,8 +54,9 @@ def test_stop_recording_anonymous(): assert Recording.objects.filter(status=RecordingStatusChoices.ACTIVE).count() == 1 -def test_stop_recording_non_owner_and_non_administrator(): +def test_stop_recording_non_owner_and_non_administrator(settings): """Non-owner and Non-Administrator users should not be allowed to stop room recordings.""" + settings.RECORDING_ENABLE = True room = RoomFactory() user = UserFactory() RecordingFactory(room=room, status=RecordingStatusChoices.ACTIVE) @@ -84,8 +85,8 @@ def test_stop_recording_recording_disabled(settings): response = client.post(f"/api/v1.0/rooms/{room.id}/stop-recording/") - assert response.status_code == 403 - assert response.json() == {"detail": "Access denied, recording is disabled."} + assert response.status_code == 404 + assert response.json() == {"detail": "Not found."} # Verify no recording exists assert Recording.objects.count() == 0 diff --git a/src/backend/core/tests/rooms/test_api_rooms_subtitle.py b/src/backend/core/tests/rooms/test_api_rooms_subtitle.py index 6f97137d..9178692f 100644 --- a/src/backend/core/tests/rooms/test_api_rooms_subtitle.py +++ b/src/backend/core/tests/rooms/test_api_rooms_subtitle.py @@ -128,8 +128,8 @@ def test_start_subtitle_disabled_by_default(mock_livekit_token): {"token": mock_livekit_token}, ) - assert response.status_code == 403 - assert response.json() == {"detail": "Access denied, subtitles are disabled."} + assert response.status_code == 404 + assert response.json() == {"detail": "Not found."} def test_start_subtitle_valid_token(