Adds in call to new rate limit method in the redis client

- both V1 and V2 APIs
- Rate limiting wrapped into a new method - check_rate_limiting
	- delegates to the previous daily limit and the new though put limit
- Rate limiting done on key type. Each key has it's own limit (number of requests) and interval (time period of requests)
- Configured in the config. Not done on a per-env basis though could be in the future.
This commit is contained in:
Martyn Inglis
2017-04-24 14:15:08 +01:00
parent 6c703840c2
commit 926b8a60f9
8 changed files with 260 additions and 37 deletions

View File

@@ -2,12 +2,13 @@ from datetime import timedelta
from celery.schedules import crontab from celery.schedules import crontab
from kombu import Exchange, Queue from kombu import Exchange, Queue
import os import os
from app.models import KEY_TYPE_NORMAL, KEY_TYPE_TEAM, KEY_TYPE_TEST
if os.environ.get('VCAP_SERVICES'): if os.environ.get('VCAP_SERVICES'):
# on cloudfoundry, config is a json blob in VCAP_SERVICES - unpack it, and populate # on cloudfoundry, config is a json blob in VCAP_SERVICES - unpack it, and populate
# standard environment variables from it # standard environment variables from it
from app.cloudfoundry_config import extract_cloudfoundry_config from app.cloudfoundry_config import extract_cloudfoundry_config
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')) 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 ### # Config overrides ###
@@ -223,6 +239,21 @@ class Test(Config):
REDIS_ENABLED = True REDIS_ENABLED = True
API_HOST_NAME = "http://localhost:6011" 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): class Preview(Config):
NOTIFY_EMAIL_DOMAIN = 'notify.works' NOTIFY_EMAIL_DOMAIN = 'notify.works'

View File

@@ -21,9 +21,9 @@ from app.notifications.process_client_response import (
from app.notifications.process_notifications import (persist_notification, from app.notifications.process_notifications import (persist_notification,
send_notification_to_queue, send_notification_to_queue,
simulated_recipient) 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_for_notification_type,
check_template_is_active) check_template_is_active, check_rate_limiting)
from app.schemas import ( from app.schemas import (
email_notification_schema, email_notification_schema,
sms_template_notification_schema, sms_template_notification_schema,
@@ -107,7 +107,7 @@ def send_notification(notification_type):
if errors: if errors:
raise InvalidRequest(errors, status_code=400) 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'], template = templates_dao.dao_get_template_by_id_and_service_id(template_id=notification_form['template'],
service_id=service.id) service_id=service.id)

View File

@@ -4,12 +4,26 @@ from notifications_utils.recipients import validate_and_format_phone_number, val
from app.dao import services_dao from app.dao import services_dao
from app.models import KEY_TYPE_TEST, KEY_TYPE_TEAM, SMS_TYPE from app.models import KEY_TYPE_TEST, KEY_TYPE_TEAM, SMS_TYPE
from app.service.utils import service_allowed_to_send_to 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 app import redis_store
from notifications_utils.clients import redis from notifications_utils.clients import redis
def check_service_message_limit(key_type, service): def check_service_over_api_rate_limit(service, api_key):
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):
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: if key_type != KEY_TYPE_TEST:
cache_key = redis.daily_limit_cache_key(service.id) cache_key = redis.daily_limit_cache_key(service.id)
service_stats = redis_store.get(cache_key) service_stats = redis_store.get(cache_key)
@@ -20,6 +34,11 @@ def check_service_message_limit(key_type, service):
raise TooManyRequestsError(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): def check_template_is_for_notification_type(notification_type, template_type):
if notification_type != template_type: if notification_type != template_type:
message = "{0} template is not suitable for {1} notification".format(template_type, message = "{0} template is not suitable for {1} notification".format(template_type,
@@ -55,8 +74,6 @@ def validate_and_format_recipient(send_to, key_type, service, notification_type)
def check_sms_content_char_count(content_count): def check_sms_content_char_count(content_count):
char_count_limit = current_app.config.get('SMS_CHAR_COUNT_LIMIT') 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) message = 'Content for template has a character count greater than the limit of {}'.format(char_count_limit)
raise BadRequestError(message=message) raise BadRequestError(message=message)

View File

@@ -16,6 +16,19 @@ class TooManyRequestsError(InvalidRequest):
self.message = self.message_template.format(sending_limit) 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): class BadRequestError(InvalidRequest):
status_code = 400 status_code = 400
message = "An error occurred" message = "An error occurred"

