From 271cf7643da35357fd78de98986474857a67a98d Mon Sep 17 00:00:00 2001 From: Ryan Ahearn Date: Mon, 19 Sep 2022 11:19:38 -0400 Subject: [PATCH 1/4] Connect to s3 buckets created by notifications-api --- app/cloudfoundry_config.py | 21 ++++++++++ app/config.py | 39 ++++++++++--------- manifest.yml | 2 + .../s3_client/test_s3_letter_upload_client.py | 2 + tests/app/test_cloudfoundry_config.py | 17 ++++++++ tests/app/test_config.py | 22 +++-------- 6 files changed, 68 insertions(+), 35 deletions(-) diff --git a/app/cloudfoundry_config.py b/app/cloudfoundry_config.py index f493738a5..4716603f7 100644 --- a/app/cloudfoundry_config.py +++ b/app/cloudfoundry_config.py @@ -2,6 +2,27 @@ import json import os +def find_by_service_name(services, service_name): + for i in range(len(services)): + if services[i]['name'] == service_name: + return services[i] + return None + + def extract_cloudfoundry_config(): vcap_services = json.loads(os.environ['VCAP_SERVICES']) + + # Redis config os.environ['REDIS_URL'] = vcap_services['aws-elasticache-redis'][0]['credentials']['uri'].replace('redis', 'rediss') + + # CSV Upload Bucket Name + csv_bucket_service = find_by_service_name( + vcap_services['s3'], f"notifications-api-csv-upload-bucket-{os.environ['DEPLOY_ENV']}") + if csv_bucket_service: + os.environ['CSV_UPLOAD_BUCKET_NAME'] = csv_bucket_service['credentials']['bucket'] + + # Contact List Bucket Name + contact_bucket_service = find_by_service_name( + vcap_services['s3'], f"notifications-api-contact-list-bucket-{os.environ['DEPLOY_ENV']}") + if contact_bucket_service: + os.environ['CONTACT_LIST_BUCKET_NAME'] = contact_bucket_service['credentials']['bucket'] diff --git a/app/config.py b/app/config.py index f3f034c85..6c0f74c40 100644 --- a/app/config.py +++ b/app/config.py @@ -1,13 +1,11 @@ import json import os -if os.environ.get('VCAP_APPLICATION'): - # on cloudfoundry, config is a json blob in VCAP_APPLICATION - unpack it, and populate +if os.environ.get('VCAP_SERVICES'): + # on cloudfoundry, config is a json blob in VCAP_SERVICES - unpack it, and populate # standard environment variables from it - # from app.cloudfoundry_config import extract_cloudfoundry_config - # extract_cloudfoundry_config() - vcap_services = json.loads(os.environ['VCAP_SERVICES']) - os.environ['REDIS_URL'] = vcap_services['aws-elasticache-redis'][0]['credentials']['uri'].replace("redis", "rediss") + from app.cloudfoundry_config import extract_cloudfoundry_config + extract_cloudfoundry_config() class Config(object): @@ -62,7 +60,7 @@ class Config(object): NOTIFY_ENVIRONMENT = 'development' LOGO_UPLOAD_BUCKET_NAME = 'public-logos-local' MOU_BUCKET_NAME = 'local-mou' - TRANSIENT_UPLOADED_LETTERS = 'local-transient-uploaded-letters' + # TRANSIENT_UPLOADED_LETTERS = 'local-transient-uploaded-letters' 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') CHECK_PROXY_HEADER = False @@ -111,8 +109,9 @@ class Development(Config): CONTACT_LIST_UPLOAD_BUCKET_NAME = 'local-contact-list' # created in gsa sandbox LOGO_UPLOAD_BUCKET_NAME = 'local-public-logos-tools' # created in gsa sandbox MOU_BUCKET_NAME = 'local-notify-tools-mou' # created in gsa sandbox - TRANSIENT_UPLOADED_LETTERS = 'development-transient-uploaded-letters' # not created in gsa sandbox - PRECOMPILED_ORIGINALS_BACKUP_LETTERS = 'development-letters-precompiled-originals-backup' # not created in sandbox + # TRANSIENT_UPLOADED_LETTERS = 'development-transient-uploaded-letters' # not created in gsa sandbox + # PRECOMPILED_ORIGINALS_BACKUP_LETTERS = + # 'development-letters-precompiled-originals-backup' # not created in sandbox ADMIN_CLIENT_SECRET = os.environ.get('ADMIN_CLIENT_SECRET') # check for local compose orchestration variable @@ -137,8 +136,8 @@ class Test(Development): LOGO_UPLOAD_BUCKET_NAME = 'public-logos-test' LOGO_CDN_DOMAIN = 'static-logos.test.com' MOU_BUCKET_NAME = 'test-mou' - TRANSIENT_UPLOADED_LETTERS = 'test-transient-uploaded-letters' - PRECOMPILED_ORIGINALS_BACKUP_LETTERS = 'test-letters-precompiled-originals-backup' + # TRANSIENT_UPLOADED_LETTERS = 'test-transient-uploaded-letters' + # PRECOMPILED_ORIGINALS_BACKUP_LETTERS = 'test-letters-precompiled-originals-backup' NOTIFY_ENVIRONMENT = 'test' API_HOST_NAME = 'http://you-forgot-to-mock-an-api-call-to' REDIS_URL = 'redis://you-forgot-to-mock-a-redis-call-to' @@ -168,8 +167,8 @@ class Preview(Config): LOGO_UPLOAD_BUCKET_NAME = 'public-logos-preview' LOGO_CDN_DOMAIN = 'static-logos.notify.works' MOU_BUCKET_NAME = 'notify.works-mou' - TRANSIENT_UPLOADED_LETTERS = 'preview-transient-uploaded-letters' - PRECOMPILED_ORIGINALS_BACKUP_LETTERS = 'preview-letters-precompiled-originals-backup' + # TRANSIENT_UPLOADED_LETTERS = 'preview-transient-uploaded-letters' + # PRECOMPILED_ORIGINALS_BACKUP_LETTERS = 'preview-letters-precompiled-originals-backup' NOTIFY_ENVIRONMENT = 'preview' CHECK_PROXY_HEADER = False ASSET_DOMAIN = 'static.notify.works' @@ -188,8 +187,8 @@ class Staging(Config): LOGO_UPLOAD_BUCKET_NAME = 'public-logos-staging' LOGO_CDN_DOMAIN = 'static-logos.staging-notify.works' MOU_BUCKET_NAME = 'staging-notify.works-mou' - TRANSIENT_UPLOADED_LETTERS = 'staging-transient-uploaded-letters' - PRECOMPILED_ORIGINALS_BACKUP_LETTERS = 'staging-letters-precompiled-originals-backup' + # TRANSIENT_UPLOADED_LETTERS = 'staging-transient-uploaded-letters' + # PRECOMPILED_ORIGINALS_BACKUP_LETTERS = 'staging-letters-precompiled-originals-backup' NOTIFY_ENVIRONMENT = 'staging' CHECK_PROXY_HEADER = False ASSET_DOMAIN = 'static.staging-notify.works' @@ -201,12 +200,14 @@ class Live(Config): HEADER_COLOUR = '#005EA5' # $govuk-blue HTTP_PROTOCOL = 'https' # buckets - CSV_UPLOAD_BUCKET_NAME = 'notifications-prototype-csv-upload' # created in gsa sandbox - CONTACT_LIST_UPLOAD_BUCKET_NAME = 'notifications-prototype-contact-list-upload' # created in gsa sandbox + CSV_UPLOAD_BUCKET_NAME = os.environ.get( + 'CSV_UPLOAD_BUCKET_NAME', 'notifications-prototype-csv-upload') # created in gsa sandbox + CONTACT_LIST_UPLOAD_BUCKET_NAME = os.environ.get( + 'CONTACT_LIST_BUCKET_NAME', 'notifications-prototype-contact-list-upload') # created in gsa sandbox LOGO_UPLOAD_BUCKET_NAME = 'notifications-prototype-logo-upload' # created in gsa sandbox MOU_BUCKET_NAME = 'notifications-prototype-mou' # created in gsa sandbox - TRANSIENT_UPLOADED_LETTERS = 'prototype-transient-uploaded-letters' # not created in gsa sandbox - PRECOMPILED_ORIGINALS_BACKUP_LETTERS = 'prototype-letters-precompiled-originals-backup' # not in sandbox + # TRANSIENT_UPLOADED_LETTERS = 'prototype-transient-uploaded-letters' # not created in gsa sandbox + # PRECOMPILED_ORIGINALS_BACKUP_LETTERS = 'prototype-letters-precompiled-originals-backup' # not in sandbox NOTIFY_ENVIRONMENT = 'live' CHECK_PROXY_HEADER = False diff --git a/manifest.yml b/manifest.yml index b0e658864..af997762d 100644 --- a/manifest.yml +++ b/manifest.yml @@ -14,6 +14,8 @@ applications: # - notify-prometheus # - notify-splunk - notifications-admin-redis-((env)) + - notifications-api-csv-upload-bucket-((env)) + - notifications-api-contact-list-bucket-((env)) env: NOTIFY_APP_NAME: admin diff --git a/tests/app/s3_client/test_s3_letter_upload_client.py b/tests/app/s3_client/test_s3_letter_upload_client.py index e65327572..5934576e6 100644 --- a/tests/app/s3_client/test_s3_letter_upload_client.py +++ b/tests/app/s3_client/test_s3_letter_upload_client.py @@ -15,6 +15,8 @@ from app.s3_client.s3_letter_upload_client import ( upload_letter_to_s3, ) +pytest.skip(reason="Skipping letter-related features", allow_module_level=True) + def test_backup_original_letter_to_s3(mocker, notify_admin): s3_mock = mocker.patch('app.s3_client.s3_letter_upload_client.utils_s3upload') diff --git a/tests/app/test_cloudfoundry_config.py b/tests/app/test_cloudfoundry_config.py index c7ac6f3e9..0ab3bad8c 100644 --- a/tests/app/test_cloudfoundry_config.py +++ b/tests/app/test_cloudfoundry_config.py @@ -14,11 +14,28 @@ def vcap_services(): 'uri': 'redis://xxx:6379' } }], + 's3': [ + { + 'name': 'notifications-api-csv-upload-bucket-test', + 'credentials': { + 'bucket': 'csv-upload-bucket' + } + }, + { + 'name': 'notifications-api-contact-list-bucket-test', + 'credentials': { + 'bucket': 'contact-list-bucket' + } + } + ], } def test_extract_cloudfoundry_config_populates_other_vars(os_environ, vcap_services): + os.environ['DEPLOY_ENV'] = 'test' os.environ['VCAP_SERVICES'] = json.dumps(vcap_services) extract_cloudfoundry_config() assert os.environ['REDIS_URL'] == 'rediss://xxx:6379' + assert os.environ['CSV_UPLOAD_BUCKET_NAME'] == 'csv-upload-bucket' + assert os.environ['CONTACT_LIST_BUCKET_NAME'] == 'contact-list-bucket' diff --git a/tests/app/test_config.py b/tests/app/test_config.py index 8bfa429eb..152dd5f35 100644 --- a/tests/app/test_config.py +++ b/tests/app/test_config.py @@ -1,5 +1,4 @@ import importlib -import json import os from unittest import mock @@ -9,7 +8,7 @@ from app import config def cf_conf(): - os.environ['API_HOST_NAME'] = 'cf' + os.environ['REDIS_URL'] = 'rediss://xxx:6379' @pytest.fixture @@ -29,16 +28,8 @@ def reload_config(os_environ): def test_load_cloudfoundry_config_if_available(reload_config): - os.environ['API_HOST_NAME'] = 'env' os.environ['REDIS_URL'] = 'some uri' - os.environ['VCAP_APPLICATION'] = 'some json blob' - os.environ['VCAP_SERVICES'] = json.dumps({ - 'aws-elasticache-redis': [{ - 'credentials': { - 'uri': 'redis://xxx:6379' - } - }], - }) + os.environ['VCAP_SERVICES'] = 'some json blob' with mock.patch('app.cloudfoundry_config.extract_cloudfoundry_config', side_effect=cf_conf): # reload config so that its module level code (ie: all of it) is re-instantiated @@ -49,9 +40,8 @@ def test_load_cloudfoundry_config_if_available(reload_config): def test_load_config_if_cloudfoundry_not_available(reload_config): - os.environ['API_HOST_NAME'] = 'env' - - os.environ.pop('VCAP_APPLICATION', None) + os.environ['REDIS_URL'] = 'redis://xxx:6379' + os.environ.pop('VCAP_SERVICES', None) with mock.patch('app.cloudfoundry_config.extract_cloudfoundry_config') as cf_config: # reload config so that its module level code (ie: all of it) is re-instantiated @@ -59,5 +49,5 @@ def test_load_config_if_cloudfoundry_not_available(reload_config): assert not cf_config.called - assert os.environ['API_HOST_NAME'] == 'env' - assert config.Config.API_HOST_NAME == 'env' + assert os.environ['REDIS_URL'] == 'redis://xxx:6379' + assert config.Config.REDIS_URL == 'redis://xxx:6379' From b823c2d04f34286cc31c75db2d9703a6352b60e3 Mon Sep 17 00:00:00 2001 From: Ryan Ahearn Date: Mon, 19 Sep 2022 12:33:05 -0400 Subject: [PATCH 2/4] Create s3 buckets that aren't shared with api --- app/cloudfoundry_config.py | 24 ++++++++++++++++++------ app/config.py | 5 +++-- manifest.yml | 3 +++ terraform/production/main.tf | 11 +++++++++++ terraform/staging/main.tf | 22 ++++++++++++++++++++++ 5 files changed, 57 insertions(+), 8 deletions(-) diff --git a/app/cloudfoundry_config.py b/app/cloudfoundry_config.py index 4716603f7..8a53e4542 100644 --- a/app/cloudfoundry_config.py +++ b/app/cloudfoundry_config.py @@ -16,13 +16,25 @@ def extract_cloudfoundry_config(): os.environ['REDIS_URL'] = vcap_services['aws-elasticache-redis'][0]['credentials']['uri'].replace('redis', 'rediss') # CSV Upload Bucket Name - csv_bucket_service = find_by_service_name( + bucket_service = find_by_service_name( vcap_services['s3'], f"notifications-api-csv-upload-bucket-{os.environ['DEPLOY_ENV']}") - if csv_bucket_service: - os.environ['CSV_UPLOAD_BUCKET_NAME'] = csv_bucket_service['credentials']['bucket'] + if bucket_service: + os.environ['CSV_UPLOAD_BUCKET_NAME'] = bucket_service['credentials']['bucket'] # Contact List Bucket Name - contact_bucket_service = find_by_service_name( + bucket_service = find_by_service_name( vcap_services['s3'], f"notifications-api-contact-list-bucket-{os.environ['DEPLOY_ENV']}") - if contact_bucket_service: - os.environ['CONTACT_LIST_BUCKET_NAME'] = contact_bucket_service['credentials']['bucket'] + if bucket_service: + os.environ['CONTACT_LIST_BUCKET_NAME'] = bucket_service['credentials']['bucket'] + + # 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'] + + # MOU Upload Bucket Name + bucket_service = find_by_service_name( + vcap_services['s3'], f"notifications-admin-mou-upload-bucket-{os.environ['DEPLOY_ENV']}") + if bucket_service: + os.environ['MOU_UPLOAD_BUCKET_NAME'] = bucket_service['credentials']['bucket'] diff --git a/app/config.py b/app/config.py index 6c0f74c40..2ec7ee8c8 100644 --- a/app/config.py +++ b/app/config.py @@ -204,8 +204,9 @@ class Live(Config): 'CSV_UPLOAD_BUCKET_NAME', 'notifications-prototype-csv-upload') # created in gsa sandbox CONTACT_LIST_UPLOAD_BUCKET_NAME = os.environ.get( 'CONTACT_LIST_BUCKET_NAME', 'notifications-prototype-contact-list-upload') # created in gsa sandbox - LOGO_UPLOAD_BUCKET_NAME = 'notifications-prototype-logo-upload' # created in gsa sandbox - MOU_BUCKET_NAME = 'notifications-prototype-mou' # created in gsa sandbox + LOGO_UPLOAD_BUCKET_NAME = os.environ.get( + 'LOGO_UPLOAD_BUCKET_NAME', 'notifications-prototype-logo-upload') # created in gsa sandbox + 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 # PRECOMPILED_ORIGINALS_BACKUP_LETTERS = 'prototype-letters-precompiled-originals-backup' # not in sandbox diff --git a/manifest.yml b/manifest.yml index af997762d..0d437cc6e 100644 --- a/manifest.yml +++ b/manifest.yml @@ -16,6 +16,8 @@ applications: - notifications-admin-redis-((env)) - notifications-api-csv-upload-bucket-((env)) - notifications-api-contact-list-bucket-((env)) + - notifications-admin-logo-upload-bucket-((env)) + - notifications-admin-mou-upload-bucket-((env)) env: NOTIFY_APP_NAME: admin @@ -23,6 +25,7 @@ applications: NOTIFY_LOG_LEVEL: INFO FLASK_APP: application.py FLASK_ENV: production + DEPLOY_ENV: ((env)) REDIS_ENABLED: ((REDIS_ENABLED)) NOTIFY_ENVIRONMENT: live diff --git a/terraform/production/main.tf b/terraform/production/main.tf index 8f1b5574a..807f4a660 100644 --- a/terraform/production/main.tf +++ b/terraform/production/main.tf @@ -19,6 +19,17 @@ module "redis" { redis_plan_name = "TKTK-production-redis-plan" } +module "logo_upload_bucket" { + source = "github.com/18f/terraform-cloudgov//s3" + + cf_user = var.cf_user + cf_password = var.cf_password + cf_org_name = local.cf_org_name + cf_space_name = local.cf_space_name + recursive_delete = local.recursive_delete + s3_service_name = "${local.app_name}-logo-upload-bucket-${local.env}" +} + ########################################################################### # The following lines need to be commented out for the initial `terraform apply` # It can be re-enabled after: diff --git a/terraform/staging/main.tf b/terraform/staging/main.tf index 7d09ed8c0..73946a736 100644 --- a/terraform/staging/main.tf +++ b/terraform/staging/main.tf @@ -18,3 +18,25 @@ module "redis" { recursive_delete = local.recursive_delete redis_plan_name = "redis-dev" } + +module "logo_upload_bucket" { + source = "github.com/18f/terraform-cloudgov//s3" + + cf_user = var.cf_user + cf_password = var.cf_password + cf_org_name = local.cf_org_name + cf_space_name = local.cf_space_name + recursive_delete = local.recursive_delete + s3_service_name = "${local.app_name}-logo-upload-bucket-${local.env}" +} + +module "mou_upload_bucket" { + source = "github.com/18f/terraform-cloudgov//s3" + + cf_user = var.cf_user + cf_password = var.cf_password + cf_org_name = local.cf_org_name + cf_space_name = local.cf_space_name + recursive_delete = local.recursive_delete + s3_service_name = "${local.app_name}-mou-upload-bucket-${local.env}" +} From 7ce9f8cfcb456012069e30a95ba4f732313f55c1 Mon Sep 17 00:00:00 2001 From: Ryan Ahearn Date: Tue, 20 Sep 2022 13:59:21 -0400 Subject: [PATCH 3/4] Remove MOU bucket, for now --- app/cloudfoundry_config.py | 6 ---- app/config.py | 13 +++---- .../agreement/service-agreement-signed.html | 2 -- .../views/agreement/service-agreement.html | 3 -- .../organisations/organisation/billing.html | 10 ------ manifest.yml | 1 - terraform/staging/main.tf | 11 ------ .../views/organisations/test_organisations.py | 20 +++++------ tests/app/main/views/test_agreement.py | 35 ++++++------------- 9 files changed, 28 insertions(+), 73 deletions(-) diff --git a/app/cloudfoundry_config.py b/app/cloudfoundry_config.py index 8a53e4542..d63cbb986 100644 --- a/app/cloudfoundry_config.py +++ b/app/cloudfoundry_config.py @@ -32,9 +32,3 @@ def extract_cloudfoundry_config(): 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'] - - # MOU Upload Bucket Name - bucket_service = find_by_service_name( - vcap_services['s3'], f"notifications-admin-mou-upload-bucket-{os.environ['DEPLOY_ENV']}") - if bucket_service: - os.environ['MOU_UPLOAD_BUCKET_NAME'] = bucket_service['credentials']['bucket'] diff --git a/app/config.py b/app/config.py index 2ec7ee8c8..005358147 100644 --- a/app/config.py +++ b/app/config.py @@ -59,7 +59,7 @@ class Config(object): NOTIFY_ENVIRONMENT = 'development' LOGO_UPLOAD_BUCKET_NAME = 'public-logos-local' - MOU_BUCKET_NAME = 'local-mou' + # 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') ROUTE_SECRET_KEY_2 = os.environ.get('ROUTE_SECRET_KEY_2', 'dev-route-secret-key-2') @@ -108,7 +108,7 @@ class Development(Config): CSV_UPLOAD_BUCKET_NAME = 'local-notifications-csv-upload' # created in gsa sandbox CONTACT_LIST_UPLOAD_BUCKET_NAME = 'local-contact-list' # created in gsa sandbox LOGO_UPLOAD_BUCKET_NAME = 'local-public-logos-tools' # created in gsa sandbox - MOU_BUCKET_NAME = 'local-notify-tools-mou' # created in gsa sandbox + # MOU_BUCKET_NAME = 'local-notify-tools-mou' # created in gsa sandbox # TRANSIENT_UPLOADED_LETTERS = 'development-transient-uploaded-letters' # not created in gsa sandbox # PRECOMPILED_ORIGINALS_BACKUP_LETTERS = # 'development-letters-precompiled-originals-backup' # not created in sandbox @@ -135,7 +135,7 @@ class Test(Development): CONTACT_LIST_UPLOAD_BUCKET_NAME = 'test-contact-list' LOGO_UPLOAD_BUCKET_NAME = 'public-logos-test' LOGO_CDN_DOMAIN = 'static-logos.test.com' - MOU_BUCKET_NAME = 'test-mou' + # MOU_BUCKET_NAME = 'test-mou' # TRANSIENT_UPLOADED_LETTERS = 'test-transient-uploaded-letters' # PRECOMPILED_ORIGINALS_BACKUP_LETTERS = 'test-letters-precompiled-originals-backup' NOTIFY_ENVIRONMENT = 'test' @@ -166,7 +166,7 @@ class Preview(Config): CONTACT_LIST_UPLOAD_BUCKET_NAME = 'preview-contact-list' LOGO_UPLOAD_BUCKET_NAME = 'public-logos-preview' LOGO_CDN_DOMAIN = 'static-logos.notify.works' - MOU_BUCKET_NAME = 'notify.works-mou' + # MOU_BUCKET_NAME = 'notify.works-mou' # TRANSIENT_UPLOADED_LETTERS = 'preview-transient-uploaded-letters' # PRECOMPILED_ORIGINALS_BACKUP_LETTERS = 'preview-letters-precompiled-originals-backup' NOTIFY_ENVIRONMENT = 'preview' @@ -186,7 +186,7 @@ class Staging(Config): CONTACT_LIST_UPLOAD_BUCKET_NAME = 'staging-contact-list' LOGO_UPLOAD_BUCKET_NAME = 'public-logos-staging' LOGO_CDN_DOMAIN = 'static-logos.staging-notify.works' - MOU_BUCKET_NAME = 'staging-notify.works-mou' + # MOU_BUCKET_NAME = 'staging-notify.works-mou' # TRANSIENT_UPLOADED_LETTERS = 'staging-transient-uploaded-letters' # PRECOMPILED_ORIGINALS_BACKUP_LETTERS = 'staging-letters-precompiled-originals-backup' NOTIFY_ENVIRONMENT = 'staging' @@ -206,7 +206,8 @@ class Live(Config): 'CONTACT_LIST_BUCKET_NAME', 'notifications-prototype-contact-list-upload') # created in gsa sandbox LOGO_UPLOAD_BUCKET_NAME = os.environ.get( 'LOGO_UPLOAD_BUCKET_NAME', 'notifications-prototype-logo-upload') # created in gsa sandbox - MOU_BUCKET_NAME = os.environ.get('MOU_UPLOAD_BUCKET_NAME', 'notifications-prototype-mou') # created in gsa sandbox + # 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 # PRECOMPILED_ORIGINALS_BACKUP_LETTERS = 'prototype-letters-precompiled-originals-backup' # not in sandbox diff --git a/app/templates/views/agreement/service-agreement-signed.html b/app/templates/views/agreement/service-agreement-signed.html index 62ffb7a8b..7ca5b2225 100644 --- a/app/templates/views/agreement/service-agreement-signed.html +++ b/app/templates/views/agreement/service-agreement-signed.html @@ -21,8 +21,6 @@ {{ current_service.organisation.name }} has already accepted the US Notify data sharing and financial agreement.

