From 1fd90884f180683c9da043bfdbdefcadf77acd6a Mon Sep 17 00:00:00 2001 From: kahnjw Date: Mon, 11 Nov 2013 14:50:50 -0800 Subject: [PATCH 01/13] Add get_ident method to BaseThrottle class * Tries to use X_FORWARDED_FOR first * Falls back to REMOTE_ADDR --- rest_framework/throttling.py | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) diff --git a/rest_framework/throttling.py b/rest_framework/throttling.py index a946d837f..4faecff35 100644 --- a/rest_framework/throttling.py +++ b/rest_framework/throttling.py @@ -18,6 +18,13 @@ class BaseThrottle(object): """ raise NotImplementedError('.allow_request() must be overridden') + def get_ident(self, request, **kwargs): + if 'HTTP_X_FORWARDED_FOR' in request.META: + xff = request.META.get('HTTP_X_FORWARDED_FOR') + return xff.split(',')[0].strip() + + return request.META.get('REMOTE_ADDR', None) + def wait(self): """ Optionally, return a recommended number of seconds to wait before @@ -152,9 +159,7 @@ 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') + ident = self.get_ident(request) return self.cache_format % { 'scope': self.scope, @@ -176,7 +181,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 +229,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 e7b131eb20dca91ff383605f493c2f5d64a11cf7 Mon Sep 17 00:00:00 2001 From: kahnjw Date: Mon, 11 Nov 2013 14:56:26 -0800 Subject: [PATCH 02/13] Remove kwargs argument from get_ident --- rest_framework/throttling.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/rest_framework/throttling.py b/rest_framework/throttling.py index 4faecff35..df0d130df 100644 --- a/rest_framework/throttling.py +++ b/rest_framework/throttling.py @@ -18,7 +18,7 @@ class BaseThrottle(object): """ raise NotImplementedError('.allow_request() must be overridden') - def get_ident(self, request, **kwargs): + def get_ident(self, request): if 'HTTP_X_FORWARDED_FOR' in request.META: xff = request.META.get('HTTP_X_FORWARDED_FOR') return xff.split(',')[0].strip() From a1b4695cf8a12b0af022cc2bcc1504e78e408efe Mon Sep 17 00:00:00 2001 From: kahnjw Date: Mon, 11 Nov 2013 16:28:01 -0800 Subject: [PATCH 03/13] Make number of app proxies configurable by the user, default to zero --- rest_framework/settings.py | 1 + rest_framework/throttling.py | 4 +++- 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/rest_framework/settings.py b/rest_framework/settings.py index 8abaf1409..02a0cd122 100644 --- a/rest_framework/settings.py +++ b/rest_framework/settings.py @@ -63,6 +63,7 @@ DEFAULTS = { 'user': None, 'anon': None, }, + 'NUM_PROXIES': 0, # Pagination 'PAGINATE_BY': None, diff --git a/rest_framework/throttling.py b/rest_framework/throttling.py index df0d130df..1c8c6941d 100644 --- a/rest_framework/throttling.py +++ b/rest_framework/throttling.py @@ -21,7 +21,9 @@ class BaseThrottle(object): def get_ident(self, request): if 'HTTP_X_FORWARDED_FOR' in request.META: xff = request.META.get('HTTP_X_FORWARDED_FOR') - return xff.split(',')[0].strip() + num_proxies = api_settings.NUM_PROXIES + + return xff.split(',')[-min(num_proxies, len(xff))].strip() return request.META.get('REMOTE_ADDR', None) From 129cf80f5e9e9cce10855c9e5ee53fc5fe929a1b Mon Sep 17 00:00:00 2001 From: kahnjw Date: Mon, 11 Nov 2013 16:36:19 -0800 Subject: [PATCH 04/13] Add source documentation. --- rest_framework/throttling.py | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/rest_framework/throttling.py b/rest_framework/throttling.py index 1c8c6941d..982ca73d9 100644 --- a/rest_framework/throttling.py +++ b/rest_framework/throttling.py @@ -19,6 +19,10 @@ class BaseThrottle(object): raise NotImplementedError('.allow_request() must be overridden') def get_ident(self, request): + """ + Identify the machine making the request using HTTP_X_FORWARDED_FOR if + present, if not use REMOTE_ADDR. + """ if 'HTTP_X_FORWARDED_FOR' in request.META: xff = request.META.get('HTTP_X_FORWARDED_FOR') num_proxies = api_settings.NUM_PROXIES From a94ffd1e6a3172e243532b6eccbb47c3278d66d5 Mon Sep 17 00:00:00 2001 From: kahnjw Date: Mon, 11 Nov 2013 16:41:14 -0800 Subject: [PATCH 05/13] Add throttling topic documentation. --- 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 fc1525df6..baab6b737 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. +Additionally the number of application proxies can be set using `NUM_PROXIES`. This +setting will allow the throttle to correctly identify unique requests there are +multiple proxies in front of the server. + You can also set the throttling policy on a per-view or per-viewset basis, using the `APIView` class based views. From 23af9a1a2272cb17bb4a8d76d2a5b95223c20f52 Mon Sep 17 00:00:00 2001 From: kahnjw Date: Mon, 11 Nov 2013 16:42:25 -0800 Subject: [PATCH 06/13] Correct NUM_PROXIES throttling documentation wording. --- docs/api-guide/throttling.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/docs/api-guide/throttling.md b/docs/api-guide/throttling.md index baab6b737..aebc9caf5 100644 --- a/docs/api-guide/throttling.md +++ b/docs/api-guide/throttling.md @@ -42,8 +42,8 @@ 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. Additionally the number of application proxies can be set using `NUM_PROXIES`. This -setting will allow the throttle to correctly identify unique requests there are -multiple proxies in front of the server. +setting will allow the throttle to correctly identify unique requests when there are +multiple application side proxies in front of the server. You can also set the throttling policy on a per-view or per-viewset basis, using the `APIView` class based views. From 7c27561039826d434fe7e6a1e7a0b7d04333ad36 Mon Sep 17 00:00:00 2001 From: kahnjw Date: Tue, 12 Nov 2013 09:36:33 -0800 Subject: [PATCH 07/13] Only use HTTP_X_FORWARDED_FOR if NUM_PROXIES > 0 else use REMOTE_ADDR --- rest_framework/throttling.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/rest_framework/throttling.py b/rest_framework/throttling.py index 982ca73d9..efbef4e72 100644 --- a/rest_framework/throttling.py +++ b/rest_framework/throttling.py @@ -23,9 +23,10 @@ class BaseThrottle(object): Identify the machine making the request using HTTP_X_FORWARDED_FOR if present, if not use REMOTE_ADDR. """ - if 'HTTP_X_FORWARDED_FOR' in request.META: + num_proxies = api_settings.NUM_PROXIES + + if 'HTTP_X_FORWARDED_FOR' in request.META and num_proxies > 0: xff = request.META.get('HTTP_X_FORWARDED_FOR') - num_proxies = api_settings.NUM_PROXIES return xff.split(',')[-min(num_proxies, len(xff))].strip() From 87e80b7be5e772a3ebce8f232cd189910d04754e Mon Sep 17 00:00:00 2001 From: kahnjw Date: Wed, 13 Nov 2013 14:06:51 -0800 Subject: [PATCH 08/13] Add tests for using HTTP_X_FORWARDED_FOR to ID machines for throttling --- rest_framework/tests/test_throttling.py | 78 +++++++++++++++++++++++++ 1 file changed, 78 insertions(+) diff --git a/rest_framework/tests/test_throttling.py b/rest_framework/tests/test_throttling.py index 41bff6926..9ba06c7c4 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,80 @@ 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_no_app_proxies(self): + self.config_proxy(0) + self.view(self.request) + self.request.META['HTTP_X_FORWARDED_FOR'] = '4.4.4.4, 5.5.5.5, 6.6.6.6' + self.assertEqual(429, self.view(self.request).status_code) + + 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_no_proxies(self): + self.config_proxy(0) + self.view(self.request) + self.request.META['REMOTE_ADDR'] = '7.7.7.7' + self.assertEqual(200, self.view(self.request).status_code) + + 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 900cb676e51511d09101bf434f0180585acebc50 Mon Sep 17 00:00:00 2001 From: kahnjw Date: Fri, 22 Nov 2013 14:51:22 -0800 Subject: [PATCH 09/13] Default NUM_PROXIES to None instead of zero. --- rest_framework/settings.py | 2 +- rest_framework/throttling.py | 3 +-- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/rest_framework/settings.py b/rest_framework/settings.py index 02a0cd122..383de72ea 100644 --- a/rest_framework/settings.py +++ b/rest_framework/settings.py @@ -63,7 +63,7 @@ DEFAULTS = { 'user': None, 'anon': None, }, - 'NUM_PROXIES': 0, + 'NUM_PROXIES': None, # Pagination 'PAGINATE_BY': None, diff --git a/rest_framework/throttling.py b/rest_framework/throttling.py index efbef4e72..ca161ee48 100644 --- a/rest_framework/throttling.py +++ b/rest_framework/throttling.py @@ -25,7 +25,7 @@ class BaseThrottle(object): """ num_proxies = api_settings.NUM_PROXIES - if 'HTTP_X_FORWARDED_FOR' in request.META and num_proxies > 0: + if 'HTTP_X_FORWARDED_FOR' in request.META and num_proxies: xff = request.META.get('HTTP_X_FORWARDED_FOR') return xff.split(',')[-min(num_proxies, len(xff))].strip() @@ -222,7 +222,6 @@ class ScopedRateThrottle(SimpleRateThrottle): # the `__init__` call. self.rate = self.get_rate() self.num_requests, self.duration = self.parse_rate(self.rate) - # We can now proceed as normal. return super(ScopedRateThrottle, self).allow_request(request, view) From 70192369809a279e3a89c1368bc3d353fea6cf5c Mon Sep 17 00:00:00 2001 From: kahnjw Date: Thu, 5 Dec 2013 11:32:56 -0800 Subject: [PATCH 10/13] Preserve current default behavior. --- rest_framework/throttling.py | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/rest_framework/throttling.py b/rest_framework/throttling.py index ca161ee48..1fea202bc 100644 --- a/rest_framework/throttling.py +++ b/rest_framework/throttling.py @@ -18,11 +18,13 @@ class BaseThrottle(object): """ raise NotImplementedError('.allow_request() must be overridden') - def get_ident(self, request): + def get_ident(self, request, default_to=None): """ Identify the machine making the request using HTTP_X_FORWARDED_FOR if - present, if not use REMOTE_ADDR. + present and number of proxies is > 0. If not use default if it is set + and available, if not fall back to REMOTE_ADDR. """ + default = request.META.get(default_to) num_proxies = api_settings.NUM_PROXIES if 'HTTP_X_FORWARDED_FOR' in request.META and num_proxies: @@ -30,7 +32,7 @@ class BaseThrottle(object): return xff.split(',')[-min(num_proxies, len(xff))].strip() - return request.META.get('REMOTE_ADDR', None) + return default if default else request.META.get('REMOTE_ADDR') def wait(self): """ @@ -166,7 +168,7 @@ class AnonRateThrottle(SimpleRateThrottle): if request.user.is_authenticated(): return None # Only throttle unauthenticated requests. - ident = self.get_ident(request) + ident = self.get_ident(request, default_to='HTTP_X_FORWARDED_FOR') return self.cache_format % { 'scope': self.scope, From f0ee72f578d6218ca30b9fa8e50d0b1adb0d828a Mon Sep 17 00:00:00 2001 From: kahnjw Date: Fri, 6 Dec 2013 13:51:30 -0800 Subject: [PATCH 11/13] Remove default_to arg and use X_FORWARDED_FOR & fallback to REMOTE_ADDR --- rest_framework/throttling.py | 19 +++++++++---------- 1 file changed, 9 insertions(+), 10 deletions(-) diff --git a/rest_framework/throttling.py b/rest_framework/throttling.py index 1fea202bc..f79e58303 100644 --- a/rest_framework/throttling.py +++ b/rest_framework/throttling.py @@ -18,21 +18,20 @@ class BaseThrottle(object): """ raise NotImplementedError('.allow_request() must be overridden') - def get_ident(self, request, default_to=None): + def get_ident(self, request): """ - Identify the machine making the request using HTTP_X_FORWARDED_FOR if - present and number of proxies is > 0. If not use default if it is set - and available, if not fall back to REMOTE_ADDR. + 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. """ - default = request.META.get(default_to) + xff = request.META.get('HTTP_X_FORWARDED_FOR') + remote_addr = request.META.get('REMOTE_ADDR') num_proxies = api_settings.NUM_PROXIES - if 'HTTP_X_FORWARDED_FOR' in request.META and num_proxies: - xff = request.META.get('HTTP_X_FORWARDED_FOR') - + if xff and num_proxies: return xff.split(',')[-min(num_proxies, len(xff))].strip() - return default if default else request.META.get('REMOTE_ADDR') + return xff if xff else remote_addr def wait(self): """ @@ -168,7 +167,7 @@ class AnonRateThrottle(SimpleRateThrottle): if request.user.is_authenticated(): return None # Only throttle unauthenticated requests. - ident = self.get_ident(request, default_to='HTTP_X_FORWARDED_FOR') + ident = self.get_ident(request) return self.cache_format % { 'scope': self.scope, From 0dcdf438280360bc52b9b14c320320cb6bbe547b Mon Sep 17 00:00:00 2001 From: kahnjw Date: Fri, 6 Dec 2013 13:51:37 -0800 Subject: [PATCH 12/13] Remove invalid test cases --- rest_framework/tests/test_throttling.py | 12 ------------ 1 file changed, 12 deletions(-) diff --git a/rest_framework/tests/test_throttling.py b/rest_framework/tests/test_throttling.py index 9ba06c7c4..8c5eefe9c 100644 --- a/rest_framework/tests/test_throttling.py +++ b/rest_framework/tests/test_throttling.py @@ -316,12 +316,6 @@ class IdWithXffBasicTests(XffTestingBase): class XffSpoofingTests(XffTestingBase): - def test_xff_spoofing_doesnt_change_machine_id_with_no_app_proxies(self): - self.config_proxy(0) - self.view(self.request) - self.request.META['HTTP_X_FORWARDED_FOR'] = '4.4.4.4, 5.5.5.5, 6.6.6.6' - self.assertEqual(429, self.view(self.request).status_code) - def test_xff_spoofing_doesnt_change_machine_id_with_one_app_proxy(self): self.config_proxy(1) self.view(self.request) @@ -336,12 +330,6 @@ class XffSpoofingTests(XffTestingBase): class XffUniqueMachinesTest(XffTestingBase): - def test_unique_clients_are_counted_independently_with_no_proxies(self): - self.config_proxy(0) - self.view(self.request) - self.request.META['REMOTE_ADDR'] = '7.7.7.7' - self.assertEqual(200, self.view(self.request).status_code) - def test_unique_clients_are_counted_independently_with_one_proxy(self): self.config_proxy(1) self.view(self.request) From 270f68457211ea0608a3ed3823f0cb526d6729bd Mon Sep 17 00:00:00 2001 From: kahnjw Date: Fri, 6 Dec 2013 14:07:08 -0800 Subject: [PATCH 13/13] Update Throttling documentation for client identification method --- docs/api-guide/throttling.md | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/docs/api-guide/throttling.md b/docs/api-guide/throttling.md index aebc9caf5..71d4c6fcb 100644 --- a/docs/api-guide/throttling.md +++ b/docs/api-guide/throttling.md @@ -41,9 +41,11 @@ 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. -Additionally 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. +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.