Restrict email reply-to domains to domains used by current team members

app/main/forms.py

@@ -53,6 +53,7 @@
     ValidCallbackUrl,
     ValidEmail,
     ValidGovEmail,
+    ValidTeamMemberDomain,
     validate_email_from,
     validate_service_name,
 )
@@ -124,7 +125,7 @@ class MultiCheckboxField(SelectMultipleField):
     option_widget = CheckboxInput()
 
 
-def email_address(label=_l("Email address"), gov_user=True, required=True):
+def email_address(label=_l("Email address"), gov_user=True, only_team_member_domains=False, required=True):
     if label == "email address":
         label = _l("email address")
     elif label == "phone number":
@@ -140,6 +141,9 @@ def email_address(label=_l("Email address"), gov_user=True, required=True):
     if required:
         validators.append(DataRequired(message=_l("Enter an email address")))
 
+    if only_team_member_domains:
+        validators.append(ValidTeamMemberDomain())
+
     return EmailField(label, validators, render_kw={"spellcheck": "false"})
 
 
@@ -1103,7 +1107,7 @@ def __init__(self, *args, **kwargs):
 
 class ServiceReplyToEmailForm(StripWhitespaceForm):
     label_text = _l("Reply-to email address")
-    email_address = email_address(label=_l(label_text), gov_user=False)
+    email_address = email_address(label=_l(label_text), only_team_member_domains=True, gov_user=False)
     is_default = BooleanField(_l("Make this email address the default"))
 
 

app/main/validators.py

@@ -62,6 +62,19 @@ def __call__(self, form, field):
             )
 
 
+class ValidTeamMemberDomain:
+    def __call__(self, form, field):
+        if not field.data:
+            return
+        email_domain = field.data.split("@")[-1]
+
+        if email_domain not in g.team_member_email_domains:
+            message = _(
+                "{} is not an email domain used by team members of this service. Only email domains found in your team list can be used as an email reply-to: {}."
+            ).format(email_domain, ", ".join(g.team_member_email_domains))
+            raise ValidationError(message)
+
+
 class ValidGovEmail:
     def __call__(self, form, field):
         if not field.data:

app/main/views/service_settings.py

@@ -5,6 +5,7 @@
     abort,
     current_app,
     flash,
+    g,
     jsonify,
     redirect,
     render_template,
@@ -580,6 +581,7 @@ def service_add_email_reply_to(service_id):
     form = ServiceReplyToEmailForm()
     first_email_address = current_service.count_email_reply_to_addresses == 0
     is_default = first_email_address if first_email_address else form.is_default.data
+    g.team_member_email_domains = set([team_member.email_domain for team_member in current_service.team_members])
     if form.validate_on_submit():
         try:
             notification_id = service_api_client.verify_reply_to_email_address(service_id, form.email_address.data)["data"]["id"]
@@ -696,6 +698,7 @@ def get_service_verify_reply_to_address_partials(service_id, notification_id):
 def service_edit_email_reply_to(service_id, reply_to_email_id):
     form = ServiceReplyToEmailForm()
     reply_to_email_address = current_service.get_email_reply_to_address(reply_to_email_id)
+    g.team_member_email_domains = set([team_member.email_domain for team_member in current_service.team_members])
     if request.method == "GET":
         form.email_address.data = reply_to_email_address["email_address"]
         form.is_default.data = reply_to_email_address["is_default"]

app/translations/csv/fr.csv

@@ -2102,4 +2102,5 @@
 "French Government of Canada signature and custom logo","Signature du gouvernement du Canada en français et logo personnalisé"
 "SVG Images only!","Images SVG uniquement"
 "English Government of Canada signature and custom logo","Signature du gouvernement du Canada en anglais et logo personnalisé"
-"Upload a PNG logo","Téléverser un logo PNG"
+"Upload a PNG logo","Téléverser un logo PNG"
+"{} is not an email domain used by team members of this service. Only email domains found in your team list can be used as an email reply-to: {}.","(FR) - {} is not an email domain used by team members of this service. Only email domains found in your team list can be used as an email reply-to: {}."

tests/__init__.py

