Merge pull request #917 from alphagov/rate-limit-api-calls

Rate limit api calls

app/config.py

@@ -2,12 +2,13 @@ from datetime import timedelta
 from celery.schedules import crontab
 from kombu import Exchange, Queue
 import os
-
+from app.models import KEY_TYPE_NORMAL, KEY_TYPE_TEAM, KEY_TYPE_TEST
 
 if os.environ.get('VCAP_SERVICES'):
     # on cloudfoundry, config is a json blob in VCAP_SERVICES - unpack it, and populate
     # standard environment variables from it
     from app.cloudfoundry_config import extract_cloudfoundry_config
+
     extract_cloudfoundry_config()
 
 
@@ -176,6 +177,21 @@ class Config(object):
 
     DVLA_UPLOAD_BUCKET_NAME = "{}-dvla-file-per-job".format(os.getenv('NOTIFY_ENVIRONMENT'))
 
+    API_KEY_LIMITS = {
+        KEY_TYPE_TEAM: {
+            "limit": 3000,
+            "interval": 60
+        },
+        KEY_TYPE_NORMAL: {
+            "limit": 3000,
+            "interval": 60
+        },
+        KEY_TYPE_TEST: {
+            "limit": 3000,
+            "interval": 60
+        }
+    }
+
 
 ######################
 # Config overrides ###
@@ -198,6 +214,7 @@ class Development(Config):
         Queue('research-mode', Exchange('default'), routing_key='research-mode')
     ]
     API_HOST_NAME = "http://localhost:6011"
+    API_RATE_LIMIT_ENABLED = True
 
 
 class Test(Config):
@@ -220,14 +237,31 @@ class Test(Config):
         Queue('research-mode', Exchange('default'), routing_key='research-mode')
     ]
     REDIS_ENABLED = True
+    API_RATE_LIMIT_ENABLED = True
     API_HOST_NAME = "http://localhost:6011"
 
+    API_KEY_LIMITS = {
+        KEY_TYPE_TEAM: {
+            "limit": 1,
+            "interval": 2
+        },
+        KEY_TYPE_NORMAL: {
+            "limit": 10,
+            "interval": 20
+        },
+        KEY_TYPE_TEST: {
+            "limit": 100,
+            "interval": 200
+        }
+    }
+
 
 class Preview(Config):
     NOTIFY_EMAIL_DOMAIN = 'notify.works'
     NOTIFY_ENVIRONMENT = 'preview'
     CSV_UPLOAD_BUCKET_NAME = 'preview-notifications-csv-upload'
     FROM_NUMBER = 'preview'
+    API_RATE_LIMIT_ENABLED = True
 
 
 class Staging(Config):
@@ -236,6 +270,7 @@ class Staging(Config):
     CSV_UPLOAD_BUCKET_NAME = 'staging-notify-csv-upload'
     STATSD_ENABLED = True
     FROM_NUMBER = 'stage'
+    API_RATE_LIMIT_ENABLED = True
 
 
 class Live(Config):
@@ -247,6 +282,7 @@ class Live(Config):
     FUNCTIONAL_TEST_PROVIDER_SERVICE_ID = '6c1d81bb-dae2-4ee9-80b0-89a4aae9f649'
     FUNCTIONAL_TEST_PROVIDER_SMS_TEMPLATE_ID = 'ba9e1789-a804-40b8-871f-cc60d4c1286f'
     PERFORMANCE_PLATFORM_ENABLED = True
+    API_RATE_LIMIT_ENABLED = False
 
 
 class CloudFoundryConfig(Config):

app/notifications/rest.py

@@ -16,9 +16,9 @@ from app.models import SMS_TYPE
 from app.notifications.process_notifications import (persist_notification,
                                                      send_notification_to_queue,
                                                      simulated_recipient)
