Skip to content

feat: Add rate limiting to identity search endpoint #6438

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
1-23-smy wants to merge 12 commits into
base: main
Choose a base branch
from
+41 −2
Open

feat: Add rate limiting to identity search endpoint #6438

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
eb6c827
feat: Add rate limiting to identity search endpoint
smy-637q Dec 22, 2025
db1cc77
fix: Apply identity search throttle only to list action
smy-637q Dec 22, 2025
14dc93f
Merge branch 'Flagsmith:main' into feature/identity-search-rate-limiting
1-23-smy Dec 22, 2025
aa244d5
fix: Add identity_search throttle rate to test settings
smy-637q Dec 31, 2025
a11a21e
Merge branch 'Flagsmith:main' into feature/identity-search-rate-limiting
1-23-smy Dec 31, 2025
c207b15
Merge branch 'feature/identity-search-rate-limiting' of https://githu…
1-23-smy Jan 18, 2026
b17cd1a
fix: Address reviewer feedback
smy-637q Jan 20, 2026
61b5b7e
Merge branch 'main' of https://github.com/1-23-smy/flagsmith
smy-637q Jan 20, 2026
761d0a8
feat: re-added-master-api-key-throttle
Zaimwa9 Jan 20, 2026
7dbb0dc
Merge branch 'main' of github.com:Flagsmith/flagsmith into feature/id…
Zaimwa9 Jan 20, 2026
ba7ab9c
feat: re-added-deleted-values
Zaimwa9 Jan 20, 2026
4d4dced
Merge branch 'main' into feature/identity-search-rate-limiting
1-23-smy Feb 5, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 4 additions & 2 deletions api/app/settings/common.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -302,8 +302,9 @@

LOGIN_THROTTLE_RATE = env("LOGIN_THROTTLE_RATE", "20/min")
SIGNUP_THROTTLE_RATE = env("SIGNUP_THROTTLE_RATE", "10000/min")
USER_THROTTLE_RATE = env("USER_THROTTLE_RATE", default=None)
MASTER_API_KEY_THROTTLE_RATE = env("MASTER_API_KEY_THROTTLE_RATE", default=None)
USER_THROTTLE_RATE = env("USER_THROTTLE_RATE", "500/min")
MASTER_API_KEY_THROTTLE_RATE = env("MASTER_API_KEY_THROTTLE_RATE", USER_THROTTLE_RATE)
Copy link

@cursor cursor bot Feb 5, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Undocumented breaking change to global throttle rate defaults

Medium Severity

The PR changes USER_THROTTLE_RATE and MASTER_API_KEY_THROTTLE_RATE defaults from None (no throttling) to "500/min", but this behavioral change isn't mentioned in the PR description. The stated scope is only adding rate limiting to the identity search endpoint. Existing deployments that had DEFAULT_THROTTLE_CLASSES configured but relied on the None default for these rates would suddenly have 500/min rate limiting applied to all admin API requests. If intentional, this change warrants explicit documentation in the PR description.

Fix in Cursor Fix in Web

IDENTITY_SEARCH_THROTTLE_RATE = env("IDENTITY_SEARCH_THROTTLE_RATE", "30/min")
Copy link

Copilot AI Feb 5, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The PR description mentions "Frontend Changes" that increase debounce from 500ms to 750ms in frontend/common/useDebouncedSearch.ts, but this file is not included in the current changes. Based on previous review feedback, there were concerns about this change affecting all components using the hook, and it was suggested to stick to 500ms if there's no conflict with the backend throttling. Either include the frontend changes in this PR or remove the mention from the PR description.