@@ -94,6 +94,7 @@ def user_json(
     id_="1234",
     name="Test User",
     email_address="test@canada.ca",
+    email_domain=None,
     mobile_number="+16502532222",
     blocked=False,
     password_changed_at=None,
@@ -123,6 +124,7 @@ def user_json(
         "id": id_,
         "name": name,
         "email_address": email_address,
+        "email_domain": email_address.split("@")[-1],
         "mobile_number": mobile_number,
         "password_changed_at": password_changed_at,
         "permissions": permissions,

tests/app/main/test_validators.py

@@ -1,9 +1,10 @@
 from unittest.mock import Mock
 
 import pytest
+from flask import g
 from wtforms import ValidationError
 
-from app.main.forms import RegisterUserForm, ServiceSmsSenderForm
+from app.main.forms import RegisterUserForm, ServiceSmsSenderForm, ValidTeamMemberDomain
 from app.main.validators import NoCommasInPlaceHolders, OnlySMSCharacters, ValidGovEmail
 
 
@@ -88,6 +89,22 @@ def test_valid_list_of_white_list_email_domains(
     email_domain_validators(None, _gen_mock_field(email))
 
 
+@pytest.mark.parametrize(
+    "email, raises", [("test@team-domain.ca", False), ("test@cds-snc.ca", False), ("test@not-my-team.ca", True)]
+)
+def test_valid_team_only_email_domains(email, app_, raises):
+    with app_.app_context(), app_.test_request_context():
+        g.team_member_email_domains = ["team-domain.ca", "cds-snc.ca"]
+
+        team_only_domain_validator = ValidTeamMemberDomain()
+
+        if raises:
+            with pytest.raises(ValidationError):
+                team_only_domain_validator(None, _gen_mock_field(email))
+        else:
+            team_only_domain_validator(None, _gen_mock_field(email))
+
+
 @pytest.mark.parametrize(
     "email",
     [

tests/app/main/views/test_service_settings.py

@@ -780,7 +780,7 @@ def test_should_raise_duplicate_name_handled(
 
 
 @pytest.mark.parametrize(
-    ("count_of_users_with_manage_service," "count_of_invites_with_manage_service," "expected_user_checklist_item"),
+    ("count_of_users_with_manage_service,count_of_invites_with_manage_service,expected_user_checklist_item"),
     [
         (1, 0, "Add a team member who can manage settings Not completed"),
         (2, 0, "Add a team member who can manage settings Completed"),
@@ -1560,7 +1560,9 @@ def test_no_senders_message_shows(
         ("testtest", "Enter a valid email address"),
     ],
 )
-def test_incorrect_reply_to_email_address_input(reply_to_input, expected_error, client_request, no_reply_to_email_addresses):
+def test_incorrect_reply_to_email_address_input(
+    reply_to_input, expected_error, client_request, mock_team_members, no_reply_to_email_addresses, app_
+):
     page = client_request.post(
         "main.service_add_email_reply_to",
         service_id=SERVICE_ONE_ID,
@@ -1571,6 +1573,24 @@ def test_incorrect_reply_to_email_address_input(reply_to_input, expected_error,
     assert normalize_spaces(page.select_one(".error-message").text) == expected_error
 
 
+def test_incorrect_reply_to_domain_not_in_team_member_list(
+    client_request, mock_team_members, no_reply_to_email_addresses, app_, mocker
+):
+    page = client_request.post(
+        "main.service_add_email_reply_to",
+        service_id=SERVICE_ONE_ID,
+        _data={"email_address": "test@not-team-member-domain.ca"},
+        _expected_status=200,
+    )
+
+    assert (
+        normalize_spaces(page.select_one(".error-message").text)
+        == "not-team-member-domain.ca is not an email domain used by team members of this service. Only email domains found in your team list can be used as an email reply-to: {}.".format(
+            ", ".join(set([member.email_domain for member in mock_team_members]))
+        )
+    )
+
+
 @pytest.mark.parametrize(
     "contact_block_input, expected_error",
     [
@@ -1640,8 +1660,11 @@ def test_incorrect_sms_sender_input(
         (create_multiple_email_reply_to_addresses(), {"is_default": "y"}, True),
     ],
 )
-def test_add_reply_to_email_address_sends_test_notification(mocker, client_request, reply_to_addresses, data, api_default_args):
+def test_add_reply_to_email_address_sends_test_notification(
+    mocker, client_request, reply_to_addresses, mock_team_members, data, api_default_args
+):
     mocker.patch("app.service_api_client.get_reply_to_email_addresses", return_value=reply_to_addresses)
+
     data["email_address"] = "test@example.com"
     mock_verify = mocker.patch(
         "app.service_api_client.verify_reply_to_email_address",
@@ -1856,7 +1879,9 @@ def test_add_sms_sender(sms_senders, data, api_default_args, mocker, client_requ
         ),
     ],
 )
-def test_default_box_doesnt_show_on_first_sender(sender_page, function_to_mock, mocker, data, checkbox_present, client_request):
+def test_default_box_doesnt_show_on_first_sender(
+    sender_page, function_to_mock, mock_team_members, mocker, data, checkbox_present, client_request
+):
     mocker.patch(function_to_mock, side_effect=lambda service_id: data)
 
     page = client_request.get(sender_page, service_id=SERVICE_ONE_ID)
@@ -1877,6 +1902,7 @@ def test_edit_reply_to_email_address_sends_verification_notification_if_address_
     reply_to_address,
     data,
     api_default_args,
+    mock_team_members,
     mocker,
     fake_uuid,
     client_request,
@@ -1886,14 +1912,14 @@ def test_edit_reply_to_email_address_sends_verification_notification_if_address_
         return_value={"data": {"id": "123"}},
     )
     mocker.patch("app.service_api_client.get_reply_to_email_address", return_value=reply_to_address)
-    data["email_address"] = "test@tbs-sct.gc.ca"
+    data["email_address"] = "test123@example.com"
     client_request.post(
         "main.service_edit_email_reply_to",
         service_id=SERVICE_ONE_ID,
         reply_to_email_id=fake_uuid,
         _data=data,
     )
-    mock_verify.assert_called_once_with(SERVICE_ONE_ID, "test@tbs-sct.gc.ca")
+    mock_verify.assert_called_once_with(SERVICE_ONE_ID, "test123@example.com")
 
 
 @pytest.mark.parametrize(
@@ -1912,6 +1938,7 @@ def test_edit_reply_to_email_address_goes_straight_to_update_if_address_not_chan
     mocker,
     fake_uuid,
     client_request,
+    mock_team_members,
     mock_update_reply_to_email_address,
 ):
     mocker.patch("app.service_api_client.get_reply_to_email_address", return_value=reply_to_address)
@@ -1941,6 +1968,7 @@ def test_edit_reply_to_email_address_goes_straight_to_update_if_address_not_chan
 def test_always_shows_delete_link_for_email_reply_to_address(
     mocker: MockerFixture,
     sender_details,
+    mock_team_members,
     fake_uuid,
     client_request,
 ):
@@ -1967,7 +1995,9 @@ def test_always_shows_delete_link_for_email_reply_to_address(
     assert link["href"] == partial_href(service_id=SERVICE_ONE_ID)
 
 
-def test_confirm_delete_reply_to_email_address(fake_uuid, client_request, get_non_default_reply_to_email_address):
+def test_confirm_delete_reply_to_email_address(
+    fake_uuid, client_request, mock_team_members, get_non_default_reply_to_email_address
+):
     page = client_request.get(
         "main.service_confirm_delete_email_reply_to",
         service_id=SERVICE_ONE_ID,
@@ -1976,14 +2006,14 @@ def test_confirm_delete_reply_to_email_address(fake_uuid, client_request, get_no
     )
 
     assert normalize_spaces(page.select_one(".banner-dangerous").text) == (
-        "Are you sure you want to delete this reply-to email address? " "Yes, delete"
+        "Are you sure you want to delete this reply-to email address? Yes, delete"
     )
     assert "action" not in page.select_one(".banner-dangerous form")
     assert page.select_one(".banner-dangerous form")["method"] == "post"
 
 
 def test_confirm_delete_default_reply_to_email_address(
-    mocker: MockerFixture, fake_uuid, client_request, get_default_reply_to_email_address
+    mocker: MockerFixture, fake_uuid, client_request, mock_team_members, get_default_reply_to_email_address
 ):
     reply_tos = create_multiple_email_reply_to_addresses()
     reply_to_for_deletion = reply_tos[1]
@@ -1998,7 +2028,7 @@ def test_confirm_delete_default_reply_to_email_address(
     )
 
     assert normalize_spaces(page.select_one(".banner-dangerous").text) == (
-        "Are you sure you want to delete this reply-to email address? " "Yes, delete"
+        "Are you sure you want to delete this reply-to email address? Yes, delete"
     )
     assert "action" not in page.select_one(".banner-dangerous form")
     assert page.select_one(".banner-dangerous form")["method"] == "post"
@@ -2100,7 +2130,7 @@ def test_confirm_delete_letter_contact_block(
     )
 
     assert normalize_spaces(page.select_one(".banner-dangerous").text) == (
-        "Are you sure you want to delete this contact block? " "Yes, delete"
+        "Are you sure you want to delete this contact block? Yes, delete"
     )
     assert "action" not in page.select_one(".banner-dangerous form")
     assert page.select_one(".banner-dangerous form")["method"] == "post"
@@ -2233,6 +2263,7 @@ def test_default_box_shows_on_non_default_sender_details_while_editing(
     sender_details,
     client_request,
     platform_admin_user,
+    mock_team_members,
     default_message,
     checkbox_present,
     is_platform_admin,
@@ -2321,7 +2352,7 @@ def test_confirm_delete_sms_sender(
     )
 
     assert normalize_spaces(page.select_one(".banner-dangerous").text) == (
-        "Are you sure you want to delete this text message sender? " "Yes, delete"
+        "Are you sure you want to delete this text message sender? Yes, delete"
     )
     assert "action" not in page.select_one(".banner-dangerous form")
     assert page.select_one(".banner-dangerous form")["method"] == "post"
@@ -3307,7 +3338,7 @@ def test_switch_service_enable_letters(
 
 
 @pytest.mark.parametrize(
-    ("initial_permissions," "expected_initial_value," "posted_value," "expected_updated_permissions"),
+    ("initial_permissions,expected_initial_value,posted_value,expected_updated_permissions"),
     [
         (
             ["email", "sms"],
@@ -3566,7 +3597,7 @@ def test_archive_service_prompts_user(
         service_id=SERVICE_ONE_ID,
     )
     assert normalize_spaces(delete_page.select_one(".banner-dangerous").text) == (
-        "Are you sure you want to delete ‘service one’? " "There’s no way to undo this. " "Yes, delete"
+        "Are you sure you want to delete ‘service one’? There’s no way to undo this. Yes, delete"
     )
     assert mocked_fn.called is False
 
@@ -4252,7 +4283,7 @@ def test_submit_email_branding_request(
         tags=["notify_action_add_branding"],
     )
     assert normalize_spaces(page.select_one(".banner-default").text) == (
-        "Thanks for your branding request. We’ll get back to you " "within one working day."
+        "Thanks for your branding request. We’ll get back to you within one working day."
     )
 
 

tests/conftest.py

@@ -91,6 +91,22 @@ def service_two(api_user_active):
     return service_json(SERVICE_TWO_ID, "service two", [api_user_active["id"]])
 
 
+@pytest.fixture(scope="function")
+def service_multi_member(api_user_active):
+    return service_json(
+        SERVICE_MULTI_MEMBER_ID,
+        "service multi-team-member",
+        [api_user_active, user_json(email_address="a-user@team-member.ca"), user_json(email_address="another-user@example.com")],
+    )
+
+
+@pytest.fixture(scope="function")
+def mock_team_members(mocker, service_multi_member):
+    mock_team_members = [Mock(email_domain=team_member["email_domain"]) for team_member in service_multi_member.users]
+    mocker.patch("app.models.service.Service.team_members", mock_team_members)
+    return mock_team_members
+
+
 @pytest.fixture(scope="function")
 def multiple_reply_to_email_addresses(mocker):
     def _get(service_id):
@@ -793,6 +809,7 @@ def _update(service_id, **kwargs):
 
 SERVICE_ONE_ID = "596364a0-858e-42c8-9062-a8fe822260eb"
 SERVICE_TWO_ID = "147ad62a-2951-4fa1-9ca0-093cd1a52c52"
+SERVICE_MULTI_MEMBER_ID = "ca1a6d94-cca9-476a-9d5a-49c21a79d938"
 ORGANISATION_ID = "c011fa40-4cbe-4524-b415-dde2f421bd9c"
 ORGANISATION_TWO_ID = "d9b5be73-0b36-4210-9d89-8f1a5c2fef26"
 TEMPLATE_ONE_ID = "b22d7d94-2197-4a7d-a8e7-fd5f9770bf48"
@@ -1432,6 +1449,7 @@ def api_user_active(fake_uuid):
         "name": "Test User",
         "password": "somepassword",
         "email_address": "test@user.canada.ca",
+        "email_domain": "user.canada.ca",
         "mobile_number": "6502532222",
         "blocked": False,
         "state": "active",