View File

@@ -4,21 +4,25 @@ from sqlalchemy.orm.exc import NoResultFound
from app import api_user from app import api_user
from app.dao import services_dao, templates_dao from app.dao import services_dao, templates_dao
from app.models import SMS_TYPE, EMAIL_TYPE, PRIORITY 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.schema_validation import validate
from app.v2.errors import BadRequestError from app.v2.errors import BadRequestError
from app.v2.notifications import v2_notification_blueprint 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']) @v2_notification_blueprint.route('/<notification_type>', methods=['POST'])
@@ -28,7 +32,9 @@ def post_notification(notification_type):
else: else:
form = validate(request.get_json(), post_sms_request) form = validate(request.get_json(), post_sms_request)
service = services_dao.dao_fetch_service_by_id(api_user.service_id) 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'] 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, send_to = validate_and_format_recipient(send_to=form_send_to,
key_type=api_user.key_type, key_type=api_user.key_type,
@@ -39,6 +45,7 @@ def post_notification(notification_type):
# Do not persist or send notification to the queue if it is a simulated recipient # Do not persist or send notification to the queue if it is a simulated recipient
simulated = simulated_recipient(send_to, notification_type) simulated = simulated_recipient(send_to, notification_type)
notification = persist_notification(template_id=template.id, notification = persist_notification(template_id=template.id,
template_version=template.version, template_version=template.version,
recipient=send_to, recipient=send_to,
@@ -49,6 +56,7 @@ def post_notification(notification_type):
key_type=api_user.key_type, key_type=api_user.key_type,
client_reference=form.get('reference', None), client_reference=form.get('reference', None),
simulated=simulated) simulated=simulated)
if not simulated: if not simulated:
queue_name = 'priority' if template.process_type == PRIORITY else None queue_name = 'priority' if template.process_type == PRIORITY else None
send_notification_to_queue(notification=notification, research_mode=service.research_mode, queue=queue_name) send_notification_to_queue(notification=notification, research_mode=service.research_mode, queue=queue_name)

View File

