diff --git a/app/service/rest.py b/app/service/rest.py index 6bf76763a..7c1b3fb0b 100644 --- a/app/service/rest.py +++ b/app/service/rest.py @@ -650,21 +650,22 @@ def get_email_reply_to_address(service_id, reply_to_id): return jsonify(result.serialize()), 200 -@service_blueprint.route('/email-reply-to/verify', methods=['POST']) -def verify_new_service_reply_to_email_address(): +@service_blueprint.route('//email-reply-to/verify', methods=['POST']) +def verify_new_service_reply_to_email_address(service_id): email_address, errors = email_data_request_schema.load(request.get_json()) + check_if_reply_to_address_already_in_use(service_id, email_address["email"]) template = dao_get_template_by_id(current_app.config['REPLY_TO_EMAIL_ADDRESS_VERIFICATION_TEMPLATE_ID']) - service = Service.query.get(current_app.config['NOTIFY_SERVICE_ID']) + notify_service = Service.query.get(current_app.config['NOTIFY_SERVICE_ID']) saved_notification = persist_notification( template_id=template.id, template_version=template.version, recipient=email_address["email"], - service=service, + service=notify_service, personalisation='', notification_type=template.template_type, api_key_id=None, key_type=KEY_TYPE_NORMAL, - reply_to_text=service.get_default_reply_to_email_address() + reply_to_text=notify_service.get_default_reply_to_email_address() ) send_notification_to_queue(saved_notification, False, queue=QueueNames.NOTIFY) @@ -677,6 +678,7 @@ def add_service_reply_to_email_address(service_id): # validate the service exists, throws ResultNotFound exception. dao_fetch_service_by_id(service_id) form = validate(request.get_json(), add_service_email_reply_to_request) + check_if_reply_to_address_already_in_use(service_id, form['email_address']) new_reply_to = add_reply_to_email_address_for_service(service_id=service_id, email_address=form['email_address'], is_default=form.get('is_default', True)) @@ -688,6 +690,7 @@ def update_service_reply_to_email_address(service_id, reply_to_email_id): # validate the service exists, throws ResultNotFound exception. dao_fetch_service_by_id(service_id) form = validate(request.get_json(), add_service_email_reply_to_request) + check_if_reply_to_address_already_in_use(service_id, form['email_address']) new_reply_to = update_reply_to_email_address(service_id=service_id, reply_to_id=reply_to_email_id, email_address=form['email_address'], @@ -901,3 +904,11 @@ def check_request_args(request): if errors: raise InvalidRequest(errors, status_code=400) return service_id, name, email_from + + +def check_if_reply_to_address_already_in_use(service_id, email_address): + existing_reply_to_addresses = dao_get_reply_to_by_service_id(service_id) + if email_address in [i.email_address for i in existing_reply_to_addresses]: + raise InvalidRequest( + "Your service already uses '{}' as an email reply-to address.".format(email_address), status_code=400 + ) diff --git a/tests/app/service/test_rest.py b/tests/app/service/test_rest.py index 3d630ad82..ecde5f097 100644 --- a/tests/app/service/test_rest.py +++ b/tests/app/service/test_rest.py @@ -2495,12 +2495,13 @@ def test_get_email_reply_to_addresses_with_multiple_email_addresses(client, noti def test_verify_new_service_reply_to_email_address_should_send_verification_email( client, sample_user, mocker, verify_reply_to_address_email_template ): + service = create_service() mocked = mocker.patch('app.celery.provider_tasks.deliver_email.apply_async') data = json.dumps({'email': 'reply-here@example.gov.uk'}) auth_header = create_authorization_header() notify_service = verify_reply_to_address_email_template.service response = client.post( - url_for('service.verify_new_service_reply_to_email_address'), + url_for('service.verify_new_service_reply_to_email_address', service_id=service.id), data=data, headers=[('Content-Type', 'application/json'), auth_header]) @@ -2512,6 +2513,21 @@ def test_verify_new_service_reply_to_email_address_should_send_verification_emai assert notification.reply_to_text == notify_service.get_default_reply_to_email_address() +def test_verify_new_service_reply_to_email_address_doesnt_allow_duplicates(client, sample_user, mocker): + data = json.dumps({'email': 'reply-here@example.gov.uk'}) + service = create_service() + create_reply_to_email(service, 'reply-here@example.gov.uk') + auth_header = create_authorization_header() + response = client.post( + url_for('service.verify_new_service_reply_to_email_address', service_id=service.id), + data=data, + headers=[('Content-Type', 'application/json'), auth_header]) + assert response.status_code == 400 + assert response.json[ + "message" + ] == "Your service already uses 'reply-here@example.gov.uk' as an email reply-to address." + + def test_add_service_reply_to_email_address(client, sample_service): data = json.dumps({"email_address": "new@reply.com", "is_default": True}) response = client.post('/service/{}/email-reply-to'.format(sample_service.id), @@ -2525,6 +2541,20 @@ def test_add_service_reply_to_email_address(client, sample_service): assert json_resp['data'] == results[0].serialize() +def test_add_service_reply_to_email_address_doesnt_allow_duplicates(client, sample_user, mocker): + data = json.dumps({"email_address": "reply-here@example.gov.uk", "is_default": True}) + service = create_service() + create_reply_to_email(service, 'reply-here@example.gov.uk') + auth_header = create_authorization_header() + response = client.post('/service/{}/email-reply-to'.format(service.id), + data=data, + headers=[('Content-Type', 'application/json'), auth_header]) + assert response.status_code == 400 + assert response.json[ + "message" + ] == "Your service already uses 'reply-here@example.gov.uk' as an email reply-to address." + + def test_add_service_reply_to_email_address_can_add_multiple_addresses(client, sample_service): data = json.dumps({"email_address": "first@reply.com", "is_default": True}) client.post('/service/{}/email-reply-to'.format(sample_service.id), @@ -2580,6 +2610,18 @@ def test_update_service_reply_to_email_address(client, sample_service): assert json_resp['data'] == results[0].serialize() +def test_update_service_reply_to_email_address_doesnt_allow_duplicates(client, sample_user, mocker): + service = create_service() + original_reply_to = create_reply_to_email(service=service, email_address="some@email.com") + data = json.dumps({"email_address": "changed@reply.com", "is_default": True}) + create_reply_to_email(service, 'changed@reply.com') + response = client.post('/service/{}/email-reply-to/{}'.format(service.id, original_reply_to.id), + data=data, + headers=[('Content-Type', 'application/json'), create_authorization_header()]) + assert response.status_code == 400 + assert response.json["message"] == "Your service already uses 'changed@reply.com' as an email reply-to address." + + def test_update_service_reply_to_email_address_returns_400_when_no_default(client, sample_service): original_reply_to = create_reply_to_email(service=sample_service, email_address="some@email.com") data = json.dumps({"email_address": "changed@reply.com", "is_default": False})