From acfe8092fc12cf6334e7f24466937501128913ce Mon Sep 17 00:00:00 2001 From: Alexey Bezhan Date: Fri, 9 Feb 2018 15:03:32 +0000 Subject: [PATCH] Add route secret key header to the API requests Currently requests to the API made from the admin app are going from PaaS admin app to the nginx router ELB, which then routes them back to the api app on PaaS. This makes sense for external requests, but for requests made from the admin app we could skip nginx and go directly to the api PaaS host, which should reduce load on the nginx instances and potentially reduce latency of the api requests. API apps on PaaS are checking the X-Custom-Forwarder header (which is set by nginx on proxy_pass requests) to only allow requests going through the proxy. This adds the custom header to the API client requests, so that they can pass that header check without going through nginx. --- app/notify_client/__init__.py | 7 +++++++ app/notify_client/api_key_api_client.py | 5 ----- app/notify_client/billing_api_client.py | 5 ----- app/notify_client/email_branding_client.py | 5 ----- app/notify_client/events_api_client.py | 5 ----- app/notify_client/inbound_number_client.py | 5 ----- app/notify_client/invite_api_client.py | 5 ++--- app/notify_client/job_api_client.py | 5 ----- app/notify_client/letter_jobs_client.py | 5 ----- app/notify_client/notification_api_client.py | 5 ----- app/notify_client/organisations_api_client.py | 5 ----- app/notify_client/provider_client.py | 5 ----- app/notify_client/service_api_client.py | 5 ----- app/notify_client/status_api_client.py | 5 ----- .../template_statistics_api_client.py | 5 ----- app/notify_client/user_api_client.py | 5 ++--- .../notify_client/test_notify_admin_api_client.py | 14 ++++++++++---- 17 files changed, 21 insertions(+), 75 deletions(-) diff --git a/app/notify_client/__init__.py b/app/notify_client/__init__.py index 504af8c9f..768b5ceda 100644 --- a/app/notify_client/__init__.py +++ b/app/notify_client/__init__.py @@ -12,10 +12,17 @@ def _attach_current_user(data): class NotifyAdminAPIClient(BaseAPIClient): + def init_app(self, app): + self.base_url = app.config['API_HOST_NAME'] + self.service_id = app.config['ADMIN_CLIENT_USER_NAME'] + self.api_key = app.config['ADMIN_CLIENT_SECRET'] + self.route_secret = app.config['ROUTE_SECRET_KEY_1'] + def generate_headers(self, api_token): headers = { "Content-type": "application/json", "Authorization": "Bearer {}".format(api_token), + "X-Custom-Forwarder": self.route_secret, "User-agent": "NOTIFY-API-PYTHON-CLIENT/{}".format(__version__) } return self._add_request_id_header(headers) diff --git a/app/notify_client/api_key_api_client.py b/app/notify_client/api_key_api_client.py index 3052ce838..404f7880d 100644 --- a/app/notify_client/api_key_api_client.py +++ b/app/notify_client/api_key_api_client.py @@ -10,11 +10,6 @@ class ApiKeyApiClient(NotifyAdminAPIClient): def __init__(self): super().__init__("a" * 73, "b") - def init_app(self, app): - self.base_url = app.config['API_HOST_NAME'] - self.service_id = app.config['ADMIN_CLIENT_USER_NAME'] - self.api_key = app.config['ADMIN_CLIENT_SECRET'] - def get_api_keys(self, service_id, key_id=None): if key_id: return self.get(url='/service/{}/api-keys/{}'.format(service_id, key_id)) diff --git a/app/notify_client/billing_api_client.py b/app/notify_client/billing_api_client.py index ab6e76e13..10e0ce948 100644 --- a/app/notify_client/billing_api_client.py +++ b/app/notify_client/billing_api_client.py @@ -7,11 +7,6 @@ class BillingAPIClient(NotifyAdminAPIClient): def __init__(self): super().__init__("a" * 73, "b") - def init_app(self, application): - self.base_url = application.config['API_HOST_NAME'] - self.service_id = application.config['ADMIN_CLIENT_USER_NAME'] - self.api_key = application.config['ADMIN_CLIENT_SECRET'] - def get_billable_units(self, service_id, year): return self.get( '/service/{0}/billing/monthly-usage'.format(service_id), diff --git a/app/notify_client/email_branding_client.py b/app/notify_client/email_branding_client.py index b82981adc..cadafecc5 100644 --- a/app/notify_client/email_branding_client.py +++ b/app/notify_client/email_branding_client.py @@ -6,11 +6,6 @@ class EmailBrandingClient(NotifyAdminAPIClient): def __init__(self): super().__init__("a" * 73, "b") - def init_app(self, app): - self.base_url = app.config['API_HOST_NAME'] - self.service_id = app.config['ADMIN_CLIENT_USER_NAME'] - self.api_key = app.config['ADMIN_CLIENT_SECRET'] - def get_email_branding(self, branding_id): return self.get(url='/email-branding/{}'.format(branding_id)) diff --git a/app/notify_client/events_api_client.py b/app/notify_client/events_api_client.py index 859f22c85..c12b8b721 100644 --- a/app/notify_client/events_api_client.py +++ b/app/notify_client/events_api_client.py @@ -5,11 +5,6 @@ class EventsApiClient(NotifyAdminAPIClient): def __init__(self): super().__init__("a" * 73, "b") - def init_app(self, app): - self.base_url = app.config['API_HOST_NAME'] - self.service_id = app.config['ADMIN_CLIENT_USER_NAME'] - self.api_key = app.config['ADMIN_CLIENT_SECRET'] - def create_event(self, event_type, event_data): data = { 'event_type': event_type, diff --git a/app/notify_client/inbound_number_client.py b/app/notify_client/inbound_number_client.py index 9ece7e746..eeadc7aca 100644 --- a/app/notify_client/inbound_number_client.py +++ b/app/notify_client/inbound_number_client.py @@ -6,11 +6,6 @@ class InboundNumberClient(NotifyAdminAPIClient): def __init__(self): super().__init__("a" * 73, "b") - def init_app(self, app): - self.base_url = app.config['API_HOST_NAME'] - self.service_id = app.config['ADMIN_CLIENT_USER_NAME'] - self.api_key = app.config['ADMIN_CLIENT_SECRET'] - def get_available_inbound_sms_numbers(self): return self.get(url='/inbound-number/available') diff --git a/app/notify_client/invite_api_client.py b/app/notify_client/invite_api_client.py index fab12a8f0..5c3a548ba 100644 --- a/app/notify_client/invite_api_client.py +++ b/app/notify_client/invite_api_client.py @@ -8,10 +8,9 @@ class InviteApiClient(NotifyAdminAPIClient): super().__init__("a" * 73, "b") def init_app(self, app): - self.base_url = app.config['API_HOST_NAME'] + super().init_app(app) + self.admin_url = app.config['ADMIN_BASE_URL'] - self.service_id = app.config['ADMIN_CLIENT_USER_NAME'] - self.api_key = app.config['ADMIN_CLIENT_SECRET'] def create_invite(self, invite_from_id, service_id, email_address, permissions, auth_type): data = { diff --git a/app/notify_client/job_api_client.py b/app/notify_client/job_api_client.py index 2766c53d8..8b4a85563 100644 --- a/app/notify_client/job_api_client.py +++ b/app/notify_client/job_api_client.py @@ -19,11 +19,6 @@ class JobApiClient(NotifyAdminAPIClient): def __init__(self): super().__init__("a" * 73, "b") - def init_app(self, app): - self.base_url = app.config['API_HOST_NAME'] - self.service_id = app.config['ADMIN_CLIENT_USER_NAME'] - self.api_key = app.config['ADMIN_CLIENT_SECRET'] - @staticmethod def __convert_statistics(job): results = defaultdict(int) diff --git a/app/notify_client/letter_jobs_client.py b/app/notify_client/letter_jobs_client.py index 62b2a7c93..efa35e696 100644 --- a/app/notify_client/letter_jobs_client.py +++ b/app/notify_client/letter_jobs_client.py @@ -6,11 +6,6 @@ class LetterJobsClient(NotifyAdminAPIClient): def __init__(self): super().__init__("a" * 73, "b") - def init_app(self, app): - self.base_url = app.config['API_HOST_NAME'] - self.service_id = app.config['ADMIN_CLIENT_USER_NAME'] - self.api_key = app.config['ADMIN_CLIENT_SECRET'] - def get_letter_jobs(self): return self.get(url='/letter-jobs')['data'] diff --git a/app/notify_client/notification_api_client.py b/app/notify_client/notification_api_client.py index bb4140066..9e80af511 100644 --- a/app/notify_client/notification_api_client.py +++ b/app/notify_client/notification_api_client.py @@ -5,11 +5,6 @@ class NotificationApiClient(NotifyAdminAPIClient): def __init__(self): super().__init__("a" * 73, "b") - def init_app(self, app): - self.base_url = app.config['API_HOST_NAME'] - self.service_id = app.config['ADMIN_CLIENT_USER_NAME'] - self.api_key = app.config['ADMIN_CLIENT_SECRET'] - def get_notifications_for_service( self, service_id, diff --git a/app/notify_client/organisations_api_client.py b/app/notify_client/organisations_api_client.py index 4583abaf8..0c4b110fc 100644 --- a/app/notify_client/organisations_api_client.py +++ b/app/notify_client/organisations_api_client.py @@ -6,11 +6,6 @@ class OrganisationsClient(NotifyAdminAPIClient): def __init__(self): super().__init__("a" * 73, "b") - def init_app(self, app): - self.base_url = app.config['API_HOST_NAME'] - self.service_id = app.config['ADMIN_CLIENT_USER_NAME'] - self.api_key = app.config['ADMIN_CLIENT_SECRET'] - def get_organisations(self): return self.get(url='/organisations') diff --git a/app/notify_client/provider_client.py b/app/notify_client/provider_client.py index c1c1cbf8e..8c9975b85 100644 --- a/app/notify_client/provider_client.py +++ b/app/notify_client/provider_client.py @@ -6,11 +6,6 @@ class ProviderClient(NotifyAdminAPIClient): def __init__(self): super().__init__("a" * 73, "b") - def init_app(self, app): - self.base_url = app.config['API_HOST_NAME'] - self.service_id = app.config['ADMIN_CLIENT_USER_NAME'] - self.api_key = app.config['ADMIN_CLIENT_SECRET'] - def get_all_providers(self): return self.get( url='/provider-details' diff --git a/app/notify_client/service_api_client.py b/app/notify_client/service_api_client.py index 7a086c61e..d6119f409 100644 --- a/app/notify_client/service_api_client.py +++ b/app/notify_client/service_api_client.py @@ -12,11 +12,6 @@ class ServiceAPIClient(NotifyAdminAPIClient): def __init__(self): super().__init__("a" * 73, "b") - def init_app(self, application): - self.base_url = application.config['API_HOST_NAME'] - self.service_id = application.config['ADMIN_CLIENT_USER_NAME'] - self.api_key = application.config['ADMIN_CLIENT_SECRET'] - def create_service( self, service_name, diff --git a/app/notify_client/status_api_client.py b/app/notify_client/status_api_client.py index 5f8a88bad..1a69e7281 100644 --- a/app/notify_client/status_api_client.py +++ b/app/notify_client/status_api_client.py @@ -6,10 +6,5 @@ class StatusApiClient(NotifyAdminAPIClient): def __init__(self): super().__init__("a" * 73, "b") - def init_app(self, app): - self.base_url = app.config['API_HOST_NAME'] - self.service_id = app.config['ADMIN_CLIENT_USER_NAME'] - self.api_key = app.config['ADMIN_CLIENT_SECRET'] - def get_status(self, *params): return self.get(url='/_status', *params) diff --git a/app/notify_client/template_statistics_api_client.py b/app/notify_client/template_statistics_api_client.py index e6afb411a..749fdf0d4 100644 --- a/app/notify_client/template_statistics_api_client.py +++ b/app/notify_client/template_statistics_api_client.py @@ -5,11 +5,6 @@ class TemplateStatisticsApiClient(NotifyAdminAPIClient): def __init__(self): super().__init__("a" * 73, "b") - def init_app(self, app): - self.base_url = app.config['API_HOST_NAME'] - self.service_id = app.config['ADMIN_CLIENT_USER_NAME'] - self.api_key = app.config['ADMIN_CLIENT_SECRET'] - def get_template_statistics_for_service(self, service_id, limit_days=None): params = {} if limit_days is not None: diff --git a/app/notify_client/user_api_client.py b/app/notify_client/user_api_client.py index 42bb8268e..8b9e5643a 100644 --- a/app/notify_client/user_api_client.py +++ b/app/notify_client/user_api_client.py @@ -16,9 +16,8 @@ class UserApiClient(NotifyAdminAPIClient): super().__init__("a" * 73, "b") def init_app(self, app): - self.base_url = app.config['API_HOST_NAME'] - self.service_id = app.config['ADMIN_CLIENT_USER_NAME'] - self.api_key = app.config['ADMIN_CLIENT_SECRET'] + super().init_app(app) + self.max_failed_login_count = app.config["MAX_FAILED_LOGIN_COUNT"] self.admin_url = app.config['ADMIN_BASE_URL'] diff --git a/tests/app/notify_client/test_notify_admin_api_client.py b/tests/app/notify_client/test_notify_admin_api_client.py index 398208b9c..112e43ced 100644 --- a/tests/app/notify_client/test_notify_admin_api_client.py +++ b/tests/app/notify_client/test_notify_admin_api_client.py @@ -4,7 +4,7 @@ from unittest.mock import patch import pytest import werkzeug from tests import service_json -from tests.conftest import api_user_active, platform_admin_user +from tests.conftest import api_user_active, platform_admin_user, set_config from app.notify_client import NotifyAdminAPIClient @@ -76,23 +76,29 @@ def test_inactive_service_can_be_modified_by_platform_admin(app_, platform_admin assert ret == request.return_value -def test_generate_headers_sets_standard_headers(): +def test_generate_headers_sets_standard_headers(app_): api_client = NotifyAdminAPIClient(SAMPLE_API_KEY, 'base_url') + with set_config(app_, 'ROUTE_SECRET_KEY_1', 'proxy-secret'): + api_client.init_app(app_) # with patch('app.notify_client.has_request_context', return_value=False): headers = api_client.generate_headers('api_token') - assert set(headers.keys()) == {'Authorization', 'Content-type', 'User-agent'} + assert set(headers.keys()) == {'Authorization', 'Content-type', 'User-agent', 'X-Custom-Forwarder'} assert headers['Authorization'] == 'Bearer api_token' assert headers['Content-type'] == 'application/json' assert headers['User-agent'].startswith('NOTIFY-API-PYTHON-CLIENT') + assert headers['X-Custom-Forwarder'] == 'proxy-secret' def test_generate_headers_sets_request_id_if_in_request_context(app_): api_client = NotifyAdminAPIClient(SAMPLE_API_KEY, 'base_url') + api_client.init_app(app_) with app_.test_request_context() as request_context: headers = api_client.generate_headers('api_token') - assert set(headers.keys()) == {'Authorization', 'Content-type', 'User-agent', 'NotifyRequestID'} + assert set(headers.keys()) == { + 'Authorization', 'Content-type', 'User-agent', 'X-Custom-Forwarder', 'NotifyRequestID' + } assert headers['NotifyRequestID'] == request_context.request.request_id