diff --git a/Makefile b/Makefile index bebfe42280..8833fd26e4 100644 --- a/Makefile +++ b/Makefile @@ -58,6 +58,7 @@ run-dev: .PHONY: format format: ruff check --select I --fix . + ruff check ruff format . mypy ./ npx prettier --write app/assets/javascripts app/assets/stylesheets tests_cypress/cypress/e2e diff --git a/app/main/forms.py b/app/main/forms.py index 6effcd55b6..809a1f3b67 100644 --- a/app/main/forms.py +++ b/app/main/forms.py @@ -23,6 +23,7 @@ SelectField, SelectMultipleField, StringField, + SubmitField, TextAreaField, ValidationError, validators, @@ -49,6 +50,7 @@ LettersNumbersAndFullStopsOnly, NoCommasInPlaceHolders, OnlySMSCharacters, + ValidCallbackUrl, ValidEmail, ValidGovEmail, validate_email_from, @@ -342,7 +344,8 @@ def bind_field(self, form, unbound_field, options): filters = [strip_whitespace] if not issubclass(unbound_field.field_class, no_filter_fields) else [] filters += unbound_field.kwargs.get("filters", []) bound = unbound_field.bind(form=form, filters=filters, **options) - bound.get_form = weakref.ref(form) # GC won't collect the form if we don't use a weakref + # GC won't collect the form if we don't use a weakref + bound.get_form = weakref.ref(form) return bound @@ -1407,7 +1410,7 @@ def __init__(self, *args, **kwargs): class CallbackForm(StripWhitespaceForm): def validate(self, extra_validators=None): - return super().validate(extra_validators) or self.url.data == "" + return super().validate(extra_validators) class ServiceReceiveMessagesCallbackForm(CallbackForm): @@ -1415,7 +1418,8 @@ class ServiceReceiveMessagesCallbackForm(CallbackForm): "URL", validators=[ DataRequired(message=_l("This cannot be empty")), - Regexp(regex="^https.*", message=_l("Must be a valid https URL")), + Regexp(regex="^https.*", message=_l("Enter a URL that starts with https://")), + ValidCallbackUrl(), ], ) bearer_token = PasswordFieldShowHasContent( @@ -1432,7 +1436,8 @@ class ServiceDeliveryStatusCallbackForm(CallbackForm): "URL", validators=[ DataRequired(message=_l("This cannot be empty")), - Regexp(regex="^https.*", message=_l("Must be a valid https URL")), + Regexp(regex="^https.*", message=_l("Enter a URL that starts with https://")), + ValidCallbackUrl(), ], ) bearer_token = PasswordFieldShowHasContent( @@ -1442,6 +1447,7 @@ class ServiceDeliveryStatusCallbackForm(CallbackForm): Length(min=10, message=_l("Must be at least 10 characters")), ], ) + test_response_time = SubmitField() class InternationalSMSForm(StripWhitespaceForm): @@ -1885,7 +1891,8 @@ class BrandingPoolForm(StripWhitespaceForm): pool_branding = RadioField( _l("Select alternate logo"), - choices=[], # Choices by default, override to get more refined options. + # Choices by default, override to get more refined options. + choices=[], validators=[DataRequired(message=_l("You must select an option to continue"))], ) diff --git a/app/main/validators.py b/app/main/validators.py index 238e68cc91..77ad8365b4 100644 --- a/app/main/validators.py +++ b/app/main/validators.py @@ -2,7 +2,9 @@ import time import pwnedpasswords -from flask import current_app +import requests +import validators +from flask import current_app, g from flask_babel import _ from flask_babel import lazy_gettext as _l from notifications_utils.field import Field @@ -11,7 +13,7 @@ from wtforms import ValidationError from wtforms.validators import Email -from app import formatted_list, service_api_client +from app import current_service, formatted_list, service_api_client from app.main._blocked_passwords import blocked_passwords from app.utils import Spreadsheet, email_safe, email_safe_name, is_gov_user @@ -25,7 +27,8 @@ def __init__(self, message=None): def __call__(self, form, field): if current_app.config.get("HIPB_ENABLED", None): hibp_bad_password_found = False - for i in range(0, 3): # Try 3 times. If the HIPB API is down then fall back to the old banlist. + # Try 3 times. If the HIPB API is down then fall back to the old banlist. + for i in range(0, 3): try: response = pwnedpasswords.check(field.data) if response > 0: @@ -141,6 +144,57 @@ def __call__(self, form, field): raise ValidationError(self.message) +class ValidCallbackUrl: + def __init__(self, message="Enter a URL that starts with https://"): + self.message = message + + def __call__(self, form, field): + if field.data: + validate_callback_url(field.data, form.bearer_token.data) + + +def validate_callback_url(service_callback_url, bearer_token): + """Validates a callback URL, checking that it is https and by sending a POST request to the URL with a health_check parameter. + 4xx responses are considered invalid. 5xx responses are considered valid as it indicates there is at least a service running + at the URL, and we are sending a payload that the service will not understand. + + Args: + service_callback_url (str): The url to validate. + bearer_token (str): The bearer token to use in the request, specified by the user requesting callbacks. + + Raises: + ValidationError: If the URL is not HTTPS or the http response is 4xx. + """ + if not validators.url(service_callback_url): + current_app.logger.warning( + f"Unable to create callback for service: {current_service.id}. Error: Invalid callback URL format: URL: {service_callback_url}" + ) + raise ValidationError(_l("Enter a URL that starts with https://")) + + try: + response = requests.post( + url=service_callback_url, + allow_redirects=True, + data={"health_check": "true"}, + headers={"Content-Type": "application/json", "Authorization": f"Bearer {bearer_token}"}, + timeout=2, + ) + + g.callback_response_time = response.elapsed.total_seconds() + + if response.status_code < 500 and response.status_code >= 400: + current_app.logger.warning( + f"Unable to create callback for service: {current_service.id} Error: Callback URL not reachable URL: {service_callback_url}" + ) + raise ValidationError(_l("Check your service is running and not using a proxy we cannot access")) + + except requests.RequestException as e: + current_app.logger.warning( + f"Unable to create callback for service: {current_service.id} Error: Callback URL not reachable URL: {service_callback_url} Exception: {e}" + ) + raise ValidationError(_l("Check your service is running and not using a proxy we cannot access")) + + def validate_email_from(form, field): if email_safe(field.data) != field.data.lower(): # fix their data instead of only warning them diff --git a/app/main/views/api_keys.py b/app/main/views/api_keys.py index d3c9a35277..b50286dce2 100644 --- a/app/main/views/api_keys.py +++ b/app/main/views/api_keys.py @@ -1,4 +1,4 @@ -from flask import Markup, abort, flash, redirect, render_template, request, url_for +from flask import Markup, abort, flash, g, redirect, render_template, request, url_for from flask_babel import lazy_gettext as _l from flask_login import current_user @@ -28,11 +28,13 @@ @main.route("/services//api") @user_has_permissions("manage_api_keys") def api_integration(service_id): - callbacks_link = ".api_callbacks" if current_service.has_permission("inbound_sms") else ".delivery_status_callback" + callbacks_link = ".api_callbacks" if current_service.has_permission( + "inbound_sms") else ".delivery_status_callback" return render_template( "views/api/index.html", callbacks_link=callbacks_link, - api_notifications=notification_api_client.get_api_notifications_for_service(service_id), + api_notifications=notification_api_client.get_api_notifications_for_service( + service_id), ) @@ -87,7 +89,8 @@ def create_api_key(service_id): disabled_options, option_hints = [], {} if current_service.trial_mode: disabled_options = [KEY_TYPE_NORMAL] - option_hints[KEY_TYPE_NORMAL] = Markup(_l("Not available because your service is in trial mode.")) + option_hints[KEY_TYPE_NORMAL] = Markup( + _l("Not available because your service is in trial mode.")) if current_service.has_permission("letter"): option_hints[KEY_TYPE_TEAM] = "" if form.validate_on_submit(): @@ -119,7 +122,8 @@ def revoke_api_key(service_id, key_id): if request.method == "GET": flash( [ - "{} ‘{}’?".format(_l("Are you sure you want to revoke"), key_name), + "{} ‘{}’?".format( + _l("Are you sure you want to revoke"), key_name), _l("You will not be able to use this API key to connect to GC Notify"), ], "revoke this API key", @@ -137,9 +141,11 @@ def get_apis(): callback_api = None inbound_api = None if current_service.service_callback_api: - callback_api = service_api_client.get_service_callback_api(current_service.id, current_service.service_callback_api[0]) + callback_api = service_api_client.get_service_callback_api( + current_service.id, current_service.service_callback_api[0]) if current_service.inbound_api: - inbound_api = service_api_client.get_service_inbound_api(current_service.id, current_service.inbound_api[0]) + inbound_api = service_api_client.get_service_inbound_api( + current_service.id, current_service.inbound_api[0]) return (callback_api, inbound_api) @@ -161,7 +167,8 @@ def api_callbacks(service_id): return render_template( "views/api/callbacks.html", - received_text_messages_callback=received_text_messages_callback["url"] if received_text_messages_callback else None, + received_text_messages_callback=received_text_messages_callback[ + "url"] if received_text_messages_callback else None, delivery_status_callback=delivery_status_callback["url"] if delivery_status_callback else None, ) @@ -171,6 +178,44 @@ def get_delivery_status_callback_details(): return service_api_client.get_service_callback_api(current_service.id, current_service.service_callback_api[0]) +@main.route("/services//api/callbacks/delivery-status-callback/delete", methods=["GET", "POST"]) +@user_has_permissions("manage_api_keys") +def delete_delivery_status_callback(service_id): + delivery_status_callback = get_delivery_status_callback_details() + back_link = ".api_callbacks" if current_service.has_permission( + "inbound_sms") else ".api_integration" + url_hint_txt = "Must start with https://" + + if request.method == "POST": + if delivery_status_callback: + service_api_client.delete_service_callback_api( + service_id, + delivery_status_callback["id"], + ) + + flash(_l("Your Callback configuration has been deleted."), + "default_with_tick") + return redirect(url_for(back_link, service_id=service_id)) + + flash(["{}".format( + _l("Are you sure you want to delete this callback configuration?"))], "delete") + + form = ServiceDeliveryStatusCallbackForm( + url=delivery_status_callback.get( + "url") if delivery_status_callback else "", + bearer_token=dummy_bearer_token if delivery_status_callback else "", + ) + + return render_template( + "views/api/callbacks/delivery-status-callback.html", + back_link=".api_callbacks" if current_service.has_permission( + "inbound_sms") else ".delivery_status_callback", + hint_text=url_hint_txt, + is_deleting=True, + form=form, + ) + + @main.route( "/services//api/callbacks/delivery-status-callback", methods=["GET", "POST"], @@ -178,28 +223,54 @@ def get_delivery_status_callback_details(): @user_has_permissions("manage_api_keys") def delivery_status_callback(service_id): delivery_status_callback = get_delivery_status_callback_details() - back_link = ".api_callbacks" if current_service.has_permission("inbound_sms") else ".api_integration" + back_link = ".api_callbacks" if current_service.has_permission( + "inbound_sms") else ".api_integration" + url_hint_txt = _l("Must start with https://") form = ServiceDeliveryStatusCallbackForm( - url=delivery_status_callback.get("url") if delivery_status_callback else "", + url=delivery_status_callback.get( + "url") if delivery_status_callback else "", bearer_token=dummy_bearer_token if delivery_status_callback else "", ) if form.validate_on_submit(): + # As part of the ValidCallbackUrl validation, we ping their callback URL to check if it's up and set the response time in g + response_time = "{:.2f}".format(g.callback_response_time) + url_hostname = form.url.data.split("https://")[1] + + # Update existing callback if delivery_status_callback and form.url.data: if delivery_status_callback.get("url") != form.url.data or form.bearer_token.data != dummy_bearer_token: service_api_client.update_service_callback_api( service_id, url=form.url.data, - bearer_token=check_token_against_dummy_bearer(form.bearer_token.data), + bearer_token=check_token_against_dummy_bearer( + form.bearer_token.data), user_id=current_user.id, callback_api_id=delivery_status_callback.get("id"), ) - elif delivery_status_callback and not form.url.data: - service_api_client.delete_service_callback_api( - service_id, - delivery_status_callback["id"], - ) + + # If the user is just testing their URL, don't send them back to the API Integration page + if request.form.get("button_pressed") == "test_response_time": + flash( + _l("The service {} responded in {} seconds.").format( + url_hostname, + response_time, + ), + "default_with_tick", + ) + return redirect(url_for("main.delivery_status_callback", service_id=service_id)) + + flash( + _l("We’ve saved your callback configuration. {} responded in {} seconds.").format( + url_hostname, + response_time, + ), + "default_with_tick", + ) + + return redirect(url_for(back_link, service_id=service_id)) + # Create a new callback elif form.url.data: service_api_client.create_service_callback_api( service_id, @@ -207,17 +278,45 @@ def delivery_status_callback(service_id): bearer_token=form.bearer_token.data, user_id=current_user.id, ) + + flash( + _l("We’ve set up your callback configuration. {} responded in {} seconds.").format( + url_hostname, + response_time, + ), + "default_with_tick", + ) + + return redirect(url_for(back_link, service_id=service_id)) else: # If no callback is set up and the user chooses to continue # having no callback (ie both fields empty) then there’s # nothing for us to do here pass - return redirect(url_for(back_link, service_id=service_id)) + if request.form.get("button_pressed") == "test_response_time" and g.callback_response_time >= 1: + flash( + _l("The service {} took longer than 1 second to respond.").format( + url_hostname, + ), + "error", + ) + return redirect(url_for("main.delivery_status_callback", service_id=service_id)) + else: + flash( + _l("The service {} responded in {} seconds.").format( + url_hostname, + response_time, + ), + "default_with_tick", + ) + return redirect(url_for("main.delivery_status_callback", service_id=service_id)) return render_template( "views/api/callbacks/delivery-status-callback.html", + has_callback_config=delivery_status_callback is not None, back_link=back_link, + hint_text=url_hint_txt, form=form, ) @@ -235,23 +334,52 @@ def get_received_text_messages_callback(): def received_text_messages_callback(service_id): if not current_service.has_permission("inbound_sms"): return redirect(url_for(".api_integration", service_id=service_id)) + back_link = ".api_callbacks" if current_service.has_permission( + "inbound_sms") else ".api_integration" received_text_messages_callback = get_received_text_messages_callback() form = ServiceReceiveMessagesCallbackForm( - url=received_text_messages_callback.get("url") if received_text_messages_callback else "", + url=received_text_messages_callback.get( + "url") if received_text_messages_callback else "", bearer_token=dummy_bearer_token if received_text_messages_callback else "", ) + url_hint_txt = _l("Must start with https://") if form.validate_on_submit(): + # As part of the ValidCallbackUrl validation, we ping their callback URL to check if it's up and set the response time in g + response_time = "{:.2f}".format(g.callback_response_time) + url_hostname = form.url.data.split("https://")[1] + if received_text_messages_callback and form.url.data: if received_text_messages_callback.get("url") != form.url.data or form.bearer_token.data != dummy_bearer_token: service_api_client.update_service_inbound_api( service_id, url=form.url.data, - bearer_token=check_token_against_dummy_bearer(form.bearer_token.data), + bearer_token=check_token_against_dummy_bearer( + form.bearer_token.data), user_id=current_user.id, inbound_api_id=received_text_messages_callback.get("id"), ) + + # If the user is just testing their URL, don't send them back to the API Integration page + if request.form.get("button_pressed") == "test_response_time": + flash( + _l("The service {} responded in {} seconds.").format( + url_hostname, + response_time, + ), + "default_with_tick", + ) + return redirect(url_for("main.received_text_messages_callback", service_id=service_id)) + + flash( + _l("We’ve saved your callback configuration. {} responded in {} seconds.").format( + url_hostname, + response_time, + ), + "default_with_tick", + ) + elif received_text_messages_callback and not form.url.data: service_api_client.delete_service_inbound_api( service_id, @@ -264,8 +392,75 @@ def received_text_messages_callback(service_id): bearer_token=form.bearer_token.data, user_id=current_user.id, ) - return redirect(url_for(".api_callbacks", service_id=service_id)) + flash( + _l("We’ve set up your callback configuration. {} responded in {} seconds.").format( + url_hostname, + response_time, + ), + "default_with_tick", + ) + + return redirect(url_for(back_link, service_id=service_id)) + + if request.form.get("button_pressed") == "test_response_time" and g.callback_response_time >= 1: + flash( + _l("The service {} took longer than 1 second to respond.").format( + url_hostname, + ), + "error", + ) + return redirect(url_for("main.received_text_messages_callback", service_id=service_id)) + else: + flash( + _l("The service {} responded in {} seconds.").format( + url_hostname, + response_time, + ), + "default_with_tick", + ) + return redirect(url_for("main.received_text_messages_callback", service_id=service_id)) + return render_template( "views/api/callbacks/received-text-messages-callback.html", + has_callback_config=received_text_messages_callback is not None, + form=form, + hint_text=url_hint_txt, + ) + + +@main.route("/services//api/callbacks/received-text-messages-callback/delete", methods=["GET", "POST"]) +@user_has_permissions("manage_api_keys") +def delete_received_text_messages_callback(service_id): + received_text_messages_callback = get_received_text_messages_callback() + back_link = ".api_callbacks" if current_service.has_permission( + "inbound_sms") else ".api_integration" + url_hint_txt = "Must start with https://" + + if request.method == "POST": + if received_text_messages_callback: + service_api_client.delete_service_inbound_api( + service_id, + received_text_messages_callback["id"], + ) + + flash(_l("Your Callback configuration has been deleted."), + "default_with_tick") + return redirect(url_for(back_link, service_id=service_id)) + + flash(["{}".format( + _l("Are you sure you want to delete this callback configuration?"))], "delete") + + form = ServiceReceiveMessagesCallbackForm( + url=received_text_messages_callback.get( + "url") if delivery_status_callback else "", + bearer_token=dummy_bearer_token if received_text_messages_callback else "", + ) + + return render_template( + "views/api/callbacks/delivery-status-callback.html", + back_link=".api_callbacks" if current_service.has_permission( + "inbound_sms") else ".delivery_status_callback", + hint_text=url_hint_txt, + is_deleting=True, form=form, ) diff --git a/app/templates/components/page-footer.html b/app/templates/components/page-footer.html index 13929520a3..144aaaccab 100644 --- a/app/templates/components/page-footer.html +++ b/app/templates/components/page-footer.html @@ -67,3 +67,44 @@ {% endmacro %} + +{% macro sticky_page_footer_two_submit_buttons_and_delete_link( + button1_text="b1", + button1_value="b1", + button2_text=None, + button2_value=None, + delete_link=False, + delete_link_text=_("Delete")) %} +
+
+ + + + {% if button2_text and button2_value %} + + {% endif %} + + {% if delete_link %} + + {{ delete_link_text }} + + {% endif %} +
+
+{% endmacro %} \ No newline at end of file diff --git a/app/templates/views/api/callbacks/delivery-status-callback.html b/app/templates/views/api/callbacks/delivery-status-callback.html index ed20b80b18..211ed01a39 100644 --- a/app/templates/views/api/callbacks/delivery-status-callback.html +++ b/app/templates/views/api/callbacks/delivery-status-callback.html @@ -1,8 +1,9 @@ {% extends "admin_template.html" %} {% from "components/textbox.html" import textbox %} {% from "components/page-header.html" import page_header %} -{% from "components/page-footer.html" import page_footer %} +{% from "components/page-footer.html" import sticky_page_footer_two_submit_buttons_and_delete_link %} {% from "components/form.html" import form_wrapper %} +{% from "components/ajax-block.html" import ajax_block %} {% block service_page_title %} {{ _('Callbacks for delivery receipts') }} @@ -23,12 +24,11 @@

