From ed8a2c1c9b4d42146d120b588a5bd62e88d6947e Mon Sep 17 00:00:00 2001 From: Tushar Goel Date: Sat, 29 Oct 2022 16:21:09 +0530 Subject: [PATCH 1/4] Enable throttling Signed-off-by: Tushar Goel --- vulnerabilities/tests/test_fix_api.py | 7 ++-- vulnerabilities/throttling.py | 52 +++++++++++++++++++++++++++ vulnerablecode/settings.py | 4 +++ 3 files changed, 60 insertions(+), 3 deletions(-) create mode 100644 vulnerabilities/throttling.py diff --git a/vulnerabilities/tests/test_fix_api.py b/vulnerabilities/tests/test_fix_api.py index 4b13fccc9..673c91e75 100644 --- a/vulnerabilities/tests/test_fix_api.py +++ b/vulnerabilities/tests/test_fix_api.py @@ -259,12 +259,13 @@ def test_api_with_single_vulnerability_and_vulnerable_package(self): } def test_api_with_all_vulnerable_packages(self): - with self.assertNumQueries(4): + with self.assertNumQueries(5): # There are 4 queries: # 1. SAVEPOINT # 2. Authenticating user - # 3. Get all vulnerable packages - # 4. RELEASE SAVEPOINT + # 3. Checking if user is staff user for throttling purposes + # 4. Get all vulnerable packages + # 5. RELEASE SAVEPOINT response = self.csrf_client.get(f"/api/packages/all", format="json").data assert len(response) == 11 assert response == [ diff --git a/vulnerabilities/throttling.py b/vulnerabilities/throttling.py new file mode 100644 index 000000000..97a9508f3 --- /dev/null +++ b/vulnerabilities/throttling.py @@ -0,0 +1,52 @@ +# +# Copyright (c) nexB Inc. and others. All rights reserved. +# VulnerableCode is a trademark of nexB Inc. +# SPDX-License-Identifier: Apache-2.0 +# See http://www.apache.org/licenses/LICENSE-2.0 for the license text. +# See https://github.com/nexB/vulnerablecode for support or download. +# See https://aboutcode.org for more information about nexB OSS projects. +# + +from django.contrib.auth import get_user_model +from rest_framework.throttling import UserRateThrottle + +User = get_user_model() + + +class ExceptionalUserRateThrottle(UserRateThrottle): + def allow_request(self, request, view): + """ + Give special access to a few special accounts. + + Mirrors code in super class with minor tweaks. + """ + if self.rate is None: + return True + + self.key = self.get_cache_key(request, view) + if self.key is None: + return True + + self.history = self.cache.get(self.key, []) + self.now = self.timer() + + # Adjust if user has special privileges. + + user = User.objects.get(username=request.user.username) + + if user: + if user.is_superuser or user.is_staff: + # No throttling for superusers or staff. + return True + + else: + self.num_requests = self.num_requests + self.duration = self.duration + + # Drop any requests from the history which have now passed the + # throttle duration + while self.history and self.history[-1] <= self.now - self.duration: + self.history.pop() + if len(self.history) >= self.num_requests: + return self.throttle_failure() + return self.throttle_success() diff --git a/vulnerablecode/settings.py b/vulnerablecode/settings.py index 2cf0172ee..638c8b049 100644 --- a/vulnerablecode/settings.py +++ b/vulnerablecode/settings.py @@ -184,6 +184,10 @@ "django_filters.rest_framework.DjangoFilterBackend", "rest_framework.filters.SearchFilter", ), + "DEFAULT_THROTTLE_CLASSES": [ + "vulnerabilities.throttling.ExceptionalUserRateThrottle", + ], + "DEFAULT_THROTTLE_RATES": {"user": "1000/hour"}, "DEFAULT_PAGINATION_CLASS": "vulnerabilities.pagination.SmallResultSetPagination", # Limit the load on the Database returning a small number of records by default. https://github.com/nexB/vulnerablecode/issues/819 "PAGE_SIZE": 10, From b604103920e9c675cc8566cf91c1b2e9880b97fc Mon Sep 17 00:00:00 2001 From: Tushar Goel Date: Sat, 29 Oct 2022 16:58:20 +0530 Subject: [PATCH 2/4] Add tests for API throttling Signed-off-by: Tushar Goel --- vulnerabilities/tests/test_throttling.py | 48 ++++++++++++++++++++++++ vulnerablecode/settings.py | 5 ++- 2 files changed, 52 insertions(+), 1 deletion(-) create mode 100644 vulnerabilities/tests/test_throttling.py diff --git a/vulnerabilities/tests/test_throttling.py b/vulnerabilities/tests/test_throttling.py new file mode 100644 index 000000000..ade4726ef --- /dev/null +++ b/vulnerabilities/tests/test_throttling.py @@ -0,0 +1,48 @@ +# +# Copyright (c) nexB Inc. and others. All rights reserved. +# VulnerableCode is a trademark of nexB Inc. +# SPDX-License-Identifier: Apache-2.0 +# See http://www.apache.org/licenses/LICENSE-2.0 for the license text. +# See https://github.com/nexB/vulnerablecode for support or download. +# See https://aboutcode.org for more information about nexB OSS projects. +# + +from django.contrib.auth import get_user_model +from rest_framework.test import APIClient +from rest_framework.test import APITestCase + +User = get_user_model() + + +class ThrottleApiTests(APITestCase): + def setUp(self): + # create a basic user + self.user = User.objects.create_user("username", "e@mail.com", "secret") + self.auth = f"Token {self.user.auth_token.key}" + self.csrf_client = APIClient(enforce_csrf_checks=True) + self.csrf_client.credentials(HTTP_AUTHORIZATION=self.auth) + + # create a staff user + self.staff_user = User.objects.create_user( + "staff", "staff@mail.com", "secret", is_staff=True + ) + self.staff_auth = f"Token {self.staff_user.auth_token.key}" + self.staff_csrf_client = APIClient(enforce_csrf_checks=True) + self.staff_csrf_client.credentials(HTTP_AUTHORIZATION=self.staff_auth) + + def test_api_throttling(self): + + # A basic user can only access API 5 times a day + for i in range(0, 5): + response = self.csrf_client.get("/api/packages") + self.assertEqual(response.status_code, 200) + response = self.staff_csrf_client.get("/api/packages") + self.assertEqual(response.status_code, 200) + + response = self.csrf_client.get("/api/packages") + # 429 - too many requests for basic user + self.assertEqual(response.status_code, 429) + + response = self.staff_csrf_client.get("/api/packages", format="json") + # 200 - staff user can access API unlimited times + self.assertEqual(response.status_code, 200) diff --git a/vulnerablecode/settings.py b/vulnerablecode/settings.py index 638c8b049..e5e95cfd1 100644 --- a/vulnerablecode/settings.py +++ b/vulnerablecode/settings.py @@ -150,9 +150,12 @@ LOGIN_REDIRECT_URL = "/" LOGOUT_REDIRECT_URL = "/" +THROTTLING_RATE = env.str("THROTTLING_RATE", default="1000/day") if IS_TESTS: VULNERABLECODEIO_REQUIRE_AUTHENTICATION = True + THROTTLING_RATE = "5/day" + USE_L10N = True @@ -187,7 +190,7 @@ "DEFAULT_THROTTLE_CLASSES": [ "vulnerabilities.throttling.ExceptionalUserRateThrottle", ], - "DEFAULT_THROTTLE_RATES": {"user": "1000/hour"}, + "DEFAULT_THROTTLE_RATES": {"user": THROTTLING_RATE}, "DEFAULT_PAGINATION_CLASS": "vulnerabilities.pagination.SmallResultSetPagination", # Limit the load on the Database returning a small number of records by default. https://github.com/nexB/vulnerablecode/issues/819 "PAGE_SIZE": 10, From ceed4e4e790175c68480286c29f5b16e31fc4782 Mon Sep 17 00:00:00 2001 From: Tushar Goel Date: Mon, 31 Oct 2022 22:46:13 +0530 Subject: [PATCH 3/4] Add CHANGELOG Signed-off-by: Tushar Goel --- CHANGELOG.rst | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/CHANGELOG.rst b/CHANGELOG.rst index c8093726d..814febff3 100644 --- a/CHANGELOG.rst +++ b/CHANGELOG.rst @@ -3,6 +3,13 @@ Release notes +Version v30.2.2 +---------------- + +- We enabled API throttling for a basic user and for a staff user + they can have unlimited access on API. + + Version v30.2.1 ---------------- From 11d6bd045183fb99a74834ab454c165b0ae5182c Mon Sep 17 00:00:00 2001 From: Tushar Goel Date: Tue, 1 Nov 2022 19:10:26 +0530 Subject: [PATCH 4/4] Address review comments Signed-off-by: Tushar Goel --- vulnerabilities/tests/test_fix_api.py | 7 +++--- vulnerabilities/throttling.py | 36 +++------------------------ vulnerablecode/settings.py | 2 +- 3 files changed, 8 insertions(+), 37 deletions(-) diff --git a/vulnerabilities/tests/test_fix_api.py b/vulnerabilities/tests/test_fix_api.py index 673c91e75..4b13fccc9 100644 --- a/vulnerabilities/tests/test_fix_api.py +++ b/vulnerabilities/tests/test_fix_api.py @@ -259,13 +259,12 @@ def test_api_with_single_vulnerability_and_vulnerable_package(self): } def test_api_with_all_vulnerable_packages(self): - with self.assertNumQueries(5): + with self.assertNumQueries(4): # There are 4 queries: # 1. SAVEPOINT # 2. Authenticating user - # 3. Checking if user is staff user for throttling purposes - # 4. Get all vulnerable packages - # 5. RELEASE SAVEPOINT + # 3. Get all vulnerable packages + # 4. RELEASE SAVEPOINT response = self.csrf_client.get(f"/api/packages/all", format="json").data assert len(response) == 11 assert response == [ diff --git a/vulnerabilities/throttling.py b/vulnerabilities/throttling.py index 97a9508f3..e98db3806 100644 --- a/vulnerabilities/throttling.py +++ b/vulnerabilities/throttling.py @@ -13,40 +13,12 @@ User = get_user_model() -class ExceptionalUserRateThrottle(UserRateThrottle): +class StaffUserRateThrottle(UserRateThrottle): def allow_request(self, request, view): """ - Give special access to a few special accounts. - - Mirrors code in super class with minor tweaks. + Do not apply throttling for superusers and admins. """ - if self.rate is None: - return True - - self.key = self.get_cache_key(request, view) - if self.key is None: + if request.user.is_superuser or request.user.is_staff: return True - self.history = self.cache.get(self.key, []) - self.now = self.timer() - - # Adjust if user has special privileges. - - user = User.objects.get(username=request.user.username) - - if user: - if user.is_superuser or user.is_staff: - # No throttling for superusers or staff. - return True - - else: - self.num_requests = self.num_requests - self.duration = self.duration - - # Drop any requests from the history which have now passed the - # throttle duration - while self.history and self.history[-1] <= self.now - self.duration: - self.history.pop() - if len(self.history) >= self.num_requests: - return self.throttle_failure() - return self.throttle_success() + return super().allow_request(request, view) diff --git a/vulnerablecode/settings.py b/vulnerablecode/settings.py index e5e95cfd1..99b52a23a 100644 --- a/vulnerablecode/settings.py +++ b/vulnerablecode/settings.py @@ -188,7 +188,7 @@ "rest_framework.filters.SearchFilter", ), "DEFAULT_THROTTLE_CLASSES": [ - "vulnerabilities.throttling.ExceptionalUserRateThrottle", + "vulnerabilities.throttling.StaffUserRateThrottle", ], "DEFAULT_THROTTLE_RATES": {"user": THROTTLING_RATE}, "DEFAULT_PAGINATION_CLASS": "vulnerabilities.pagination.SmallResultSetPagination",