From a90dcc918b6184d29788e3744bfeb4adfa9e9cdd Mon Sep 17 00:00:00 2001 From: Ryan Ahearn Date: Thu, 22 Sep 2022 12:33:55 -0400 Subject: [PATCH] Provide s3 credentials for each individual bucket --- app/cloudfoundry_config.py | 6 ++ app/config.py | 12 +++ app/models/contact_list.py | 71 +++++++++------ app/s3_client/__init__.py | 46 ++++++++++ app/s3_client/s3_csv_client.py | 60 +++++-------- app/s3_client/s3_logo_client.py | 35 +++++--- requirements.in | 2 +- requirements.txt | 2 +- tests/app/main/views/test_send.py | 12 +-- .../views/uploads/test_upload_contact_list.py | 89 ++++++++----------- tests/app/s3_client/test_s3_csv_client.py | 4 +- tests/app/s3_client/test_s3_logo_client.py | 11 ++- tests/app/test_cloudfoundry_config.py | 8 +- 13 files changed, 211 insertions(+), 147 deletions(-) diff --git a/app/cloudfoundry_config.py b/app/cloudfoundry_config.py index d63cbb986..53cf61462 100644 --- a/app/cloudfoundry_config.py +++ b/app/cloudfoundry_config.py @@ -20,15 +20,21 @@ def extract_cloudfoundry_config(): vcap_services['s3'], f"notifications-api-csv-upload-bucket-{os.environ['DEPLOY_ENV']}") if bucket_service: os.environ['CSV_UPLOAD_BUCKET_NAME'] = bucket_service['credentials']['bucket'] + os.environ['CSV_UPLOAD_ACCESS_KEY'] = bucket_service['credentials']['access_key_id'] + os.environ['CSV_UPLOAD_SECRET_KEY'] = bucket_service['credentials']['secret_access_key'] # Contact List Bucket Name bucket_service = find_by_service_name( vcap_services['s3'], f"notifications-api-contact-list-bucket-{os.environ['DEPLOY_ENV']}") if bucket_service: os.environ['CONTACT_LIST_BUCKET_NAME'] = bucket_service['credentials']['bucket'] + os.environ['CONTACT_LIST_ACCESS_KEY'] = bucket_service['credentials']['access_key_id'] + os.environ['CONTACT_LIST_SECRET_KEY'] = bucket_service['credentials']['secret_access_key'] # Logo Upload Bucket Name bucket_service = find_by_service_name( vcap_services['s3'], f"notifications-admin-logo-upload-bucket-{os.environ['DEPLOY_ENV']}") if bucket_service: os.environ['LOGO_UPLOAD_BUCKET_NAME'] = bucket_service['credentials']['bucket'] + os.environ['LOGO_UPLOAD_ACCESS_KEY'] = bucket_service['credentials']['access_key_id'] + os.environ['LOGO_UPLOAD_SECRET_KEY'] = bucket_service['credentials']['secret_access_key'] diff --git a/app/config.py b/app/config.py index 005358147..107fe54f6 100644 --- a/app/config.py +++ b/app/config.py @@ -52,13 +52,19 @@ class Config(object): WTF_CSRF_ENABLED = True WTF_CSRF_TIME_LIMIT = None CSV_UPLOAD_BUCKET_NAME = 'local-notifications-csv-upload' + CSV_UPLOAD_ACCESS_KEY = os.environ.get('AWS_ACCESS_KEY_ID') + CSV_UPLOAD_SECRET_KEY = os.environ.get('AWS_SECRET_ACCESS_KEY') CONTACT_LIST_UPLOAD_BUCKET_NAME = 'local-contact-list' + CONTACT_LIST_UPLOAD_ACCESS_KEY = os.environ.get('AWS_ACCESS_KEY_ID') + CONTACT_LIST_UPLOAD_SECRET_KEY = os.environ.get('AWS_SECRET_ACCESS_KEY') ACTIVITY_STATS_LIMIT_DAYS = 7 REPLY_TO_EMAIL_ADDRESS_VALIDATION_TIMEOUT = 45 NOTIFY_ENVIRONMENT = 'development' LOGO_UPLOAD_BUCKET_NAME = 'public-logos-local' + LOGO_UPLOAD_ACCESS_KEY = os.environ.get('AWS_ACCESS_KEY_ID') + LOGO_UPLOAD_SECRET_KEY = os.environ.get('AWS_SECRET_ACCESS_KEY') # MOU_BUCKET_NAME = 'local-mou' # TRANSIENT_UPLOADED_LETTERS = 'local-transient-uploaded-letters' ROUTE_SECRET_KEY_1 = os.environ.get('ROUTE_SECRET_KEY_1', 'dev-route-secret-key-1') @@ -202,10 +208,16 @@ class Live(Config): # buckets CSV_UPLOAD_BUCKET_NAME = os.environ.get( 'CSV_UPLOAD_BUCKET_NAME', 'notifications-prototype-csv-upload') # created in gsa sandbox + CSV_UPLOAD_ACCESS_KEY = os.environ.get('CSV_UPLOAD_ACCESS_KEY') + CSV_UPLOAD_SECRET_KEY = os.environ.get('CSV_UPLOAD_SECRET_KEY') CONTACT_LIST_UPLOAD_BUCKET_NAME = os.environ.get( 'CONTACT_LIST_BUCKET_NAME', 'notifications-prototype-contact-list-upload') # created in gsa sandbox + CONTACT_LIST_UPLOAD_ACCESS_KEY = os.environ.get('CONTACT_LIST_ACCESS_KEY') + CONTACT_LIST_UPLOAD_SECRET_KEY = os.environ.get('CONTACT_LIST_SECRET_KEY') LOGO_UPLOAD_BUCKET_NAME = os.environ.get( 'LOGO_UPLOAD_BUCKET_NAME', 'notifications-prototype-logo-upload') # created in gsa sandbox + LOGO_UPLOAD_ACCESS_KEY = os.environ.get('LOGO_UPLOAD_ACCESS_KEY') + LOGO_UPLOAD_SECRET_KEY = os.environ.get('LOGO_UPLOAD_SECRET_KEY') # MOU_BUCKET_NAME = os.environ.get( # 'MOU_UPLOAD_BUCKET_NAME', 'notifications-prototype-mou') # created in gsa sandbox # TRANSIENT_UPLOADED_LETTERS = 'prototype-transient-uploaded-letters' # not created in gsa sandbox diff --git a/app/models/contact_list.py b/app/models/contact_list.py index 6bd104510..7dec2537c 100644 --- a/app/models/contact_list.py +++ b/app/models/contact_list.py @@ -1,20 +1,23 @@ from functools import partial from os import path +from uuid import uuid4 from flask import abort, current_app from notifications_utils.formatters import strip_all_whitespace from notifications_utils.recipients import RecipientCSV +from notifications_utils.s3 import s3upload as utils_s3upload from werkzeug.utils import cached_property from app.models import JSONModel, ModelList from app.models.job import PaginatedJobsAndScheduledJobs from app.notify_client.contact_list_api_client import contact_list_api_client -from app.s3_client.s3_csv_client import ( - get_csv_metadata, - s3download, - s3upload, - set_metadata_on_csv_upload, +from app.s3_client import ( + get_s3_contents, + get_s3_metadata, + get_s3_object, + set_s3_metadata, ) +from app.s3_client.s3_csv_client import s3upload, set_metadata_on_csv_upload from app.utils.templates import get_sample_template @@ -46,38 +49,52 @@ class ContactList(JSONModel): return current_app.config['CONTACT_LIST_UPLOAD_BUCKET_NAME'] @staticmethod - def upload(service_id, file_dict): - return s3upload( - service_id, - file_dict, - current_app.config['AWS_REGION'], - bucket=ContactList.get_bucket_name(), + def get_access_key(): + return current_app.config['CONTACT_LIST_UPLOAD_ACCESS_KEY'] + + @staticmethod + def get_secret_key(): + return current_app.config['CONTACT_LIST_UPLOAD_SECRET_KEY'] + + @staticmethod + def get_filename(service_id, upload_id): + return f"service-{service_id}-notify/{upload_id}.csv" + + @staticmethod + def get_s3_arguments(service_id, upload_id): + return ( + ContactList.get_bucket_name(), + ContactList.get_filename(service_id, upload_id), + ContactList.get_access_key(), + ContactList.get_secret_key(), ) + @staticmethod + def upload(service_id, file_dict): + upload_id = str(uuid4()) + utils_s3upload( + filedata=file_dict['data'], + region=current_app.config['AWS_REGION'], + bucket_name=ContactList.get_bucket_name(), + file_location=ContactList.get_filename(service_id, upload_id), + access_key=ContactList.get_access_key(), + secret_key=ContactList.get_secret_key(), + ) + return upload_id + @staticmethod def download(service_id, upload_id): - return strip_all_whitespace(s3download( - service_id, - upload_id, - bucket=ContactList.get_bucket_name(), - )) + return strip_all_whitespace( + get_s3_contents( + get_s3_object(*ContactList.get_s3_arguments(service_id, upload_id)))) @staticmethod def set_metadata(service_id, upload_id, **kwargs): - return set_metadata_on_csv_upload( - service_id, - upload_id, - bucket=ContactList.get_bucket_name(), - **kwargs, - ) + return set_s3_metadata(get_s3_object(*ContactList.get_s3_arguments(service_id, upload_id)), **kwargs) @staticmethod def get_metadata(service_id, upload_id): - return get_csv_metadata( - service_id, - upload_id, - bucket=ContactList.get_bucket_name(), - ) + return get_s3_metadata(get_s3_object(*ContactList.get_s3_arguments(service_id, upload_id))) def copy_to_uploads(self): metadata = self.get_metadata(self.service_id, self.id) diff --git a/app/s3_client/__init__.py b/app/s3_client/__init__.py index e69de29bb..da17f1a64 100644 --- a/app/s3_client/__init__.py +++ b/app/s3_client/__init__.py @@ -0,0 +1,46 @@ +import os + +import botocore +from boto3 import Session +from flask import current_app + +default_access_key = os.environ.get('AWS_ACCESS_KEY_ID') +default_secret_key = os.environ.get('AWS_SECRET_ACCESS_KEY') + + +def get_s3_object(bucket_name, filename, access_key=default_access_key, secret_key=default_secret_key): + # To inspect contents: obj.get()['Body'].read().decode('utf-8') + session = Session(aws_access_key_id=access_key, aws_secret_access_key=secret_key) + s3 = session.resource('s3') + obj = s3.Object(bucket_name, filename) + return obj + + +def get_s3_metadata(obj): + try: + return obj.get()['Metadata'] + except botocore.exceptions.ClientError as client_error: + current_app.logger.error(f"Unable to download s3 file {obj.bucket_name}/{obj.key}") + raise client_error + + +def set_s3_metadata(obj, **kwargs): + copy_from_object_result = obj.copy_from( + CopySource=f"{obj.bucket_name}/{obj.key}", + ServerSideEncryption='AES256', + Metadata={ + key: str(value) for key, value in kwargs.items() + }, + MetadataDirective='REPLACE', + ) + return copy_from_object_result + + +def get_s3_contents(obj): + contents = '' + try: + contents = obj.get()['Body'].read().decode('utf-8') + except botocore.exceptions.ClientError as client_error: + current_app.logger.error(f"Unable to download s3 file {obj.bucket_name}/{obj.key}") + raise client_error + return contents diff --git a/app/s3_client/s3_csv_client.py b/app/s3_client/s3_csv_client.py index da12c7dbd..f29c638fe 100644 --- a/app/s3_client/s3_csv_client.py +++ b/app/s3_client/s3_csv_client.py @@ -1,68 +1,52 @@ import uuid -import botocore from flask import current_app from notifications_utils.s3 import s3upload as utils_s3upload -from app.s3_client.s3_logo_client import get_s3_object +from app.s3_client import ( + get_s3_contents, + get_s3_metadata, + get_s3_object, + set_s3_metadata, +) FILE_LOCATION_STRUCTURE = 'service-{}-notify/{}.csv' -def get_csv_location(service_id, upload_id, bucket=None): +def get_csv_location(service_id, upload_id): return ( - bucket or current_app.config['CSV_UPLOAD_BUCKET_NAME'], + current_app.config['CSV_UPLOAD_BUCKET_NAME'], FILE_LOCATION_STRUCTURE.format(service_id, upload_id), + current_app.config['CSV_UPLOAD_ACCESS_KEY'], + current_app.config['CSV_UPLOAD_SECRET_KEY'], ) -def get_csv_upload(service_id, upload_id, bucket=None): - return get_s3_object(*get_csv_location(service_id, upload_id, bucket)) +def get_csv_upload(service_id, upload_id): + return get_s3_object(*get_csv_location(service_id, upload_id)) -def s3upload(service_id, filedata, region, bucket=None): +def s3upload(service_id, filedata, region): upload_id = str(uuid.uuid4()) - bucket_name, file_location = get_csv_location(service_id, upload_id, bucket) + bucket_name, file_location, access_key, secret_key = get_csv_location(service_id, upload_id) utils_s3upload( filedata=filedata['data'], region=region, bucket_name=bucket_name, file_location=file_location, + access_key=access_key, + secret_key=secret_key, ) return upload_id -def s3download(service_id, upload_id, bucket=None): - contents = '' - try: - key = get_csv_upload(service_id, upload_id, bucket) - contents = key.get()['Body'].read().decode('utf-8') - except botocore.exceptions.ClientError as e: - current_app.logger.error("Unable to download s3 file {}".format( - FILE_LOCATION_STRUCTURE.format(service_id, upload_id))) - raise e - return contents +def s3download(service_id, upload_id): + return get_s3_contents(get_csv_upload(service_id, upload_id)) -def set_metadata_on_csv_upload(service_id, upload_id, bucket=None, **kwargs): - copy_from_object_result = get_csv_upload( - service_id, upload_id, bucket=bucket - ).copy_from( - CopySource='{}/{}'.format(*get_csv_location(service_id, upload_id, bucket=bucket)), - ServerSideEncryption='AES256', - Metadata={ - key: str(value) for key, value in kwargs.items() - }, - MetadataDirective='REPLACE', - ) - return copy_from_object_result +def set_metadata_on_csv_upload(service_id, upload_id, **kwargs): + return set_s3_metadata(get_csv_upload(service_id, upload_id), **kwargs) -def get_csv_metadata(service_id, upload_id, bucket=None): - try: - key = get_csv_upload(service_id, upload_id, bucket) - return key.get()['Metadata'] - except botocore.exceptions.ClientError as e: - current_app.logger.error("Unable to download s3 file {}".format( - FILE_LOCATION_STRUCTURE.format(service_id, upload_id))) - raise e +def get_csv_metadata(service_id, upload_id): + return get_s3_metadata(get_csv_upload(service_id, upload_id)) diff --git a/app/s3_client/s3_logo_client.py b/app/s3_client/s3_logo_client.py index d7596b849..53aa9ab7d 100644 --- a/app/s3_client/s3_logo_client.py +++ b/app/s3_client/s3_logo_client.py @@ -1,9 +1,11 @@ import uuid -from boto3 import resource +from boto3 import Session from flask import current_app from notifications_utils.s3 import s3upload as utils_s3upload +from app.s3_client import get_s3_object + TEMP_TAG = 'temp-{user_id}_' EMAIL_LOGO_LOCATION_STRUCTURE = '{temp}{unique_id}-{filename}' LETTER_PREFIX = 'letters/static/images/letter-template/' @@ -11,30 +13,33 @@ LETTER_TEMP_TAG = LETTER_PREFIX + TEMP_TAG LETTER_TEMP_LOGO_LOCATION = 'letters/static/images/letter-template/temp-{user_id}_{unique_id}-{filename}' -def get_s3_object(bucket_name, filename): - # To inspect contents: obj.get()['Body'].read().decode('utf-8') - s3 = resource('s3') - obj = s3.Object(bucket_name, filename) - return obj +def get_logo_location(filename=None): + return ( + current_app.config['LOGO_UPLOAD_BUCKET_NAME'], + filename, + current_app.config['LOGO_UPLOAD_ACCESS_KEY'], + current_app.config['LOGO_UPLOAD_SECRET_KEY'], + ) def delete_s3_object(filename): - bucket_name = current_app.config['LOGO_UPLOAD_BUCKET_NAME'] - get_s3_object(bucket_name, filename).delete() + get_s3_object(*get_logo_location(filename)).delete() def persist_logo(old_name, new_name): if old_name == new_name: return - bucket_name = current_app.config['LOGO_UPLOAD_BUCKET_NAME'] - get_s3_object(bucket_name, new_name).copy_from( + bucket_name, filename, access_key, secret_key = get_logo_location(new_name) + get_s3_object(bucket_name, filename, access_key, secret_key).copy_from( CopySource='{}/{}'.format(bucket_name, old_name)) delete_s3_object(old_name) def get_s3_objects_filter_by_prefix(prefix): bucket_name = current_app.config['LOGO_UPLOAD_BUCKET_NAME'] - s3 = resource('s3') + session = Session(aws_access_key_id=current_app.config['LOGO_UPLOAD_ACCESS_KEY'], + aws_secret_access_key=current_app.config['LOGO_UPLOAD_SECRET_KEY']) + s3 = session.resource('s3') return s3.Bucket(bucket_name).objects.filter(Prefix=prefix) @@ -58,7 +63,9 @@ def upload_email_logo(filename, filedata, region, user_id): region=region, bucket_name=bucket_name, file_location=upload_file_name, - content_type='image/png' + content_type='image/png', + access_key=current_app.config['LOGO_UPLOAD_ACCESS_KEY'], + secret_key=current_app.config['LOGO_UPLOAD_SECRET_KEY'], ) return upload_file_name @@ -76,7 +83,9 @@ def upload_letter_temp_logo(filename, filedata, region, user_id): region=region, bucket_name=bucket_name, file_location=upload_filename, - content_type='image/svg+xml' + content_type='image/svg+xml', + access_key=current_app.config['LOGO_UPLOAD_ACCESS_KEY'], + secret_key=current_app.config['LOGO_UPLOAD_SECRET_KEY'], ) return upload_filename diff --git a/requirements.in b/requirements.in index 4508546d6..e2e7bb205 100644 --- a/requirements.in +++ b/requirements.in @@ -30,7 +30,7 @@ python-dotenv==0.20.0 # PaaS awscli-cwlogs>=1.4,<1.5 itsdangerous==2.1.2 -notifications-utils @ git+https://github.com/GSA/notifications-utils.git +notifications-utils @ git+https://github.com/GSA/notifications-utils.git@s3-credentials govuk-frontend-jinja @ git+https://github.com/alphagov/govuk-frontend-jinja.git@v0.5.8-alpha # gds-metrics requires prometheseus 0.2.0, override that requirement as later versions bring significant performance gains diff --git a/requirements.txt b/requirements.txt index 99f974fdd..adcef5503 100644 --- a/requirements.txt +++ b/requirements.txt @@ -126,7 +126,7 @@ mistune==0.8.4 # via notifications-utils notifications-python-client==6.3.0 # via -r requirements.in -notifications-utils @ git+https://github.com/GSA/notifications-utils.git +notifications-utils @ git+https://github.com/GSA/notifications-utils.git@s3-credentials # via -r requirements.in openpyxl==3.0.10 # via pyexcel-xlsx diff --git a/tests/app/main/views/test_send.py b/tests/app/main/views/test_send.py index 2c95b37e0..982cd3833 100644 --- a/tests/app/main/views/test_send.py +++ b/tests/app/main/views/test_send.py @@ -4368,8 +4368,8 @@ def test_send_from_contact_list( mock_get_contact_list, ): new_uuid = uuid.uuid4() - mock_download = mocker.patch('app.models.contact_list.s3download', return_value='contents') - mock_get_metadata = mocker.patch('app.models.contact_list.get_csv_metadata', return_value={ + mock_download = mocker.patch('app.models.contact_list.get_s3_contents', return_value='contents') + mock_get_metadata = mocker.patch('app.models.contact_list.get_s3_metadata', return_value={ 'example_key': 'example value', }) mock_upload = mocker.patch('app.models.contact_list.s3upload', return_value=new_uuid) @@ -4388,12 +4388,8 @@ def test_send_from_contact_list( contact_list_id=fake_uuid, ) ) - mock_download.assert_called_once_with( - SERVICE_ONE_ID, fake_uuid, bucket='test-contact-list' - ) - mock_get_metadata.assert_called_once_with( - SERVICE_ONE_ID, fake_uuid, bucket='test-contact-list' - ) + mock_download.assert_called_once() + mock_get_metadata.assert_called_once() mock_upload.assert_called_once_with( SERVICE_ONE_ID, {'data': 'contents'}, ANY ) diff --git a/tests/app/main/views/uploads/test_upload_contact_list.py b/tests/app/main/views/uploads/test_upload_contact_list.py index cfd6eef8c..1a58c7453 100644 --- a/tests/app/main/views/uploads/test_upload_contact_list.py +++ b/tests/app/main/views/uploads/test_upload_contact_list.py @@ -7,6 +7,7 @@ from flask import url_for from freezegun import freeze_time from app.formatters import normalize_spaces +from app.s3_client import default_access_key, default_secret_key from tests import contact_list_json from tests.conftest import SERVICE_ONE_ID @@ -186,17 +187,15 @@ def test_upload_csv_file_shows_error_banner( expected_thead, expected_tbody, ): - mock_upload = mocker.patch( - 'app.models.contact_list.s3upload', - return_value=fake_uuid, - ) + mocker.patch('app.models.contact_list.uuid4', return_value=fake_uuid) + mock_upload = mocker.patch('app.models.contact_list.utils_s3upload') mock_download = mocker.patch( - 'app.models.contact_list.s3download', + 'app.models.contact_list.get_s3_contents', return_value=file_contents, ) - mock_set_metadata = mocker.patch('app.models.contact_list.set_metadata_on_csv_upload') + mock_set_metadata = mocker.patch('app.models.contact_list.set_s3_metadata') mock_get_metadata = mocker.patch( - 'app.models.contact_list.get_csv_metadata', + 'app.models.contact_list.get_s3_metadata', return_value={'original_file_name': 'invalid.csv'}, ) @@ -207,27 +206,19 @@ def test_upload_csv_file_shows_error_banner( _follow_redirects=True, ) mock_upload.assert_called_once_with( - SERVICE_ONE_ID, - {'data': '', 'file_name': 'invalid.csv'}, - ANY, - bucket='test-contact-list', + filedata='', + region='us-west-2', + bucket_name='test-contact-list', + file_location=f"service-{SERVICE_ONE_ID}-notify/{fake_uuid}.csv", + access_key=default_access_key, + secret_key=default_secret_key, ) mock_set_metadata.assert_called_once_with( - SERVICE_ONE_ID, - fake_uuid, - bucket='test-contact-list', + ANY, original_file_name='invalid.csv' ) - mock_download.assert_called_once_with( - SERVICE_ONE_ID, - fake_uuid, - bucket='test-contact-list', - ) - mock_get_metadata.assert_called_once_with( - SERVICE_ONE_ID, - fake_uuid, - bucket='test-contact-list', - ) + mock_download.assert_called_once() + mock_get_metadata.assert_called_once() assert normalize_spaces(page.select_one('.banner-dangerous').text) == expected_error @@ -250,12 +241,12 @@ def test_upload_csv_file_shows_error_banner_for_too_many_rows( mock_get_users_by_service, fake_uuid, ): - mocker.patch('app.models.contact_list.s3upload', return_value=fake_uuid) - mocker.patch('app.models.contact_list.set_metadata_on_csv_upload') - mocker.patch('app.models.contact_list.s3download', return_value='\n'.join( + mocker.patch('app.models.contact_list.utils_s3upload', return_value=fake_uuid) + mocker.patch('app.models.contact_list.set_s3_metadata') + mocker.patch('app.models.contact_list.get_s3_contents', return_value='\n'.join( ['phone number'] + (['07700900986'] * 100_001) )) - mocker.patch('app.models.contact_list.get_csv_metadata', + mocker.patch('app.models.contact_list.get_s3_metadata', return_value={'original_file_name': 'invalid.csv'}) page = client_request.post( @@ -299,14 +290,14 @@ def test_upload_csv_file_sanitises_and_truncates_file_name_in_metadata( mock_get_users_by_service, fake_uuid, ): - mocker.patch('app.models.contact_list.s3upload', return_value=fake_uuid) - mock_set_metadata = mocker.patch('app.models.contact_list.set_metadata_on_csv_upload') - mocker.patch('app.models.contact_list.s3download', return_value='\n'.join( + mocker.patch('app.models.contact_list.utils_s3upload', return_value=fake_uuid) + mock_set_metadata = mocker.patch('app.models.contact_list.set_s3_metadata') + mocker.patch('app.models.contact_list.get_s3_contents', return_value='\n'.join( ['phone number'] + (['07700900986'] * 100_001) )) filename = f"😁{'a' * 2000}.csv" - mocker.patch('app.models.contact_list.get_csv_metadata', + mocker.patch('app.models.contact_list.get_s3_metadata', return_value={'original_file_name': filename}) client_request.post( 'main.upload_contact_list', @@ -329,12 +320,12 @@ def test_upload_csv_shows_trial_mode_error( fake_uuid, mocker ): - mocker.patch('app.models.contact_list.s3upload', return_value=fake_uuid) - mocker.patch('app.models.contact_list.s3download', return_value=( + mocker.patch('app.models.contact_list.utils_s3upload', return_value=fake_uuid) + mocker.patch('app.models.contact_list.get_s3_contents', return_value=( 'phone number\n' '07900900321' # Not in team )) - mocker.patch('app.models.contact_list.get_csv_metadata', + mocker.patch('app.models.contact_list.get_s3_metadata', return_value={'original_file_name': 'invalid.csv'}) page = client_request.get( @@ -361,12 +352,12 @@ def test_upload_csv_shows_ok_page( fake_uuid, mocker ): - mocker.patch('app.models.contact_list.s3download', return_value='\n'.join( + mocker.patch('app.models.contact_list.get_s3_contents', return_value='\n'.join( ['email address'] + ['test@example.com'] * 51 )) - mocker.patch('app.models.contact_list.get_csv_metadata', + mocker.patch('app.models.contact_list.get_s3_metadata', return_value={'original_file_name': 'good times.xlsx'}) - mock_metadata_set = mocker.patch('app.models.contact_list.set_metadata_on_csv_upload') + mock_metadata_set = mocker.patch('app.models.contact_list.set_s3_metadata') page = client_request.get( 'main.check_contact_list', @@ -376,9 +367,7 @@ def test_upload_csv_shows_ok_page( ) mock_metadata_set.assert_called_once_with( - SERVICE_ONE_ID, - fake_uuid, - bucket='test-contact-list', + mocker.ANY, row_count=51, original_file_name='good times.xlsx', template_type='email', @@ -418,7 +407,7 @@ def test_save_contact_list( fake_uuid, mock_create_contact_list, ): - mock_get_metadata = mocker.patch('app.models.contact_list.get_csv_metadata', return_value={ + mock_get_metadata = mocker.patch('app.models.contact_list.get_s3_metadata', return_value={ 'row_count': 999, 'valid': True, 'original_file_name': 'example.csv', @@ -434,11 +423,7 @@ def test_save_contact_list( service_id=SERVICE_ONE_ID, ) ) - mock_get_metadata.assert_called_once_with( - SERVICE_ONE_ID, - fake_uuid, - bucket='test-contact-list', - ) + mock_get_metadata.assert_called_once() mock_create_contact_list.assert_called_once_with( service_id=SERVICE_ONE_ID, upload_id=fake_uuid, @@ -454,7 +439,7 @@ def test_cant_save_bad_contact_list( fake_uuid, mock_create_contact_list, ): - mocker.patch('app.models.contact_list.get_csv_metadata', return_value={ + mocker.patch('app.models.contact_list.get_s3_metadata', return_value={ 'row_count': 999, 'valid': False, 'original_file_name': 'example.csv', @@ -492,7 +477,7 @@ def test_view_contact_list( has_jobs=has_jobs ) ) - mocker.patch('app.models.contact_list.s3download', return_value='\n'.join( + mocker.patch('app.models.contact_list.get_s3_contents', return_value='\n'.join( ['email address'] + [ f'test-{i}@example.com' for i in range(51) ] @@ -576,7 +561,7 @@ def test_view_jobs_for_contact_list( 'template_type': 'email', }, ) - mocker.patch('app.models.contact_list.s3download', return_value='\n'.join( + mocker.patch('app.models.contact_list.get_s3_contents', return_value='\n'.join( ['email address'] + ['test@example.com'] * 51 )) page = client_request.get( @@ -656,7 +641,7 @@ def test_download_contact_list( mock_get_contact_list, ): mocker.patch( - 'app.models.contact_list.s3download', + 'app.models.contact_list.get_s3_contents', return_value='phone number\n07900900321' ) response = client_request.get_response( @@ -687,7 +672,7 @@ def test_confirm_delete_contact_list( mock_get_contact_list, ): mocker.patch( - 'app.models.contact_list.s3download', + 'app.models.contact_list.get_s3_contents', return_value='phone number\n07900900321' ) page = client_request.get( diff --git a/tests/app/s3_client/test_s3_csv_client.py b/tests/app/s3_client/test_s3_csv_client.py index 82071124d..1ffb498b0 100644 --- a/tests/app/s3_client/test_s3_csv_client.py +++ b/tests/app/s3_client/test_s3_csv_client.py @@ -4,7 +4,7 @@ from app.s3_client.s3_csv_client import set_metadata_on_csv_upload def test_sets_metadata(client_request, mocker): - mocked_s3_object = Mock() + mocked_s3_object = Mock(bucket_name='test-notifications-csv-upload', key='service-1234-notify/5678.csv') mocked_get_s3_object = mocker.patch( 'app.s3_client.s3_csv_client.get_csv_upload', return_value=mocked_s3_object, @@ -12,7 +12,7 @@ def test_sets_metadata(client_request, mocker): set_metadata_on_csv_upload('1234', '5678', foo='bar', baz=True) - mocked_get_s3_object.assert_called_once_with('1234', '5678', bucket=None) + mocked_get_s3_object.assert_called_once_with('1234', '5678') mocked_s3_object.copy_from.assert_called_once_with( CopySource='test-notifications-csv-upload/service-1234-notify/5678.csv', Metadata={'baz': 'True', 'foo': 'bar'}, diff --git a/tests/app/s3_client/test_s3_logo_client.py b/tests/app/s3_client/test_s3_logo_client.py index 580c5f17a..20ddbe42a 100644 --- a/tests/app/s3_client/test_s3_logo_client.py +++ b/tests/app/s3_client/test_s3_logo_client.py @@ -3,6 +3,7 @@ from unittest.mock import call import pytest +from app.s3_client import default_access_key, default_secret_key from app.s3_client.s3_logo_client import ( EMAIL_LOGO_LOCATION_STRUCTURE, LETTER_TEMP_LOGO_LOCATION, @@ -54,7 +55,9 @@ def test_upload_email_logo_calls_correct_args(client_request, mocker, fake_uuid, region=region, file_location=upload_filename, bucket_name=bucket, - content_type='image/png' + content_type='image/png', + access_key=default_access_key, + secret_key=default_secret_key, ) @@ -70,7 +73,9 @@ def test_upload_letter_temp_logo_calls_correct_args(mocker, fake_uuid, letter_up region=region, bucket_name=bucket, file_location=letter_upload_filename, - content_type='image/svg+xml' + content_type='image/svg+xml', + access_key=default_access_key, + secret_key=default_secret_key, ) assert new_filename == 'letters/static/images/letter-template/temp-{}_test_uuid-test.svg'.format(fake_uuid) @@ -84,7 +89,7 @@ def test_persist_logo(client_request, mocker, fake_uuid, upload_filename): persist_logo(upload_filename, new_filename) - mocked_get_s3_object.assert_called_once_with(bucket, new_filename) + mocked_get_s3_object.assert_called_once_with(bucket, new_filename, default_access_key, default_secret_key) mocked_delete_s3_object.assert_called_once_with(upload_filename) diff --git a/tests/app/test_cloudfoundry_config.py b/tests/app/test_cloudfoundry_config.py index 0ab3bad8c..52f9103a6 100644 --- a/tests/app/test_cloudfoundry_config.py +++ b/tests/app/test_cloudfoundry_config.py @@ -18,13 +18,17 @@ def vcap_services(): { 'name': 'notifications-api-csv-upload-bucket-test', 'credentials': { - 'bucket': 'csv-upload-bucket' + 'access_key_id': 'csv-access', + 'bucket': 'csv-upload-bucket', + 'secret_access_key': 'csv-secret' } }, { 'name': 'notifications-api-contact-list-bucket-test', 'credentials': { - 'bucket': 'contact-list-bucket' + 'access_key_id': 'contact-list-access', + 'bucket': 'contact-list-bucket', + 'secret_access_key': 'contact-list-secret' } } ],