Merge pull request #1279 from GSA/notify-admin-1870

Change phone number in Notify.gov w/o entering password

app/clients/sms/aws_sns.py

@@ -48,7 +48,6 @@ class AwsSnsClient(SmsClient):
 
     def send_sms(self, to, content, reference, sender=None, international=False):
         matched = False
-
         for match in phonenumbers.PhoneNumberMatcher(to, "US"):
             matched = True
             to = phonenumbers.format_number(

app/delivery/send_to_providers.py

@@ -1,4 +1,5 @@
 import json
+from contextlib import suppress
 from urllib import parse
 
 from cachetools import TTLCache, cached
@@ -81,27 +82,15 @@ def send_sms_to_provider(notification):
                 # We start by trying to get the phone number from a job in s3.  If we fail, we assume
                 # the phone number is for the verification code on login, which is not a job.
                 recipient = None
-                try:
+                # It is our 2facode, maybe
+                recipient = _get_verify_code(notification)
+
+                if recipient is None:
                     recipient = get_phone_number_from_s3(
                         notification.service_id,
                         notification.job_id,
                         notification.job_row_number,
                     )
-                except Exception:
-                    # It is our 2facode, maybe
-                    key = f"2facode-{notification.id}".replace(" ", "")
-                    recipient = redis_store.get(key)
-
-                    if recipient:
-                        recipient = recipient.decode("utf-8")
-
-                if recipient is None:
-                    si = notification.service_id
-                    ji = notification.job_id
-                    jrn = notification.job_row_number
-                    raise Exception(
-                        f"The recipient for (Service ID: {si}; Job ID: {ji}; Job Row Number {jrn} was not found."
-                    )
 
                 sender_numbers = get_sender_numbers(notification)
                 if notification.reply_to_text not in sender_numbers:
@@ -138,6 +127,14 @@ def send_sms_to_provider(notification):
     return message_id
 
 
+def _get_verify_code(notification):
+    key = f"2facode-{notification.id}".replace(" ", "")
+    recipient = redis_store.get(key)
+    with suppress(AttributeError):
+        recipient = recipient.decode("utf-8")
+    return recipient
+
+
 def get_sender_numbers(notification):
     possible_senders = dao_get_sms_senders_by_service_id(notification.service_id)
     sender_numbers = []

app/user/rest.py

@@ -308,7 +308,6 @@ def send_user_2fa_code(user_id, code_type):
 
 def send_user_sms_code(user_to_send_to, data):
     recipient = data.get("to") or user_to_send_to.mobile_number
-
     secret_code = create_secret_code()
     personalisation = {"verify_code": secret_code}
 

tests/app/delivery/test_send_to_providers.py

@@ -75,6 +75,8 @@ def test_provider_to_use_raises_if_no_active_providers(
 def test_should_send_personalised_template_to_correct_sms_provider_and_persist(
     sample_sms_template_with_html, mocker
 ):
+
+    mocker.patch("app.delivery.send_to_providers._get_verify_code", return_value=None)
     db_notification = create_notification(
         template=sample_sms_template_with_html,
         personalisation={},
@@ -114,6 +116,7 @@ def test_should_send_personalised_template_to_correct_sms_provider_and_persist(
 def test_should_send_personalised_template_to_correct_email_provider_and_persist(
     sample_email_template_with_html, mocker
 ):
+
     mock_redis = mocker.patch("app.delivery.send_to_providers.redis_store")
     utf8_encoded_email = "jo.smith@example.com".encode("utf-8")
     mock_redis.get.return_value = utf8_encoded_email
@@ -213,6 +216,8 @@ def test_should_not_send_sms_message_when_service_is_inactive_notification_is_in
 def test_send_sms_should_use_template_version_from_notification_not_latest(
     sample_template, mocker
 ):
+
+    mocker.patch("app.delivery.send_to_providers._get_verify_code", return_value=None)
     db_notification = create_notification(
         template=sample_template,
         to_field="2028675309",
@@ -318,6 +323,8 @@ def test_should_send_sms_with_downgraded_content(notify_db_session, mocker):
     # é, o, and u are in GSM.
     # ī, grapes, tabs, zero width space and ellipsis are not
     # ó isn't in GSM, but it is in the welsh alphabet so will still be sent
+
+    mocker.patch("app.delivery.send_to_providers.redis_store", return_value=None)
     mocker.patch(
         "app.delivery.send_to_providers.get_sender_numbers", return_value=["testing"]
     )
@@ -352,6 +359,8 @@ def test_should_send_sms_with_downgraded_content(notify_db_session, mocker):
 def test_send_sms_should_use_service_sms_sender(
     sample_service, sample_template, mocker
 ):
+
+    mocker.patch("app.delivery.send_to_providers.redis_store", return_value=None)
     mocker.patch("app.aws_sns_client.send_sms")
 
     sms_sender = create_service_sms_sender(
@@ -614,6 +623,7 @@ def test_should_update_billable_units_and_status_according_to_research_mode_and_
     sample_template, mocker, research_mode, key_type, billable_units, expected_status
 ):
 
+    mocker.patch("app.delivery.send_to_providers.redis_store", return_value=None)
     mocker.patch(
         "app.delivery.send_to_providers.get_sender_numbers", return_value=["testing"]
     )
@@ -676,6 +686,8 @@ def test_should_set_notification_billable_units_and_reduces_provider_priority_if
 def test_should_send_sms_to_international_providers(
     sample_template, sample_user, mocker
 ):
+
+    mocker.patch("app.delivery.send_to_providers._get_verify_code", return_value=None)
     mocker.patch("app.aws_sns_client.send_sms")
 
     notification_international = create_notification(
@@ -725,6 +737,8 @@ def test_should_send_sms_to_international_providers(
 def test_should_handle_sms_sender_and_prefix_message(
     mocker, sms_sender, prefix_sms, expected_sender, expected_content, notify_db_session
 ):
+
+    mocker.patch("app.delivery.send_to_providers.redis_store", return_value=None)
     mocker.patch("app.aws_sns_client.send_sms")
     service = create_service_with_defined_sms_sender(
         sms_sender_value=sms_sender, prefix_sms=prefix_sms
@@ -781,6 +795,7 @@ def test_send_email_to_provider_uses_reply_to_from_notification(
 
 def test_send_sms_to_provider_should_use_normalised_to(mocker, client, sample_template):
 
+    mocker.patch("app.delivery.send_to_providers._get_verify_code", return_value=None)
     mocker.patch(
         "app.delivery.send_to_providers.get_sender_numbers", return_value=["testing"]
     )
@@ -843,6 +858,7 @@ def test_send_sms_to_provider_should_return_template_if_found_in_redis(
     mocker, client, sample_template
 ):
 
+    mocker.patch("app.delivery.send_to_providers._get_verify_code", return_value=None)
     mocker.patch(
         "app.delivery.send_to_providers.get_sender_numbers", return_value=["testing"]
     )