From e77534fb1751e7cab0f95cac64e23c2d2328da4d Mon Sep 17 00:00:00 2001 From: Rebecca Law Date: Wed, 17 Feb 2021 09:52:04 +0000 Subject: [PATCH 1/2] Send text message that are to an international number from a number rather than "Notify" Update `send_user_2fa_code` to send from number when recipient is international Update `update_user_attribute` to send from number when recipient is international --- app/config.py | 1 + app/user/rest.py | 11 +++++- .../versions/0346_notify_number_sms_sender.py | 39 +++++++++++++++++++ tests/app/user/test_rest.py | 24 +++++++++++- tests/app/user/test_rest_verify.py | 18 +++++++++ 5 files changed, 90 insertions(+), 3 deletions(-) create mode 100644 migrations/versions/0346_notify_number_sms_sender.py diff --git a/app/config.py b/app/config.py index 1ffdddefd..e9307c94e 100644 --- a/app/config.py +++ b/app/config.py @@ -179,6 +179,7 @@ class Config(object): MOU_SIGNED_ON_BEHALF_SIGNER_RECEIPT_TEMPLATE_ID = 'c20206d5-bf03-4002-9a90-37d5032d9e84' MOU_SIGNED_ON_BEHALF_ON_BEHALF_RECEIPT_TEMPLATE_ID = '522b6657-5ca5-4368-a294-6b527703bd0b' MOU_NOTIFY_TEAM_ALERT_TEMPLATE_ID = 'd0e66c4c-0c50-43f0-94f5-f85b613202d4' + NOTIFY_NUMBER_SMS_SENDER = '07984404008' BROKER_URL = 'sqs://' BROKER_TRANSPORT_OPTIONS = { diff --git a/app/user/rest.py b/app/user/rest.py index 68cf78f1a..81644aa71 100644 --- a/app/user/rest.py +++ b/app/user/rest.py @@ -4,6 +4,7 @@ from datetime import datetime from urllib.parse import urlencode from flask import (jsonify, request, Blueprint, current_app, abort) +from notifications_utils.recipients import is_uk_phone_number from sqlalchemy.exc import IntegrityError from app.config import QueueNames @@ -104,7 +105,10 @@ def update_user_attribute(user_id): elif 'mobile_number' in update_dct: template = dao_get_template_by_id(current_app.config['TEAM_MEMBER_EDIT_MOBILE_TEMPLATE_ID']) recipient = user_to_update.mobile_number - reply_to = template.service.get_default_sms_sender() + if is_uk_phone_number(recipient): + reply_to = template.service.get_default_sms_sender() + else: + reply_to = current_app.config['NOTIFY_NUMBER_SMS_SENDER'] else: return jsonify(data=user_to_update.serialize()), 200 service = Service.query.get(current_app.config['NOTIFY_SERVICE_ID']) @@ -267,7 +271,10 @@ def create_2fa_code(template_id, user_to_send_to, secret_code, recipient, person create_user_code(user_to_send_to, secret_code, template.template_type) reply_to = None if template.template_type == SMS_TYPE: - reply_to = template.service.get_default_sms_sender() + if is_uk_phone_number(recipient): + reply_to = template.service.get_default_sms_sender() + else: + reply_to = current_app.config['NOTIFY_NUMBER_SMS_SENDER'] elif template.template_type == EMAIL_TYPE: reply_to = template.service.get_default_reply_to_email_address() saved_notification = persist_notification( diff --git a/migrations/versions/0346_notify_number_sms_sender.py b/migrations/versions/0346_notify_number_sms_sender.py new file mode 100644 index 000000000..73d990e37 --- /dev/null +++ b/migrations/versions/0346_notify_number_sms_sender.py @@ -0,0 +1,39 @@ +""" + +Revision ID: 0346_notify_number_sms_sender +Revises: 0345_move_broadcast_provider +Create Date: 2021-02-17 10:40:10.181087 + +""" +import uuid + +from alembic import op +from flask import current_app + +revision = '0346_notify_number_sms_sender' +down_revision = '0345_move_broadcast_provider' + +SMS_SENDER_ID = 'd24b830b-57b4-4f14-bd80-02f46f8d54de' +NOTIFY_SERVICE_ID = current_app.config['NOTIFY_SERVICE_ID'] +INBOUND_NUMBER = current_app.config['NOTIFY_NUMBER_SMS_SENDER'] + + +def upgrade(): + + sql = f"""INSERT INTO service_sms_senders (id, sms_sender, service_id, is_default, created_at) + VALUES ('{SMS_SENDER_ID}', '{INBOUND_NUMBER}', '{NOTIFY_SERVICE_ID}',false, now())""" + + op.execute(sql) + inbound_number_id = uuid.uuid4() + # by adding a row in inbound_number we ensure the number isn't added to the table and assigned to a service. + inbound_number_sql = f"""INSERT INTO INBOUND_NUMBERS (id, number, provider, active, created_at) + VALUES('{inbound_number_id}', '{INBOUND_NUMBER}', 'mmg', false, now()) + """ + op.execute(inbound_number_sql) + + +def downgrade(): + delete_sms_sender = f"delete from service_sms_senders where id = '{SMS_SENDER_ID}'" + delete_inbound_number = f"delete from inbound_numbers where number = '{INBOUND_NUMBER}'" + op.execute(delete_sms_sender) + op.execute(delete_inbound_number) diff --git a/tests/app/user/test_rest.py b/tests/app/user/test_rest.py index 4c447d8c7..81299cb1f 100644 --- a/tests/app/user/test_rest.py +++ b/tests/app/user/test_rest.py @@ -4,7 +4,7 @@ import mock from uuid import UUID from datetime import datetime -from flask import url_for +from flask import url_for, current_app from freezegun import freeze_time from app.models import ( @@ -310,6 +310,28 @@ def test_post_user_attribute_with_updated_by( mock_persist_notification.assert_not_called() +def test_post_user_attribute_with_updated_by_sends_notification_to_international_from_number( + client, mocker, sample_user, team_member_mobile_edit_template +): + updater = create_user(name="Service Manago") + update_dict = { + 'mobile_number': '601117224412', + 'updated_by': str(updater.id) + } + auth_header = create_authorization_header() + headers = [('Content-Type', 'application/json'), auth_header] + mocker.patch('app.user.rest.send_notification_to_queue') + resp = client.post( + url_for('user.update_user_attribute', user_id=sample_user.id), + data=json.dumps(update_dict), + headers=headers) + + assert resp.status_code == 200, resp.get_data(as_text=True) + + notification = Notification.query.first() + assert notification.reply_to_text == current_app.config['NOTIFY_NUMBER_SMS_SENDER'] + + def test_archive_user(mocker, client, sample_user): archive_mock = mocker.patch('app.user.rest.dao_archive_user') diff --git a/tests/app/user/test_rest_verify.py b/tests/app/user/test_rest_verify.py index cfdcc9c11..f32c2cdbc 100644 --- a/tests/app/user/test_rest_verify.py +++ b/tests/app/user/test_rest_verify.py @@ -477,3 +477,21 @@ def test_user_verify_email_code_fails_if_code_already_used(admin_request, sample assert verify_code.code_used assert sample_user.logged_in_at is None assert sample_user.current_session_id is None + + +def test_send_user_2fa_code_sends_from_number_for_international_numbers( + client, sample_user, mocker, sms_code_template +): + sample_user.mobile_number = "601117224412" + auth_header = create_authorization_header() + mocker.patch('app.user.rest.create_secret_code', return_value='11111') + mocker.patch('app.user.rest.send_notification_to_queue') + + resp = client.post( + url_for('user.send_user_2fa_code', code_type='sms', user_id=sample_user.id), + data=json.dumps({}), + headers=[('Content-Type', 'application/json'), auth_header]) + assert resp.status_code == 204 + + notification = Notification.query.first() + assert notification.reply_to_text == current_app.config['NOTIFY_NUMBER_SMS_SENDER'] From 77b76ea0a497ba1ce236beaa0f28ac2b08b78fc8 Mon Sep 17 00:00:00 2001 From: Rebecca Law Date: Wed, 17 Feb 2021 13:15:29 +0000 Subject: [PATCH 2/2] Rename variable, it's a better name now. --- app/config.py | 2 +- app/user/rest.py | 4 ++-- migrations/versions/0346_notify_number_sms_sender.py | 2 +- tests/app/user/test_rest.py | 2 +- tests/app/user/test_rest_verify.py | 2 +- 5 files changed, 6 insertions(+), 6 deletions(-) diff --git a/app/config.py b/app/config.py index e9307c94e..f3de3baaf 100644 --- a/app/config.py +++ b/app/config.py @@ -179,7 +179,7 @@ class Config(object): MOU_SIGNED_ON_BEHALF_SIGNER_RECEIPT_TEMPLATE_ID = 'c20206d5-bf03-4002-9a90-37d5032d9e84' MOU_SIGNED_ON_BEHALF_ON_BEHALF_RECEIPT_TEMPLATE_ID = '522b6657-5ca5-4368-a294-6b527703bd0b' MOU_NOTIFY_TEAM_ALERT_TEMPLATE_ID = 'd0e66c4c-0c50-43f0-94f5-f85b613202d4' - NOTIFY_NUMBER_SMS_SENDER = '07984404008' + NOTIFY_INTERNATIONAL_SMS_SENDER = '07984404008' BROKER_URL = 'sqs://' BROKER_TRANSPORT_OPTIONS = { diff --git a/app/user/rest.py b/app/user/rest.py index 81644aa71..32aa7b532 100644 --- a/app/user/rest.py +++ b/app/user/rest.py @@ -108,7 +108,7 @@ def update_user_attribute(user_id): if is_uk_phone_number(recipient): reply_to = template.service.get_default_sms_sender() else: - reply_to = current_app.config['NOTIFY_NUMBER_SMS_SENDER'] + reply_to = current_app.config['NOTIFY_INTERNATIONAL_SMS_SENDER'] else: return jsonify(data=user_to_update.serialize()), 200 service = Service.query.get(current_app.config['NOTIFY_SERVICE_ID']) @@ -274,7 +274,7 @@ def create_2fa_code(template_id, user_to_send_to, secret_code, recipient, person if is_uk_phone_number(recipient): reply_to = template.service.get_default_sms_sender() else: - reply_to = current_app.config['NOTIFY_NUMBER_SMS_SENDER'] + reply_to = current_app.config['NOTIFY_INTERNATIONAL_SMS_SENDER'] elif template.template_type == EMAIL_TYPE: reply_to = template.service.get_default_reply_to_email_address() saved_notification = persist_notification( diff --git a/migrations/versions/0346_notify_number_sms_sender.py b/migrations/versions/0346_notify_number_sms_sender.py index 73d990e37..fca7ddbcd 100644 --- a/migrations/versions/0346_notify_number_sms_sender.py +++ b/migrations/versions/0346_notify_number_sms_sender.py @@ -15,7 +15,7 @@ down_revision = '0345_move_broadcast_provider' SMS_SENDER_ID = 'd24b830b-57b4-4f14-bd80-02f46f8d54de' NOTIFY_SERVICE_ID = current_app.config['NOTIFY_SERVICE_ID'] -INBOUND_NUMBER = current_app.config['NOTIFY_NUMBER_SMS_SENDER'] +INBOUND_NUMBER = current_app.config['NOTIFY_INTERNATIONAL_SMS_SENDER'] def upgrade(): diff --git a/tests/app/user/test_rest.py b/tests/app/user/test_rest.py index 81299cb1f..64ad80d15 100644 --- a/tests/app/user/test_rest.py +++ b/tests/app/user/test_rest.py @@ -329,7 +329,7 @@ def test_post_user_attribute_with_updated_by_sends_notification_to_international assert resp.status_code == 200, resp.get_data(as_text=True) notification = Notification.query.first() - assert notification.reply_to_text == current_app.config['NOTIFY_NUMBER_SMS_SENDER'] + assert notification.reply_to_text == current_app.config['NOTIFY_INTERNATIONAL_SMS_SENDER'] def test_archive_user(mocker, client, sample_user): diff --git a/tests/app/user/test_rest_verify.py b/tests/app/user/test_rest_verify.py index f32c2cdbc..16661a57f 100644 --- a/tests/app/user/test_rest_verify.py +++ b/tests/app/user/test_rest_verify.py @@ -494,4 +494,4 @@ def test_send_user_2fa_code_sends_from_number_for_international_numbers( assert resp.status_code == 204 notification = Notification.query.first() - assert notification.reply_to_text == current_app.config['NOTIFY_NUMBER_SMS_SENDER'] + assert notification.reply_to_text == current_app.config['NOTIFY_INTERNATIONAL_SMS_SENDER']