{{ _('Add an endpoint where GC Notify should send POST requests') }}

- {% set hint_txt = _('Must start with https://') %} {% call form_wrapper() %} {{ textbox( form.url, width='w-full', - hint=hint_txt + hint=hint_text ) }}

{{ _('Add a secret value for authentication') }} @@ -40,10 +40,20 @@

hint=hint_txt, autocomplete='new-password' ) }} - {% set button_txt = _('Save') %} - {{ page_footer(button_txt) }} + {% set test_response_txt = _('Test response time') if has_callback_config else None %} + {% set test_response_value = _('test_response_time') if has_callback_config else None %} + {% set display_footer = is_deleting if is_deleting else False %} + {% if not display_footer %} + {{ sticky_page_footer_two_submit_buttons_and_delete_link( + button1_text=_('Save'), + button1_value=_('save'), + button2_text=test_response_txt, + button2_value=test_response_value, + delete_link=url_for('.delete_delivery_status_callback', service_id=current_service.id), + delete_link_text=_('Delete') + ) }} + {% endif %} {% endcall %} - {% endblock %} diff --git a/app/templates/views/api/callbacks/received-text-messages-callback.html b/app/templates/views/api/callbacks/received-text-messages-callback.html index 488dad293f..49fab276b6 100644 --- a/app/templates/views/api/callbacks/received-text-messages-callback.html +++ b/app/templates/views/api/callbacks/received-text-messages-callback.html @@ -1,7 +1,7 @@ {% extends "admin_template.html" %} {% from "components/textbox.html" import textbox %} {% from "components/page-header.html" import page_header %} -{% from "components/page-footer.html" import page_footer %} +{% from "components/page-footer.html" import sticky_page_footer_two_submit_buttons_and_delete_link %} {% from "components/form.html" import form_wrapper %} {% block service_page_title %} @@ -18,13 +18,16 @@