-from app.notifications.validators import (check_service_message_limit,
+from app.notifications.validators import (check_service_over_daily_message_limit,
                                           check_template_is_for_notification_type,
-                                          check_template_is_active)
+                                          check_template_is_active, check_rate_limiting)
 from app.schemas import (
     email_notification_schema,
     sms_template_notification_schema,
@@ -105,7 +105,7 @@ def send_notification(notification_type):
     if errors:
         raise InvalidRequest(errors, status_code=400)
 
-    check_service_message_limit(api_user.key_type, service)
+    check_rate_limiting(service, api_user)
 
     template = templates_dao.dao_get_template_by_id_and_service_id(
         template_id=notification_form['template'],

app/notifications/validators.py

@@ -8,12 +8,22 @@ from notifications_utils.recipients import (
 from app.dao import services_dao
 from app.models import KEY_TYPE_TEST, KEY_TYPE_TEAM, SMS_TYPE
 from app.service.utils import service_allowed_to_send_to
-from app.v2.errors import TooManyRequestsError, BadRequestError
+from app.v2.errors import TooManyRequestsError, BadRequestError, RateLimitError
 from app import redis_store
 from notifications_utils.clients import redis
 
 
-def check_service_message_limit(key_type, service):
+def check_service_over_api_rate_limit(service, api_key):
+    if current_app.config['API_RATE_LIMIT_ENABLED']:
+        cache_key = redis.rate_limit_cache_key(service.id, api_key.key_type)
+        rate_limit = current_app.config['API_KEY_LIMITS'][api_key.key_type]['limit']
+        interval = current_app.config['API_KEY_LIMITS'][api_key.key_type]['interval']
+        if redis_store.exceeded_rate_limit(cache_key, rate_limit, interval):
+            current_app.logger.error("service {} has been rate limited for throughput".format(service.id))
+            raise RateLimitError(rate_limit, interval, api_key.key_type)
+
+
+def check_service_over_daily_message_limit(key_type, service):
     if key_type != KEY_TYPE_TEST:
         cache_key = redis.daily_limit_cache_key(service.id)
         service_stats = redis_store.get(cache_key)
@@ -21,9 +31,18 @@ def check_service_message_limit(key_type, service):
             service_stats = services_dao.fetch_todays_total_message_count(service.id)
             redis_store.set(cache_key, service_stats, ex=3600)
         if int(service_stats) >= service.message_limit:
+            current_app.logger.error(
+                "service {} has been rate limited for daily use sent {} limit {}".format(
+                    service.id, int(service_stats), service.message_limit)
+            )
             raise TooManyRequestsError(service.message_limit)
 
 
+def check_rate_limiting(service, api_key):
+    check_service_over_api_rate_limit(service, api_key)
+    check_service_over_daily_message_limit(api_key.key_type, service)
+
+
 def check_template_is_for_notification_type(notification_type, template_type):
     if notification_type != template_type:
         message = "{0} template is not suitable for {1} notification".format(template_type,
@@ -68,8 +87,6 @@ def validate_and_format_recipient(send_to, key_type, service, notification_type)
 
 def check_sms_content_char_count(content_count):
     char_count_limit = current_app.config.get('SMS_CHAR_COUNT_LIMIT')
-    if (
+    if content_count > char_count_limit:
-        content_count > char_count_limit
-    ):
         message = 'Content for template has a character count greater than the limit of {}'.format(char_count_limit)
         raise BadRequestError(message=message)

app/v2/errors.py

@@ -16,6 +16,19 @@ class TooManyRequestsError(InvalidRequest):
         self.message = self.message_template.format(sending_limit)
 
 
+class RateLimitError(InvalidRequest):
+    status_code = 429
+    message_template = 'Exceeded rate limit for key type {} of {} requests per {} seconds'
+
+    def __init__(self, sending_limit, interval, key_type):
+        # normal keys are spoken of as "live" in the documentation
+        # so using this in the error messaging
+        if key_type == 'normal':
+            key_type = 'live'
+
+        self.message = self.message_template.format(key_type.upper(), sending_limit, interval)
+
+
 class BadRequestError(InvalidRequest):
     status_code = 400
     message = "An error occurred"

app/v2/notifications/post_notifications.py

@@ -4,21 +4,25 @@ from sqlalchemy.orm.exc import NoResultFound
 from app import api_user
 from app.dao import services_dao, templates_dao
 from app.models import SMS_TYPE, EMAIL_TYPE, PRIORITY
-from app.notifications.process_notifications import (create_content_for_notification,
+from app.notifications.process_notifications import (
-                                                     persist_notification,
+    create_content_for_notification,
-                                                     send_notification_to_queue,
+    persist_notification,
-                                                     simulated_recipient)
+    send_notification_to_queue,
-from app.notifications.validators import (check_service_message_limit,
+    simulated_recipient)
-                                          check_template_is_for_notification_type,
+from app.notifications.validators import (
-                                          check_template_is_active,
+    check_template_is_for_notification_type,
-                                          check_sms_content_char_count,
+    check_template_is_active,
-                                          validate_and_format_recipient)
+    check_sms_content_char_count,
+    validate_and_format_recipient,
+    check_rate_limiting)
 from app.schema_validation import validate
 from app.v2.errors import BadRequestError
 from app.v2.notifications import v2_notification_blueprint
-from app.v2.notifications.notification_schemas import (post_sms_request,
+from app.v2.notifications.notification_schemas import (
-                                                       create_post_sms_response_from_notification, post_email_request,
+    post_sms_request,
-                                                       create_post_email_response_from_notification)
+    create_post_sms_response_from_notification,
+    post_email_request,
+    create_post_email_response_from_notification)
 
 
 @v2_notification_blueprint.route('/<notification_type>', methods=['POST'])
@@ -29,7 +33,9 @@ def post_notification(notification_type):
         form = validate(request.get_json(), post_sms_request)
 
     service = services_dao.dao_fetch_service_by_id(api_user.service_id)
-    check_service_message_limit(api_user.key_type, service)
+
+    check_rate_limiting(service, api_user)
+
     form_send_to = form['phone_number'] if notification_type == SMS_TYPE else form['email_address']
     send_to = validate_and_format_recipient(send_to=form_send_to,
                                             key_type=api_user.key_type,
@@ -40,6 +46,7 @@ def post_notification(notification_type):
 
     # Do not persist or send notification to the queue if it is a simulated recipient
     simulated = simulated_recipient(send_to, notification_type)
+
     notification = persist_notification(template_id=template.id,
                                         template_version=template.version,
                                         recipient=form_send_to,
@@ -50,6 +57,7 @@ def post_notification(notification_type):
                                         key_type=api_user.key_type,
                                         client_reference=form.get('reference', None),
                                         simulated=simulated)
+
     if not simulated:
         queue_name = 'priority' if template.process_type == PRIORITY else None
         send_notification_to_queue(notification=notification, research_mode=service.research_mode, queue=queue_name)

tests/app/notifications/rest/test_send_notification.py

@@ -13,6 +13,7 @@ from app.models import ApiKey, KEY_TYPE_NORMAL, KEY_TYPE_TEAM, KEY_TYPE_TEST, No
 from app.dao.templates_dao import dao_get_all_templates_for_service, dao_update_template
 from app.dao.services_dao import dao_update_service
 from app.dao.api_key_dao import save_model_api_key
+from app.v2.errors import RateLimitError
 from tests import create_authorization_header
 from tests.app.conftest import (
     sample_notification as create_sample_notification,
@@ -1048,6 +1049,52 @@ def test_send_notification_uses_priority_queue_when_template_is_marked_as_priori
     mocked.assert_called_once_with([notification_id], queue='priority')
 
 
+@pytest.mark.parametrize(
+    "notification_type, send_to",
+    [("sms", "07700 900 855"), ("email", "sample@email.com")]
+)
+def test_returns_a_429_limit_exceeded_if_rate_limit_exceeded(
+    client,
+    notify_db,
+    notify_db_session,
+    mocker,
+    notification_type,
+    send_to
+):
+    sample = create_sample_template(
+        notify_db,
+        notify_db_session,
+        template_type=notification_type
+    )
+    persist_mock = mocker.patch('app.notifications.rest.persist_notification')
+    deliver_mock = mocker.patch('app.notifications.rest.send_notification_to_queue')
+
+    mocker.patch(
+        'app.notifications.rest.check_rate_limiting',
+        side_effect=RateLimitError("LIMIT", "INTERVAL", "TYPE"))
+
+    data = {
+        'to': send_to,
+        'template': str(sample.id)
+    }
+
+    auth_header = create_authorization_header(service_id=sample.service_id)
+
+    response = client.post(
+        path='/notifications/{}'.format(notification_type),
+        data=json.dumps(data),
+        headers=[('Content-Type', 'application/json'), auth_header])
+
+    message = json.loads(response.data)['message']
+    result = json.loads(response.data)['result']
+    assert response.status_code == 429
+    assert result == 'error'
+    assert message == 'Exceeded rate limit for key type TYPE of LIMIT requests per INTERVAL seconds'
+
+    assert not persist_mock.called
+    assert not deliver_mock.called
+
+
 def test_should_allow_store_original_number_on_sms_notification(client, sample_template, mocker):
         mocked = mocker.patch('app.celery.provider_tasks.deliver_sms.apply_async')
         mocker.patch('app.encryption.encrypt', return_value="something_encrypted")

tests/app/notifications/test_validators.py

@@ -1,25 +1,26 @@
 import pytest
 from freezegun import freeze_time
-
+from flask import current_app
 import app
-from app.models import KEY_TYPE_NORMAL
 from app.notifications.validators import (
-    check_service_message_limit,
+    check_service_over_daily_message_limit,
     check_template_is_for_notification_type,
     check_template_is_active,
     service_can_send_to_recipient,
     check_sms_content_char_count,
+    check_service_over_api_rate_limit,
     validate_and_format_recipient
 )
+
 from app.v2.errors import (
     BadRequestError,
-    TooManyRequestsError
+    TooManyRequestsError,
-)
+    RateLimitError)
 from tests.app.conftest import (
     sample_notification as create_notification,
     sample_service as create_service,
-    sample_service_whitelist
+    sample_service_whitelist,
-)
+    sample_api_key)
 
 
 @pytest.mark.parametrize('key_type', ['team', 'normal'])
@@ -39,7 +40,7 @@ def test_exception_thrown_by_redis_store_get_should_not_be_fatal(
             create_notification(notify_db, notify_db_session, service=service)
 
         with pytest.raises(TooManyRequestsError) as e:
-            check_service_message_limit(key_type, service)
+            check_service_over_daily_message_limit(key_type, service)
         assert e.value.status_code == 429
         assert e.value.message == 'Exceeded send limits (4) for today'
         assert e.value.fields == []
@@ -55,7 +56,7 @@ def test_exception_thown_by_redis_store_set_should_not_be_fatal(
         mocker):
     mocker.patch('app.notifications.validators.redis_store.redis_store.set', side_effect=Exception("broken redis"))
     mocker.patch('app.notifications.validators.redis_store.get', return_value=None)
-    assert not check_service_message_limit(key_type, sample_service)
+    assert not check_service_over_daily_message_limit(key_type, sample_service)
 
 
 @pytest.mark.parametrize('key_type', ['test', 'team', 'normal'])
@@ -67,7 +68,7 @@ def test_check_service_message_limit_in_cache_with_unrestricted_service_is_allow
     mocker.patch('app.notifications.validators.redis_store.set')
     mocker.patch('app.notifications.validators.services_dao')
 
-    check_service_message_limit(key_type, sample_service)
+    check_service_over_daily_message_limit(key_type, sample_service)
     app.notifications.validators.redis_store.set.assert_not_called()
     assert not app.notifications.validators.services_dao.mock_calls
 
@@ -80,14 +81,14 @@ def test_check_service_message_limit_in_cache_under_message_limit_passes(
     mocker.patch('app.notifications.validators.redis_store.get', return_value=1)
     mocker.patch('app.notifications.validators.redis_store.set')
     mocker.patch('app.notifications.validators.services_dao')
-    check_service_message_limit(key_type, sample_service)
+    check_service_over_daily_message_limit(key_type, sample_service)
     app.notifications.validators.redis_store.set.assert_not_called()
     assert not app.notifications.validators.services_dao.mock_calls
 
 
 def test_should_not_interact_with_cache_for_test_key(sample_service, mocker):
     mocker.patch('app.notifications.validators.redis_store')
-    check_service_message_limit('test', sample_service)
+    check_service_over_daily_message_limit('test', sample_service)
     assert not app.notifications.validators.redis_store.mock_calls
 
 
@@ -104,7 +105,7 @@ def test_should_set_cache_value_as_value_from_database_if_cache_not_set(
             create_notification(notify_db, notify_db_session, service=sample_service)
         mocker.patch('app.notifications.validators.redis_store.get', return_value=None)
         mocker.patch('app.notifications.validators.redis_store.set')
-        check_service_message_limit(key_type, sample_service)
+        check_service_over_daily_message_limit(key_type, sample_service)
         app.notifications.validators.redis_store.set.assert_called_with(
             str(sample_service.id) + "-2016-01-01-count", 5, ex=3600
         )
@@ -120,7 +121,7 @@ def test_check_service_message_limit_over_message_limit_fails(key_type, notify_d
         for x in range(5):
             create_notification(notify_db, notify_db_session, service=service)
         with pytest.raises(TooManyRequestsError) as e:
-            check_service_message_limit(key_type, service)
+            check_service_over_daily_message_limit(key_type, service)
         assert e.value.status_code == 429
         assert e.value.message == 'Exceeded send limits (4) for today'
         assert e.value.fields == []
@@ -142,7 +143,7 @@ def test_check_service_message_limit_in_cache_over_message_limit_fails(
 
         service = create_service(notify_db, notify_db_session, restricted=True, limit=4)
         with pytest.raises(TooManyRequestsError) as e:
-            check_service_message_limit(key_type, service)
+            check_service_over_daily_message_limit(key_type, service)
         assert e.value.status_code == 429
         assert e.value.message == 'Exceeded send limits (4) for today'
         assert e.value.fields == []
@@ -266,6 +267,81 @@ def test_check_sms_content_char_count_fails(char_count, notify_api):
     assert e.value.fields == []
 
 
+@pytest.mark.parametrize('key_type, limit, interval', [('team', 1, 2), ('live', 10, 20), ('test', 100, 200)])
+def test_that_when_exceed_rate_limit_request_fails(
+        notify_db,
+        notify_db_session,
+        key_type,
+        limit,
+        interval,
+        mocker):
+    with freeze_time("2016-01-01 12:00:00.000000"):
+
+        if key_type == 'live':
+            api_key_type = 'normal'
+        else:
+            api_key_type = key_type
+
+        mocker.patch('app.redis_store.exceeded_rate_limit', return_value=True)
+        mocker.patch('app.notifications.validators.services_dao')
+
+        service = create_service(notify_db, notify_db_session, restricted=True)
+        api_key = sample_api_key(notify_db, notify_db_session, service=service, key_type=api_key_type)
+        with pytest.raises(RateLimitError) as e:
+            check_service_over_api_rate_limit(service, api_key)
+
+        assert app.redis_store.exceeded_rate_limit.called_with(
+            "{}-{}".format(str(service.id), api_key.key_type),
+            limit,
+            interval
+        )
+        assert e.value.status_code == 429
+        assert e.value.message == 'Exceeded rate limit for key type {} of {} requests per {} seconds'.format(
+            key_type.upper(), limit, interval
+        )
+        assert e.value.fields == []
+
+
+@pytest.mark.parametrize('key_type, limit, interval', [('team', 1, 2), ('normal', 10, 20), ('test', 100, 200)])
+def test_that_when_not_exceeded_rate_limit_request_succeeds(
+        notify_db,
+        notify_db_session,
+        key_type,
+        limit,
+        interval,
+        mocker):
+    with freeze_time("2016-01-01 12:00:00.000000"):
+        mocker.patch('app.redis_store.exceeded_rate_limit', return_value=False)
+        mocker.patch('app.notifications.validators.services_dao')
+
+        service = create_service(notify_db, notify_db_session, restricted=True)
+        api_key = sample_api_key(notify_db, notify_db_session, service=service, key_type=key_type)
+
+        check_service_over_api_rate_limit(service, api_key)
+        assert app.redis_store.exceeded_rate_limit.called_with(
+            "{}-{}".format(str(service.id), api_key.key_type),
+            limit,
+            interval
+        )
+
+
+def test_should_not_rate_limit_if_limiting_is_disabled(
+        notify_db,
+        notify_db_session,
+        mocker):
+    with freeze_time("2016-01-01 12:00:00.000000"):
+        current_app.config['API_RATE_LIMIT_ENABLED'] = False
+
+        mocker.patch('app.redis_store.exceeded_rate_limit', return_value=False)
+        mocker.patch('app.notifications.validators.services_dao')
+
+        service = create_service(notify_db, notify_db_session, restricted=True)
+        api_key = sample_api_key(notify_db, notify_db_session, service=service)
+
+        check_service_over_api_rate_limit(service, api_key)
+        assert not app.redis_store.exceeded_rate_limit.called
+
+
 @pytest.mark.parametrize('key_type', ['test', 'normal'])
 def test_rejects_api_calls_with_international_numbers_if_service_does_not_allow_int_sms(sample_service, key_type):
     with pytest.raises(BadRequestError) as e:

tests/app/v2/notifications/test_post_notifications.py

@@ -2,6 +2,7 @@ import uuid
 import pytest
 from flask import json
 from app.models import Notification
+from app.v2.errors import RateLimitError
 from tests import create_authorization_header
 from tests.app.conftest import sample_template as create_sample_template, sample_service
 
@@ -229,6 +230,54 @@ def test_send_notification_uses_priority_queue_when_template_is_marked_as_priori
     mocked.assert_called_once_with([notification_id], queue='priority')
 
 
+@pytest.mark.parametrize(
+    "notification_type, key_send_to, send_to",
+    [("sms", "phone_number", "07700 900 855"), ("email", "email_address", "sample@email.com")]
+)
+def test_returns_a_429_limit_exceeded_if_rate_limit_exceeded(
+    client,
+    notify_db,
+    notify_db_session,
+    mocker,
+    notification_type,
+    key_send_to,
+    send_to
+):
+    sample = create_sample_template(
+        notify_db,
+        notify_db_session,
+        template_type=notification_type
+    )
+    persist_mock = mocker.patch('app.v2.notifications.post_notifications.persist_notification')
+    deliver_mock = mocker.patch('app.v2.notifications.post_notifications.send_notification_to_queue')
+    mocker.patch(
+        'app.v2.notifications.post_notifications.check_rate_limiting',
+        side_effect=RateLimitError("LIMIT", "INTERVAL", "TYPE"))
+
+    data = {
+        key_send_to: send_to,
+        'template_id': str(sample.id)
+    }
+
+    auth_header = create_authorization_header(service_id=sample.service_id)
+
+    response = client.post(
+        path='/v2/notifications/{}'.format(notification_type),
+        data=json.dumps(data),
+        headers=[('Content-Type', 'application/json'), auth_header])
+
+    error = json.loads(response.data)['errors'][0]['error']
+    message = json.loads(response.data)['errors'][0]['message']
+    status_code = json.loads(response.data)['status_code']
+    assert response.status_code == 429
+    assert error == 'RateLimitError'
+    assert message == 'Exceeded rate limit for key type TYPE of LIMIT requests per INTERVAL seconds'
+    assert status_code == 429
+
+    assert not persist_mock.called
+    assert not deliver_mock.called
+
+
 def test_post_sms_notification_returns_400_if_not_allowed_to_send_int_sms(client, sample_service, sample_template):
     data = {
         'phone_number': '20-12-1234-1234',