Copilot uses AI. Check for mistakes.
DEFAULT_THROTTLE_CLASSES = env.list("DEFAULT_THROTTLE_CLASSES", subcast=str, default=[])
REST_FRAMEWORK = {
"DEFAULT_PERMISSION_CLASSES": ["rest_framework.permissions.IsAuthenticated"],
Expand All @@ -324,6 +325,7 @@
"invite": "10/min",
"user": USER_THROTTLE_RATE,
"influx_query": "5/min",
"identity_search": IDENTITY_SEARCH_THROTTLE_RATE,
},
"DEFAULT_FILTER_BACKENDS": ["django_filters.rest_framework.DjangoFilterBackend"],
"DEFAULT_RENDERER_CLASSES": [
Expand Down
1 change: 1 addition & 0 deletions api/app/settings/test.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
"user": "100000/day",
"master_api_key": "100000/day",
"influx_query": "50/min",
"identity_search": "100/min",
}

AWS_SSE_LOGS_BUCKET_NAME = "test_bucket"
Expand Down
11 changes: 11 additions & 0 deletions api/environments/identities/views.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
from rest_framework import status, viewsets
from rest_framework.permissions import IsAuthenticated
from rest_framework.response import Response
from rest_framework.throttling import ScopedRateThrottle

from app.pagination import CustomPagination
from core.constants import FLAGSMITH_UPDATED_AT_HEADER, SDK_ENVIRONMENT_KEY_HEADER
Expand All @@ -41,6 +42,16 @@
class IdentityViewSet(viewsets.ModelViewSet): # type: ignore[type-arg]
serializer_class = IdentitySerializer
pagination_class = CustomPagination
throttle_scope = "identity_search"

def get_throttles(self): # type: ignore[no-untyped-def]
"""
Apply identity_search throttle only to list (search) requests.
For other actions, return the global default throttle classes.
"""
if getattr(self, "action", None) == "list":
Copy link

Copilot AI Feb 5, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The comment states "Apply identity_search throttle only to list (search) requests" but the implementation applies throttling to ALL list requests, regardless of whether they include a search query parameter. The current implementation throttles both /identities/ and /identities/?q=test equally.

If the intent is to only throttle search requests (when the 'q' parameter is present), the condition should check for the presence of the query parameter. Otherwise, if throttling all list requests is the intended behavior, update the comment to accurately reflect this.

Suggested change
if getattr(self, "action", None) == "list":
if getattr(self, "action", None) == "list" and "q" in self.request.query_params:

Copilot uses AI. Check for mistakes.
return [ScopedRateThrottle()]
return super().get_throttles()

def get_queryset(self): # type: ignore[no-untyped-def]
if getattr(self, "swagger_fake_view", False):
Expand Down
25 changes: 25 additions & 0 deletions api/tests/unit/environments/identities/test_unit_identities_views.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -305,6 +305,31 @@ def test_search_identities_still_allows_paging(
assert response2.data["results"]


def test_identity_search_is_throttled(
admin_client: APIClient,
environment: Environment,
reset_cache: None,
mocker: MockerFixture,
) -> None:
# Given - mock the throttle rate to be restrictive for testing
mocker.patch(
"rest_framework.throttling.ScopedRateThrottle.get_rate", return_value="1/minute"
)
base_url = reverse(
"api-v1:environments:environment-identities-list",
args=[environment.api_key],
)
url = f"{base_url}?q=test"

# When - make 2 requests in quick succession
response1 = admin_client.get(url)
response2 = admin_client.get(url)

# Then - first should succeed, second should be throttled
assert response1.status_code == status.HTTP_200_OK
assert response2.status_code == status.HTTP_429_TOO_MANY_REQUESTS

Comment on lines +308 to +331
Copy link

Copilot AI Feb 5, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The test only verifies that the list action with a search query is throttled, but it doesn't verify that other actions (create, retrieve, update, delete) are NOT throttled. Following the pattern from api/tests/integration/custom_auth/end_to_end/test_custom_auth_integration.py:563-575 (test_get_user_is_not_throttled), consider adding a test to ensure other actions on the IdentityViewSet are not affected by the throttle.

Copilot uses AI. Check for mistakes.

def test_can_delete_identity(
environment: Environment,
admin_client: APIClient,
Expand Down