From 4d4842176775e8c7abaf6b395fc6af2ecf8375c1 Mon Sep 17 00:00:00 2001 From: Alexey Bezhan Date: Thu, 23 Nov 2017 17:17:37 +0000 Subject: [PATCH 1/2] Abort unauthenticated requests for Firetext inbound SMS Switches on authentication checks for Firetext inbound SMS callbacks. This should only be released once Firetext callback URLs have been updated with authentication details. --- app/notifications/receive_notifications.py | 4 ++-- tests/app/notifications/test_receive_notification.py | 1 - 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/app/notifications/receive_notifications.py b/app/notifications/receive_notifications.py index 3167134e3..e55c02647 100644 --- a/app/notifications/receive_notifications.py +++ b/app/notifications/receive_notifications.py @@ -60,10 +60,10 @@ def receive_firetext_sms(): auth = request.authorization if not auth: current_app.logger.warning("Inbound sms no auth header") - # abort(401) + 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) + abort(403) inbound_number = strip_leading_forty_four(post_data['destination']) diff --git a/tests/app/notifications/test_receive_notification.py b/tests/app/notifications/test_receive_notification.py index 7f784135c..7d6dca486 100644 --- a/tests/app/notifications/test_receive_notification.py +++ b/tests/app/notifications/test_receive_notification.py @@ -400,7 +400,6 @@ def test_strip_leading_country_code(number, expected): ["", [], 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") From adfba208c44aa04530f4f787335d4275db6c4a0d Mon Sep 17 00:00:00 2001 From: Richard Chapman Date: Mon, 27 Nov 2017 11:05:47 +0000 Subject: [PATCH 2/2] Removed the templates/monthly endpoint Removed the REST endpoint and the DAO that it uses as the endpoint is no longer used by the Admin UI and the DAO is not reused anywhere else. - Removed REST endpoint - Removed DAO which gets the stats - Removed associated tests of both methods --- app/dao/services_dao.py | 34 ------------ app/service/rest.py | 13 ----- tests/app/dao/test_services_dao.py | 89 ------------------------------ tests/app/service/test_rest.py | 45 --------------- 4 files changed, 181 deletions(-) diff --git a/app/dao/services_dao.py b/app/dao/services_dao.py index 416ed87a8..851348271 100644 --- a/app/dao/services_dao.py +++ b/app/dao/services_dao.py @@ -297,40 +297,6 @@ def _stats_for_service_query(service_id): ) -@statsd(namespace="dao") -def dao_fetch_monthly_historical_stats_by_template_for_service(service_id, year): - month = get_london_month_from_utc_column(NotificationHistory.created_at) - - start_date, end_date = get_financial_year(year) - sq = db.session.query( - NotificationHistory.template_id, - NotificationHistory.status, - month.label('month'), - func.count().label('count') - ).filter( - NotificationHistory.service_id == service_id, - NotificationHistory.created_at.between(start_date, end_date) - ).group_by( - month, - NotificationHistory.template_id, - NotificationHistory.status - ).subquery() - - rows = db.session.query( - Template.id.label('template_id'), - Template.name, - Template.template_type, - sq.c.status, - sq.c.count.label('count'), - sq.c.month - ).join( - sq, - sq.c.template_id == Template.id - ).all() - - return format_monthly_template_notification_stats(year, rows) - - @statsd(namespace="dao") def dao_fetch_monthly_historical_stats_for_service(service_id, year): month = get_london_month_from_utc_column(NotificationHistory.created_at) diff --git a/app/service/rest.py b/app/service/rest.py index 2dda86991..a662b8526 100644 --- a/app/service/rest.py +++ b/app/service/rest.py @@ -37,7 +37,6 @@ from app.dao.services_dao import ( dao_fetch_all_services, dao_fetch_all_services_by_user, dao_fetch_monthly_historical_stats_for_service, - dao_fetch_monthly_historical_stats_by_template_for_service, dao_fetch_monthly_historical_usage_by_template_for_service, dao_fetch_service_by_id, dao_fetch_stats_for_service, @@ -541,18 +540,6 @@ def get_monthly_template_usage(service_id): raise InvalidRequest('Year must be a number', status_code=400) -@service_blueprint.route('//notifications/templates/monthly', methods=['GET']) -def get_monthly_template_stats(service_id): - service = dao_fetch_service_by_id(service_id) - try: - return jsonify(data=dao_fetch_monthly_historical_stats_by_template_for_service( - service.id, - int(request.args.get('year', 'NaN')) - )) - except ValueError: - raise InvalidRequest('Year must be a number', status_code=400) - - @service_blueprint.route('//inbound-api', methods=['POST']) def create_service_inbound_api(service_id): data = request.get_json() diff --git a/tests/app/dao/test_services_dao.py b/tests/app/dao/test_services_dao.py index beefc6b86..4eb51725f 100644 --- a/tests/app/dao/test_services_dao.py +++ b/tests/app/dao/test_services_dao.py @@ -25,7 +25,6 @@ from app.dao.services_dao import ( dao_fetch_stats_for_service, dao_fetch_todays_stats_for_service, dao_fetch_monthly_historical_stats_for_service, - dao_fetch_monthly_historical_stats_by_template_for_service, fetch_todays_total_message_count, dao_fetch_todays_stats_for_all_services, fetch_stats_by_date_range_for_all_services, @@ -77,7 +76,6 @@ from tests.app.conftest import ( def test_should_have_decorated_services_dao_functions(): - assert dao_fetch_monthly_historical_stats_by_template_for_service.__wrapped__.__name__ == 'dao_fetch_monthly_historical_stats_by_template_for_service' # noqa assert dao_fetch_monthly_historical_stats_for_service.__wrapped__.__name__ == 'dao_fetch_monthly_historical_stats_for_service' # noqa assert dao_fetch_todays_stats_for_service.__wrapped__.__name__ == 'dao_fetch_todays_stats_for_service' # noqa assert dao_fetch_stats_for_service.__wrapped__.__name__ == 'dao_fetch_stats_for_service' # noqa @@ -850,93 +848,6 @@ def test_dao_resume_service_marks_service_as_active_and_api_keys_are_still_revok assert api_key.expiry_date == datetime(2001, 1, 1, 23, 59, 00) -def test_fetch_monthly_historical_template_stats_for_service_includes_financial_year_only( - notify_db, - notify_db_session, - sample_template -): - notification_history = functools.partial( - create_notification_history, - notify_db, - notify_db_session, - sample_template - ) - - notification_history(created_at=datetime(2016, 4, 1), status='sending') # Start of financial year - notification_history(created_at=datetime(2016, 5, 30), status='created') - notification_history(created_at=datetime(2016, 6, 1), status='created') - notification_history(created_at=datetime(2017, 3, 31), status='created') # End of financial year - notification_history(created_at=datetime(2017, 4, 1), status='created') - notification_history(created_at=datetime(2017, 5, 1), status='created') - - result = dao_fetch_monthly_historical_stats_by_template_for_service(sample_template.service_id, 2016) - - notifications_count = 0 - for dict in result.values(): - notifications_count += sum(dict.get(str(sample_template.id), {}).get("counts", {}).values()) - - assert '2017-04' not in result - assert '2017-05' not in result - assert notifications_count == 4 - - -def test_fetch_monthly_historical_template_stats_for_service_separates_months( - notify_db, - notify_db_session, - sample_template -): - notification_history = functools.partial( - create_notification_history, - notify_db, - notify_db_session, - sample_template - ) - - notification_history(created_at=datetime(2016, 4, 1), status='sending') # Start of financial year - notification_history(created_at=datetime(2016, 5, 30), status='created') - notification_history(created_at=datetime(2016, 6, 1), status='delivered') - notification_history(created_at=datetime(2016, 6, 1), status='created') - notification_history(created_at=datetime(2016, 12, 1), status='created') - notification_history(created_at=datetime(2017, 3, 30), status='sending') - notification_history(created_at=datetime(2017, 3, 31), status='sending') - - result = dao_fetch_monthly_historical_stats_by_template_for_service(sample_template.service_id, 2016) - - financial_year_month_keys = \ - ['2016-{:02}'.format(month) for month in range(4, 13)] + ['2017-{:02}'.format(month) for month in range(1, 4)] - - assert set(financial_year_month_keys) == set(result.keys()) - assert sum(result["2016-04"][str(sample_template.id)]["counts"].values()) == 1 - assert sum(result["2016-05"][str(sample_template.id)]["counts"].values()) == 1 - assert sum(result["2016-06"][str(sample_template.id)]["counts"].values()) == 2 - assert sum(result["2016-12"][str(sample_template.id)]["counts"].values()) == 1 - assert sum(result["2017-03"][str(sample_template.id)]["counts"].values()) == 2 - - -def test_fetch_monthly_historical_template_stats_for_service_separates_templates( - notify_db, - notify_db_session -): - notification_history = functools.partial( - create_notification_history, - notify_db, - notify_db_session, - status='delivered' - ) - - template_one = create_sample_template(notify_db, notify_db_session) - template_two = create_sample_template(notify_db, notify_db_session) - - notification_history(created_at=datetime(2016, 4, 1), sample_template=template_one) - notification_history(created_at=datetime(2016, 4, 1), sample_template=template_two) - - result = dao_fetch_monthly_historical_stats_by_template_for_service(template_one.service_id, 2016) - - assert len(result.get('2016-04').keys()) == 2 - assert str(template_one.id) in result.get('2016-04').keys() - assert str(template_two.id) in result.get('2016-04').keys() - - def test_dao_fetch_active_users_for_service_returns_active_only(notify_db, notify_db_session): active_user = create_user(email='active@foo.com', state='active') pending_user = create_user(email='pending@foo.com', state='pending') diff --git a/tests/app/service/test_rest.py b/tests/app/service/test_rest.py index 05a1d8734..060f6e0a3 100644 --- a/tests/app/service/test_rest.py +++ b/tests/app/service/test_rest.py @@ -1810,33 +1810,6 @@ def test_get_service_provider_aggregate_statistics( assert json.loads(response.get_data(as_text=True)) == expected_json -def test_get_template_stats_by_month_returns_correct_data(notify_db, notify_db_session, client, sample_template): - notification_history = partial( - create_notification_history, - notify_db, - notify_db_session, - sample_template - ) - with freeze_time('2016-05-01T12:00:00'): - not1 = notification_history(status='sending') - notification_history(status='sending') - notification_history(status='permanent-failure') - notification_history(status='temporary-failure') - - resp = client.get( - '/service/{}/notifications/templates/monthly?year=2016'.format(not1.service_id), - headers=[create_authorization_header()] - ) - resp_json = json.loads(resp.get_data(as_text=True)).get('data') - - assert resp.status_code == 200 - assert resp_json["2016-05"][str(sample_template.id)]["name"] == "Template Name" - assert resp_json["2016-05"][str(sample_template.id)]["type"] == "sms" - assert resp_json["2016-05"][str(sample_template.id)]["counts"]["sending"] == 2 - assert resp_json["2016-05"][str(sample_template.id)]["counts"]["temporary-failure"] == 1 - assert resp_json["2016-05"][str(sample_template.id)]["counts"]["permanent-failure"] == 1 - - @freeze_time('2017-11-11 02:00') def test_get_template_usage_by_month_returns_correct_data( notify_db, @@ -2048,24 +2021,6 @@ def test_get_template_usage_by_month_returns_two_templates( assert resp_json[2]["count"] == 1 -@pytest.mark.parametrize('query_string, expected_status, expected_json', [ - ('?year=abcd', 400, {'message': 'Year must be a number', 'result': 'error'}), -]) -def test_get_template_stats_by_month_returns_error_for_incorrect_year( - client, - sample_service, - query_string, - expected_status, - expected_json -): - response = client.get( - '/service/{}/notifications/templates/monthly{}'.format(sample_service.id, query_string), - headers=[create_authorization_header()] - ) - assert response.status_code == expected_status - assert json.loads(response.get_data(as_text=True)) == expected_json - - def test_search_for_notification_by_to_field(client, notify_db, notify_db_session): create_notification = partial(create_sample_notification, notify_db, notify_db_session) notification1 = create_notification(to_field='+447700900855', normalised_to='447700900855')