{{ _('When you receive a text message in GC Notify, we can forward it to your system. Learn more in the') }} {{ _('callbacks documentation') }} .

- {% set hint_txt = _('Must start with https://') %} + {% call form_wrapper() %} {{ textbox( form.url, width='w-full', - hint=hint_txt + hint=hint_text ) }} +

+ {{ _('Add a secret value for authentication') }} +

{% set hint_txt = _('At least 10 characters') %} {{ textbox( form.bearer_token, @@ -32,8 +35,19 @@ hint=hint_txt, autocomplete='new-password' ) }} - {% set button_txt = _('Save') %} - {{ page_footer(button_txt) }} + {% set test_response_txt = _('Test response time') if has_callback_config else None %} + {% set test_response_value = _('test_response_time') if has_callback_config else None %} + {% set display_footer = is_deleting if is_deleting else False %} + {% if not display_footer %} + {{ sticky_page_footer_two_submit_buttons_and_delete_link( + button1_text=_('Save'), + button1_value=_('save'), + button2_text=test_response_txt, + button2_value=test_response_value, + delete_link=url_for('.delete_received_text_messages_callback', service_id=current_service.id), + delete_link_text=_('Delete') + ) }} + {% endif %} {% endcall %} diff --git a/app/translations/csv/fr.csv b/app/translations/csv/fr.csv index 2548e148c9..d5a0d2c3b8 100644 --- a/app/translations/csv/fr.csv +++ b/app/translations/csv/fr.csv @@ -789,6 +789,7 @@ "When you send an email or text message, find out if GC Notify was able to deliver it with callbacks. Learn more in the callbacks documentation.","Lorsque vous envoyez un courriel ou un message texte, vérifiez si Notification GC a réussi à le livrer avec des fonctions de rappel. Pour en savoir plus, consultez la documentation sur les fonctions de rappel." "Add an endpoint where GC Notify should send POST requests","Ajouter un point de terminaison où Notification GC peut envoyer les requêtes ‘POST’" "Must start with https://","Doit commencer par https://" +"Enter a URL that starts with https://","Entrez une URL qui commence par https://" "Add a secret value for authentication","Ajouter une valeur secrète pour l’authentification" "At least 10 characters","Au moins 10 caractères" "Callbacks for received text messages","Fonctions de rappel pour les messages texte reçus" @@ -1895,6 +1896,8 @@ "You have been signed out.","Déconnexion réussie" "Your session ends after 8 hours of inactivity","Votre session se termine au bout de 8 heures d’inactivité" "total sends in the last 7 days","total des envois au cours des 7 derniers jours" +"Getting started","Découvrez comment fonctionne Notification GC" +"Check your service is running and not using a proxy we cannot access","Vérifiez que votre service est en cours d'exécution et n'utilise pas un proxy auquel nous ne pouvons pas accéder" "Know your responsibilities","Ayez connaissance de vos responsabilités" "By using GC Notify, you accept our ","En utilisant Notification GC, vous acceptez ses " "This includes your agreement to:","Cela implique notamment les engagement suivants :" @@ -1907,7 +1910,6 @@ "Keep accurate contact information","Maintenir à jour les coordonnées" "Send information that’s unclassified or Protected A only","N’envoyer que des renseignements non classifiés ou de niveau Protégé A" "Practice continuous security","Pratiquer la sécurité continue" -"Getting started","Découvrez comment fonctionne Notification GC" "Read and agree to the terms of use","Lisez et acceptez les conditions d’utilisation" "Read and agree to continue","Lisez et acceptez les conditions d’utilisation" "Agree follows terms of use","Accepter suite aux conditions d'utilisation" @@ -1977,4 +1979,10 @@ "Change your filters or search for more templates","Modifiez vos filtres ou recherchez d’autres gabarits" "Review your activity","Examinez votre activité" "Set sensitive service","Définir un service sensible" -"Suspend Callback","Suspension du rappel" \ No newline at end of file +"Suspend Callback","Suspension du rappel" +"We’ve set up your callback configuration. {} responded in {} seconds.","Nous avons mis en place votre configuration de rappel. {} a répondu en {} secondes." +"We’ve saved your callback configuration. {} responded in {} seconds.","Nous avons enregistré votre configuration de rappel. {} a répondu en {} secondes." +"The service {} responded in {} seconds.","Le service {} a répondu en {} secondes." +"Are you sure you want to delete this callback configuration?","Voulez-vous vraiment supprimer cette configuration de rappel?" +"Your Callback configuration has been deleted.","Votre configuration de rappel a été supprimée." +"The service {} took longer than 1 second to respond.","Le service {} a mis plus d’une seconde à répondre." \ No newline at end of file diff --git a/poetry.lock b/poetry.lock index 1b0bcdc6a5..97b58710c0 100644 --- a/poetry.lock +++ b/poetry.lock @@ -2484,6 +2484,17 @@ files = [ [package.dependencies] ua-parser = ">=0.10.0" +[[package]] +name = "validators" +version = "0.28.3" +description = "Python Data Validation for Humans™" +optional = false +python-versions = ">=3.8" +files = [ + {file = "validators-0.28.3-py3-none-any.whl", hash = "sha256:53cafa854f13850156259d9cc479b864ee901f6a96e6b109e6fc33f98f37d99f"}, + {file = "validators-0.28.3.tar.gz", hash = "sha256:c6c79840bcde9ba77b19f6218f7738188115e27830cbaff43264bc4ed24c429d"}, +] + [[package]] name = "virtualenv" version = "20.26.4" @@ -2733,4 +2744,4 @@ testing = ["coverage (>=5.0.3)", "zope.event", "zope.testing"] [metadata] lock-version = "2.0" python-versions = "~3.10.9" -content-hash = "d2c34103040f4bab5095aeb643611f85860024a73674fe04879031c27452fdd2" +content-hash = "9b46717b57c769f0fc18a5e86bcb7f0445bc7ac0ff71382d5ff53b98b9726aa9" diff --git a/pyproject.toml b/pyproject.toml index b1864c112a..815861c64a 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -60,6 +60,7 @@ itsdangerous = "2.2.0" # pyup: <1.0.0 newrelic = "8.10.0" # Pinned to 8.10.0 for now, 8.11.0 caused a performance regression: https://gcdigital.slack.com/archives/C012W5K734Y/p1709668046344929 aws-xray-sdk = "^2.14.0" +validators = "^0.28.3" [tool.poetry.group.test.dependencies] pytest = "7.4.4" @@ -212,7 +213,7 @@ args = { port = { options = ["--port", "-p"], default = "6012"}, host = { option [tool.poe.tasks.format] help = "Formats the python code with isort, black, and flake8, and checks typing with mypy and formats the JS code with prettier." shell = """ - isort ./app ./tests + isort ./app ./tests black ./app ./tests flake8 ./app ./tests isort --check-only ./app ./tests diff --git a/tests/app/main/views/test_api_integration.py b/tests/app/main/views/test_api_integration.py index 4711cad886..02d1e2ea93 100644 --- a/tests/app/main/views/test_api_integration.py +++ b/tests/app/main/views/test_api_integration.py @@ -1,6 +1,7 @@ +import datetime import uuid from collections import OrderedDict -from unittest.mock import call +from unittest.mock import Mock, call import pytest from bs4 import BeautifulSoup @@ -158,725 +159,753 @@ def test_api_documentation_page_should_redirect( ) -def test_should_show_empty_api_keys_page( - client, - api_user_active, - mock_login, - mock_get_no_api_keys, - mock_get_service, - mock_has_permissions, -): - client.login(api_user_active) - service_id = str(uuid.uuid4()) - response = client.get(url_for("main.api_keys", service_id=service_id)) - - assert response.status_code == 200 - assert "You have not created any API keys yet" in response.get_data(as_text=True) - assert "Create API key" in response.get_data(as_text=True) - mock_get_no_api_keys.assert_called_once_with(service_id) - - -def test_should_show_api_keys_page( - client_request, - mock_get_api_keys, - mock_get_api_key_statistics, -): - page = client_request.get("main.api_keys", service_id=SERVICE_ONE_ID) - rows = [normalize_spaces(row.text) for row in page.select("main tr")] - - assert rows[0] == "API keys Action" - assert "another key name 20 total sends in the last 7 days (20 email, 0 sms)" in rows[1] - assert "Revoke API key some key name" in rows[2] - - mock_get_api_keys.assert_called_once_with(SERVICE_ONE_ID) - - -@pytest.mark.parametrize( - "restricted, can_send_letters, expected_options", - [ - ( - True, - False, - [ - ("Live – sends to anyone " "Not available because your service is in trial mode."), - "Team and safelist – limits who you can send to", - "Test – pretends to send messages", - ], - ), - ( - False, - False, - [ - "Live – sends to anyone", - "Team and safelist – limits who you can send to", - "Test – pretends to send messages", - ], - ), - ( - False, - True, - [ - "Live – sends to anyone", - ("Team and safelist – limits who you can send to" ""), - "Test – pretends to send messages", - ], - ), - ], -) -def test_should_show_create_api_key_page( - client_request, - mocker, - api_user_active, - mock_get_api_keys, - restricted, - can_send_letters, - expected_options, - service_one, -): - service_one["restricted"] = restricted - if can_send_letters: - service_one["permissions"].append("letter") - - mocker.patch("app.service_api_client.get_service", return_value={"data": service_one}) - - page = client_request.get("main.create_api_key", service_id=SERVICE_ONE_ID) - - for index, option in enumerate(expected_options): - assert normalize_spaces(page.select(".block-label")[index].text) == option - - -def test_should_create_api_key_with_type_normal( - client_request, - api_user_active, - mock_login, - mock_get_api_keys, - mock_get_live_service, - mock_has_permissions, - fake_uuid, - mocker, -): - key_name_from_user = "Some default key name 1/2" - key_name_fixed = "some_default_key_name_12" - post = mocker.patch( - "app.notify_client.api_key_api_client.ApiKeyApiClient.post", - return_value={"data": {"key": fake_uuid, "key_name": key_name_fixed}}, - ) - - page = client_request.post( - "main.create_api_key", - service_id=SERVICE_ONE_ID, - _data={"key_name": key_name_from_user, "key_type": "normal"}, - _expected_status=200, - ) - - assert page.select_one("span.api-key-key").text.strip() == ("{}-{}-{}".format(key_name_fixed, SERVICE_ONE_ID, fake_uuid)) - - post.assert_called_once_with( - url="/service/{}/api-key".format(SERVICE_ONE_ID), - data={ - "name": key_name_from_user, - "key_type": "normal", - "created_by": api_user_active["id"], - }, - ) - - -def test_cant_create_normal_api_key_in_trial_mode( - client_request, - api_user_active, - mock_login, - mock_get_api_keys, - mock_get_service, - mock_has_permissions, - fake_uuid, - mocker, -): - mock_post = mocker.patch("app.notify_client.api_key_api_client.ApiKeyApiClient.post") +class TestApiKeys: + def test_should_show_api_keys_page( + self, + client_request, + mock_get_api_keys, + mock_get_api_key_statistics, + ): + page = client_request.get("main.api_keys", service_id=SERVICE_ONE_ID) + rows = [normalize_spaces(row.text) for row in page.select("main tr")] + + assert rows[0] == "API keys Action" + assert "another key name 20 total sends in the last 7 days (20 email, 0 sms)" in rows[1] + assert "Revoke API key some key name" in rows[2] + + mock_get_api_keys.assert_called_once_with(SERVICE_ONE_ID) + + def test_should_show_empty_api_keys_page( + self, + client, + api_user_active, + mock_login, + mock_get_no_api_keys, + mock_get_service, + mock_has_permissions, + ): + client.login(api_user_active) + service_id = str(uuid.uuid4()) + response = client.get(url_for("main.api_keys", service_id=service_id)) + + assert response.status_code == 200 + assert "You have not created any API keys yet" in response.get_data(as_text=True) + assert "Create API key" in response.get_data(as_text=True) + mock_get_no_api_keys.assert_called_once_with(service_id) + + @pytest.mark.parametrize( + "restricted, can_send_letters, expected_options", + [ + ( + True, + False, + [ + ("Live – sends to anyone " "Not available because your service is in trial mode."), + "Team and safelist – limits who you can send to", + "Test – pretends to send messages", + ], + ), + ( + False, + False, + [ + "Live – sends to anyone", + "Team and safelist – limits who you can send to", + "Test – pretends to send messages", + ], + ), + ( + False, + True, + [ + "Live – sends to anyone", + ("Team and safelist – limits who you can send to" ""), + "Test – pretends to send messages", + ], + ), + ], + ) + def test_should_show_create_api_key_page( + self, + client_request, + mocker, + api_user_active, + mock_get_api_keys, + restricted, + can_send_letters, + expected_options, + service_one, + ): + service_one["restricted"] = restricted + if can_send_letters: + service_one["permissions"].append("letter") + + mocker.patch("app.service_api_client.get_service", return_value={"data": service_one}) + + page = client_request.get("main.create_api_key", service_id=SERVICE_ONE_ID) + + for index, option in enumerate(expected_options): + assert normalize_spaces(page.select(".block-label")[index].text) == option + + def test_should_create_api_key_with_type_normal( + self, + client_request, + api_user_active, + mock_login, + mock_get_api_keys, + mock_get_live_service, + mock_has_permissions, + fake_uuid, + mocker, + ): + key_name_from_user = "Some default key name 1/2" + key_name_fixed = "some_default_key_name_12" + post = mocker.patch( + "app.notify_client.api_key_api_client.ApiKeyApiClient.post", + return_value={"data": {"key": fake_uuid, "key_name": key_name_fixed}}, + ) - client_request.post( - "main.create_api_key", - service_id=SERVICE_ONE_ID, - _data={"key_name": "some default key name", "key_type": "normal"}, - _expected_status=400, - ) - mock_post.assert_not_called() + page = client_request.post( + "main.create_api_key", + service_id=SERVICE_ONE_ID, + _data={"key_name": key_name_from_user, "key_type": "normal"}, + _expected_status=200, + ) + assert page.select_one("span.api-key-key").text.strip() == ("{}-{}-{}".format(key_name_fixed, SERVICE_ONE_ID, fake_uuid)) -def test_should_show_confirm_revoke_api_key( - client_request, - mock_get_api_keys, - fake_uuid, -): - page = client_request.get( - "main.revoke_api_key", - service_id=SERVICE_ONE_ID, - key_id=fake_uuid, - _test_page_title=False, - ) - assert normalize_spaces(page.select(".banner-dangerous")[0].text) == ( - "Are you sure you want to revoke ‘some key name’? " - "You will not be able to use this API key to connect to GC Notify " - "Yes, revoke this API key" - ) - assert mock_get_api_keys.call_args_list == [ - call("596364a0-858e-42c8-9062-a8fe822260eb"), - ] + post.assert_called_once_with( + url="/service/{}/api-key".format(SERVICE_ONE_ID), + data={ + "name": key_name_from_user, + "key_type": "normal", + "created_by": api_user_active["id"], + }, + ) + def test_cant_create_normal_api_key_in_trial_mode( + self, + client_request, + api_user_active, + mock_login, + mock_get_api_keys, + mock_get_service, + mock_has_permissions, + fake_uuid, + mocker, + ): + mock_post = mocker.patch("app.notify_client.api_key_api_client.ApiKeyApiClient.post") + + client_request.post( + "main.create_api_key", + service_id=SERVICE_ONE_ID, + _data={"key_name": "some default key name", "key_type": "normal"}, + _expected_status=400, + ) + mock_post.assert_not_called() + + def test_should_show_confirm_revoke_api_key( + self, + client_request, + mock_get_api_keys, + fake_uuid, + ): + page = client_request.get( + "main.revoke_api_key", + service_id=SERVICE_ONE_ID, + key_id=fake_uuid, + _test_page_title=False, + ) + assert normalize_spaces(page.select(".banner-dangerous")[0].text) == ( + "Are you sure you want to revoke ‘some key name’? " + "You will not be able to use this API key to connect to GC Notify " + "Yes, revoke this API key" + ) + assert mock_get_api_keys.call_args_list == [ + call("596364a0-858e-42c8-9062-a8fe822260eb"), + ] -def test_should_show_confirm_revoke_api_key_for_platform_admin( - platform_admin_client, - mock_get_api_keys, - fake_uuid, -): - url = url_for( - "main.revoke_api_key", - service_id=SERVICE_ONE_ID, - key_id=fake_uuid, - _test_page_title=False, - ) - response = platform_admin_client.get(url) - page = BeautifulSoup(response.data.decode("utf-8"), "html.parser") - assert normalize_spaces(page.select(".banner-dangerous")[0].text) == ( - "Are you sure you want to revoke ‘some key name’? " - "You will not be able to use this API key to connect to GC Notify " - "Yes, revoke this API key" - ) - assert mock_get_api_keys.call_args_list == [ - call("596364a0-858e-42c8-9062-a8fe822260eb"), - ] + def test_should_show_confirm_revoke_api_key_for_platform_admin( + self, + platform_admin_client, + mock_get_api_keys, + fake_uuid, + ): + url = url_for( + "main.revoke_api_key", + service_id=SERVICE_ONE_ID, + key_id=fake_uuid, + _test_page_title=False, + ) + response = platform_admin_client.get(url) + page = BeautifulSoup(response.data.decode("utf-8"), "html.parser") + assert normalize_spaces(page.select(".banner-dangerous")[0].text) == ( + "Are you sure you want to revoke ‘some key name’? " + "You will not be able to use this API key to connect to GC Notify " + "Yes, revoke this API key" + ) + assert mock_get_api_keys.call_args_list == [ + call("596364a0-858e-42c8-9062-a8fe822260eb"), + ] + def test_should_404_for_api_key_that_doesnt_exist( + self, + client_request, + mock_get_api_keys, + ): + client_request.get( + "main.revoke_api_key", + service_id=SERVICE_ONE_ID, + key_id="key-doesn’t-exist", + _expected_status=404, + ) -def test_should_404_for_api_key_that_doesnt_exist( - client_request, - mock_get_api_keys, -): - client_request.get( - "main.revoke_api_key", - service_id=SERVICE_ONE_ID, - key_id="key-doesn’t-exist", - _expected_status=404, - ) + def test_should_redirect_after_revoking_api_key( + self, + client_request, + api_user_active, + mock_login, + mock_revoke_api_key, + mock_get_api_keys, + mock_get_service, + mock_has_permissions, + fake_uuid, + ): + client_request.post( + "main.revoke_api_key", + service_id=SERVICE_ONE_ID, + key_id=fake_uuid, + _expected_status=302, + _expected_redirect=url_for( + ".api_keys", + service_id=SERVICE_ONE_ID, + ), + ) + mock_revoke_api_key.assert_called_once_with(service_id=SERVICE_ONE_ID, key_id=fake_uuid) + mock_get_api_keys.assert_called_once_with( + SERVICE_ONE_ID, + ) + @pytest.mark.parametrize("route", ["main.api_keys", "main.create_api_key", "main.revoke_api_key"]) + def test_route_permissions( + self, + mocker, + app_, + fake_uuid, + api_user_active, + service_one, + mock_get_api_keys, + route, + mock_get_api_key_statistics, + ): + with app_.test_request_context(): + validate_route_permission( + mocker, + app_, + "GET", + 200, + url_for(route, service_id=service_one["id"], key_id=fake_uuid), + ["manage_api_keys"], + api_user_active, + service_one, + ) + + @pytest.mark.parametrize("route", ["main.api_keys", "main.create_api_key", "main.revoke_api_key"]) + def test_route_invalid_permissions( + self, + mocker, + app_, + fake_uuid, + api_user_active, + service_one, + mock_get_api_keys, + route, + ): + with app_.test_request_context(): + validate_route_permission( + mocker, + app_, + "GET", + 403, + url_for(route, service_id=service_one["id"], key_id=fake_uuid), + ["view_activity"], + api_user_active, + service_one, + ) + + +class TestSafelist: + def test_should_update_safelist( + self, + client_request, + mock_update_safelist, + ): + data = OrderedDict( + [ + ("email_addresses-1", "test@example.com"), + ("email_addresses-3", "test@example.com"), + ("phone_numbers-0", "6502532222"), + ("phone_numbers-2", "+4966921809"), + ] + ) -def test_should_redirect_after_revoking_api_key( - client_request, - api_user_active, - mock_login, - mock_revoke_api_key, - mock_get_api_keys, - mock_get_service, - mock_has_permissions, - fake_uuid, -): - client_request.post( - "main.revoke_api_key", - service_id=SERVICE_ONE_ID, - key_id=fake_uuid, - _expected_status=302, - _expected_redirect=url_for( - ".api_keys", + client_request.post( + "main.safelist", service_id=SERVICE_ONE_ID, - ), - ) - mock_revoke_api_key.assert_called_once_with(service_id=SERVICE_ONE_ID, key_id=fake_uuid) - mock_get_api_keys.assert_called_once_with( - SERVICE_ONE_ID, - ) + _data=data, + ) + mock_update_safelist.assert_called_once_with( + SERVICE_ONE_ID, + { + "email_addresses": ["test@example.com", "test@example.com"], + "phone_numbers": ["6502532222", "+4966921809"], + }, + ) -@pytest.mark.parametrize("route", ["main.api_keys", "main.create_api_key", "main.revoke_api_key"]) -def test_route_permissions( - mocker, - app_, - fake_uuid, - api_user_active, - service_one, - mock_get_api_keys, - route, - mock_get_api_key_statistics, -): - with app_.test_request_context(): - validate_route_permission( - mocker, - app_, - "GET", - 200, - url_for(route, service_id=service_one["id"], key_id=fake_uuid), - ["manage_api_keys"], - api_user_active, - service_one, + def test_should_show_safelist_page( + self, + client_request, + mock_login, + api_user_active, + mock_get_service, + mock_has_permissions, + mock_get_safelist, + ): + page = client_request.get( + "main.safelist", + service_id=SERVICE_ONE_ID, + ) + textboxes = page.find_all("input", {"type": "email"}) + page.find_all("input", {"type": "tel"}) + for index, value in enumerate(["test@example.com"] + [""] * 4 + ["6502532222"] + [""] * 4): + assert textboxes[index]["value"] == value + + def test_should_validate_safelist_items( + self, + client_request, + mock_update_safelist, + ): + page = client_request.post( + "main.safelist", + service_id=SERVICE_ONE_ID, + _data=OrderedDict([("email_addresses-1", "abc"), ("phone_numbers-0", "123")]), + _expected_status=200, ) + assert page.select_one(".banner-title").string.strip() == "There was a problem with your safelist" + jump_links = page.select(".banner-dangerous a") -@pytest.mark.parametrize("route", ["main.api_keys", "main.create_api_key", "main.revoke_api_key"]) -def test_route_invalid_permissions( - mocker, - app_, - fake_uuid, - api_user_active, - service_one, - mock_get_api_keys, - route, -): - with app_.test_request_context(): - validate_route_permission( - mocker, - app_, - "GET", - 403, - url_for(route, service_id=service_one["id"], key_id=fake_uuid), - ["view_activity"], - api_user_active, - service_one, - ) + assert jump_links[0].string.strip() == "Enter valid email addresses" + assert jump_links[0]["href"] == "#email_addresses" + assert jump_links[1].string.strip() == "Enter valid phone numbers" + assert jump_links[1]["href"] == "#phone_numbers" -def test_should_show_safelist_page( - client_request, - mock_login, - api_user_active, - mock_get_service, - mock_has_permissions, - mock_get_safelist, -): - page = client_request.get( - "main.safelist", - service_id=SERVICE_ONE_ID, - ) - textboxes = page.find_all("input", {"type": "email"}) + page.find_all("input", {"type": "tel"}) - for index, value in enumerate(["test@example.com"] + [""] * 4 + ["6502532222"] + [""] * 4): - assert textboxes[index]["value"] == value + assert mock_update_safelist.called is False -def test_should_update_safelist( - client_request, - mock_update_safelist, -): - data = OrderedDict( +class TestApiCallbacks: + @pytest.mark.parametrize( + "endpoint", [ - ("email_addresses-1", "test@example.com"), - ("email_addresses-3", "test@example.com"), - ("phone_numbers-0", "6502532222"), - ("phone_numbers-2", "+4966921809"), - ] - ) - - client_request.post( - "main.safelist", - service_id=SERVICE_ONE_ID, - _data=data, + ("main.delivery_status_callback"), + ("main.received_text_messages_callback"), + ], ) + @pytest.mark.parametrize( + "url, bearer_token, response, expected_errors", + [ + ("https://example.com", "", None, "This cannot be empty"), + ("http://not_https.com", "1234567890", None, "Enter a URL that starts with https://"), + ( + "https://test.com", + "123456789", + {"content": "a", "status_code": 500, "headers": {"a": "a"}}, + "Must be at least 10 characters", + ), + ( + "https://test.ee", + "1234567890", + {"content": "a", "status_code": 404, "headers": {"a": "a"}}, + "Check your service is running and not using a proxy we cannot access", + ), + ], + ) + def test_callback_forms_validation( + self, + client_request, + service_one, + mock_get_valid_service_callback_api, + mock_validate_callback_url, + endpoint, + url, + bearer_token, + response, + expected_errors, + mocker, + ): + if endpoint == "main.received_text_messages_callback": + service_one["permissions"] = ["inbound_sms"] + + data = { + "url": url, + "bearer_token": bearer_token, + } + if response: + resp = Mock(content=response["content"], status_code=response["status_code"], headers=response["headers"]) + mocker.patch("app.main.validators.requests.post", return_value=resp) - mock_update_safelist.assert_called_once_with( - SERVICE_ONE_ID, - { - "email_addresses": ["test@example.com", "test@example.com"], - "phone_numbers": ["6502532222", "+4966921809"], - }, - ) + response = client_request.post(endpoint, service_id=service_one["id"], _data=data, _expected_status=200) + error_msgs = " ".join(msg.text.strip() for msg in response.select(".error-message")) + assert error_msgs == expected_errors -def test_should_validate_safelist_items( - client_request, - mock_update_safelist, -): - page = client_request.post( - "main.safelist", - service_id=SERVICE_ONE_ID, - _data=OrderedDict([("email_addresses-1", "abc"), ("phone_numbers-0", "123")]), - _expected_status=200, + @pytest.mark.parametrize( + "endpoint", + [ + ("main.delivery_status_callback"), + ("main.received_text_messages_callback"), + ], ) + def test_callback_response_time_banner_shows_error_when_response_time_greater_than_one_second( + self, + endpoint, + fake_uuid, + client_request, + service_one, + mock_get_valid_service_callback_api, + mock_get_valid_service_inbound_api, + mocker, + ): + mocker.patch( + "app.main.validators.requests.post", return_value=Mock(elapsed=datetime.timedelta(seconds=1.1), status_code=200) + ) - assert page.select_one(".banner-title").string.strip() == "There was a problem with your safelist" - jump_links = page.select(".banner-dangerous a") - - assert jump_links[0].string.strip() == "Enter valid email addresses" - assert jump_links[0]["href"] == "#email_addresses" - - assert jump_links[1].string.strip() == "Enter valid phone numbers" - assert jump_links[1]["href"] == "#phone_numbers" + if endpoint == "main.received_text_messages_callback": + service_one["permissions"] = ["inbound_sms"] + service_one["inbound_api"] = [fake_uuid] + url = "https://hello3.canada.ca" + else: + service_one["service_callback_api"] = [fake_uuid] + url = "https://hello2.canada.ca" + + data = { + "url": url, + "bearer_token": "bearer_token_set", + "button_pressed": "test_response_time", + } + + response = client_request.post( + endpoint, + service_id=service_one["id"], + _data=data, + _follow_redirects=True, + ) - assert mock_update_safelist.called is False + expected_banner_msg = f"The service {url.split('https://')[1]} took longer than 1 second to respond." + page = BeautifulSoup(response.decode("utf-8"), "html.parser") + banner_msg = normalize_spaces(page.select(".banner-dangerous")[0].text) + assert banner_msg == expected_banner_msg -@pytest.mark.parametrize( - "endpoint", - [ - ("main.delivery_status_callback"), - ("main.received_text_messages_callback"), - ], -) -@pytest.mark.parametrize( - "url, bearer_token, expected_errors", - [ - ("https://example.com", "", "This cannot be empty"), - ("http://not_https.com", "1234567890", "Must be a valid https URL"), - ("https://test.com", "123456789", "Must be at least 10 characters"), - ], -) -def test_callback_forms_validation( - client_request, - service_one, - mock_get_valid_service_callback_api, - endpoint, - url, - bearer_token, - expected_errors, -): - if endpoint == "main.received_text_messages_callback": + @pytest.mark.parametrize( + "endpoint, expected_delete_url", + [ + ( + "main.delete_delivery_status_callback", + "/service/{}/delivery-receipt-api/{}", + ), + ( + "main.delete_received_text_messages_callback", + "/service/{}/inbound-api/{}", + ), + ], + ) + def test_delete_delivery_status_and_receive_text_message_callbacks( + self, + client_request, + service_one, + endpoint, + expected_delete_url, + mocker, + fake_uuid, + mock_get_valid_service_callback_api, + mock_get_valid_service_inbound_api, + ): + service_one["service_callback_api"] = [fake_uuid] + service_one["inbound_api"] = [fake_uuid] service_one["permissions"] = ["inbound_sms"] + mocked_delete = mocker.patch("app.service_api_client.delete") - data = { - "url": url, - "bearer_token": bearer_token, - } - - response = client_request.post(endpoint, service_id=service_one["id"], _data=data, _expected_status=200) - error_msgs = " ".join(msg.text.strip() for msg in response.select(".error-message")) - - assert error_msgs == expected_errors - - -@pytest.mark.parametrize("bearer_token", ["", "some-bearer-token"]) -@pytest.mark.parametrize( - "endpoint, expected_delete_url", - [ - ( - "main.delivery_status_callback", - "/service/{}/delivery-receipt-api/{}", - ), - ( - "main.received_text_messages_callback", - "/service/{}/inbound-api/{}", - ), - ], -) -def test_callback_forms_can_be_cleared( - client_request, - service_one, - endpoint, - expected_delete_url, - bearer_token, - mocker, - fake_uuid, - mock_get_valid_service_callback_api, - mock_get_valid_service_inbound_api, -): - service_one["service_callback_api"] = [fake_uuid] - service_one["inbound_api"] = [fake_uuid] - service_one["permissions"] = ["inbound_sms"] - mocked_delete = mocker.patch("app.service_api_client.delete") - - page = client_request.post( - endpoint, - service_id=service_one["id"], - _data={ - "url": "", - "bearer_token": bearer_token, - }, - _expected_redirect=url_for( - "main.api_callbacks", + page = client_request.post( + endpoint, service_id=service_one["id"], - ), - ) + _follow_redirects=True, + ) - assert not page.select(".error-message") - mocked_delete.assert_called_once_with(expected_delete_url.format(service_one["id"], fake_uuid)) + assert not page.select(".error-message") + mocked_delete.assert_called_once_with(expected_delete_url.format(service_one["id"], fake_uuid)) + @pytest.mark.parametrize( + "has_inbound_sms, expected_link", + [ + (True, "main.api_callbacks"), + (False, "main.delivery_status_callback"), + ], + ) + def test_callbacks_button_links_straight_to_delivery_status_if_service_has_no_inbound_sms( + self, + client_request, + service_one, + mocker, + mock_get_notifications, + has_inbound_sms, + expected_link, + ): + if has_inbound_sms: + service_one["permissions"] = ["inbound_sms"] + + page = client_request.get( + "main.api_integration", + service_id=service_one["id"], + ) -@pytest.mark.parametrize("bearer_token", ["", "some-bearer-token"]) -@pytest.mark.parametrize( - "endpoint, expected_delete_url", - [ - ( - "main.delivery_status_callback", - "/service/{}/delivery-receipt-api/{}", - ), - ( - "main.received_text_messages_callback", - "/service/{}/inbound-api/{}", - ), - ], -) -def test_callback_forms_can_be_cleared_when_callback_and_inbound_apis_are_empty( - client_request, - service_one, - endpoint, - expected_delete_url, - bearer_token, - mocker, - mock_get_empty_service_callback_api, - mock_get_empty_service_inbound_api, -): - service_one["permissions"] = ["inbound_sms"] - mocked_delete = mocker.patch("app.service_api_client.delete") + assert page.select(".api-header-links")[2]["href"] == url_for(expected_link, service_id=service_one["id"]) - page = client_request.post( - endpoint, - service_id=service_one["id"], - _data={ - "url": "", - "bearer_token": bearer_token, - }, - _expected_redirect=url_for( + def test_callbacks_page_redirects_to_delivery_status_if_service_has_no_inbound_sms( + self, + client_request, + service_one, + mocker, + mock_get_valid_service_callback_api, + ): + page = client_request.get( "main.api_callbacks", service_id=service_one["id"], - ), - ) - - assert not page.select(".error-message") - assert mocked_delete.call_args_list == [] - - -@pytest.mark.parametrize( - "has_inbound_sms, expected_link", - [ - (True, "main.api_callbacks"), - (False, "main.delivery_status_callback"), - ], -) -def test_callbacks_button_links_straight_to_delivery_status_if_service_has_no_inbound_sms( - client_request, - service_one, - mocker, - mock_get_notifications, - has_inbound_sms, - expected_link, -): - if has_inbound_sms: - service_one["permissions"] = ["inbound_sms"] - - page = client_request.get( - "main.api_integration", - service_id=service_one["id"], - ) - - assert page.select(".api-header-links")[2]["href"] == url_for(expected_link, service_id=service_one["id"]) - - -def test_callbacks_page_redirects_to_delivery_status_if_service_has_no_inbound_sms( - client_request, - service_one, - mocker, - mock_get_valid_service_callback_api, -): - page = client_request.get( - "main.api_callbacks", - service_id=service_one["id"], - _follow_redirects=True, - ) - - assert normalize_spaces(page.select_one("h1").text) == "Callbacks for delivery receipts" - + _follow_redirects=True, + ) -@pytest.mark.parametrize( - "has_inbound_sms, expected_link", - [ - (True, "main.api_callbacks"), - (False, "main.api_integration"), - ], -) -def test_back_link_directs_to_api_integration_from_delivery_callback_if_no_inbound_sms( - client_request, service_one, mocker, has_inbound_sms, expected_link -): - if has_inbound_sms: - service_one["permissions"] = ["inbound_sms"] + assert normalize_spaces(page.select_one("h1").text) == "Callbacks for delivery receipts" - page = client_request.get( - "main.delivery_status_callback", - service_id=service_one["id"], - _follow_redirects=True, + @pytest.mark.parametrize( + "has_inbound_sms, expected_link", + [ + (True, "main.api_callbacks"), + (False, "main.api_integration"), + ], ) + def test_back_link_directs_to_api_integration_from_delivery_callback_if_no_inbound_sms( + self, client_request, service_one, mocker, has_inbound_sms, expected_link + ): + if has_inbound_sms: + service_one["permissions"] = ["inbound_sms"] - assert page.select_one(".back-link")["href"] == url_for(expected_link, service_id=service_one["id"]) - - -@pytest.mark.parametrize( - "endpoint", - [ - ("main.delivery_status_callback"), - ("main.received_text_messages_callback"), - ], -) -def test_create_delivery_status_and_receive_text_message_callbacks( - client_request, - service_one, - mocker, - mock_get_notifications, - mock_create_service_inbound_api, - mock_create_service_callback_api, - endpoint, - fake_uuid, -): - if endpoint == "main.received_text_messages_callback": - service_one["permissions"] = ["inbound_sms"] + page = client_request.get( + "main.delivery_status_callback", + service_id=service_one["id"], + _follow_redirects=True, + ) - data = { - "url": "https://test.url.com/", - "bearer_token": "1234567890", - "user_id": fake_uuid, - } + assert page.select_one(".back-link")["href"] == url_for(expected_link, service_id=service_one["id"]) - client_request.post( + @pytest.mark.parametrize( + "endpoint", + [ + ("main.delivery_status_callback"), + ("main.received_text_messages_callback"), + ], + ) + def test_create_delivery_status_and_receive_text_message_callbacks( + self, + client_request, + service_one, + mocker, + mock_get_notifications, + mock_create_service_inbound_api, + mock_create_service_callback_api, endpoint, - service_id=service_one["id"], - _data=data, - ) + fake_uuid, + mock_validate_callback_url, + ): + if endpoint == "main.received_text_messages_callback": + service_one["permissions"] = ["inbound_sms"] + + data = { + "url": "https://test.url.com/", + "bearer_token": "1234567890", + "user_id": fake_uuid, + } + + client_request.post( + endpoint, + service_id=service_one["id"], + _data=data, + ) - if endpoint == "main.received_text_messages_callback": - mock_create_service_inbound_api.assert_called_once_with( - service_one["id"], - url="https://test.url.com/", - bearer_token="1234567890", - user_id=fake_uuid, + if endpoint == "main.received_text_messages_callback": + mock_create_service_inbound_api.assert_called_once_with( + service_one["id"], + url="https://test.url.com/", + bearer_token="1234567890", + user_id=fake_uuid, + ) + else: + mock_create_service_callback_api.assert_called_once_with( + service_one["id"], + url="https://test.url.com/", + bearer_token="1234567890", + user_id=fake_uuid, + ) + + def test_update_delivery_status_callback_details( + self, + client_request, + service_one, + mock_update_service_callback_api, + mock_get_valid_service_callback_api, + fake_uuid, + mock_validate_callback_url, + mocker, + ): + service_one["service_callback_api"] = [fake_uuid] + + data = { + "url": "https://test.url.com/", + "bearer_token": "1234567890", + "user_id": fake_uuid, + } + + client_request.post( + "main.delivery_status_callback", + service_id=service_one["id"], + _data=data, ) - else: - mock_create_service_callback_api.assert_called_once_with( + + mock_update_service_callback_api.assert_called_once_with( service_one["id"], url="https://test.url.com/", bearer_token="1234567890", user_id=fake_uuid, + callback_api_id=fake_uuid, ) + def test_update_receive_text_message_callback_details( + self, + client_request, + service_one, + mock_update_service_inbound_api, + mock_get_valid_service_inbound_api, + fake_uuid, + mock_validate_callback_url, + ): + service_one["inbound_api"] = [fake_uuid] + service_one["permissions"] = ["inbound_sms"] -def test_update_delivery_status_callback_details( - client_request, - service_one, - mock_update_service_callback_api, - mock_get_valid_service_callback_api, - fake_uuid, -): - service_one["service_callback_api"] = [fake_uuid] - - data = { - "url": "https://test.url.com/", - "bearer_token": "1234567890", - "user_id": fake_uuid, - } - - client_request.post( - "main.delivery_status_callback", - service_id=service_one["id"], - _data=data, - ) - - mock_update_service_callback_api.assert_called_once_with( - service_one["id"], url="https://test.url.com/", bearer_token="1234567890", user_id=fake_uuid, callback_api_id=fake_uuid - ) - - -def test_update_receive_text_message_callback_details( - client_request, - service_one, - mock_update_service_inbound_api, - mock_get_valid_service_inbound_api, - fake_uuid, -): - service_one["inbound_api"] = [fake_uuid] - service_one["permissions"] = ["inbound_sms"] - - data = {"url": "https://test.url.com/", "bearer_token": "1234567890", "user_id": fake_uuid} - - client_request.post( - "main.received_text_messages_callback", - service_id=service_one["id"], - _data=data, - ) - - mock_update_service_inbound_api.assert_called_once_with( - service_one["id"], - url="https://test.url.com/", - bearer_token="1234567890", - user_id=fake_uuid, - inbound_api_id=fake_uuid, - ) - - -def test_update_delivery_status_callback_without_changes_does_not_update( - client_request, - service_one, - mock_update_service_callback_api, - fake_uuid, - mock_get_valid_service_callback_api, -): - service_one["service_callback_api"] = [fake_uuid] - data = {"user_id": fake_uuid, "url": "https://hello2.canada.ca", "bearer_token": "bearer_token_set"} - - client_request.post( - "main.delivery_status_callback", - service_id=service_one["id"], - _data=data, - ) - - assert mock_update_service_callback_api.called is False + data = {"url": "https://test.url.com/", "bearer_token": "1234567890", "user_id": fake_uuid} + client_request.post( + "main.received_text_messages_callback", + service_id=service_one["id"], + _data=data, + ) -def test_update_receive_text_message_callback_without_changes_does_not_update( - client_request, - service_one, - mock_update_service_inbound_api, - fake_uuid, - mock_get_valid_service_inbound_api, -): - service_one["inbound_api"] = [fake_uuid] - service_one["permissions"] = ["inbound_sms"] - data = {"user_id": fake_uuid, "url": "https://hello3.canada.ca", "bearer_token": "bearer_token_set"} - - client_request.post( - "main.received_text_messages_callback", - service_id=service_one["id"], - _data=data, - ) + mock_update_service_inbound_api.assert_called_once_with( + service_one["id"], + url="https://test.url.com/", + bearer_token="1234567890", + user_id=fake_uuid, + inbound_api_id=fake_uuid, + ) - assert mock_update_service_inbound_api.called is False + def test_update_delivery_status_callback_without_changes_does_not_update( + self, + client_request, + service_one, + mock_update_service_callback_api, + fake_uuid, + mock_get_valid_service_callback_api, + mock_validate_callback_url, + mocker, + ): + service_one["service_callback_api"] = [fake_uuid] + data = {"user_id": fake_uuid, "url": "https://hello2.canada.ca", "bearer_token": "bearer_token_set"} + + client_request.post( + "main.delivery_status_callback", + service_id=service_one["id"], + _data=data, + ) + assert mock_update_service_callback_api.called is False + + def test_update_receive_text_message_callback_without_changes_does_not_update( + self, + client_request, + service_one, + mock_update_service_inbound_api, + fake_uuid, + mock_get_valid_service_inbound_api, + mock_validate_callback_url, + ): + service_one["inbound_api"] = [fake_uuid] + service_one["permissions"] = ["inbound_sms"] + data = {"user_id": fake_uuid, "url": "https://hello3.canada.ca", "bearer_token": "bearer_token_set"} -@pytest.mark.parametrize( - "service_callback_api, delivery_url, expected_1st_table_row", - [ - (None, {}, "Delivery receipts Not set Change"), - ( - sample_uuid(), - {"url": "https://delivery.receipts"}, - "Delivery receipts https://delivery.receipts Change", - ), - ], -) -@pytest.mark.parametrize( - "inbound_api, inbound_url, expected_2nd_table_row", - [ - (None, {}, "Received text messages Not set Change"), - ( - sample_uuid(), - {"url": "https://inbound.sms"}, - "Received text messages https://inbound.sms Change", - ), - ], -) -def test_callbacks_page_works_when_no_apis_set( - client_request, - service_one, - mocker, - service_callback_api, - delivery_url, - expected_1st_table_row, - inbound_api, - inbound_url, - expected_2nd_table_row, -): - service_one["permissions"] = ["inbound_sms"] - service_one["inbound_api"] = inbound_api - service_one["service_callback_api"] = service_callback_api + client_request.post( + "main.received_text_messages_callback", + service_id=service_one["id"], + _data=data, + ) - mocker.patch("app.service_api_client.get_service_callback_api", return_value=delivery_url) - mocker.patch("app.service_api_client.get_service_inbound_api", return_value=inbound_url) + assert mock_update_service_inbound_api.called is False - page = client_request.get("main.api_callbacks", service_id=service_one["id"], _follow_redirects=True) - expected_rows = [ + @pytest.mark.parametrize( + "service_callback_api, delivery_url, expected_1st_table_row", + [ + (None, {}, "Delivery receipts Not set Change"), + ( + sample_uuid(), + {"url": "https://delivery.receipts"}, + "Delivery receipts https://delivery.receipts Change", + ), + ], + ) + @pytest.mark.parametrize( + "inbound_api, inbound_url, expected_2nd_table_row", + [ + (None, {}, "Received text messages Not set Change"), + ( + sample_uuid(), + {"url": "https://inbound.sms"}, + "Received text messages https://inbound.sms Change", + ), + ], + ) + def test_callbacks_page_works_when_no_apis_set( + self, + client_request, + service_one, + mocker, + service_callback_api, + delivery_url, expected_1st_table_row, + inbound_api, + inbound_url, expected_2nd_table_row, - ] - rows = page.select("tbody tr") - assert len(rows) == 2 - for index, row in enumerate(expected_rows): - assert row == normalize_spaces(rows[index].text) + ): + service_one["permissions"] = ["inbound_sms"] + service_one["inbound_api"] = inbound_api + service_one["service_callback_api"] = service_callback_api + + mocker.patch("app.service_api_client.get_service_callback_api", return_value=delivery_url) + mocker.patch("app.service_api_client.get_service_inbound_api", return_value=inbound_url) + + page = client_request.get("main.api_callbacks", service_id=service_one["id"], _follow_redirects=True) + expected_rows = [ + expected_1st_table_row, + expected_2nd_table_row, + ] + rows = page.select("tbody tr") + assert len(rows) == 2 + for index, row in enumerate(expected_rows): + assert row == normalize_spaces(rows[index].text) diff --git a/tests/conftest.py b/tests/conftest.py index d2016b8207..529587d870 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -3734,6 +3734,14 @@ def mock_get_empty_service_callback_api(mocker): ) +@pytest.fixture(scope="function") +def mock_validate_callback_url(mocker): + return mocker.patch( + "app.main.validators.requests.post", + return_value=Mock(content="a", status_code=200, headers={"a": "a"}, elapsed=timedelta(seconds=0.3)), + ) + + @pytest.fixture(scope="function") def mock_create_service_inbound_api(mocker): def _create_service_inbound_api(service_id, url, bearer_token, user_id):