mirror of
https://github.com/GSA/notifications-admin.git
synced 2026-06-20 13:12:27 -04:00
Don’t allow adding unknown domains to branding lookup
We should make sure we’re not putting typos in the branding list. We can validate what gets entered here against our known list of public-sector domains.
This commit is contained in:
@@ -35,6 +35,7 @@ from app.main.validators import (
|
||||
Blacklist,
|
||||
CsvFileValidator,
|
||||
DoesNotStartWithDoubleZero,
|
||||
KnownGovernmentDomain,
|
||||
LettersNumbersAndFullStopsOnly,
|
||||
NoCommasInPlaceHolders,
|
||||
OnlyGSMCharacters,
|
||||
@@ -722,7 +723,7 @@ class ServicePreviewBranding(StripWhitespaceForm):
|
||||
class ServiceUpdateEmailBranding(StripWhitespaceForm):
|
||||
name = StringField('Name of brand')
|
||||
text = StringField('Text')
|
||||
domain = StringField('Domain')
|
||||
domain = StringField('Domain', validators=[KnownGovernmentDomain()])
|
||||
colour = StringField(
|
||||
'Colour',
|
||||
validators=[
|
||||
|
||||
@@ -11,7 +11,7 @@ from wtforms.validators import Email
|
||||
|
||||
from app import formatted_list
|
||||
from app.main._blacklisted_passwords import blacklisted_passwords
|
||||
from app.utils import Spreadsheet, is_gov_user
|
||||
from app.utils import AgreementInfo, Spreadsheet, is_gov_user
|
||||
|
||||
|
||||
class Blacklist:
|
||||
@@ -102,3 +102,12 @@ class DoesNotStartWithDoubleZero:
|
||||
def __call__(self, form, field):
|
||||
if field.data and field.data.startswith("00"):
|
||||
raise ValidationError(self.message)
|
||||
|
||||
|
||||
class KnownGovernmentDomain:
|
||||
|
||||
message = 'Not a known government domain (you might need to update domains.yml)'
|
||||
|
||||
def __call__(self, form, field):
|
||||
if field.data and AgreementInfo(field.data).owner is None:
|
||||
raise ValidationError(self.message)
|
||||
|
||||
@@ -6,7 +6,11 @@ from bs4 import BeautifulSoup
|
||||
from flask import url_for
|
||||
|
||||
from app.main.s3_client import LOGO_LOCATION_STRUCTURE, TEMP_TAG
|
||||
from tests.conftest import mock_get_email_branding, normalize_spaces
|
||||
from tests.conftest import (
|
||||
mock_get_email_branding,
|
||||
normalize_spaces,
|
||||
platform_admin_user,
|
||||
)
|
||||
|
||||
|
||||
def test_email_branding_page_shows_full_branding_list(
|
||||
@@ -91,7 +95,7 @@ def test_create_new_email_branding_without_logo(
|
||||
'colour': '#ff0000',
|
||||
'text': 'new text',
|
||||
'name': 'new name',
|
||||
'domain': 'sample.com',
|
||||
'domain': 'voa.gov.uk',
|
||||
'brand_type': 'org'
|
||||
}
|
||||
|
||||
@@ -116,6 +120,38 @@ def test_create_new_email_branding_without_logo(
|
||||
assert mock_persist.call_args_list == []
|
||||
|
||||
|
||||
def test_cant_create_new_email_branding_with_unknown_domain(
|
||||
client_request,
|
||||
mocker,
|
||||
fake_uuid,
|
||||
mock_create_email_branding
|
||||
):
|
||||
mock_persist = mocker.patch('app.main.views.email_branding.persist_logo')
|
||||
mocker.patch('app.main.views.email_branding.delete_temp_files_created_by')
|
||||
|
||||
client_request.login(platform_admin_user(fake_uuid))
|
||||
page = client_request.post(
|
||||
'.create_email_branding',
|
||||
content_type='multipart/form-data',
|
||||
_data={
|
||||
'logo': None,
|
||||
'colour': '#ff0000',
|
||||
'text': 'new text',
|
||||
'name': 'new name',
|
||||
'domain': 'example.gov.uk',
|
||||
'brand_type': 'org',
|
||||
},
|
||||
_expected_status=200,
|
||||
)
|
||||
|
||||
assert mock_create_email_branding.called is False
|
||||
assert mock_persist.called is False
|
||||
|
||||
assert page.select_one('.error-message').text.strip() == (
|
||||
'Not a known government domain (you might need to update domains.yml)'
|
||||
)
|
||||
|
||||
|
||||
def test_create_new_email_branding_when_branding_saved(
|
||||
logged_in_platform_admin_client,
|
||||
mocker,
|
||||
@@ -130,7 +166,7 @@ def test_create_new_email_branding_when_branding_saved(
|
||||
'colour': '#ff0000',
|
||||
'text': 'new text',
|
||||
'name': 'new name',
|
||||
'domain': 'sample.com',
|
||||
'domain': 'voa.gov.uk',
|
||||
'brand_type': 'org_banner'
|
||||
}
|
||||
|
||||
@@ -229,7 +265,7 @@ def test_update_existing_branding(
|
||||
'colour': '#0000ff',
|
||||
'text': 'new text',
|
||||
'name': 'new name',
|
||||
'domain': 'sample.com',
|
||||
'domain': 'voa.gov.uk',
|
||||
'brand_type': 'both'
|
||||
}
|
||||
|
||||
@@ -341,7 +377,7 @@ def test_colour_regex_validation(
|
||||
'colour': colour_hex,
|
||||
'text': 'new text',
|
||||
'name': 'new name',
|
||||
'domain': 'sample.com',
|
||||
'domain': 'voa.gov.uk',
|
||||
'brand_type': 'org'
|
||||
}
|
||||
|
||||
|
||||
Reference in New Issue
Block a user