Skip to content

Commit

Permalink
Merge pull request #1279 from GSA/notify-admin-1870
Browse files Browse the repository at this point in the history
Change phone number in Notify.gov w/o entering password
  • Loading branch information
ccostino authored Aug 20, 2024
2 parents 791a4cc + 6a7717c commit d144cae
Show file tree
Hide file tree
Showing 4 changed files with 29 additions and 18 deletions.
1 change: 0 additions & 1 deletion app/clients/sms/aws_sns.py
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,6 @@ def _valid_sender_number(self, sender):

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(
Expand Down
29 changes: 13 additions & 16 deletions app/delivery/send_to_providers.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import json
from contextlib import suppress
from urllib import parse

from cachetools import TTLCache, cached
Expand Down Expand Up @@ -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:
Expand Down Expand Up @@ -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 = []
Expand Down
1 change: 0 additions & 1 deletion app/user/rest.py
Original file line number Diff line number Diff line change
Expand Up @@ -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}

Expand Down
16 changes: 16 additions & 0 deletions tests/app/delivery/test_send_to_providers.py
Original file line number Diff line number Diff line change
Expand Up @@ -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={},
Expand Down Expand Up @@ -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 = "[email protected]".encode("utf-8")
mock_redis.get.return_value = utf8_encoded_email
Expand Down Expand Up @@ -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",
Expand Down Expand Up @@ -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"]
)
Expand Down Expand Up @@ -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(
Expand Down Expand Up @@ -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"]
)
Expand Down Expand Up @@ -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(
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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"]
)
Expand Down Expand Up @@ -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"]
)
Expand Down

0 comments on commit d144cae

Please sign in to comment.