From 9ab0759e38492d9950d66299ed5c58155d39e696 Mon Sep 17 00:00:00 2001 From: kahnjw Date: Fri, 6 Dec 2013 14:21:33 -0800 Subject: [PATCH 1/6] Add tests to pass for get_ident method in BaseThrottle class. --- rest_framework/tests/test_throttling.py | 65 +++++++++++++++++++++++++ 1 file changed, 65 insertions(+) diff --git a/rest_framework/tests/test_throttling.py b/rest_framework/tests/test_throttling.py index 41bff6926..031276968 100644 --- a/rest_framework/tests/test_throttling.py +++ b/rest_framework/tests/test_throttling.py @@ -5,6 +5,7 @@ from __future__ import unicode_literals from django.test import TestCase from django.contrib.auth.models import User from django.core.cache import cache +from rest_framework.settings import api_settings from rest_framework.test import APIRequestFactory from rest_framework.views import APIView from rest_framework.throttling import BaseThrottle, UserRateThrottle, ScopedRateThrottle @@ -275,3 +276,67 @@ class ScopedRateThrottleTests(TestCase): self.increment_timer() response = self.unscoped_view(request) self.assertEqual(200, response.status_code) + +class XffTestingBase(TestCase): + def setUp(self): + + class Throttle(ScopedRateThrottle): + THROTTLE_RATES = {'test_limit': '1/day'} + TIMER_SECONDS = 0 + timer = lambda self: self.TIMER_SECONDS + + class View(APIView): + throttle_classes = (Throttle,) + throttle_scope = 'test_limit' + + def get(self, request): + return Response('test_limit') + + cache.clear() + self.throttle = Throttle() + self.view = View.as_view() + self.request = APIRequestFactory().get('/some_uri') + self.request.META['REMOTE_ADDR'] = '3.3.3.3' + self.request.META['HTTP_X_FORWARDED_FOR'] = '0.0.0.0, 1.1.1.1, 2.2.2.2' + + def config_proxy(self, num_proxies): + setattr(api_settings, 'NUM_PROXIES', num_proxies) + + +class IdWithXffBasicTests(XffTestingBase): + def test_accepts_request_under_limit(self): + self.config_proxy(0) + self.assertEqual(200, self.view(self.request).status_code) + + def test_denies_request_over_limit(self): + self.config_proxy(0) + self.view(self.request) + self.assertEqual(429, self.view(self.request).status_code) + + +class XffSpoofingTests(XffTestingBase): + def test_xff_spoofing_doesnt_change_machine_id_with_one_app_proxy(self): + self.config_proxy(1) + self.view(self.request) + self.request.META['HTTP_X_FORWARDED_FOR'] = '4.4.4.4, 5.5.5.5, 2.2.2.2' + self.assertEqual(429, self.view(self.request).status_code) + + def test_xff_spoofing_doesnt_change_machine_id_with_two_app_proxies(self): + self.config_proxy(2) + self.view(self.request) + self.request.META['HTTP_X_FORWARDED_FOR'] = '4.4.4.4, 1.1.1.1, 2.2.2.2' + self.assertEqual(429, self.view(self.request).status_code) + + +class XffUniqueMachinesTest(XffTestingBase): + def test_unique_clients_are_counted_independently_with_one_proxy(self): + self.config_proxy(1) + self.view(self.request) + self.request.META['HTTP_X_FORWARDED_FOR'] = '0.0.0.0, 1.1.1.1, 7.7.7.7' + self.assertEqual(200, self.view(self.request).status_code) + + def test_unique_clients_are_counted_independently_with_two_proxies(self): + self.config_proxy(2) + self.view(self.request) + self.request.META['HTTP_X_FORWARDED_FOR'] = '0.0.0.0, 7.7.7.7, 2.2.2.2' + self.assertEqual(200, self.view(self.request).status_code) From 89f26c5e040febd27bc9142b0096ca119bb3fa32 Mon Sep 17 00:00:00 2001 From: kahnjw Date: Fri, 6 Dec 2013 14:21:52 -0800 Subject: [PATCH 2/6] Add get_ident method to pass new tests. --- rest_framework/settings.py | 1 + rest_framework/throttling.py | 25 ++++++++++++++++++------- 2 files changed, 19 insertions(+), 7 deletions(-) diff --git a/rest_framework/settings.py b/rest_framework/settings.py index 8abaf1409..383de72ea 100644 --- a/rest_framework/settings.py +++ b/rest_framework/settings.py @@ -63,6 +63,7 @@ DEFAULTS = { 'user': None, 'anon': None, }, + 'NUM_PROXIES': None, # Pagination 'PAGINATE_BY': None, diff --git a/rest_framework/throttling.py b/rest_framework/throttling.py index a946d837f..60e46d479 100644 --- a/rest_framework/throttling.py +++ b/rest_framework/throttling.py @@ -18,6 +18,21 @@ class BaseThrottle(object): """ raise NotImplementedError('.allow_request() must be overridden') + def get_ident(self, request): + """ + Identify the machine making the request by parsing HTTP_X_FORWARDED_FOR + if present and number of proxies is > 0. If not use all of + HTTP_X_FORWARDED_FOR if it is available, if not use REMOTE_ADDR. + """ + xff = request.META.get('HTTP_X_FORWARDED_FOR') + remote_addr = request.META.get('REMOTE_ADDR') + num_proxies = api_settings.NUM_PROXIES + + if xff and num_proxies: + return xff.split(',')[-min(num_proxies, len(xff))].strip() + + return xff if xff else remote_addr + def wait(self): """ Optionally, return a recommended number of seconds to wait before @@ -152,13 +167,9 @@ class AnonRateThrottle(SimpleRateThrottle): if request.user.is_authenticated(): return None # Only throttle unauthenticated requests. - ident = request.META.get('HTTP_X_FORWARDED_FOR') - if ident is None: - ident = request.META.get('REMOTE_ADDR') - return self.cache_format % { 'scope': self.scope, - 'ident': ident + 'ident': self.get_ident(request) } @@ -176,7 +187,7 @@ class UserRateThrottle(SimpleRateThrottle): if request.user.is_authenticated(): ident = request.user.id else: - ident = request.META.get('REMOTE_ADDR', None) + ident = self.get_ident(request) return self.cache_format % { 'scope': self.scope, @@ -224,7 +235,7 @@ class ScopedRateThrottle(SimpleRateThrottle): if request.user.is_authenticated(): ident = request.user.id else: - ident = request.META.get('REMOTE_ADDR', None) + ident = self.get_ident(request) return self.cache_format % { 'scope': self.scope, From 100a933279e3119e2627d744cd7eb472b542f6fe Mon Sep 17 00:00:00 2001 From: kahnjw Date: Fri, 6 Dec 2013 14:22:08 -0800 Subject: [PATCH 3/6] Add documentation to explain what effect these changes have. --- docs/api-guide/throttling.md | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/docs/api-guide/throttling.md b/docs/api-guide/throttling.md index cc4692171..ee57383cd 100644 --- a/docs/api-guide/throttling.md +++ b/docs/api-guide/throttling.md @@ -35,11 +35,16 @@ The default throttling policy may be set globally, using the `DEFAULT_THROTTLE_C 'DEFAULT_THROTTLE_RATES': { 'anon': '100/day', 'user': '1000/day' - } + }, + 'NUM_PROXIES': 2, } The rate descriptions used in `DEFAULT_THROTTLE_RATES` may include `second`, `minute`, `hour` or `day` as the throttle period. +By default Django REST Framework will try to use the `HTTP_X_FORWARDED_FOR` header to uniquely identify client machines for throttling. If HTTP_X_FORWARDED_FOR is not present `REMOTE_ADDR` header value will be used. + +To help Django REST Framework identify unique clients the number of application proxies can be set using `NUM_PROXIES`. This setting will allow the throttle to correctly identify unique requests whenthere are multiple application side proxies in front of the server. `NUM_PROXIES` should be set to an integer. It is important to understand that if you configure `NUM_PROXIES > 0` all clients behind a unique [NAT'd](http://en.wikipedia.org/wiki/Network_address_translation) gateway will be treated as a single client. + You can also set the throttling policy on a per-view or per-viewset basis, using the `APIView` class based views. From 196c5952e4f610054e832aef36cb2383b8c129c0 Mon Sep 17 00:00:00 2001 From: kahnjw Date: Fri, 6 Dec 2013 14:24:16 -0800 Subject: [PATCH 4/6] Fix typo --- docs/api-guide/throttling.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/api-guide/throttling.md b/docs/api-guide/throttling.md index ee57383cd..69b15a829 100644 --- a/docs/api-guide/throttling.md +++ b/docs/api-guide/throttling.md @@ -43,7 +43,7 @@ The rate descriptions used in `DEFAULT_THROTTLE_RATES` may include `second`, `mi By default Django REST Framework will try to use the `HTTP_X_FORWARDED_FOR` header to uniquely identify client machines for throttling. If HTTP_X_FORWARDED_FOR is not present `REMOTE_ADDR` header value will be used. -To help Django REST Framework identify unique clients the number of application proxies can be set using `NUM_PROXIES`. This setting will allow the throttle to correctly identify unique requests whenthere are multiple application side proxies in front of the server. `NUM_PROXIES` should be set to an integer. It is important to understand that if you configure `NUM_PROXIES > 0` all clients behind a unique [NAT'd](http://en.wikipedia.org/wiki/Network_address_translation) gateway will be treated as a single client. +To help Django REST Framework identify unique clients the number of application proxies can be set using `NUM_PROXIES`. This setting will allow the throttle to correctly identify unique requests when there are multiple application side proxies in front of the server. `NUM_PROXIES` should be set to an integer. It is important to understand that if you configure `NUM_PROXIES > 0` all clients behind a unique [NAT'd](http://en.wikipedia.org/wiki/Network_address_translation) gateway will be treated as a single client. You can also set the throttling policy on a per-view or per-viewset basis, using the `APIView` class based views. From 887da7f6c5a9e7b5007f5e4af32a6b93b18c70ea Mon Sep 17 00:00:00 2001 From: kahnjw Date: Fri, 6 Dec 2013 14:30:33 -0800 Subject: [PATCH 5/6] Add missing tick marks --- docs/api-guide/throttling.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/api-guide/throttling.md b/docs/api-guide/throttling.md index 69b15a829..34418e84d 100644 --- a/docs/api-guide/throttling.md +++ b/docs/api-guide/throttling.md @@ -41,7 +41,7 @@ The default throttling policy may be set globally, using the `DEFAULT_THROTTLE_C The rate descriptions used in `DEFAULT_THROTTLE_RATES` may include `second`, `minute`, `hour` or `day` as the throttle period. -By default Django REST Framework will try to use the `HTTP_X_FORWARDED_FOR` header to uniquely identify client machines for throttling. If HTTP_X_FORWARDED_FOR is not present `REMOTE_ADDR` header value will be used. +By default Django REST Framework will try to use the `HTTP_X_FORWARDED_FOR` header to uniquely identify client machines for throttling. If `HTTP_X_FORWARDED_FOR` is not present `REMOTE_ADDR` header value will be used. To help Django REST Framework identify unique clients the number of application proxies can be set using `NUM_PROXIES`. This setting will allow the throttle to correctly identify unique requests when there are multiple application side proxies in front of the server. `NUM_PROXIES` should be set to an integer. It is important to understand that if you configure `NUM_PROXIES > 0` all clients behind a unique [NAT'd](http://en.wikipedia.org/wiki/Network_address_translation) gateway will be treated as a single client. From 23db6c98495d7b3c18a3069c6cb770d5cbc18ee1 Mon Sep 17 00:00:00 2001 From: kahnjw Date: Fri, 6 Dec 2013 14:52:39 -0800 Subject: [PATCH 6/6] PEP8 Compliance --- rest_framework/tests/test_throttling.py | 1 + 1 file changed, 1 insertion(+) diff --git a/rest_framework/tests/test_throttling.py b/rest_framework/tests/test_throttling.py index 031276968..8c5eefe9c 100644 --- a/rest_framework/tests/test_throttling.py +++ b/rest_framework/tests/test_throttling.py @@ -277,6 +277,7 @@ class ScopedRateThrottleTests(TestCase): response = self.unscoped_view(request) self.assertEqual(200, response.status_code) + class XffTestingBase(TestCase): def setUp(self):