-

For more information, you can download a copy of the agreement (PDF). -

The agreement is confidential and should not be shared outside your organisation.

diff --git a/app/templates/views/agreement/service-agreement.html b/app/templates/views/agreement/service-agreement.html index 91d762fe3..d9465fd49 100644 --- a/app/templates/views/agreement/service-agreement.html +++ b/app/templates/views/agreement/service-agreement.html @@ -27,9 +27,6 @@

Once accepted, the agreement covers all Notify services from {{ current_service.organisation.name }}.

-

- Download a copy of the data sharing and financial agreement (PDF). -

The agreement is confidential and should not be shared outside your organisation.

diff --git a/app/templates/views/organisations/organisation/billing.html b/app/templates/views/organisations/organisation/billing.html index d48bb9b0e..96f15cb7a 100644 --- a/app/templates/views/organisations/organisation/billing.html +++ b/app/templates/views/organisations/organisation/billing.html @@ -22,20 +22,10 @@ {{ current_org.agreement_signed_on_behalf_of_name or current_org.agreement_signed_by.name }} signed the agreement on behalf of {{ current_org.name}}.

-

- Download the current version of the agreement (PDF) - -

{% elif current_org.agreement_signed %}

{{ current_org.name}} has accepted the US Notify data sharing and financial agreement.

