Merge pull request #149 from pmclanahan/master

Disable IP rate limit on sms view
mozmeao · Nov 13, 2015 · aa5eedc · aa5eedc
2 parents d27329e + e8d5c4d
commit aa5eedc
Show file tree

Hide file tree

Showing 2 changed files with 1 addition and 66 deletions.
diff --git a/news/tests/test_views.py b/news/tests/test_views.py
@@ -15,7 +15,7 @@
 from news.models import APIUser
 from news.newsletters import newsletter_languages, newsletter_fields
 from news.tasks import SUBSCRIBE
-from news.utils import email_block_list_cache, HttpResponseJSON
+from news.utils import email_block_list_cache
 
 
 none_mock = Mock(return_value=None)
@@ -302,23 +302,6 @@ def test_invalid_message_name(self):
         self.assertEqual(errors.BASKET_USAGE_ERROR, data['code'])
         self.assertIn('msg_name', data['desc'])
 
-    @patch('news.views.IP_RATE_LIMIT_EXTERNAL', '1/m')
-    def test_ip_rate_limit(self):
-        self.client.post('/news/subscribe_sms/', {'mobile_number': '9198675309'},
-                         HTTP_X_CLUSTER_CLIENT_IP='1.1.1.1')
-        self.add_sms_user.delay.assert_called_with('SMS_Android', '19198675309', False)
-        self.add_sms_user.reset_mock()
-
-        resp = self.client.post('/news/subscribe_sms/', {'mobile_number': '9195555555'},
-                                HTTP_X_CLUSTER_CLIENT_IP='1.1.1.1')
-        data = json.loads(resp.content)
-        self.assertEqual(data, {
-            'status': 'error',
-            'desc': 'rate limit reached',
-            'code': errors.BASKET_USAGE_ERROR,
-        })
-        self.assertFalse(self.add_sms_user.delay.called)
-
     def test_phone_number_rate_limit(self):
         self.client.post('/news/subscribe_sms/', {'mobile_number': '9198675309'})
         self.add_sms_user.delay.assert_called_with('SMS_Android', '19198675309', False)
@@ -601,48 +584,6 @@ def test_success_sync_optin_lowercase(self):
             self.update_user_task.assert_called_with(request, SUBSCRIBE, data=request_data,
                                                      optin=True, sync=True)
 
-    @patch('news.views.IP_RATE_LIMIT_EXTERNAL', '1/m')
-    def test_ip_rate_limit(self):
-        self.skipTest('Bug 1154584')
-        self.update_user_task.return_value = HttpResponseJSON({'status': 'ok'})
-        request_data = {'newsletters': 'news,lets', 'optin': 'N', 'sync': 'N',
-                        'email': 'dude@example.com'}
-        self.client.post('/news/subscribe/', request_data,
-                         HTTP_X_CLUSTER_CLIENT_IP='1.1.1.1')
-        self.assertTrue(self.update_user_task.called)
-        self.update_user_task.reset_mock()
-
-        resp = self.client.post('/news/subscribe/', request_data,
-                                HTTP_X_CLUSTER_CLIENT_IP='1.1.1.1')
-        data = json.loads(resp.content)
-        self.assertEqual(data, {
-            'status': 'error',
-            'desc': 'rate limit reached',
-            'code': errors.BASKET_USAGE_ERROR,
-        })
-        self.assertFalse(self.update_user_task.delay.called)
-
-    @patch('news.views.IP_RATE_LIMIT_EXTERNAL', '1/m')
-    def test_ip_rate_limit_source_ip(self):
-        self.skipTest('Bug 1154584')
-        self.update_user_task.return_value = HttpResponseJSON({'status': 'ok'})
-        request_data = {'newsletters': 'news,lets', 'optin': 'N', 'sync': 'N',
-                        'email': 'dude@example.com'}
-        self.client.post('/news/subscribe/', request_data,
-                         HTTP_X_SOURCE_IP='1.1.1.1')
-        self.assertTrue(self.update_user_task.called)
-        self.update_user_task.reset_mock()
-
-        resp = self.client.post('/news/subscribe/', request_data,
-                                HTTP_X_SOURCE_IP='1.1.1.1')
-        data = json.loads(resp.content)
-        self.assertEqual(data, {
-            'status': 'error',
-            'desc': 'rate limit reached',
-            'code': errors.BASKET_USAGE_ERROR,
-        })
-        self.assertFalse(self.update_user_task.delay.called)
-
 
 class TestRateLimitingFunctions(ViewsPatcherMixin, TestCase):
     def setUp(self):

diff --git a/news/views.py b/news/views.py
@@ -11,7 +11,6 @@
 # Get error codes from basket-client so users see the same definitions
 from basket import errors
 from django_statsd.clients import statsd
-from ratelimit.decorators import ratelimit
 from ratelimit.exceptions import Ratelimited
 from ratelimit.utils import is_ratelimited
 
@@ -258,9 +257,6 @@ def get_involved(request):
 
 @require_POST
 @csrf_exempt
-# disabled for bug 1154584
-# @ratelimit(key=ip_rate_limit_key, rate=ip_rate_limit_rate, block=True)
-# @ratelimit(key=source_ip_rate_limit_key, rate=source_ip_rate_limit_rate, block=True)
 def subscribe(request):
     data = request.POST.dict()
     newsletters = data.get('newsletters', None)
@@ -339,8 +335,6 @@ def invalid_email_response(e):
 
 @require_POST
 @csrf_exempt
-@ratelimit(key=ip_rate_limit_key, rate=ip_rate_limit_rate, block=True)
-@ratelimit(key=source_ip_rate_limit_key, rate=source_ip_rate_limit_rate, block=True)
 def subscribe_sms(request):
     if 'mobile_number' not in request.POST:
         return HttpResponseJSON({