From 5e53d781e0ee2604a1d6110ba4e294eb5ca05af6 Mon Sep 17 00:00:00 2001 From: Alexey Bezhan Date: Mon, 20 Nov 2017 12:25:01 +0000 Subject: [PATCH 1/2] Add FIRETEXT_INBOUND_SMS_AUTH config variable and auth check Checks authentication header value on inbound SMS requests from Firetext against a list of allowed API keys set in the application config. At the moment, we're only logging the attempts without aborting the requests. Once this is rolled out to production and we've checked the logs we'll switch on the aborts and add the tests for 401 and 403 responses. --- app/cloudfoundry_config.py | 1 + app/config.py | 3 +++ app/notifications/receive_notifications.py | 12 +++++++----- tests/app/test_cloudfoundry_config.py | 8 ++++++++ 4 files changed, 19 insertions(+), 5 deletions(-) diff --git a/app/cloudfoundry_config.py b/app/cloudfoundry_config.py index ee6d2ba0c..4244823a8 100644 --- a/app/cloudfoundry_config.py +++ b/app/cloudfoundry_config.py @@ -45,6 +45,7 @@ def extract_notify_config(notify_config): os.environ['SECRET_KEY'] = notify_config['credentials']['secret_key'] os.environ['DANGEROUS_SALT'] = notify_config['credentials']['dangerous_salt'] os.environ['SMS_INBOUND_WHITELIST'] = json.dumps(notify_config['credentials']['allow_ip_inbound_sms']) + os.environ['FIRETEXT_INBOUND_SMS_AUTH'] = json.dumps(notify_config['credentials']['firetext_inbound_sms_auth']) os.environ['ROUTE_SECRET_KEY_1'] = notify_config['credentials']['route_secret_key_1'] os.environ['ROUTE_SECRET_KEY_2'] = notify_config['credentials']['route_secret_key_2'] diff --git a/app/config.py b/app/config.py index 8d7ea4ac7..bac6db38f 100644 --- a/app/config.py +++ b/app/config.py @@ -295,6 +295,8 @@ class Config(object): FREE_SMS_TIER_FRAGMENT_COUNT = 250000 SMS_INBOUND_WHITELIST = json.loads(os.environ.get('SMS_INBOUND_WHITELIST', '[]')) + FIRETEXT_INBOUND_SMS_AUTH = json.loads(os.environ.get('FIRETEXT_INBOUND_SMS_AUTH', '[]')) + ROUTE_SECRET_KEY_1 = os.environ.get('ROUTE_SECRET_KEY_1', '') ROUTE_SECRET_KEY_2 = os.environ.get('ROUTE_SECRET_KEY_2', '') @@ -364,6 +366,7 @@ class Test(Config): } SMS_INBOUND_WHITELIST = ['203.0.113.195'] + FIRETEXT_INBOUND_SMS_AUTH = ['testkey'] class Preview(Config): diff --git a/app/notifications/receive_notifications.py b/app/notifications/receive_notifications.py index 4b6e2ebb8..3167134e3 100644 --- a/app/notifications/receive_notifications.py +++ b/app/notifications/receive_notifications.py @@ -1,7 +1,7 @@ from urllib.parse import unquote import iso8601 -from flask import jsonify, Blueprint, current_app, request +from flask import jsonify, Blueprint, current_app, request, abort from notifications_utils.recipients import validate_and_format_phone_number from app import statsd_client, firetext_client, mmg_client @@ -58,10 +58,12 @@ def receive_firetext_sms(): # This is pre-implementation test code to validate the provider is basic auth headers. auth = request.authorization - if auth: - current_app.logger.info("Inbound sms username: {}".format(auth.username)) - else: - current_app.logger.info("Inbound sms no auth header") + if not auth: + current_app.logger.warning("Inbound sms no auth header") + # abort(401) + elif auth.username != 'notify' or auth.password not in current_app.config['FIRETEXT_INBOUND_SMS_AUTH']: + current_app.logger.warning("Inbound sms incorrect username ({}) or password".format(auth.username)) + # abort(403) inbound_number = strip_leading_forty_four(post_data['destination']) diff --git a/tests/app/test_cloudfoundry_config.py b/tests/app/test_cloudfoundry_config.py index f20a0300d..dabfc1e83 100644 --- a/tests/app/test_cloudfoundry_config.py +++ b/tests/app/test_cloudfoundry_config.py @@ -17,6 +17,7 @@ def notify_config(): 'secret_key': 'secret key', 'dangerous_salt': 'dangerous salt', 'allow_ip_inbound_sms': ['111.111.111.111', '100.100.100.100'], + 'firetext_inbound_sms_auth': ['testkey'], 'route_secret_key_1': "key_1", 'route_secret_key_2': "" } @@ -211,6 +212,13 @@ def test_sms_inbound_config(): assert os.environ['SMS_INBOUND_WHITELIST'] == json.dumps(['111.111.111.111', '100.100.100.100']) +@pytest.mark.usefixtures('os_environ', 'cloudfoundry_environ') +def test_firetext_inbound_sms_auth_config(): + extract_cloudfoundry_config() + + assert os.environ['FIRETEXT_INBOUND_SMS_AUTH'] == json.dumps(['testkey']) + + @pytest.mark.usefixtures('os_environ', 'cloudfoundry_environ') def test_performance_platform_config(): extract_cloudfoundry_config() From 3bcde5437bce9b01dc83fc04940e512f17d8c34a Mon Sep 17 00:00:00 2001 From: Alexey Bezhan Date: Mon, 20 Nov 2017 15:56:31 +0000 Subject: [PATCH 2/2] Add tests for aborting unauthenticated requests Tests are disabled until aborts are switched on --- .../test_receive_notification.py | 44 ++++++++++++++++--- 1 file changed, 39 insertions(+), 5 deletions(-) diff --git a/tests/app/notifications/test_receive_notification.py b/tests/app/notifications/test_receive_notification.py index 8db765b2c..7f784135c 100644 --- a/tests/app/notifications/test_receive_notification.py +++ b/tests/app/notifications/test_receive_notification.py @@ -1,4 +1,5 @@ import uuid +import base64 from datetime import datetime from unittest.mock import call @@ -16,18 +17,26 @@ from app.notifications.receive_notifications import ( ) from app.models import InboundSms, EMAIL_TYPE, SMS_TYPE, INBOUND_SMS_TYPE +from tests.conftest import set_config from tests.app.db import create_inbound_number, create_service, create_service_with_inbound_number from tests.app.conftest import sample_service -def firetext_post(client, data): +def firetext_post(client, data, auth=True, password='testkey'): + headers = [ + ('Content-Type', 'application/x-www-form-urlencoded'), + ('X-Forwarded-For', '203.0.113.195, 70.41.3.18, 150.172.238.178') + ] + + if auth: + auth_value = base64.b64encode("notify:{}".format(password).encode('utf-8')).decode('utf-8') + headers.append(('Authorization', 'Basic ' + auth_value)) + return client.post( path='/notifications/sms/receive/firetext', data=data, - headers=[ - ('Content-Type', 'application/x-www-form-urlencoded'), - ('X-Forwarded-For', '203.0.113.195, 70.41.3.18, 150.172.238.178') - ]) + headers=headers + ) def mmg_post(client, data): @@ -379,3 +388,28 @@ def test_returns_ok_to_firetext_if_mismatched_sms_sender(notify_db_session, clie ) def test_strip_leading_country_code(number, expected): assert strip_leading_forty_four(number) == expected + + +@pytest.mark.parametrize("auth, keys, status_code", [ + ["testkey", ["testkey"], 200], + ["", ["testkey"], 401], + ["wrong", ["testkey"], 403], + ["testkey1", ["testkey1", "testkey2"], 200], + ["testkey2", ["testkey1", "testkey2"], 200], + ["wrong", ["testkey1", "testkey2"], 403], + ["", [], 401], + ["testkey", [], 403], +]) +@pytest.mark.skip(reason="aborts are disabled at the moment") +def test_firetext_inbound_sms_auth(notify_db_session, notify_api, client, mocker, auth, keys, status_code): + mocker.patch("app.notifications.receive_notifications.tasks.send_inbound_sms_to_service.apply_async") + + create_service_with_inbound_number( + service_name='b', inbound_number='07111111111', service_permissions=[EMAIL_TYPE, SMS_TYPE, INBOUND_SMS_TYPE] + ) + + data = "source=07999999999&destination=07111111111&message=this is a message&time=2017-01-01 12:00:00" + + with set_config(notify_api, 'FIRETEXT_INBOUND_SMS_AUTH', keys): + response = firetext_post(client, data, auth=bool(auth), password=auth) + assert response.status_code == status_code