diff --git a/CHANGELOG.md b/CHANGELOG.md index 3fa4b68e..b83d0825 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -21,6 +21,8 @@ and this project adheres to - 🐛(back) allow only images to be used with the cors-proxy #781 - 🐛(backend) stop returning inactive users on the list endpoint #636 - 🔒️(backend) require at least 5 characters to search for users #636 +- 🔒️(back) throttle user list endpoint #636 +- 🔒️(back) remove pagination and limit to 5 for user list endpoint #636 ## [2.5.0] - 2025-03-18 diff --git a/src/backend/core/api/viewsets.py b/src/backend/core/api/viewsets.py index d52c0ad8..20f96eb8 100644 --- a/src/backend/core/api/viewsets.py +++ b/src/backend/core/api/viewsets.py @@ -143,6 +143,7 @@ class UserViewSet( permission_classes = [permissions.IsSelf] queryset = models.User.objects.filter(is_active=True) serializer_class = serializers.UserSerializer + pagination_class = None def get_queryset(self): """ @@ -157,10 +158,10 @@ class UserViewSet( return queryset # Exclude all users already in the given document - if document_id := self.request.GET.get("document_id", ""): + if document_id := self.request.query_params.get("document_id", ""): queryset = queryset.exclude(documentaccess__document_id=document_id) - if not (query := self.request.GET.get("q", "")) or len(query) < 5: + if not (query := self.request.query_params.get("q", "")) or len(query) < 5: return queryset.none() # For emails, match emails by Levenstein distance to prevent typing errors @@ -170,7 +171,7 @@ class UserViewSet( distance=RawSQL("levenshtein(email::text, %s::text)", (query,)) ) .filter(distance__lte=3) - .order_by("distance", "email") + .order_by("distance", "email")[: settings.API_USERS_LIST_LIMIT] ) # Use trigram similarity for non-email-like queries @@ -180,7 +181,7 @@ class UserViewSet( queryset.filter(email__trigram_word_similar=query) .annotate(similarity=TrigramSimilarity("email", query)) .filter(similarity__gt=0.2) - .order_by("-similarity", "email") + .order_by("-similarity", "email")[: settings.API_USERS_LIST_LIMIT] ) @drf.decorators.action( diff --git a/src/backend/core/tests/test_api_users.py b/src/backend/core/tests/test_api_users.py index eb3fd14d..37129dc8 100644 --- a/src/backend/core/tests/test_api_users.py +++ b/src/backend/core/tests/test_api_users.py @@ -37,7 +37,7 @@ def test_api_users_list_authenticated(): ) assert response.status_code == 200 content = response.json() - assert content["results"] == [] + assert content == [] def test_api_users_list_query_email(): @@ -58,24 +58,54 @@ def test_api_users_list_query_email(): "/api/v1.0/users/?q=david.bowman@work.com", ) assert response.status_code == 200 - user_ids = [user["id"] for user in response.json()["results"]] + user_ids = [user["id"] for user in response.json()] assert user_ids == [str(dave.id)] response = client.get( "/api/v1.0/users/?q=davig.bovman@worm.com", ) assert response.status_code == 200 - user_ids = [user["id"] for user in response.json()["results"]] + user_ids = [user["id"] for user in response.json()] assert user_ids == [str(dave.id)] response = client.get( "/api/v1.0/users/?q=davig.bovman@worm.cop", ) assert response.status_code == 200 - user_ids = [user["id"] for user in response.json()["results"]] + user_ids = [user["id"] for user in response.json()] assert user_ids == [] +def test_api_users_list_limit(settings): + """ + Authenticated users should be able to list users and the number of results + should be limited to 10. + """ + user = factories.UserFactory() + + client = APIClient() + client.force_login(user) + + # Use a base name with a length equal 5 to test that the limit is applied + base_name = "alice" + for i in range(15): + factories.UserFactory(email=f"{base_name}.{i}@example.com") + + response = client.get( + "/api/v1.0/users/?q=alice", + ) + assert response.status_code == 200 + assert len(response.json()) == 5 + + # if the limit is changed, all users should be returned + settings.API_USERS_LIST_LIMIT = 100 + response = client.get( + "/api/v1.0/users/?q=alice", + ) + assert response.status_code == 200 + assert len(response.json()) == 15 + + def test_api_users_list_query_email_matching(): """While filtering by email, results should be filtered and sorted by Levenstein distance.""" user = factories.UserFactory() @@ -94,13 +124,13 @@ def test_api_users_list_query_email_matching(): "/api/v1.0/users/?q=alice.johnson@example.gouv.fr", ) assert response.status_code == 200 - user_ids = [user["id"] for user in response.json()["results"]] + user_ids = [user["id"] for user in response.json()] assert user_ids == [str(user1.id), str(user2.id), str(user3.id), str(user4.id)] response = client.get("/api/v1.0/users/?q=alicia.johnnson@example.gouv.fr") assert response.status_code == 200 - user_ids = [user["id"] for user in response.json()["results"]] + user_ids = [user["id"] for user in response.json()] assert user_ids == [str(user4.id), str(user2.id), str(user1.id), str(user5.id)] @@ -126,7 +156,7 @@ def test_api_users_list_query_email_exclude_doc_user(): ) assert response.status_code == 200 - user_ids = [user["id"] for user in response.json()["results"]] + user_ids = [user["id"] for user in response.json()] assert user_ids == [str(nicole_fool.id)] @@ -143,15 +173,15 @@ def test_api_users_list_query_short_queries(): response = client.get("/api/v1.0/users/?q=jo") assert response.status_code == 200 - assert response.json()["results"] == [] + assert response.json() == [] response = client.get("/api/v1.0/users/?q=john") assert response.status_code == 200 - assert response.json()["results"] == [] + assert response.json() == [] response = client.get("/api/v1.0/users/?q=john.") assert response.status_code == 200 - assert len(response.json()["results"]) == 2 + assert len(response.json()) == 2 def test_api_users_list_query_inactive(): @@ -166,7 +196,7 @@ def test_api_users_list_query_inactive(): response = client.get("/api/v1.0/users/?q=john.") assert response.status_code == 200 - user_ids = [user["id"] for user in response.json()["results"]] + user_ids = [user["id"] for user in response.json()] assert user_ids == [str(lennon.id)] diff --git a/src/backend/impress/settings.py b/src/backend/impress/settings.py index 4a741f7c..c8979f74 100755 --- a/src/backend/impress/settings.py +++ b/src/backend/impress/settings.py @@ -604,6 +604,12 @@ class Base(Configuration): }, } + API_USERS_LIST_LIMIT = values.PositiveIntegerValue( + default=5, + environ_name="API_USERS_LIST_LIMIT", + environ_prefix=None, + ) + # pylint: disable=invalid-name @property def ENVIRONMENT(self):