diff --git a/mainsite/settings/__init__.py b/mainsite/settings/__init__.py index e4fe5123..72a181c9 100644 --- a/mainsite/settings/__init__.py +++ b/mainsite/settings/__init__.py @@ -432,13 +432,21 @@ MEDIA_URL = f"/m/{PEERINGDB_VERSION}/" STATIC_ROOT = os.path.abspath(os.path.join(BASE_DIR, "static")) STATIC_URL = f"/s/{PEERINGDB_VERSION}/" +# maximum number of entries in the cache +set_option("CACHE_MAX_ENTRIES", 5000) + +# dont allow going below 5000 (#1151) +if CACHE_MAX_ENTRIES < 5000: + raise ValueError("CACHE_MAX_ENTRIES needs to be >= 5000 (#1151)") + + CACHES = { "default": { "BACKEND": "django.core.cache.backends.db.DatabaseCache", "LOCATION": "django_cache", "OPTIONS": { # maximum number of entries in the cache - "MAX_ENTRIES": 5000, + "MAX_ENTRIES": CACHE_MAX_ENTRIES, # once max entries are reach delete 500 of the oldest entries "CULL_FREQUENCY": 10, }, diff --git a/peeringdb_server/exceptions.py b/peeringdb_server/exceptions.py index 5c83ba46..4a3cfb32 100644 --- a/peeringdb_server/exceptions.py +++ b/peeringdb_server/exceptions.py @@ -6,6 +6,7 @@ def format_wait_time(wait_time): """ Format wait time in seconds to human readable format """ + if wait_time < 60: return f"{wait_time} seconds" elif wait_time < 3600 and wait_time > 60: diff --git a/peeringdb_server/middleware.py b/peeringdb_server/middleware.py index ec248a21..7b1827c5 100644 --- a/peeringdb_server/middleware.py +++ b/peeringdb_server/middleware.py @@ -106,7 +106,7 @@ class PDBPermissionMiddleware(MiddlewareMixin): # session auth already exists, set x-auth-id value and return if request.user.is_authenticated: - request.auth_id = request.user.username + request.auth_id = f"u{request.user.id}" # request attempting to provide separate authentication while # already authenticated through session cookie, fail with @@ -131,14 +131,17 @@ class PDBPermissionMiddleware(MiddlewareMixin): user = authenticate(username=username, password=password) # return username input in x-auth-id header - request.auth_id = username + if user: + request.auth_id = f"u{user.id}" # if user is not authenticated return 401 Unauthorized - if not user: + else: # truncate the username if needed. if len(username) > 255: request.auth_id = username[:255] + else: + request.auth_id = username return self.response_unauthorized( request, message="Invalid username or password", status=401 @@ -168,8 +171,14 @@ class PDBPermissionMiddleware(MiddlewareMixin): ) # If API key is provided, check if the user has an active session - if api_key: - request.auth_id = f"apikey_{api_key.prefix}" + else: + + if isinstance(api_key, OrganizationAPIKey): + prefix = f"o{api_key.org_id}" + else: + prefix = f"u{api_key.user_id}" + + request.auth_id = f"{prefix}_apikey_{api_key.prefix}" if request.session.get("_auth_user_id") and request.user.id: if int(request.user.id) == int( request.session.get("_auth_user_id") diff --git a/peeringdb_server/rest_throttles.py b/peeringdb_server/rest_throttles.py index 56b8c8d9..4df142da 100644 --- a/peeringdb_server/rest_throttles.py +++ b/peeringdb_server/rest_throttles.py @@ -55,6 +55,42 @@ class TargetedRateThrottle(throttling.SimpleRateThrottle): msg_setting ) + def wait(self): + """ + Returns the recommended next request time in seconds. + + This is a custom implmentation of the original wait() logic that can + also handle dynamic downward adjustments of rate limits (through + changing EnvironmentSetting variables for example) + """ + if self.history: + remaining_duration = self.duration - (self.now - self.history[-1]) + else: + remaining_duration = self.duration + + available_requests = self.num_requests - len(self.history) + 1 + if available_requests < 1: + + # a negative/zero value only occurs when rate limit has been adjusted + # while already being tracked for the requesting client + + remaining_duration = self.duration + new_history = [] + for entry in self.history[: self.num_requests]: + diff = self.now - entry + if diff < self.duration: + new_history.append(entry) + remaining_duration = self.duration - diff + + self.history = new_history + self.cache.set(self.key, self.history, self.duration) + available_requests = self.num_requests - len(self.history) + 1 + + if available_requests < 1: + return None + + return remaining_duration / float(available_requests) + def _allow_request_user_auth(self, request, view, ident_prefix=""): self.ident = f"{ident_prefix}user:{self.user.pk}" self.scope = self.scope_user diff --git a/tests/test_api_throttle.py b/tests/test_api_throttle.py index bdb90198..2c26fee0 100644 --- a/tests/test_api_throttle.py +++ b/tests/test_api_throttle.py @@ -66,11 +66,11 @@ class APIThrottleTests(TestCase): cache.clear() self.factory = APIRequestFactory() - env = models.EnvironmentSetting( + self.rate_anon = env = models.EnvironmentSetting( setting="API_THROTTLE_RATE_ANON", value_str="10/minute" ) env.save() - env = models.EnvironmentSetting( + self.rate_user = env = models.EnvironmentSetting( setting="API_THROTTLE_RATE_USER", value_str="10/minute" ) env.save() @@ -139,6 +139,64 @@ class APIThrottleTests(TestCase): assert response.status_code == 429 assert "Rate limit exceeded (anon)" in response.data["message"] + def test_anon_requests_above_throttle_rate_dynamic_changes(self): + """ + Ensure request rate is limited for anonymous users while + changing the rate between requests + """ + + request = self.factory.get("/") + for dummy in range(11): + response = MockView.as_view({"get": "get"})(request) + assert response.status_code == 429 + assert "Rate limit exceeded (anon)" in response.data["message"] + + # adjust rate limit downwards + + self.rate_anon.value_str = "1/minute" + self.rate_anon.save() + + # still rate limited, no error + + response = MockView.as_view({"get": "get"})(request) + assert response.status_code == 429 + + # adjust rate limit upwards + + self.rate_anon.value_str = "100/minute" + self.rate_anon.save() + + # no longer rate limited, no error + + response = MockView.as_view({"get": "get"})(request) + assert response.status_code == 200 + + # adjust rate limit downwards (change duration) + + self.rate_anon.value_str = "1/hour" + self.rate_anon.save() + + # rate limited again, no error + + response = MockView.as_view({"get": "get"})(request) + assert response.status_code == 429 + + # adjust rate limit upwards (change duration) + + self.rate_anon.value_str = "20/hour" + self.rate_anon.save() + + # no longer rate limited (19 attempts), no error + + for idx in range(19): + response = MockView.as_view({"get": "get"})(request) + assert response.status_code == 200 + + # rate limited again, no error + + response = MockView.as_view({"get": "get"})(request) + assert response.status_code == 429 + def test_authenticated_requests_above_throttle_rate(self): """ Ensure request rate is not limited for authenticated users @@ -153,6 +211,66 @@ class APIThrottleTests(TestCase): assert response.status_code == 429 assert "Rate limit exceeded (user)" in response.data["message"] + def test_authenticated_requests_above_throttle_rate_dynamic_changes(self): + """ + Ensure request rate is not limited for authenticated users + """ + + user = models.User(username="test") + user.save() + request = self.factory.get("/") + request.user = user + for dummy in range(11): + response = MockView.as_view({"get": "get"})(request) + assert response.status_code == 429 + assert "Rate limit exceeded (user)" in response.data["message"] + + # adjust rate limit downwards + + self.rate_user.value_str = "1/minute" + self.rate_user.save() + + # still rate limited, no error + + response = MockView.as_view({"get": "get"})(request) + assert response.status_code == 429 + + # adjust rate limit upwards + + self.rate_user.value_str = "100/minute" + self.rate_user.save() + + # no longer rate limited, no error + + response = MockView.as_view({"get": "get"})(request) + assert response.status_code == 200 + + # adjust rate limit downwards (change duration) + + self.rate_user.value_str = "1/hour" + self.rate_user.save() + + # rate limited again, no error + + response = MockView.as_view({"get": "get"})(request) + assert response.status_code == 429 + + # adjust rate limit upwards (change duration) + + self.rate_user.value_str = "20/hour" + self.rate_user.save() + + # no longer rate limited (19 attempts), no error + + for idx in range(19): + response = MockView.as_view({"get": "get"})(request) + assert response.status_code == 200 + + # rate limited again, no error + + response = MockView.as_view({"get": "get"})(request) + assert response.status_code == 429 + def test_response_size_ip_block(self): """ Ensure request rate is limited based on response size diff --git a/tests/test_middleware.py b/tests/test_middleware.py index c047efef..578068b9 100644 --- a/tests/test_middleware.py +++ b/tests/test_middleware.py @@ -10,7 +10,8 @@ from django.test import ( from rest_framework.test import APIClient, APITestCase from peeringdb_server.middleware import PDBCommonMiddleware -from peeringdb_server.models import User, UserAPIKey +from peeringdb_server.models import Organization, OrganizationAPIKey, User, UserAPIKey +from .util import reset_group_ids def get_response_empty(request): @@ -67,7 +68,31 @@ class PDBPermissionMiddlewareTest(APITestCase): self.client.credentials(HTTP_AUTHORIZATION=f"Api-Key {key}") response = self.client.get("/api/fac") self.assertEqual(response.status_code, 200) - assert response.headers.get("X-Auth-ID").startswith("apikey_") + assert ( + response.headers.get("X-Auth-ID") == f"u{user.id}_apikey_{api_key.prefix}" + ) + + # test that header gets cleared between requests + other_client = APIClient() + response = other_client.get("/api/fac") + self.assertEqual(response.status_code, 200) + assert response.headers.get("X-Auth-ID") is None + + def test_auth_id_org_api_key(self): + reset_group_ids() + + org = Organization.objects.create(name="Test org", status="ok") + + # Create an API key for the user + api_key, key = OrganizationAPIKey.objects.create_key( + name="test", + org=org, + ) + + self.client.credentials(HTTP_AUTHORIZATION=f"Api-Key {key}") + response = self.client.get("/api/fac") + self.assertEqual(response.status_code, 200) + assert response.headers.get("X-Auth-ID") == f"o{org.id}_apikey_{api_key.prefix}" # test that header gets cleared between requests other_client = APIClient() @@ -83,7 +108,7 @@ class PDBPermissionMiddlewareTest(APITestCase): self.client.force_login(user) response = self.client.get("/api/fac") self.assertEqual(response.status_code, 200) - assert response.headers.get("X-Auth-ID") == user.username + assert response.headers.get("X-Auth-ID") == f"u{user.id}" # test that header gets cleared between requests other_client = APIClient() @@ -101,7 +126,7 @@ class PDBPermissionMiddlewareTest(APITestCase): response = self.client.get("/api/fac") self.assertEqual(response.status_code, 200) - assert response.headers.get("X-Auth-ID") == user.username + assert response.headers.get("X-Auth-ID") == f"u{user.id}" # test that header gets cleared between requests other_client = APIClient()