@@ -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.templates_dao import dao_get_all_templates_for_service, dao_update_template
from app.dao.services_dao import dao_update_service from app.dao.services_dao import dao_update_service
from app.dao.api_key_dao import save_model_api_key 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 import create_authorization_header
from tests.app.conftest import ( from tests.app.conftest import (
sample_notification as create_sample_notification, sample_notification as create_sample_notification,
@@ -1046,3 +1047,49 @@ def test_send_notification_uses_priority_queue_when_template_is_marked_as_priori
assert response.status_code == 201 assert response.status_code == 201
mocked.assert_called_once_with([notification_id], queue='priority') 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

View File

@@ -3,21 +3,21 @@ from freezegun import freeze_time
import app import app
from app.notifications.validators import ( from app.notifications.validators import (
check_service_message_limit, check_service_over_daily_message_limit,
check_template_is_for_notification_type, check_template_is_for_notification_type,
check_template_is_active, check_template_is_active,
service_can_send_to_recipient, service_can_send_to_recipient,
check_sms_content_char_count check_sms_content_char_count,
) check_service_over_api_rate_limit)
from app.v2.errors import ( from app.v2.errors import (
BadRequestError, BadRequestError,
TooManyRequestsError TooManyRequestsError,
) RateLimitError)
from tests.app.conftest import ( from tests.app.conftest import (
sample_notification as create_notification, sample_notification as create_notification,
sample_service as create_service, sample_service as create_service,
sample_service_whitelist sample_service_whitelist,
) sample_api_key)
@pytest.mark.parametrize('key_type', ['team', 'normal']) @pytest.mark.parametrize('key_type', ['team', 'normal'])
@@ -37,7 +37,7 @@ def test_exception_thrown_by_redis_store_get_should_not_be_fatal(
create_notification(notify_db, notify_db_session, service=service) create_notification(notify_db, notify_db_session, service=service)
with pytest.raises(TooManyRequestsError) as e: 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.status_code == 429
assert e.value.message == 'Exceeded send limits (4) for today' assert e.value.message == 'Exceeded send limits (4) for today'
assert e.value.fields == [] assert e.value.fields == []
@@ -53,7 +53,7 @@ def test_exception_thown_by_redis_store_set_should_not_be_fatal(
mocker): mocker):
mocker.patch('app.notifications.validators.redis_store.redis_store.set', side_effect=Exception("broken redis")) 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) 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']) @pytest.mark.parametrize('key_type', ['test', 'team', 'normal'])
@@ -65,7 +65,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.redis_store.set')
mocker.patch('app.notifications.validators.services_dao') 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() app.notifications.validators.redis_store.set.assert_not_called()
assert not app.notifications.validators.services_dao.mock_calls assert not app.notifications.validators.services_dao.mock_calls
@@ -78,14 +78,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.get', return_value=1)
mocker.patch('app.notifications.validators.redis_store.set') mocker.patch('app.notifications.validators.redis_store.set')
mocker.patch('app.notifications.validators.services_dao') 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() app.notifications.validators.redis_store.set.assert_not_called()
assert not app.notifications.validators.services_dao.mock_calls assert not app.notifications.validators.services_dao.mock_calls
def test_should_not_interact_with_cache_for_test_key(sample_service, mocker): def test_should_not_interact_with_cache_for_test_key(sample_service, mocker):
mocker.patch('app.notifications.validators.redis_store') 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 assert not app.notifications.validators.redis_store.mock_calls
@@ -102,7 +102,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) 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.get', return_value=None)
mocker.patch('app.notifications.validators.redis_store.set') 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( app.notifications.validators.redis_store.set.assert_called_with(
str(sample_service.id) + "-2016-01-01-count", 5, ex=3600 str(sample_service.id) + "-2016-01-01-count", 5, ex=3600
) )
@@ -118,7 +118,7 @@ def test_check_service_message_limit_over_message_limit_fails(key_type, notify_d
for x in range(5): for x in range(5):
create_notification(notify_db, notify_db_session, service=service) create_notification(notify_db, notify_db_session, service=service)
with pytest.raises(TooManyRequestsError) as e: 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.status_code == 429
assert e.value.message == 'Exceeded send limits (4) for today' assert e.value.message == 'Exceeded send limits (4) for today'
assert e.value.fields == [] assert e.value.fields == []
@@ -140,7 +140,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) service = create_service(notify_db, notify_db_session, restricted=True, limit=4)
with pytest.raises(TooManyRequestsError) as e: 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.status_code == 429
assert e.value.message == 'Exceeded send limits (4) for today' assert e.value.message == 'Exceeded send limits (4) for today'
assert e.value.fields == [] assert e.value.fields == []
@@ -262,3 +262,61 @@ def test_check_sms_content_char_count_fails(char_count, notify_api):
assert e.value.message == 'Content for template has a character count greater than the limit of {}'.format( assert e.value.message == 'Content for template has a character count greater than the limit of {}'.format(
notify_api.config['SMS_CHAR_COUNT_LIMIT']) notify_api.config['SMS_CHAR_COUNT_LIMIT'])
assert e.value.fields == [] 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
)

View File

@@ -2,6 +2,7 @@ import uuid
import pytest import pytest
from flask import json from flask import json
from app.models import Notification from app.models import Notification
from app.v2.errors import RateLimitError
from tests import create_authorization_header from tests import create_authorization_header
from tests.app.conftest import sample_template as create_sample_template from tests.app.conftest import sample_template as create_sample_template
@@ -224,3 +225,51 @@ def test_send_notification_uses_priority_queue_when_template_is_marked_as_priori
assert response.status_code == 201 assert response.status_code == 201
mocked.assert_called_once_with([notification_id], queue='priority') 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