From 906087a89e13278a9c175a984b678ffe6e2e4bd9 Mon Sep 17 00:00:00 2001 From: Alberto Islas Date: Tue, 15 Oct 2024 13:46:31 -0600 Subject: [PATCH 1/8] feat(api): Block v3 usage for new users Fixes: #4489 --- cl/api/api_permissions.py | 58 +++++++++++ cl/api/tests.py | 116 +++++++++++++++++++--- cl/api/utils.py | 6 +- cl/audio/api_views.py | 2 + cl/settings/third_party/rest_framework.py | 2 + 5 files changed, 169 insertions(+), 15 deletions(-) diff --git a/cl/api/api_permissions.py b/cl/api/api_permissions.py index 79f4d4d417..851c68ba47 100644 --- a/cl/api/api_permissions.py +++ b/cl/api/api_permissions.py @@ -1,4 +1,12 @@ +from django.conf import settings +from django.contrib.auth.models import User +from django.http import HttpRequest from rest_framework import permissions +from rest_framework.exceptions import PermissionDenied +from rest_framework.views import APIView + +from cl.api.utils import get_logging_prefix +from cl.lib.redis_utils import get_redis_interface class IsOwner(permissions.BasePermission): @@ -8,3 +16,53 @@ def has_object_permission(self, request, view, obj): if request.method in permissions.SAFE_METHODS: return True return obj.user == request.user + + +class V3APIPermission(permissions.BasePermission): + + r = get_redis_interface("STATS") + + def is_new_v3_user(self, user: User) -> bool: + """Check if the user is new for V3 by determining their presence in the + V3 API stats. + + :param user: The User to check. + :return: True if the user is new for V3, False otherwise. + """ + api_prefix = get_logging_prefix("v3") + set_key = f"{api_prefix}.user.counts" + score = self.r.zscore(set_key, user.id) + return score is None + + @staticmethod + def is_v3_api_request(request: HttpRequest) -> bool: + return getattr(request, "version", None) == "v3" + + def has_permission(self, request: HttpRequest, view: APIView) -> bool: + """Check if the user has permission to access the V3 API. + + :param request: The HTTPRequest object. + :param view: The APIview being accessed. + :return: True if the user has permission to access V3, False if not. + """ + + if ( + not self.is_v3_api_request(request) + or not settings.BLOCK_NEW_V3_USERS + ): + # Allow the request if it is not a V3 request + return True + + user = request.user + if not user.pk: + # Block V3 for Anonymous users. + raise PermissionDenied( + "Anonymous users don't have permission to access V3 of the API. " + "Please use V4 instead." + ) + if user.is_authenticated and self.is_new_v3_user(user): + raise PermissionDenied( + "As a new user, you don't have permission to access V3 of the API. " + "Please use V4 instead." + ) + return True diff --git a/cl/api/tests.py b/cl/api/tests.py index f550de3b4a..9bd20d1ca4 100644 --- a/cl/api/tests.py +++ b/cl/api/tests.py @@ -11,6 +11,7 @@ from django.contrib.humanize.templatetags.humanize import intcomma, ordinal from django.db import connection from django.http import HttpRequest, JsonResponse +from django.test import override_settings from django.test.client import AsyncClient, AsyncRequestFactory from django.test.utils import CaptureQueriesContext from django.urls import reverse @@ -24,6 +25,7 @@ from cl.api.factories import WebhookEventFactory, WebhookFactory from cl.api.models import WEBHOOK_EVENT_STATUS, WebhookEvent, WebhookEventType from cl.api.pagination import VersionBasedPagination +from cl.api.utils import get_logging_prefix from cl.api.views import coverage_data from cl.api.webhooks import send_webhook_event from cl.audio.api_views import AudioViewSet @@ -326,8 +328,8 @@ def flush_stats(self) -> None: if keys: self.r.delete(*keys) - async def hit_the_api(self) -> None: - path = reverse("audio-list", kwargs={"version": "v3"}) + async def hit_the_api(self, api_version) -> None: + path = reverse("audio-list", kwargs={"version": f"{api_version}"}) request = AsyncRequestFactory().get(path) # Create the view and change the milestones to be something we can test @@ -337,8 +339,7 @@ async def hit_the_api(self) -> None: # Set the attributes needed in the absence of middleware request.user = self.user - - await sync_to_async(view)(request) + await sync_to_async(view)(request, **{"version": f"{api_version}"}) @mock.patch( "cl.api.utils.get_logging_prefix", @@ -348,7 +349,7 @@ async def test_are_events_created_properly( self, mock_logging_prefix ) -> None: """Are event objects created as API requests are made?""" - await self.hit_the_api() + await self.hit_the_api("v3") expected_event_count = 1 self.assertEqual(expected_event_count, await Event.objects.acount()) @@ -357,36 +358,127 @@ async def test_are_events_created_properly( # run in parallel do not affect this one. @mock.patch( "cl.api.utils.get_logging_prefix", - return_value="api:Test", + side_effect=lambda *args, **kwargs: f"{get_logging_prefix(*args, **kwargs)}-Test", ) async def test_api_logged_correctly(self, mock_logging_prefix) -> None: # Global stats self.assertEqual(mock_logging_prefix.called, 0) - await self.hit_the_api() + await self.hit_the_api("v3") + self.assertEqual(mock_logging_prefix.called, 1) + self.assertEqual(int(self.r.get("api:v3-Test.count")), 1) + + # User stats + self.assertEqual( + self.r.zscore("api:v3-Test.user.counts", self.user.pk), 1.0 + ) + + # IP address + keys = self.r.keys("api:v3-Test.d:*") + ip_key = [k for k in keys if k.endswith("ip_map")][0] + self.assertEqual(self.r.hlen(ip_key), 1) + + # Endpoints + self.assertEqual( + self.r.zscore("api:v3-Test.endpoint.counts", self.endpoint_name), 1 + ) + + # Timings + self.assertAlmostEqual( + int(self.r.get("api:v3-Test.timing")), 10, delta=2000 + ) + + @mock.patch( + "cl.api.utils.get_logging_prefix", + side_effect=lambda *args, **kwargs: f"{get_logging_prefix(*args, **kwargs)}-Test", + ) + async def test_api_logged_correctly_v4(self, mock_logging_prefix) -> None: + # Global stats + self.assertEqual(mock_logging_prefix.called, 0) + await self.hit_the_api("v4") + self.assertEqual(mock_logging_prefix.called, 1) - self.assertEqual(int(self.r.get("api:Test.count")), 1) + self.assertEqual(int(self.r.get("api:v4-Test.count")), 1) # User stats self.assertEqual( - self.r.zscore("api:Test.user.counts", self.user.pk), 1.0 + self.r.zscore("api:v4-Test.user.counts", self.user.pk), 1.0 ) # IP address - keys = self.r.keys("api:Test.d:*") + keys = self.r.keys("api:v4-Test.d:*") ip_key = [k for k in keys if k.endswith("ip_map")][0] self.assertEqual(self.r.hlen(ip_key), 1) # Endpoints self.assertEqual( - self.r.zscore("api:Test.endpoint.counts", self.endpoint_name), 1 + self.r.zscore("api:v4-Test.endpoint.counts", self.endpoint_name), 1 ) # Timings self.assertAlmostEqual( - int(self.r.get("api:Test.timing")), 10, delta=2000 + int(self.r.get("api:v4-Test.timing")), 10, delta=2000 ) +@override_settings(BLOCK_NEW_V3_USERS=True) +@mock.patch( + "cl.api.api_permissions.get_logging_prefix", return_value="api-block:v3" +) +class BlockV3APITests(TestCase): + """Check that V3 API is restricted for anonymous and new users.""" + + @classmethod + def setUpTestData(cls) -> None: + cls.user_1 = UserFactory() + cls.user_2 = UserFactory() + cls.client_1 = make_client(cls.user_1.pk) + cls.client_2 = make_client(cls.user_2.pk) + + cls.audio_path_v3 = reverse("audio-list", kwargs={"version": "v3"}) + cls.audio_path_v4 = reverse("audio-list", kwargs={"version": "v4"}) + + def setUp(self) -> None: + self.r = get_redis_interface("STATS") + self.flush_stats() + + def tearDown(self) -> None: + self.flush_stats() + + def flush_stats(self) -> None: + keys = self.r.keys("api-block:*") + if keys: + self.r.delete(*keys) + + async def test_block_v3_for_new_users(self, mock_api_prefix) -> None: + """Confirm new v3 API users are blocked""" + response = await self.client_1.get(self.audio_path_v3, format="json") + self.assertEqual(response.status_code, HTTPStatus.FORBIDDEN) + self.assertEqual( + response.json()["detail"], + "As a new user, you don't have permission to access V3 of the API. Please use V4 instead.", + ) + + async def test_allow_v3_for_existing_users(self, mock_api_prefix) -> None: + """Confirm that existing v3 API users are granted access to use it""" + self.r.zincrby("api-block:v3.user.counts", 1, self.user_2.pk) + response = await self.client_2.get(self.audio_path_v3, format="json") + self.assertEqual(response.status_code, HTTPStatus.OK) + + async def test_block_v3_for_anonymous_users(self, mock_api_prefix) -> None: + """Confirm anonymous v3 API users are blocked""" + response = await self.async_client.get(self.audio_path_v3) + self.assertEqual(response.status_code, HTTPStatus.FORBIDDEN) + self.assertEqual( + response.json()["detail"], + "Anonymous users don't have permission to access V3 of the API. Please use V4 instead.", + ) + + async def test_allow_v4_for_new_users(self, mock_api_prefix) -> None: + """Confirm new API users are allowed to use V4 of the API""" + response = await self.client_2.get(self.audio_path_v4, format="json") + self.assertEqual(response.status_code, HTTPStatus.OK) + + class DRFOrderingTests(TestCase): """Does ordering work generally and specifically?""" diff --git a/cl/api/utils.py b/cl/api/utils.py index f2013164e9..fe3ba4be72 100644 --- a/cl/api/utils.py +++ b/cl/api/utils.py @@ -186,11 +186,11 @@ def determine_actions(self, request, view): return actions -def get_logging_prefix() -> str: +def get_logging_prefix(api_version: str) -> str: """Simple tool for getting the prefix for logging API requests. Useful for mocking the logger. """ - return "api:v3" + return f"api:{api_version}" class LoggingMixin: @@ -255,7 +255,7 @@ def _log_request(self, request): r = get_redis_interface("STATS") pipe = r.pipeline() - api_prefix = get_logging_prefix() + api_prefix = get_logging_prefix(request.version) # Global and daily tallies for all URLs. pipe.incr(f"{api_prefix}.count") diff --git a/cl/audio/api_views.py b/cl/audio/api_views.py index e5478a09d6..a444db4a98 100644 --- a/cl/audio/api_views.py +++ b/cl/audio/api_views.py @@ -1,5 +1,6 @@ from rest_framework import viewsets +from cl.api.api_permissions import V3APIPermission from cl.api.utils import LoggingMixin from cl.audio.api_serializers import AudioSerializer from cl.audio.filters import AudioFilter @@ -9,6 +10,7 @@ class AudioViewSet(LoggingMixin, viewsets.ModelViewSet): serializer_class = AudioSerializer filterset_class = AudioFilter + permission_classes = [V3APIPermission] ordering_fields = ( "id", "date_created", diff --git a/cl/settings/third_party/rest_framework.py b/cl/settings/third_party/rest_framework.py index 7f618af82a..7ce0afd4b2 100644 --- a/cl/settings/third_party/rest_framework.py +++ b/cl/settings/third_party/rest_framework.py @@ -116,3 +116,5 @@ if DEVELOPMENT: REST_FRAMEWORK["DEFAULT_THROTTLE_RATES"]["anon"] = "10000/day" # type: ignore + +BLOCK_NEW_V3_USERS = env.bool("BLOCK_NEW_V3_USERS", default=False) From 6623cb349568ff659ec78c332135f08f13b96b5e Mon Sep 17 00:00:00 2001 From: Alberto Islas Date: Tue, 15 Oct 2024 14:53:00 -0600 Subject: [PATCH 2/8] fix(api): Introduced v3-user-list to check v3 users --- cl/api/api_permissions.py | 10 ++++------ cl/api/tests.py | 22 +++++++++++++++++++--- 2 files changed, 23 insertions(+), 9 deletions(-) diff --git a/cl/api/api_permissions.py b/cl/api/api_permissions.py index 851c68ba47..afd5588d06 100644 --- a/cl/api/api_permissions.py +++ b/cl/api/api_permissions.py @@ -5,7 +5,6 @@ from rest_framework.exceptions import PermissionDenied from rest_framework.views import APIView -from cl.api.utils import get_logging_prefix from cl.lib.redis_utils import get_redis_interface @@ -24,15 +23,14 @@ class V3APIPermission(permissions.BasePermission): def is_new_v3_user(self, user: User) -> bool: """Check if the user is new for V3 by determining their presence in the - V3 API stats. + v3-user-list. :param user: The User to check. :return: True if the user is new for V3, False otherwise. """ - api_prefix = get_logging_prefix("v3") - set_key = f"{api_prefix}.user.counts" - score = self.r.zscore(set_key, user.id) - return score is None + + is_new_user = self.r.sismember("v3-user-list", user.id) != 1 + return is_new_user @staticmethod def is_v3_api_request(request: HttpRequest) -> bool: diff --git a/cl/api/tests.py b/cl/api/tests.py index 9bd20d1ca4..db2dea9c16 100644 --- a/cl/api/tests.py +++ b/cl/api/tests.py @@ -422,7 +422,8 @@ async def test_api_logged_correctly_v4(self, mock_logging_prefix) -> None: @override_settings(BLOCK_NEW_V3_USERS=True) @mock.patch( - "cl.api.api_permissions.get_logging_prefix", return_value="api-block:v3" + "cl.api.api_permissions.get_logging_prefix", + return_value="api-block-test:v3", ) class BlockV3APITests(TestCase): """Check that V3 API is restricted for anonymous and new users.""" @@ -445,10 +446,21 @@ def tearDown(self) -> None: self.flush_stats() def flush_stats(self) -> None: - keys = self.r.keys("api-block:*") + keys = self.r.keys("api-block-test:*") if keys: self.r.delete(*keys) + v3_user_list = self.r.keys("v3-user-list") + if v3_user_list: + self.r.delete(*v3_user_list) + + def create_v3_user_list(self) -> None: + v3_stats_members = self.r.zrange( + "api-block-test:v3.user.counts", 0, -1 + ) + if v3_stats_members: + self.r.sadd("v3-user-list", *v3_stats_members) + async def test_block_v3_for_new_users(self, mock_api_prefix) -> None: """Confirm new v3 API users are blocked""" response = await self.client_1.get(self.audio_path_v3, format="json") @@ -460,7 +472,11 @@ async def test_block_v3_for_new_users(self, mock_api_prefix) -> None: async def test_allow_v3_for_existing_users(self, mock_api_prefix) -> None: """Confirm that existing v3 API users are granted access to use it""" - self.r.zincrby("api-block:v3.user.counts", 1, self.user_2.pk) + + # Simulate v3 existing user and create v3 user list. + self.r.zincrby("api-block-test:v3.user.counts", 1, self.user_2.pk) + self.create_v3_user_list() + response = await self.client_2.get(self.audio_path_v3, format="json") self.assertEqual(response.status_code, HTTPStatus.OK) From 37d757e5f683016af3c16cae21b1e91572999be2 Mon Sep 17 00:00:00 2001 From: Alberto Islas Date: Tue, 15 Oct 2024 18:14:44 -0600 Subject: [PATCH 3/8] fix(api): Introduced v3-blocked-users-list --- cl/api/api_permissions.py | 66 +++++++++++++++++++++++++++++---------- cl/api/tests.py | 47 +++++++++++++++++++++++----- 2 files changed, 90 insertions(+), 23 deletions(-) diff --git a/cl/api/api_permissions.py b/cl/api/api_permissions.py index afd5588d06..bfc5628104 100644 --- a/cl/api/api_permissions.py +++ b/cl/api/api_permissions.py @@ -1,5 +1,7 @@ +import time + from django.conf import settings -from django.contrib.auth.models import User +from django.contrib.auth.models import AnonymousUser, User from django.http import HttpRequest from rest_framework import permissions from rest_framework.exceptions import PermissionDenied @@ -20,22 +22,47 @@ def has_object_permission(self, request, view, obj): class V3APIPermission(permissions.BasePermission): r = get_redis_interface("STATS") + v3_blocked_message = ( + "As a new user, you don't have permission to access V3 of the API. " + "Please use V4 instead." + ) + v3_blocked_list_key = "v3-blocked-users-list" + v3_users_list_key = "v3-users-list" def is_new_v3_user(self, user: User) -> bool: """Check if the user is new for V3 by determining their presence in the - v3-user-list. + v3-users-list. :param user: The User to check. :return: True if the user is new for V3, False otherwise. """ - - is_new_user = self.r.sismember("v3-user-list", user.id) != 1 + is_new_user = self.r.sismember(self.v3_users_list_key, user.id) != 1 return is_new_user + def is_user_v3_blocked(self, user: User) -> bool: + """Check if the user is already blocked form V3 by determining their + presence in the v3-blocked-users-list. + + :param user: The User to check. + :return: True if the user is blocked for V3, False otherwise. + """ + is_blocked_user = ( + self.r.sismember(self.v3_blocked_list_key, user.id) == 1 + ) + return is_blocked_user + @staticmethod def is_v3_api_request(request: HttpRequest) -> bool: return getattr(request, "version", None) == "v3" + @staticmethod + def check_request() -> bool: + # Check requests every 10 seconds. + # Notice that all requests within that second will be checked. + if int(time.time()) % 10 == 0: + return True + return False + def has_permission(self, request: HttpRequest, view: APIView) -> bool: """Check if the user has permission to access the V3 API. @@ -44,23 +71,30 @@ def has_permission(self, request: HttpRequest, view: APIView) -> bool: :return: True if the user has permission to access V3, False if not. """ + user = request.user + if isinstance(user, AnonymousUser): + # Block V3 for all Anonymous users. + raise PermissionDenied( + "Anonymous users don't have permission to access V3 of the API. " + "Please use V4 instead." + ) + + if self.is_user_v3_blocked(user): + # Check if user has been blocked from V3. + raise PermissionDenied(self.v3_blocked_message) + if ( not self.is_v3_api_request(request) - or not settings.BLOCK_NEW_V3_USERS + or not settings.BLOCK_NEW_V3_USERS # type: ignore ): # Allow the request if it is not a V3 request return True - user = request.user - if not user.pk: - # Block V3 for Anonymous users. - raise PermissionDenied( - "Anonymous users don't have permission to access V3 of the API. " - "Please use V4 instead." - ) + if not self.check_request(): + return True + if user.is_authenticated and self.is_new_v3_user(user): - raise PermissionDenied( - "As a new user, you don't have permission to access V3 of the API. " - "Please use V4 instead." - ) + # This a new user. Block request and add it to v3-blocked-list + self.r.sadd(self.v3_blocked_list_key, user.id) + raise PermissionDenied(self.v3_blocked_message) return True diff --git a/cl/api/tests.py b/cl/api/tests.py index db2dea9c16..ddee6953cc 100644 --- a/cl/api/tests.py +++ b/cl/api/tests.py @@ -5,6 +5,7 @@ from unittest import mock from urllib.parse import parse_qs, urlparse +import time_machine from asgiref.sync import async_to_sync, sync_to_async from django.contrib.auth.hashers import make_password from django.contrib.auth.models import Permission @@ -422,7 +423,7 @@ async def test_api_logged_correctly_v4(self, mock_logging_prefix) -> None: @override_settings(BLOCK_NEW_V3_USERS=True) @mock.patch( - "cl.api.api_permissions.get_logging_prefix", + "cl.api.utils.get_logging_prefix", return_value="api-block-test:v3", ) class BlockV3APITests(TestCase): @@ -438,6 +439,9 @@ def setUpTestData(cls) -> None: cls.audio_path_v3 = reverse("audio-list", kwargs={"version": "v3"}) cls.audio_path_v4 = reverse("audio-list", kwargs={"version": "v4"}) + cls.date_check_request = now().replace(second=10) + cls.date_dont_check_request = now().replace(second=3) + def setUp(self) -> None: self.r = get_redis_interface("STATS") self.flush_stats() @@ -450,7 +454,7 @@ def flush_stats(self) -> None: if keys: self.r.delete(*keys) - v3_user_list = self.r.keys("v3-user-list") + v3_user_list = self.r.keys("v3-users-list") if v3_user_list: self.r.delete(*v3_user_list) @@ -459,30 +463,59 @@ def create_v3_user_list(self) -> None: "api-block-test:v3.user.counts", 0, -1 ) if v3_stats_members: - self.r.sadd("v3-user-list", *v3_stats_members) + self.r.sadd("v3-users-list", *v3_stats_members) async def test_block_v3_for_new_users(self, mock_api_prefix) -> None: """Confirm new v3 API users are blocked""" - response = await self.client_1.get(self.audio_path_v3, format="json") + + # A new user request is not detected because we only check some + # requests. In this case, it's not checked. + with time_machine.travel(self.date_dont_check_request, tick=False): + response = await self.client_1.get( + self.audio_path_v3, format="json" + ) + self.assertEqual(response.status_code, HTTPStatus.OK) + + # Now check the request and block the request and user. + with time_machine.travel(self.date_check_request, tick=False): + response = await self.client_1.get( + self.audio_path_v3, format="json" + ) self.assertEqual(response.status_code, HTTPStatus.FORBIDDEN) self.assertEqual( response.json()["detail"], "As a new user, you don't have permission to access V3 of the API. Please use V4 instead.", ) + # Once the user is caught, they are added to the V3 blocked list, so + # every consecutive request from the user is blocked. + response = await self.client_1.get(self.audio_path_v3, format="json") + self.assertEqual( + response.status_code, + HTTPStatus.FORBIDDEN, + msg="Consecutive request.", + ) + self.assertEqual( + response.json()["detail"], + "As a new user, you don't have permission to access V3 of the API. Please use V4 instead.", + ) + async def test_allow_v3_for_existing_users(self, mock_api_prefix) -> None: """Confirm that existing v3 API users are granted access to use it""" # Simulate v3 existing user and create v3 user list. self.r.zincrby("api-block-test:v3.user.counts", 1, self.user_2.pk) self.create_v3_user_list() - - response = await self.client_2.get(self.audio_path_v3, format="json") + with time_machine.travel(self.date_check_request, tick=False): + response = await self.client_2.get( + self.audio_path_v3, format="json" + ) self.assertEqual(response.status_code, HTTPStatus.OK) async def test_block_v3_for_anonymous_users(self, mock_api_prefix) -> None: """Confirm anonymous v3 API users are blocked""" - response = await self.async_client.get(self.audio_path_v3) + with time_machine.travel(self.date_check_request, tick=False): + response = await self.async_client.get(self.audio_path_v3) self.assertEqual(response.status_code, HTTPStatus.FORBIDDEN) self.assertEqual( response.json()["detail"], From 4c9e9fef857046a6e8119b3c72d0b1fbe3e55cb1 Mon Sep 17 00:00:00 2001 From: Alberto Islas Date: Tue, 15 Oct 2024 18:43:30 -0600 Subject: [PATCH 4/8] fix(api): Added V3APIPermission to API viewsets --- cl/alerts/api_views.py | 6 +++--- cl/citations/api_views.py | 3 ++- cl/disclosures/api_views.py | 10 ++++++++++ cl/favorites/api_views.py | 3 ++- cl/people_db/api_views.py | 12 ++++++++++++ cl/recap/views.py | 4 +++- cl/search/api_views.py | 13 +++++++++++-- cl/visualizations/api_views.py | 10 +++++++--- 8 files changed, 50 insertions(+), 11 deletions(-) diff --git a/cl/alerts/api_views.py b/cl/alerts/api_views.py index 05c35c415f..60d2ff65b6 100644 --- a/cl/alerts/api_views.py +++ b/cl/alerts/api_views.py @@ -7,7 +7,7 @@ ) from cl.alerts.filters import DocketAlertFilter, SearchAlertFilter from cl.alerts.models import Alert, DocketAlert -from cl.api.api_permissions import IsOwner +from cl.api.api_permissions import IsOwner, V3APIPermission from cl.api.pagination import MediumAdjustablePagination from cl.api.utils import LoggingMixin @@ -15,7 +15,7 @@ class SearchAlertViewSet(LoggingMixin, ModelViewSet): """A ModelViewset to handle CRUD operations for SearchAlerts.""" - permission_classes = [IsOwner, IsAuthenticated] + permission_classes = [IsOwner, IsAuthenticated, V3APIPermission] serializer_class = SearchAlertSerializer pagination_class = MediumAdjustablePagination filterset_class = SearchAlertFilter @@ -42,7 +42,7 @@ def get_queryset(self): class DocketAlertViewSet(LoggingMixin, ModelViewSet): """A ModelViewset to handle CRUD operations for DocketAlerts.""" - permission_classes = [IsOwner, IsAuthenticated] + permission_classes = [IsOwner, IsAuthenticated, V3APIPermission] serializer_class = DocketAlertSerializer pagination_class = MediumAdjustablePagination filterset_class = DocketAlertFilter diff --git a/cl/citations/api_views.py b/cl/citations/api_views.py index 24f8a278a2..556f5fe01f 100644 --- a/cl/citations/api_views.py +++ b/cl/citations/api_views.py @@ -13,6 +13,7 @@ from rest_framework.response import Response from rest_framework.viewsets import GenericViewSet +from cl.api.api_permissions import V3APIPermission from cl.api.utils import CitationCountRateThrottle, LoggingMixin from cl.citations.api_serializers import ( CitationAPIRequestSerializer, @@ -27,7 +28,7 @@ class CitationLookupViewSet(LoggingMixin, CreateModelMixin, GenericViewSet): queryset = OpinionCluster.objects.all() serializer_class = CitationAPIRequestSerializer - permission_classes = [IsAuthenticated] + permission_classes = [IsAuthenticated, V3APIPermission] throttle_classes = [CitationCountRateThrottle] citation_list: list[FullCaseCitation | ShortCaseCitation] = [] diff --git a/cl/disclosures/api_views.py b/cl/disclosures/api_views.py index b6500581f7..1c1be6f3a4 100644 --- a/cl/disclosures/api_views.py +++ b/cl/disclosures/api_views.py @@ -1,5 +1,6 @@ from rest_framework import viewsets +from cl.api.api_permissions import V3APIPermission from cl.api.utils import LoggingMixin from cl.disclosures.api_serializers import ( AgreementSerializer, @@ -39,6 +40,7 @@ class AgreementViewSet(LoggingMixin, viewsets.ModelViewSet): queryset = Agreement.objects.all().order_by("-id") serializer_class = AgreementSerializer + permission_classes = [V3APIPermission] ordering_fields = ("id", "date_created", "date_modified") filterset_class = AgreementFilter # Default cursor ordering key @@ -54,6 +56,7 @@ class AgreementViewSet(LoggingMixin, viewsets.ModelViewSet): class DebtViewSet(LoggingMixin, viewsets.ModelViewSet): queryset = Debt.objects.all().order_by("-id") serializer_class = DebtSerializer + permission_classes = [V3APIPermission] ordering_fields = ("id", "date_created", "date_modified") filterset_class = DebtFilter # Default cursor ordering key @@ -84,6 +87,7 @@ class FinancialDisclosureViewSet(LoggingMixin, viewsets.ModelViewSet): ) serializer_class = FinancialDisclosureSerializer filterset_class = FinancialDisclosureFilter + permission_classes = [V3APIPermission] ordering_fields = ("id", "date_created", "date_modified") # Default cursor ordering key ordering = "-id" @@ -99,6 +103,7 @@ class GiftViewSet(LoggingMixin, viewsets.ModelViewSet): queryset = Gift.objects.all().order_by("-id") serializer_class = GiftSerializer filterset_class = GiftFilter + permission_classes = [V3APIPermission] ordering_fields = ("id", "date_created", "date_modified") # Default cursor ordering key ordering = "-id" @@ -114,6 +119,7 @@ class InvestmentViewSet(LoggingMixin, viewsets.ModelViewSet): queryset = Investment.objects.all().order_by("-id") serializer_class = InvestmentSerializer filterset_class = InvestmentFilter + permission_classes = [V3APIPermission] ordering_fields = ("id", "date_created", "date_modified") # Default cursor ordering key ordering = "-id" @@ -129,6 +135,7 @@ class NonInvestmentIncomeViewSet(LoggingMixin, viewsets.ModelViewSet): queryset = NonInvestmentIncome.objects.all().order_by("-id") serializer_class = NonInvestmentIncomeSerializer filterset_class = NonInvestmentIncomeFilter + permission_classes = [V3APIPermission] ordering_fields = ("id", "date_created", "date_modified") # Default cursor ordering key ordering = "-id" @@ -144,6 +151,7 @@ class PositionViewSet(LoggingMixin, viewsets.ModelViewSet): queryset = Position.objects.all().order_by("-id") serializer_class = PositionSerializer filterset_class = PositionFilter + permission_classes = [V3APIPermission] ordering_fields = ("id", "date_created", "date_modified") # Default cursor ordering key ordering = "-id" @@ -159,6 +167,7 @@ class ReimbursementViewSet(LoggingMixin, viewsets.ModelViewSet): queryset = Reimbursement.objects.all().order_by("-id") serializer_class = ReimbursementSerializer filterset_class = ReimbursementFilter + permission_classes = [V3APIPermission] ordering_fields = ("id", "date_created", "date_modified") # Default cursor ordering key ordering = "-id" @@ -174,6 +183,7 @@ class SpouseIncomeViewSet(LoggingMixin, viewsets.ModelViewSet): queryset = SpouseIncome.objects.all().order_by("-id") serializer_class = SpouseIncomeSerializer filterset_class = SpouseIncomeFilter + permission_classes = [V3APIPermission] ordering_fields = ("id", "date_created", "date_modified") # Default cursor ordering key ordering = "-id" diff --git a/cl/favorites/api_views.py b/cl/favorites/api_views.py index 981c1d3ebb..f27dc1381f 100644 --- a/cl/favorites/api_views.py +++ b/cl/favorites/api_views.py @@ -3,6 +3,7 @@ from rest_framework.permissions import IsAuthenticated from rest_framework.viewsets import ModelViewSet +from cl.api.api_permissions import V3APIPermission from cl.api.pagination import MediumAdjustablePagination from cl.api.utils import LoggingMixin from cl.favorites.api_permissions import IsTagOwner @@ -61,7 +62,7 @@ def get_queryset(self): class PrayerViewSet(LoggingMixin, ModelViewSet): """A ModelViewset to handle CRUD operations for Prayer.""" - permission_classes = [IsAuthenticated] + permission_classes = [IsAuthenticated, V3APIPermission] serializer_class = PrayerSerializer pagination_class = MediumAdjustablePagination filterset_class = PrayerFilter diff --git a/cl/people_db/api_views.py b/cl/people_db/api_views.py index e292a26cac..001d181f15 100644 --- a/cl/people_db/api_views.py +++ b/cl/people_db/api_views.py @@ -1,6 +1,7 @@ from django.db.models import Exists, OuterRef, Prefetch from rest_framework import viewsets +from cl.api.api_permissions import V3APIPermission from cl.api.pagination import TinyAdjustablePagination from cl.api.utils import LoggingMixin, RECAPUsersReadOnly from cl.disclosures.models import FinancialDisclosure @@ -89,6 +90,7 @@ class PersonDisclosureViewSet(viewsets.ModelViewSet): serializer_class = PersonDisclosureSerializer filterset_class = PersonDisclosureFilter pagination_class = TinyAdjustablePagination + permission_classes = [V3APIPermission] ordering_fields = ( "id", "date_created", @@ -120,6 +122,7 @@ class PersonViewSet(LoggingMixin, viewsets.ModelViewSet): ) serializer_class = PersonSerializer filterset_class = PersonFilter + permission_classes = [V3APIPermission] ordering_fields = ( "id", "date_created", @@ -142,6 +145,7 @@ class PositionViewSet(LoggingMixin, viewsets.ModelViewSet): queryset = Position.objects.all().order_by("-id") serializer_class = PositionSerializer filterset_class = PositionFilter + permission_classes = [V3APIPermission] ordering_fields = ( "id", "date_created", @@ -171,6 +175,7 @@ class RetentionEventViewSet(LoggingMixin, viewsets.ModelViewSet): queryset = RetentionEvent.objects.all().order_by("-id") serializer_class = RetentionEventSerializer filterset_class = RetentionEventFilter + permission_classes = [V3APIPermission] ordering_fields = ("id", "date_created", "date_modified", "date_retention") # Default cursor ordering key ordering = "-id" @@ -186,6 +191,7 @@ class EducationViewSet(LoggingMixin, viewsets.ModelViewSet): queryset = Education.objects.all().order_by("-id") serializer_class = EducationSerializer filterset_class = EducationFilter + permission_classes = [V3APIPermission] ordering_fields = ("id", "date_created", "date_modified") # Default cursor ordering key ordering = "-id" @@ -201,6 +207,7 @@ class SchoolViewSet(LoggingMixin, viewsets.ModelViewSet): queryset = School.objects.all().order_by("-id") serializer_class = SchoolSerializer filterset_class = SchoolFilter + permission_classes = [V3APIPermission] ordering_fields = ("id", "date_created", "date_modified", "name") # Default cursor ordering key ordering = "-id" @@ -216,6 +223,7 @@ class PoliticalAffiliationViewSet(LoggingMixin, viewsets.ModelViewSet): queryset = PoliticalAffiliation.objects.all().order_by("-id") serializer_class = PoliticalAffiliationSerializer filterset_class = PoliticalAffiliationFilter + permission_classes = [V3APIPermission] ordering_fields = ( "id", "date_created", @@ -237,6 +245,7 @@ class SourceViewSet(LoggingMixin, viewsets.ModelViewSet): queryset = Source.objects.all().order_by("-id") serializer_class = SourceSerializer filterset_class = SourceFilter + permission_classes = [V3APIPermission] ordering_fields = ( "id", "date_modified", @@ -252,6 +261,7 @@ class ABARatingViewSet(LoggingMixin, viewsets.ModelViewSet): queryset = ABARating.objects.all().order_by("-id") serializer_class = ABARatingSerializer filterset_class = ABARatingFilter + permission_classes = [V3APIPermission] ordering_fields = ( "id", "date_created", @@ -272,6 +282,7 @@ class PartyViewSet(LoggingMixin, viewsets.ModelViewSet): permission_classes = (RECAPUsersReadOnly,) serializer_class = PartySerializer filterset_class = PartyFilter + permission_classes = [V3APIPermission] ordering_fields = ( "id", "date_created", @@ -297,6 +308,7 @@ class AttorneyViewSet(LoggingMixin, viewsets.ModelViewSet): permission_classes = (RECAPUsersReadOnly,) serializer_class = AttorneySerializer filterset_class = AttorneyFilter + permission_classes = [V3APIPermission] ordering_fields = ( "id", "date_created", diff --git a/cl/recap/views.py b/cl/recap/views.py index 6779fd576f..9bb70cb6cf 100644 --- a/cl/recap/views.py +++ b/cl/recap/views.py @@ -6,6 +6,7 @@ from rest_framework.permissions import IsAuthenticatedOrReadOnly from rest_framework.viewsets import ModelViewSet +from cl.api.api_permissions import V3APIPermission from cl.api.pagination import BigPagination from cl.api.utils import ( EmailProcessingQueueAPIUsers, @@ -115,7 +116,7 @@ class PacerFetchRequestViewSet(LoggingMixin, ModelViewSet): queryset = PacerFetchQueue.objects.all().order_by("-id") serializer_class = PacerFetchQueueSerializer filterset_class = PacerFetchQueueFilter - permission_classes = (IsAuthenticatedOrReadOnly,) + permission_classes = (IsAuthenticatedOrReadOnly, V3APIPermission) ordering_fields = ( "id", "date_created", @@ -178,6 +179,7 @@ class FjcIntegratedDatabaseViewSet(LoggingMixin, ModelViewSet): queryset = FjcIntegratedDatabase.objects.all().order_by("-id") serializer_class = FjcIntegratedDatabaseSerializer filterset_class = FjcIntegratedDatabaseFilter + permission_classes = [V3APIPermission] ordering_fields = ( "id", "date_created", diff --git a/cl/search/api_views.py b/cl/search/api_views.py index b909f0210c..22e7d3b542 100644 --- a/cl/search/api_views.py +++ b/cl/search/api_views.py @@ -5,6 +5,7 @@ from rest_framework.exceptions import NotFound from rest_framework.pagination import PageNumberPagination +from cl.api.api_permissions import V3APIPermission from cl.api.pagination import ESCursorPagination from cl.api.utils import CacheListMixin, LoggingMixin, RECAPUsersReadOnly from cl.lib.elasticsearch_utils import do_es_api_query @@ -78,6 +79,7 @@ class OriginatingCourtInformationViewSet(viewsets.ModelViewSet): class DocketViewSet(LoggingMixin, viewsets.ModelViewSet): serializer_class = DocketSerializer filterset_class = DocketFilter + permission_classes = [V3APIPermission] ordering_fields = ( "id", "date_created", @@ -112,6 +114,7 @@ class DocketEntryViewSet(LoggingMixin, viewsets.ModelViewSet): permission_classes = (RECAPUsersReadOnly,) serializer_class = DocketEntrySerializer filterset_class = DocketEntryFilter + permission_classes = [V3APIPermission] ordering_fields = ("id", "date_created", "date_modified", "date_filed") # Default cursor ordering key ordering = "-id" @@ -140,6 +143,7 @@ class RECAPDocumentViewSet( permission_classes = (RECAPUsersReadOnly,) serializer_class = RECAPDocumentSerializer filterset_class = RECAPDocumentFilter + permission_classes = [V3APIPermission] ordering_fields = ("id", "date_created", "date_modified", "date_upload") # Default cursor ordering key ordering = "-id" @@ -161,6 +165,7 @@ class RECAPDocumentViewSet( class CourtViewSet(LoggingMixin, viewsets.ModelViewSet): serializer_class = CourtSerializer filterset_class = CourtFilter + permission_classes = [V3APIPermission] ordering_fields = ( "id", "date_modified", @@ -180,6 +185,7 @@ class CourtViewSet(LoggingMixin, viewsets.ModelViewSet): class OpinionClusterViewSet(LoggingMixin, viewsets.ModelViewSet): serializer_class = OpinionClusterSerializer filterset_class = OpinionClusterFilter + permission_classes = [V3APIPermission] ordering_fields = ( "id", "date_created", @@ -204,6 +210,7 @@ class OpinionClusterViewSet(LoggingMixin, viewsets.ModelViewSet): class OpinionViewSet(LoggingMixin, viewsets.ModelViewSet): serializer_class = OpinionSerializer filterset_class = OpinionFilter + permission_classes = [V3APIPermission] ordering_fields = ( "id", "date_created", @@ -227,6 +234,7 @@ class OpinionViewSet(LoggingMixin, viewsets.ModelViewSet): class OpinionsCitedViewSet(LoggingMixin, viewsets.ModelViewSet): serializer_class = OpinionsCitedSerializer filterset_class = OpinionsCitedFilter + permission_classes = [V3APIPermission] # Default cursor ordering key ordering = "-id" # Additional cursor ordering fields @@ -237,6 +245,7 @@ class OpinionsCitedViewSet(LoggingMixin, viewsets.ModelViewSet): class TagViewSet(LoggingMixin, viewsets.ModelViewSet): permission_classes = (RECAPUsersReadOnly,) serializer_class = TagSerializer + permission_classes = [V3APIPermission] # Default cursor ordering key ordering = "-id" # Additional cursor ordering fields @@ -251,7 +260,7 @@ class TagViewSet(LoggingMixin, viewsets.ModelViewSet): class SearchViewSet(LoggingMixin, viewsets.ViewSet): # Default permissions use Django permissions, so here we AllowAny, # but folks will need to log in to get past the thresholds. - permission_classes = (permissions.AllowAny,) + permission_classes = (permissions.AllowAny, V3APIPermission) def list(self, request, *args, **kwargs): @@ -306,7 +315,7 @@ def list(self, request, *args, **kwargs): class SearchV4ViewSet(LoggingMixin, viewsets.ViewSet): # Default permissions use Django permissions, so here we AllowAny, # but folks will need to log in to get past the thresholds. - permission_classes = (permissions.AllowAny,) + permission_classes = (permissions.AllowAny, V3APIPermission) supported_search_types = { SEARCH_TYPES.RECAP: { diff --git a/cl/visualizations/api_views.py b/cl/visualizations/api_views.py index 05e80c37cc..22fccc03d9 100644 --- a/cl/visualizations/api_views.py +++ b/cl/visualizations/api_views.py @@ -6,7 +6,7 @@ from rest_framework.response import Response from rest_framework.viewsets import ModelViewSet -from cl.api.api_permissions import IsOwner +from cl.api.api_permissions import IsOwner, V3APIPermission from cl.api.utils import LoggingMixin from cl.visualizations.api_permissions import IsParentVisualizationOwner from cl.visualizations.api_serializers import ( @@ -19,7 +19,11 @@ class JSONViewSet(LoggingMixin, ModelViewSet): - permission_classes = [IsAuthenticated, IsParentVisualizationOwner] + permission_classes = [ + IsAuthenticated, + IsParentVisualizationOwner, + V3APIPermission, + ] serializer_class = JSONVersionSerializer ordering_fields = ("id", "date_created", "date_modified") # Default cursor ordering key @@ -38,7 +42,7 @@ def get_queryset(self): class VisualizationViewSet(LoggingMixin, ModelViewSet): - permission_classes = [IsAuthenticated, IsOwner] + permission_classes = [IsAuthenticated, IsOwner, V3APIPermission] serializer_class = VisualizationSerializer ordering_fields = ("id", "date_created", "date_modified", "user") # Default cursor ordering key From 853db32db7a733ca8dacaeb0ef8a9f12c0684f63 Mon Sep 17 00:00:00 2001 From: Alberto Islas Date: Wed, 16 Oct 2024 10:35:43 -0600 Subject: [PATCH 5/8] fix(api): Fixed failing tests and permission_classes --- cl/api/api_permissions.py | 14 +++++++------- cl/api/tests.py | 15 ++++++++++++++- cl/favorites/api_views.py | 7 +++++-- cl/people_db/api_views.py | 6 ++---- cl/search/api_views.py | 10 ++++------ 5 files changed, 32 insertions(+), 20 deletions(-) diff --git a/cl/api/api_permissions.py b/cl/api/api_permissions.py index bfc5628104..7ab20df700 100644 --- a/cl/api/api_permissions.py +++ b/cl/api/api_permissions.py @@ -71,6 +71,13 @@ def has_permission(self, request: HttpRequest, view: APIView) -> bool: :return: True if the user has permission to access V3, False if not. """ + if ( + not self.is_v3_api_request(request) + or not settings.BLOCK_NEW_V3_USERS # type: ignore + ): + # Allow the request if it is not a V3 request + return True + user = request.user if isinstance(user, AnonymousUser): # Block V3 for all Anonymous users. @@ -83,13 +90,6 @@ def has_permission(self, request: HttpRequest, view: APIView) -> bool: # Check if user has been blocked from V3. raise PermissionDenied(self.v3_blocked_message) - if ( - not self.is_v3_api_request(request) - or not settings.BLOCK_NEW_V3_USERS # type: ignore - ): - # Allow the request if it is not a V3 request - return True - if not self.check_request(): return True diff --git a/cl/api/tests.py b/cl/api/tests.py index ddee6953cc..3ab24f8859 100644 --- a/cl/api/tests.py +++ b/cl/api/tests.py @@ -458,6 +458,10 @@ def flush_stats(self) -> None: if v3_user_list: self.r.delete(*v3_user_list) + v3_user_blocked_list = self.r.keys("v3-blocked-users-list") + if v3_user_blocked_list: + self.r.delete(*v3_user_blocked_list) + def create_v3_user_list(self) -> None: v3_stats_members = self.r.zrange( "api-block-test:v3.user.counts", 0, -1 @@ -524,7 +528,16 @@ async def test_block_v3_for_anonymous_users(self, mock_api_prefix) -> None: async def test_allow_v4_for_new_users(self, mock_api_prefix) -> None: """Confirm new API users are allowed to use V4 of the API""" - response = await self.client_2.get(self.audio_path_v4, format="json") + with time_machine.travel(self.date_check_request, tick=False): + response = await self.client_2.get( + self.audio_path_v4, format="json" + ) + self.assertEqual(response.status_code, HTTPStatus.OK) + + async def test_allow_v4_for_anonymous_users(self, mock_api_prefix) -> None: + """Confirm V4 anonymous API users are allowed to use V4 of the API""" + with time_machine.travel(self.date_check_request, tick=False): + response = await self.async_client.get(self.audio_path_v4) self.assertEqual(response.status_code, HTTPStatus.OK) diff --git a/cl/favorites/api_views.py b/cl/favorites/api_views.py index f27dc1381f..d2d02b6a14 100644 --- a/cl/favorites/api_views.py +++ b/cl/favorites/api_views.py @@ -17,7 +17,10 @@ class UserTagViewSet(ModelViewSet): - permission_classes = [permissions.IsAuthenticatedOrReadOnly] + permission_classes = [ + permissions.IsAuthenticatedOrReadOnly, + V3APIPermission, + ] serializer_class = UserTagSerializer pagination_class = MediumAdjustablePagination filterset_class = UserTagFilter @@ -44,7 +47,7 @@ def get_queryset(self): class DocketTagViewSet(ModelViewSet): - permission_classes = [IsAuthenticated, IsTagOwner] + permission_classes = [IsAuthenticated, IsTagOwner, V3APIPermission] serializer_class = DocketTagSerializer filterset_class = DocketTagFilter pagination_class = MediumAdjustablePagination diff --git a/cl/people_db/api_views.py b/cl/people_db/api_views.py index 001d181f15..c593c1789a 100644 --- a/cl/people_db/api_views.py +++ b/cl/people_db/api_views.py @@ -279,10 +279,9 @@ class ABARatingViewSet(LoggingMixin, viewsets.ModelViewSet): class PartyViewSet(LoggingMixin, viewsets.ModelViewSet): - permission_classes = (RECAPUsersReadOnly,) + permission_classes = (RECAPUsersReadOnly, V3APIPermission) serializer_class = PartySerializer filterset_class = PartyFilter - permission_classes = [V3APIPermission] ordering_fields = ( "id", "date_created", @@ -305,10 +304,9 @@ class PartyViewSet(LoggingMixin, viewsets.ModelViewSet): class AttorneyViewSet(LoggingMixin, viewsets.ModelViewSet): - permission_classes = (RECAPUsersReadOnly,) + permission_classes = (RECAPUsersReadOnly, V3APIPermission) serializer_class = AttorneySerializer filterset_class = AttorneyFilter - permission_classes = [V3APIPermission] ordering_fields = ( "id", "date_created", diff --git a/cl/search/api_views.py b/cl/search/api_views.py index 22e7d3b542..1a3ee78dea 100644 --- a/cl/search/api_views.py +++ b/cl/search/api_views.py @@ -65,6 +65,7 @@ class OriginatingCourtInformationViewSet(viewsets.ModelViewSet): serializer_class = OriginalCourtInformationSerializer + permission_classes = [V3APIPermission] # Default cursor ordering key ordering = "-id" # Additional cursor ordering fields @@ -111,10 +112,9 @@ class DocketViewSet(LoggingMixin, viewsets.ModelViewSet): class DocketEntryViewSet(LoggingMixin, viewsets.ModelViewSet): - permission_classes = (RECAPUsersReadOnly,) + permission_classes = (RECAPUsersReadOnly, V3APIPermission) serializer_class = DocketEntrySerializer filterset_class = DocketEntryFilter - permission_classes = [V3APIPermission] ordering_fields = ("id", "date_created", "date_modified", "date_filed") # Default cursor ordering key ordering = "-id" @@ -140,10 +140,9 @@ class DocketEntryViewSet(LoggingMixin, viewsets.ModelViewSet): class RECAPDocumentViewSet( LoggingMixin, CacheListMixin, viewsets.ModelViewSet ): - permission_classes = (RECAPUsersReadOnly,) + permission_classes = (RECAPUsersReadOnly, V3APIPermission) serializer_class = RECAPDocumentSerializer filterset_class = RECAPDocumentFilter - permission_classes = [V3APIPermission] ordering_fields = ("id", "date_created", "date_modified", "date_upload") # Default cursor ordering key ordering = "-id" @@ -243,9 +242,8 @@ class OpinionsCitedViewSet(LoggingMixin, viewsets.ModelViewSet): class TagViewSet(LoggingMixin, viewsets.ModelViewSet): - permission_classes = (RECAPUsersReadOnly,) + permission_classes = (RECAPUsersReadOnly, V3APIPermission) serializer_class = TagSerializer - permission_classes = [V3APIPermission] # Default cursor ordering key ordering = "-id" # Additional cursor ordering fields From b9fa9007fe9f8fa4cadb2f6349670873868abcd4 Mon Sep 17 00:00:00 2001 From: Alberto Islas Date: Wed, 16 Oct 2024 11:00:20 -0600 Subject: [PATCH 6/8] fix(api): Removed V3APIPermission from SearchV4ViewSet --- cl/search/api_views.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cl/search/api_views.py b/cl/search/api_views.py index 1a3ee78dea..7fb44eb083 100644 --- a/cl/search/api_views.py +++ b/cl/search/api_views.py @@ -313,7 +313,7 @@ def list(self, request, *args, **kwargs): class SearchV4ViewSet(LoggingMixin, viewsets.ViewSet): # Default permissions use Django permissions, so here we AllowAny, # but folks will need to log in to get past the thresholds. - permission_classes = (permissions.AllowAny, V3APIPermission) + permission_classes = (permissions.AllowAny,) supported_search_types = { SEARCH_TYPES.RECAP: { From 999ef2d3f2983e028e1fbf2c18cf69318168c557 Mon Sep 17 00:00:00 2001 From: Alberto Islas Date: Wed, 16 Oct 2024 12:37:27 -0600 Subject: [PATCH 7/8] fix(api): Create API events for v3 and v4 independently --- cl/api/tests.py | 41 +++++++++++++++++++++++++++++++++++++---- cl/api/utils.py | 14 +++++++++----- 2 files changed, 46 insertions(+), 9 deletions(-) diff --git a/cl/api/tests.py b/cl/api/tests.py index 3ab24f8859..bdbef55290 100644 --- a/cl/api/tests.py +++ b/cl/api/tests.py @@ -26,7 +26,7 @@ from cl.api.factories import WebhookEventFactory, WebhookFactory from cl.api.models import WEBHOOK_EVENT_STATUS, WebhookEvent, WebhookEventType from cl.api.pagination import VersionBasedPagination -from cl.api.utils import get_logging_prefix +from cl.api.utils import LoggingMixin, get_logging_prefix from cl.api.views import coverage_data from cl.api.webhooks import send_webhook_event from cl.audio.api_views import AudioViewSet @@ -346,14 +346,47 @@ async def hit_the_api(self, api_version) -> None: "cl.api.utils.get_logging_prefix", return_value="api:Test", ) - async def test_are_events_created_properly( + @mock.patch.object(LoggingMixin, "milestones", new=[1]) + async def test_are_v3_events_created_properly( self, mock_logging_prefix ) -> None: - """Are event objects created as API requests are made?""" + """Are event objects created as v3 API requests are made?""" await self.hit_the_api("v3") - expected_event_count = 1 + expected_event_count = 2 self.assertEqual(expected_event_count, await Event.objects.acount()) + event_descriptions = { + event.description async for event in Event.objects.all() + } + expected_descriptions = set() + expected_descriptions.add("API v3 has logged 1 total requests.") + expected_descriptions.add( + f"User '{self.user.username}' has placed their 1st API v3 request." + ) + self.assertEqual(event_descriptions, expected_descriptions) + + @mock.patch( + "cl.api.utils.get_logging_prefix", + return_value="api:Test", + ) + @mock.patch.object(LoggingMixin, "milestones", new=[1]) + async def test_are_v4_events_created_properly( + self, mock_logging_prefix + ) -> None: + """Are event objects created as V4 API requests are made?""" + await self.hit_the_api("v4") + + expected_event_count = 2 + self.assertEqual(expected_event_count, await Event.objects.acount()) + event_descriptions = { + event.description async for event in Event.objects.all() + } + expected_descriptions = set() + expected_descriptions.add("API v4 has logged 1 total requests.") + expected_descriptions.add( + f"User '{self.user.username}' has placed their 1st API v4 request." + ) + self.assertEqual(event_descriptions, expected_descriptions) # Set the api prefix so that other tests # run in parallel do not affect this one. diff --git a/cl/api/utils.py b/cl/api/utils.py index fe3ba4be72..3068cd4369 100644 --- a/cl/api/utils.py +++ b/cl/api/utils.py @@ -227,7 +227,7 @@ def finalize_response(self, request, response, *args, **kwargs): # noinspection PyBroadException try: results = self._log_request(request) - self._handle_events(results, request.user) + self._handle_events(results, request.user, request.version) except Exception as e: logger.exception( "Unable to log API response timing info: %s", e @@ -291,19 +291,23 @@ def _log_request(self, request): results = pipe.execute() return results - def _handle_events(self, results, user): + def _handle_events(self, results, user, api_version): total_count = results[0] user_count = results[4] if total_count in MILESTONES_FLAT: Event.objects.create( - description=f"API has logged {total_count} total requests." + description=f"API {api_version} has logged {total_count} total requests." ) if user.is_authenticated: if user_count in self.milestones: Event.objects.create( - description="User '%s' has placed their %s API request." - % (user.username, intcomma(ordinal(user_count))), + description="User '%s' has placed their %s API %s request." + % ( + user.username, + intcomma(ordinal(user_count)), + api_version, + ), user=user, ) From ae73dfcb05c06d08d765d3d53c9cee3c8d44f6ac Mon Sep 17 00:00:00 2001 From: Alberto Islas Date: Wed, 16 Oct 2024 18:35:17 -0600 Subject: [PATCH 8/8] fix(api): Changed approach for checking random requests in V3APIPermission --- cl/api/api_permissions.py | 7 +++---- cl/api/tests.py | 29 +++++++++++++++++++---------- 2 files changed, 22 insertions(+), 14 deletions(-) diff --git a/cl/api/api_permissions.py b/cl/api/api_permissions.py index 7ab20df700..6562da789f 100644 --- a/cl/api/api_permissions.py +++ b/cl/api/api_permissions.py @@ -1,4 +1,4 @@ -import time +import random from django.conf import settings from django.contrib.auth.models import AnonymousUser, User @@ -57,9 +57,8 @@ def is_v3_api_request(request: HttpRequest) -> bool: @staticmethod def check_request() -> bool: - # Check requests every 10 seconds. - # Notice that all requests within that second will be checked. - if int(time.time()) % 10 == 0: + # Check 1 in 50 requests. + if random.randint(1, 50) == 1: return True return False diff --git a/cl/api/tests.py b/cl/api/tests.py index bdbef55290..9037486b86 100644 --- a/cl/api/tests.py +++ b/cl/api/tests.py @@ -5,7 +5,6 @@ from unittest import mock from urllib.parse import parse_qs, urlparse -import time_machine from asgiref.sync import async_to_sync, sync_to_async from django.contrib.auth.hashers import make_password from django.contrib.auth.models import Permission @@ -23,6 +22,7 @@ from rest_framework.test import APIRequestFactory from cl.alerts.api_views import DocketAlertViewSet, SearchAlertViewSet +from cl.api.api_permissions import V3APIPermission from cl.api.factories import WebhookEventFactory, WebhookFactory from cl.api.models import WEBHOOK_EVENT_STATUS, WebhookEvent, WebhookEventType from cl.api.pagination import VersionBasedPagination @@ -472,9 +472,6 @@ def setUpTestData(cls) -> None: cls.audio_path_v3 = reverse("audio-list", kwargs={"version": "v3"}) cls.audio_path_v4 = reverse("audio-list", kwargs={"version": "v4"}) - cls.date_check_request = now().replace(second=10) - cls.date_dont_check_request = now().replace(second=3) - def setUp(self) -> None: self.r = get_redis_interface("STATS") self.flush_stats() @@ -507,14 +504,18 @@ async def test_block_v3_for_new_users(self, mock_api_prefix) -> None: # A new user request is not detected because we only check some # requests. In this case, it's not checked. - with time_machine.travel(self.date_dont_check_request, tick=False): + with mock.patch.object( + V3APIPermission, "check_request", return_value=False + ): response = await self.client_1.get( self.audio_path_v3, format="json" ) self.assertEqual(response.status_code, HTTPStatus.OK) # Now check the request and block the request and user. - with time_machine.travel(self.date_check_request, tick=False): + with mock.patch.object( + V3APIPermission, "check_request", return_value=True + ): response = await self.client_1.get( self.audio_path_v3, format="json" ) @@ -543,7 +544,9 @@ async def test_allow_v3_for_existing_users(self, mock_api_prefix) -> None: # Simulate v3 existing user and create v3 user list. self.r.zincrby("api-block-test:v3.user.counts", 1, self.user_2.pk) self.create_v3_user_list() - with time_machine.travel(self.date_check_request, tick=False): + with mock.patch.object( + V3APIPermission, "check_request", return_value=True + ): response = await self.client_2.get( self.audio_path_v3, format="json" ) @@ -551,7 +554,9 @@ async def test_allow_v3_for_existing_users(self, mock_api_prefix) -> None: async def test_block_v3_for_anonymous_users(self, mock_api_prefix) -> None: """Confirm anonymous v3 API users are blocked""" - with time_machine.travel(self.date_check_request, tick=False): + with mock.patch.object( + V3APIPermission, "check_request", return_value=True + ): response = await self.async_client.get(self.audio_path_v3) self.assertEqual(response.status_code, HTTPStatus.FORBIDDEN) self.assertEqual( @@ -561,7 +566,9 @@ async def test_block_v3_for_anonymous_users(self, mock_api_prefix) -> None: async def test_allow_v4_for_new_users(self, mock_api_prefix) -> None: """Confirm new API users are allowed to use V4 of the API""" - with time_machine.travel(self.date_check_request, tick=False): + with mock.patch.object( + V3APIPermission, "check_request", return_value=True + ): response = await self.client_2.get( self.audio_path_v4, format="json" ) @@ -569,7 +576,9 @@ async def test_allow_v4_for_new_users(self, mock_api_prefix) -> None: async def test_allow_v4_for_anonymous_users(self, mock_api_prefix) -> None: """Confirm V4 anonymous API users are allowed to use V4 of the API""" - with time_machine.travel(self.date_check_request, tick=False): + with mock.patch.object( + V3APIPermission, "check_request", return_value=True + ): response = await self.async_client.get(self.audio_path_v4) self.assertEqual(response.status_code, HTTPStatus.OK)