Skip to content

Commit

Permalink
Enhance UX and add features to the Callback pages (#1855)
Browse files Browse the repository at this point in the history
* Add basic validation to callbacks URLs

- Added the [validators](https://yozachar.github.io/pyvalidators/stable/) lib to the project
- Callback URLs are now checked for validity
- Callback URLs are now tested to determine if they are reachable before saving the callback url
- Validate that callback urls return 200 responses

* Added dynamic hint text to the url field

* Various fixes

- Update error messages
- Add logic for catching 5xx and 4xx errors and raise the correct validation error + logging
- Fixed ServiceReceiveCallbackMessagesForm validation

* Fix tests

* Add placeholder translations

* Consider 5xx responses as valid

- We only want to verify that there's a service running at the specified callback URL.
- Additionally we are sending a payload the service won't understand how to response to so a 5xx is likely

* Improve the callback config UX

- There is now a dedicated delete button to remove a callback configuration instead of needing to empty the form and click save to delete a config
- Users will receive a confirmation banner before deletion occurs
- Saving, creating, and deleting a callback url now provide the user with a confirmation message that the operation succeeded

* Add callback test button

- Added a button to the callback config page so users can see the response times of their callback services
- Added some translation stubs
- The ping to their service takes place as part of the validation

* Unify delivery-status-callback and received-text-messages-callback pages

- Fix a few tests

* formatting fixes

* Fix tests

- Added placeholder FR translations
- Removed validation that allowed the form to be submitted while empty, this was how callbacks were deleted previously
- Added additional check to the make format task
- Updated EN translations
- Formatting

* Fix updated translations in code

* Add & fix tests

- Add new message content

* Update delete message

* Update french translations

* Update translations

- Fix issue with text displaying incorrectly on the delete confirmation interstitial

* Translations & refresh lock file

* Fix code QL issues

---------

Co-authored-by: Jumana B <[email protected]>
  • Loading branch information
whabanks and jzbahrai authored Sep 24, 2024
1 parent 9127f01 commit 5d71ce6
Show file tree
Hide file tree
Showing 12 changed files with 1,082 additions and 703 deletions.
1 change: 1 addition & 0 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
17 changes: 12 additions & 5 deletions app/main/forms.py
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
SelectField,
SelectMultipleField,
StringField,
SubmitField,
TextAreaField,
ValidationError,
validators,
Expand All @@ -49,6 +50,7 @@
LettersNumbersAndFullStopsOnly,
NoCommasInPlaceHolders,
OnlySMSCharacters,
ValidCallbackUrl,
ValidEmail,
ValidGovEmail,
validate_email_from,
Expand Down Expand Up @@ -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


Expand Down Expand Up @@ -1407,15 +1410,16 @@ 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):
url = StringField(
"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(
Expand All @@ -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(
Expand All @@ -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):
Expand Down Expand Up @@ -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"))],
)

Expand Down
60 changes: 57 additions & 3 deletions app/main/validators.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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

Expand All @@ -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:
Expand Down Expand Up @@ -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
Expand Down
Loading

0 comments on commit 5d71ce6

Please sign in to comment.