From 2bcae2044163d6a359ed5462a18d9d15500b915b Mon Sep 17 00:00:00 2001 From: Jim Moffet Date: Sat, 25 Jun 2022 13:05:10 -0700 Subject: [PATCH] initial sms provider cleanup --- .gitignore | 1 + app/authentication/auth.py | 1 + app/clients/__init__.py | 4 +++- app/clients/sms/aws_sns.py | 5 ++++- app/config.py | 14 +++++++------- app/delivery/send_to_providers.py | 13 ++++++++----- .../versions/0076_add_intl_flag_to_provider.py | 4 ++-- 7 files changed, 26 insertions(+), 16 deletions(-) diff --git a/.gitignore b/.gitignore index b5c8d94af..039fe0c7c 100644 --- a/.gitignore +++ b/.gitignore @@ -76,5 +76,6 @@ celerybeat-schedule # CloudFoundry .cf +varsfile* /scripts/run_my_tests.sh diff --git a/app/authentication/auth.py b/app/authentication/auth.py index 4c4b88596..d9a9a8dfb 100644 --- a/app/authentication/auth.py +++ b/app/authentication/auth.py @@ -72,6 +72,7 @@ def requires_internal_auth(expected_client_id): if expected_client_id not in current_app.config.get('INTERNAL_CLIENT_API_KEYS'): raise TypeError("Unknown client_id for internal auth") + current_app.logger.info('Authorised for internal client_id {}'.format(expected_client_id)) # TODO remove request_helper.check_proxy_header_before_request() auth_token = _get_auth_token(request) client_id = _get_token_issuer(auth_token) diff --git a/app/clients/__init__.py b/app/clients/__init__.py index ecf60204f..4d7abd1fe 100644 --- a/app/clients/__init__.py +++ b/app/clients/__init__.py @@ -1,3 +1,5 @@ +from celery import current_app + class ClientException(Exception): ''' Base Exceptions for sending notifications that fail @@ -36,7 +38,7 @@ class NotificationProviderClients(object): def get_client_by_name_and_type(self, name, notification_type): assert notification_type in ['email', 'sms'] - + if notification_type == 'email': return self.get_email_client(name) diff --git a/app/clients/sms/aws_sns.py b/app/clients/sms/aws_sns.py index ffc306e81..96d593ec0 100644 --- a/app/clients/sms/aws_sns.py +++ b/app/clients/sms/aws_sns.py @@ -20,11 +20,14 @@ class AwsSnsClient(SmsClient): self.current_app = current_app self.statsd_client = statsd_client self.long_code_regex = re.compile(r"^\+1\d{10}$") - + @property def name(self): return 'sns' + def get_name(self): + return 'sns' + def send_sms(self, to, content, reference, sender=None, international=False): matched = False diff --git a/app/config.py b/app/config.py index 00dc33d15..ded70e18a 100644 --- a/app/config.py +++ b/app/config.py @@ -92,7 +92,7 @@ class Config(object): GOVUK_ALERTS_CLIENT_ID = 'govuk-alerts' INTERNAL_CLIENT_API_KEYS = json.loads( - os.environ.get('INTERNAL_CLIENT_API_KEYS', '{}') + os.environ.get('INTERNAL_CLIENT_API_KEYS', '{"notify-admin":["dev-notify-secret-key"]}') ) # encyption secret/salt @@ -374,8 +374,8 @@ class Config(object): FIRETEXT_INBOUND_SMS_AUTH = json.loads(os.environ.get('FIRETEXT_INBOUND_SMS_AUTH', '[]')) MMG_INBOUND_SMS_AUTH = json.loads(os.environ.get('MMG_INBOUND_SMS_AUTH', '[]')) MMG_INBOUND_SMS_USERNAME = json.loads(os.environ.get('MMG_INBOUND_SMS_USERNAME', '[]')) - ROUTE_SECRET_KEY_1 = os.environ.get('ROUTE_SECRET_KEY_1', '') - ROUTE_SECRET_KEY_2 = os.environ.get('ROUTE_SECRET_KEY_2', '') + ROUTE_SECRET_KEY_1 = os.environ.get('ROUTE_SECRET_KEY_1', 'dev-route-secret-key-1') + ROUTE_SECRET_KEY_2 = os.environ.get('ROUTE_SECRET_KEY_2', 'dev-route-secret-key-2') HIGH_VOLUME_SERVICE = json.loads(os.environ.get('HIGH_VOLUME_SERVICE', '[]')) @@ -421,10 +421,10 @@ class Development(Config): TRANSIENT_UPLOADED_LETTERS = 'development-transient-uploaded-letters' LETTER_SANITISE_BUCKET_NAME = 'development-letters-sanitise' - INTERNAL_CLIENT_API_KEYS = { - Config.ADMIN_CLIENT_ID: ['dev-notify-secret-key'], - Config.GOVUK_ALERTS_CLIENT_ID: ['govuk-alerts-secret-key'] - } + # INTERNAL_CLIENT_API_KEYS = { + # Config.ADMIN_CLIENT_ID: ['dev-notify-secret-key'], + # Config.GOVUK_ALERTS_CLIENT_ID: ['govuk-alerts-secret-key'] + # } SECRET_KEY = 'dev-notify-secret-key' DANGEROUS_SALT = 'dev-notify-salt' diff --git a/app/delivery/send_to_providers.py b/app/delivery/send_to_providers.py index 0dbb66f9e..84e3f0f04 100644 --- a/app/delivery/send_to_providers.py +++ b/app/delivery/send_to_providers.py @@ -45,6 +45,9 @@ def send_sms_to_provider(notification): if notification.status == 'created': provider = provider_to_use(SMS_TYPE, notification.international) + if not provider: + technical_failure(notification=notification) + return template_model = SerialisedTemplate.from_id_and_service_id( template_id=notification.template_id, service_id=service.id, version=notification.template_version @@ -169,9 +172,10 @@ provider_cache = TTLCache(maxsize=8, ttl=10) @cached(cache=provider_cache) def provider_to_use(notification_type, international=True): - international = True # TODO: remove or resolve the functionality of this + international = False # TODO: remove or resolve the functionality of this flag + # TODO rip firetext and mmg out of early migrations and clean up the expression below active_providers = [ - p for p in get_provider_details_by_notification_type(notification_type, international) if p.active + p for p in get_provider_details_by_notification_type(notification_type, international) if p.active and p.identifier not in ['firetext','mmg'] ] if not active_providers: @@ -181,11 +185,10 @@ def provider_to_use(notification_type, international=True): raise Exception("No active {} providers".format(notification_type)) if len(active_providers) == 1: - weights = [100] + chosen_provider = active_providers[0] else: weights = [p.priority for p in active_providers] - - chosen_provider = random.choices(active_providers, weights=weights)[0] + chosen_provider = random.choices(active_providers, weights=weights)[0] return notification_provider_clients.get_client_by_name_and_type(chosen_provider.identifier, notification_type) diff --git a/migrations/versions/0076_add_intl_flag_to_provider.py b/migrations/versions/0076_add_intl_flag_to_provider.py index 921bb85dd..619f4189b 100644 --- a/migrations/versions/0076_add_intl_flag_to_provider.py +++ b/migrations/versions/0076_add_intl_flag_to_provider.py @@ -18,8 +18,8 @@ def upgrade(): op.add_column('provider_details', sa.Column('supports_international', sa.Boolean(), nullable=False, server_default=sa.false())) op.add_column('provider_details_history', sa.Column('supports_international', sa.Boolean(), nullable=False, server_default=sa.false())) - op.execute("UPDATE provider_details SET supports_international=True WHERE identifier='mmg'") - op.execute("UPDATE provider_details_history SET supports_international=True WHERE identifier='mmg'") + op.execute("UPDATE provider_details SET supports_international=True WHERE identifier='sns'") + op.execute("UPDATE provider_details_history SET supports_international=True WHERE identifier='sns'") def downgrade():