-

- Download the current version of the agreement (PDF) - -

{% elif current_org.agreement_signed is false %}

{{ current_org.name}} needs to accept the US Notify data sharing and financial agreement. diff --git a/manifest.yml b/manifest.yml index 0d437cc6e..2bd475250 100644 --- a/manifest.yml +++ b/manifest.yml @@ -17,7 +17,6 @@ applications: - notifications-api-csv-upload-bucket-((env)) - notifications-api-contact-list-bucket-((env)) - notifications-admin-logo-upload-bucket-((env)) - - notifications-admin-mou-upload-bucket-((env)) env: NOTIFY_APP_NAME: admin diff --git a/terraform/staging/main.tf b/terraform/staging/main.tf index 73946a736..f60fb31bd 100644 --- a/terraform/staging/main.tf +++ b/terraform/staging/main.tf @@ -29,14 +29,3 @@ module "logo_upload_bucket" { recursive_delete = local.recursive_delete s3_service_name = "${local.app_name}-logo-upload-bucket-${local.env}" } - -module "mou_upload_bucket" { - source = "github.com/18f/terraform-cloudgov//s3" - - cf_user = var.cf_user - cf_password = var.cf_password - cf_org_name = local.cf_org_name - cf_space_name = local.cf_space_name - recursive_delete = local.recursive_delete - s3_service_name = "${local.app_name}-mou-upload-bucket-${local.env}" -} diff --git a/tests/app/main/views/organisations/test_organisations.py b/tests/app/main/views/organisations/test_organisations.py index 331d83cd8..598948506 100644 --- a/tests/app/main/views/organisations/test_organisations.py +++ b/tests/app/main/views/organisations/test_organisations.py @@ -1669,7 +1669,7 @@ def test_organisation_billing_page_when_the_agreement_is_signed_by_a_known_perso assert '2.5 of the US Notify data sharing and financial agreement on 20 February 2020' in normalize_spaces( page.text) assert f'{expected_signatory} signed' in page.text - assert page.select_one('main a')['href'] == url_for('.organisation_download_agreement', org_id=ORGANISATION_ID) + # assert page.select_one('main a')['href'] == url_for('.organisation_download_agreement', org_id=ORGANISATION_ID) def test_organisation_billing_page_when_the_agreement_is_signed_by_an_unknown_person( @@ -1690,7 +1690,7 @@ def test_organisation_billing_page_when_the_agreement_is_signed_by_an_unknown_pe assert page.h1.string == 'Billing' assert (f'{organisation_one["name"]} has accepted the US Notify data ' 'sharing and financial agreement.') in page.text - assert page.select_one('main a')['href'] == url_for('.organisation_download_agreement', org_id=ORGANISATION_ID) + # assert page.select_one('main a')['href'] == url_for('.organisation_download_agreement', org_id=ORGANISATION_ID) @pytest.mark.parametrize('agreement_signed, expected_content', [ @@ -1719,14 +1719,14 @@ def test_organisation_billing_page_when_the_agreement_is_not_signed( @pytest.mark.parametrize('crown, expected_status, expected_file_fetched, expected_file_served', ( - ( - True, 200, 'crown.pdf', - 'US Notify data sharing and financial agreement.pdf', - ), - ( - False, 200, 'non-crown.pdf', - 'US Notify data sharing and financial agreement (non-crown).pdf', - ), + # ( + # True, 200, 'crown.pdf', + # 'US Notify data sharing and financial agreement.pdf', + # ), + # ( + # False, 200, 'non-crown.pdf', + # 'US Notify data sharing and financial agreement (non-crown).pdf', + # ), ( None, 404, None, None, diff --git a/tests/app/main/views/test_agreement.py b/tests/app/main/views/test_agreement.py index ef666a921..0744656a3 100644 --- a/tests/app/main/views/test_agreement.py +++ b/tests/app/main/views/test_agreement.py @@ -23,21 +23,12 @@ class MockS3Object(): ( True, True, partial(url_for, 'main.request_to_go_live', service_id=SERVICE_ONE_ID), - [ - ( - ['govuk-link', 'govuk-link--no-visited-state'], - partial(url_for, 'main.service_download_agreement', service_id=SERVICE_ONE_ID), - ), - ] + [] ), ( False, False, partial(url_for, 'main.request_to_go_live', service_id=SERVICE_ONE_ID), [ - ( - ['govuk-link', 'govuk-link--no-visited-state'], - partial(url_for, 'main.service_download_agreement', service_id=SERVICE_ONE_ID), - ), ( ['govuk-button'], partial(url_for, 'main.service_accept_agreement', service_id=SERVICE_ONE_ID), @@ -48,10 +39,6 @@ class MockS3Object(): False, True, partial(url_for, 'main.request_to_go_live', service_id=SERVICE_ONE_ID), [ - ( - ['govuk-link', 'govuk-link--no-visited-state'], - partial(url_for, 'main.service_download_agreement', service_id=SERVICE_ONE_ID), - ), ( ['govuk-button'], partial(url_for, 'main.service_accept_agreement', service_id=SERVICE_ONE_ID), @@ -126,14 +113,14 @@ def test_unknown_gps_and_trusts_are_redirected( @pytest.mark.parametrize('crown, expected_status, expected_file_fetched, expected_file_served', ( - ( - True, 200, 'crown.pdf', - 'US Notify data sharing and financial agreement.pdf', - ), - ( - False, 200, 'non-crown.pdf', - 'US Notify data sharing and financial agreement (non-crown).pdf', - ), + # ( + # True, 200, 'crown.pdf', + # 'US Notify data sharing and financial agreement.pdf', + # ), + # ( + # False, 200, 'non-crown.pdf', + # 'US Notify data sharing and financial agreement (non-crown).pdf', + # ), ( None, 404, None, None, @@ -490,8 +477,8 @@ def test_confirm_agreement_page_persists( 'main.public_download_agreement', )) @pytest.mark.parametrize('variant, expected_status', ( - ('crown', 200), - ('non-crown', 200), + # ('crown', 200), + # ('non-crown', 200), ('foo', 404), )) def test_show_public_agreement_page( From a90dcc918b6184d29788e3744bfeb4adfa9e9cdd Mon Sep 17 00:00:00 2001 From: Ryan Ahearn Date: Thu, 22 Sep 2022 12:33:55 -0400 Subject: [PATCH 4/4] 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' } } ],