From 4f672cb5dca8ab0b6c5c39515e563c95002bc16d Mon Sep 17 00:00:00 2001 From: Chris Hill-Scott Date: Wed, 26 Jan 2022 14:44:33 +0000 Subject: [PATCH] Make logo CDN domain into simple config MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Having this as a function which does string parsing and manipulation surprised me a bit when I was trying to figure out why something wasn’t working. It’s more in line with the way we do other config like this (for example `ASSET_PATH`) to make it a simple config variable, rather than trying to be clever and guess things based on other config variables. It’s also less code, and is explicit enough that it doesn’t need tests. --- app/__init__.py | 3 +-- app/config.py | 5 +++++ app/main/views/email_branding.py | 5 ++--- app/main/views/index.py | 5 +++-- app/main/views/letter_branding.py | 5 ++--- app/utils/__init__.py | 15 +-------------- tests/app/main/views/test_headers.py | 7 ++++--- tests/app/test_utils.py | 14 +------------- 8 files changed, 19 insertions(+), 40 deletions(-) diff --git a/app/__init__.py b/app/__init__.py index e79f98e74..14569ce0e 100644 --- a/app/__init__.py +++ b/app/__init__.py @@ -129,7 +129,6 @@ from app.url_converters import ( TemplateTypeConverter, TicketTypeConverter, ) -from app.utils import get_logo_cdn_domain login_manager = LoginManager() csrf = CSRFProtect() @@ -372,7 +371,7 @@ def useful_headers_after_request(response): " *.notifications.service.gov.uk {logo_domain} data:;" "frame-src 'self' www.youtube-nocookie.com;".format( asset_domain=current_app.config['ASSET_DOMAIN'], - logo_domain=get_logo_cdn_domain(), + logo_domain=current_app.config['LOGO_CDN_DOMAIN'], ) )) response.headers.add('Link', ( diff --git a/app/config.py b/app/config.py index 487afa31a..5b4e06566 100644 --- a/app/config.py +++ b/app/config.py @@ -101,6 +101,7 @@ class Development(Config): CSV_UPLOAD_BUCKET_NAME = 'development-notifications-csv-upload' CONTACT_LIST_UPLOAD_BUCKET_NAME = 'development-contact-list' LOGO_UPLOAD_BUCKET_NAME = 'public-logos-tools' + LOGO_CDN_DOMAIN = 'static-logos.notify.tools' MOU_BUCKET_NAME = 'notify.tools-mou' TRANSIENT_UPLOADED_LETTERS = 'development-transient-uploaded-letters' PRECOMPILED_ORIGINALS_BACKUP_LETTERS = 'development-letters-precompiled-originals-backup' @@ -125,6 +126,7 @@ class Test(Development): CSV_UPLOAD_BUCKET_NAME = 'test-notifications-csv-upload' 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' TRANSIENT_UPLOADED_LETTERS = 'test-transient-uploaded-letters' PRECOMPILED_ORIGINALS_BACKUP_LETTERS = 'test-letters-precompiled-originals-backup' @@ -145,6 +147,7 @@ class Preview(Config): CSV_UPLOAD_BUCKET_NAME = 'preview-notifications-csv-upload' 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' TRANSIENT_UPLOADED_LETTERS = 'preview-transient-uploaded-letters' PRECOMPILED_ORIGINALS_BACKUP_LETTERS = 'preview-letters-precompiled-originals-backup' @@ -163,6 +166,7 @@ class Staging(Config): CSV_UPLOAD_BUCKET_NAME = 'staging-notifications-csv-upload' 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' TRANSIENT_UPLOADED_LETTERS = 'staging-transient-uploaded-letters' PRECOMPILED_ORIGINALS_BACKUP_LETTERS = 'staging-letters-precompiled-originals-backup' @@ -178,6 +182,7 @@ class Live(Config): CSV_UPLOAD_BUCKET_NAME = 'live-notifications-csv-upload' CONTACT_LIST_UPLOAD_BUCKET_NAME = 'production-contact-list' LOGO_UPLOAD_BUCKET_NAME = 'public-logos-production' + LOGO_CDN_DOMAIN = 'static-logos.notifications.service.gov.uk' MOU_BUCKET_NAME = 'notifications.service.gov.uk-mou' TRANSIENT_UPLOADED_LETTERS = 'production-transient-uploaded-letters' PRECOMPILED_ORIGINALS_BACKUP_LETTERS = 'production-letters-precompiled-originals-backup' diff --git a/app/main/views/email_branding.py b/app/main/views/email_branding.py index c3b5b10ec..16e5adec6 100644 --- a/app/main/views/email_branding.py +++ b/app/main/views/email_branding.py @@ -11,7 +11,6 @@ from app.s3_client.s3_logo_client import ( persist_logo, upload_email_logo, ) -from app.utils import get_logo_cdn_domain from app.utils.user import user_is_platform_admin @@ -78,7 +77,7 @@ def update_email_branding(branding_id, logo=None): 'views/email-branding/manage-branding.html', form=form, email_branding=email_branding, - cdn_url=get_logo_cdn_domain(), + cdn_url=current_app.config['LOGO_CDN_DOMAIN'], logo=logo ) @@ -123,6 +122,6 @@ def create_email_branding(logo=None): return render_template( 'views/email-branding/manage-branding.html', form=form, - cdn_url=get_logo_cdn_domain(), + cdn_url=current_app.config['LOGO_CDN_DOMAIN'], logo=logo ) diff --git a/app/main/views/index.py b/app/main/views/index.py index 83d6399a4..f95c96759 100644 --- a/app/main/views/index.py +++ b/app/main/views/index.py @@ -1,5 +1,6 @@ from flask import ( abort, + current_app, make_response, redirect, render_template, @@ -16,7 +17,7 @@ from app.main.views.sub_navigation_dictionaries import ( features_nav, using_notify_nav, ) -from app.utils import get_logo_cdn_domain, hide_from_search_engines +from app.utils import hide_from_search_engines @main.route('/') @@ -87,7 +88,7 @@ def email_template(): colour = email_branding['colour'] brand_text = email_branding['text'] brand_colour = colour - brand_logo = ('https://{}/{}'.format(get_logo_cdn_domain(), email_branding['logo']) + brand_logo = (f"https://{current_app.config['LOGO_CDN_DOMAIN']}/{email_branding['logo']}" if email_branding['logo'] else None) govuk_banner = branding_type in ['govuk', 'both'] brand_banner = branding_type == 'org_banner' diff --git a/app/main/views/letter_branding.py b/app/main/views/letter_branding.py index 571581ec9..08f0332eb 100644 --- a/app/main/views/letter_branding.py +++ b/app/main/views/letter_branding.py @@ -25,7 +25,6 @@ from app.s3_client.s3_logo_client import ( persist_logo, upload_letter_temp_logo, ) -from app.utils import get_logo_cdn_domain from app.utils.user import user_is_platform_admin @@ -113,7 +112,7 @@ def update_letter_branding(branding_id, logo=None): 'views/letter-branding/manage-letter-branding.html', file_upload_form=file_upload_form, letter_branding_details_form=letter_branding_details_form, - cdn_url=get_logo_cdn_domain(), + cdn_url=current_app.config['LOGO_CDN_DOMAIN'], logo=logo, is_update=True ) @@ -169,7 +168,7 @@ def create_letter_branding(logo=None): 'views/letter-branding/manage-letter-branding.html', file_upload_form=file_upload_form, letter_branding_details_form=letter_branding_details_form, - cdn_url=get_logo_cdn_domain(), + cdn_url=current_app.config['LOGO_CDN_DOMAIN'], logo=logo ) diff --git a/app/utils/__init__.py b/app/utils/__init__.py index 6d1f3dafb..d2eff2b9a 100644 --- a/app/utils/__init__.py +++ b/app/utils/__init__.py @@ -1,8 +1,7 @@ from functools import wraps from itertools import chain -from urllib.parse import urlparse -from flask import abort, current_app, g, make_response, request +from flask import abort, g, make_response, request from flask_login import current_user from notifications_utils.field import Field from orderedset._orderedset import OrderedSet @@ -36,18 +35,6 @@ def get_help_argument(): return request.args.get('help') if request.args.get('help') in ('1', '2', '3') else None -def get_logo_cdn_domain(): - parsed_uri = urlparse(current_app.config['ADMIN_BASE_URL']) - - if parsed_uri.netloc.startswith('localhost'): - return 'static-logos.notify.tools' - - subdomain = parsed_uri.hostname.split('.')[0] - domain = parsed_uri.netloc[len(subdomain + '.'):] - - return "static-logos.{}".format(domain) - - def parse_filter_args(filter_dict): if not isinstance(filter_dict, MultiDict): filter_dict = MultiDict(filter_dict) diff --git a/tests/app/main/views/test_headers.py b/tests/app/main/views/test_headers.py index 16acf2ca3..d43e7efc9 100644 --- a/tests/app/main/views/test_headers.py +++ b/tests/app/main/views/test_headers.py @@ -6,8 +6,6 @@ def test_owasp_useful_headers_set( mock_get_service_and_organisation_counts, ): client_request.logout() - mocker.patch('app.get_logo_cdn_domain', return_value='static-logos.test.com') - response = client_request.get_response('.index') assert response.headers['X-Frame-Options'] == 'deny' @@ -36,7 +34,10 @@ def test_headers_non_ascii_characters_are_replaced( mock_get_service_and_organisation_counts, ): client_request.logout() - mocker.patch('app.get_logo_cdn_domain', return_value='static-logos€æ.test.com') + mocker.patch.dict( + 'app.current_app.config', + values={'LOGO_CDN_DOMAIN': 'static-logos€æ.test.com'}, + ) response = client_request.get_response('.index') diff --git a/tests/app/test_utils.py b/tests/app/test_utils.py index 54dbe17e5..9fa05118b 100644 --- a/tests/app/test_utils.py +++ b/tests/app/test_utils.py @@ -1,18 +1,6 @@ import pytest -from app.utils import get_logo_cdn_domain, merge_jsonlike - - -def test_get_cdn_domain_on_localhost(client_request, mocker): - mocker.patch.dict('app.current_app.config', values={'ADMIN_BASE_URL': 'http://localhost:6012'}) - domain = get_logo_cdn_domain() - assert domain == 'static-logos.notify.tools' - - -def test_get_cdn_domain_on_non_localhost(client_request, mocker): - mocker.patch.dict('app.current_app.config', values={'ADMIN_BASE_URL': 'https://some.admintest.com'}) - domain = get_logo_cdn_domain() - assert domain == 'static-logos.admintest.com' +from app.utils import merge_jsonlike @pytest.mark.parametrize("source_object, destination_object, expected_result", [