Fix: reCAPTCHA + HTML5 form validation #1071

thekaveman · 2022-10-14T22:04:38Z

Based on an answer to this SO post: https://stackoverflow.com/q/41665935/453168

What this PR does

Uses programmatic reCAPTCHA validation

This is the main issue being solved. reCAPTCHA v3 supports two options:

Automatically bind the challenge to a button (existing implementation)
Programmatically invoke the challenge (new implementation in this PR)

Using a programmatic challenge allows the browser's HTML5 form validation a chance to kick-in before reCAPTCHA's checks,
which isn't possible with the challenge button auto-bind.

Refactors how templates get reCAPTCHA data

Before we used a Django context processor, which gave the data to every request context, even those that didn't need it:

def recaptcha(request):
    """Context processor adds recaptcha information to request context."""
    return {
        "recaptcha": {
            "api_url": settings.RECAPTCHA_API_URL,
            "enabled": settings.RECAPTCHA_ENABLED,
            "site_key": settings.RECAPTCHA_SITE_KEY,
        }
    }

This PR introduces a Middleware, following our usual pattern where we can apply the middleware per-view that needs it, which adds the reCAPTCHA data directly to the request:

class RecaptchaEnabled(MiddlewareMixin):
    """Middleware configures the request with required reCAPTCHA settings."""

    def process_request(self, request):
        if settings.RECAPTCHA_ENABLED:
            request.recaptcha = {
                "data_field": recaptcha.DATA_FIELD,
                "script_api": settings.RECAPTCHA_API_KEY_URL,
                "site_key": settings.RECAPTCHA_SITE_KEY,
            }
        return None

# later in views.py

@decorator_from_middleware(RecaptchaEnabled)
def confirm(request):
    """View handler for the eligibility verification form."""
    # etc.

This makes it more explicit which views use reCAPTCHA, and also makes using reCAPTCHA data easy from templates. Like in base.html where we include the reCAPTCHA API library only once and only if needed:

<html>
    <body>
        <!-- etc -->

        {% if request.recaptcha %}<script src="{{ request.recaptcha.script_api }}"></script>{% endif %}
    </body>
</html>

Makes the `eligibility:unverified` path a redirect

This has a couple advantages:

(the reason for this change): creates a new request, clearing reCAPTCHA data from prior /eligiblity/confirm request
Mirrors how the eligibility:verified path works (ultimately a redirect to enrollment:index)
URL is now /eligiblity/unverified instead of /eligibility/confirm, a more accurate UX

Testing this PR

Setup

Add this to your .vscode/launch.json under the Django: Benefits Client, Debug=False entry's env:

{
  "name": "Django: Benefits Client, Debug=False",
  // other fields...
  "env": {
    // existing fields...
   // add these 2 entries with the values for the test reCAPTCHA site
    "DJANGO_RECAPTCHA_SITE_KEY": "<SITE KEY HERE>",
    "DJANGO_RECAPTCHA_SECRET_KEY": "<SECRET KEY HERE>"
  }
}

Eligibility index

reCAPTCHA library included once at the bottom of <body>:

HTML5 validation works, form submits with valid data:

Eligibility confirm

reCAPTCHA library included once at the bottom of <body>:

HTML5 validation works, form submits with valid data:

Other pages

Every other page should not have the reCAPTCHA library or any reCAPTCHA handling for form elements.

E.g. the homepage does not include reCAPTCHA:

Nor does the eligibility:start page:

use as selector in JS

not multiple times per-form

we don't need the recaptcha attributes on this button

allows HTML5 field validation to occur before the recaptcha check adapted from https://stackoverflow.com/a/63290578/453168

* adds settings directly to request.recaptcha * middleware instead of context processor --> can be applied per-view

only present if recaptcha enabled for the view with the form

ensure the verifier selection form checks the recaptcha value

otherwise returning the view function directly uses the same request object, with the recaptcha data from the original confirm request this also causes the URL to change from /eligibility/confirm to /eligibility/unverified (wasn't shown before, but is a valid route)

machikoyasuda · 2022-10-24T19:43:20Z

I just wrote these notes for myself to help document what the app is doing for forms and what JS each form has

These are the 4 forms that use the form.html includes on the app and their corresponding IDs:

#verifier-selection - has recaptcha
#eligibility-verification - has recaptcha, has button name change
#card-tokenize-fail - does not have recaptcha
#card-tokenize-success - does not have recaptcha

The header's language selector button is also a form, but it does not use form.html.

machikoyasuda

Really like the way the JS in form.html is cleaned up! I also really like the instructions/documentation on how to run recaptcha locally - this will be very helpful.

I added the note comment there just to make sure we never make a CSS ID that collides with the ID name of a form. (ie - Ensuring we never make another #verifier-selection ID).

Once this PR is merged, I believe the unverified page should have the proper new layout that is on dev already.

thekaveman · 2022-10-24T20:08:00Z

I added the note comment there just to make sure we never make a CSS ID that collides with the ID name of a form

That's a good point - we could prefix these new form IDs with form- to make that more clear and less likely for a collision?

angela-tran

Nice refactoring!

machikoyasuda · 2022-10-24T20:24:38Z

I added the note comment there just to make sure we never make a CSS ID that collides with the ID name of a form

That's a good point - we could prefix these new form IDs with form- to make that more clear and less likely for a collision?

Yeah. This could be done in another PR.

help avoid future CSS selector collision

thekaveman added 9 commits October 13, 2022 23:52

feat(forms): add and render a form id

bb7cf18

use as selector in JS

feat(forms): feature flag recaptcha per-form

4b016bf

refactor(recaptcha): include API library once in base

804b5b5

not multiple times per-form

chore(forms): simplify submit render

1f84f74

we don't need the recaptcha attributes on this button

refactor(recaptcha): programmatic validation + form submission

79836b0

allows HTML5 field validation to occur before the recaptcha check adapted from https://stackoverflow.com/a/63290578/453168

refactor(recaptcha): configure settings in middleware

5239379

* adds settings directly to request.recaptcha * middleware instead of context processor --> can be applied per-view

refactor(forms): use recaptcha info on request

56a0904

only present if recaptcha enabled for the view with the form

refactor(recaptcha): enable on eligibility index

7e53c2f

ensure the verifier selection form checks the recaptcha value

feat(recaptcha): enable on eligiblity confirm

4bbaf05

thekaveman requested a review from a team as a code owner October 14, 2022 22:04

thekaveman added this to the Courtesy Cards milestone Oct 14, 2022

thekaveman self-assigned this Oct 14, 2022

thekaveman added 2 commits October 14, 2022 22:15

chore(html): fix spacing on inline javascript

efac987

thekaveman force-pushed the fix/recaptcha-html5-validation branch from 12737dc to efac987 Compare October 14, 2022 22:16

machikoyasuda previously approved these changes Oct 24, 2022

View reviewed changes

angela-tran previously approved these changes Oct 24, 2022

View reviewed changes

chore(forms): prefix ID for clarity

e783ce1

help avoid future CSS selector collision

thekaveman dismissed stale reviews from angela-tran and machikoyasuda via e783ce1 October 24, 2022 20:27

angela-tran approved these changes Oct 24, 2022

View reviewed changes

thekaveman merged commit fe247eb into dev Oct 24, 2022

thekaveman deleted the fix/recaptcha-html5-validation branch October 24, 2022 20:31

machikoyasuda mentioned this pull request Oct 24, 2022

Update docs site with details about debugging with Debug=False #929

Closed

1 task

This was referenced Oct 26, 2022

Feat: spinner on Check Status button #1088

Merged

HTMLFormElement.requestSubmit lacks browser support #1111

Closed

machikoyasuda mentioned this pull request Dec 7, 2022

Docs: attaching with DEBUG=False #1160

Merged

machikoyasuda mentioned this pull request Dec 16, 2024

reCAPTCHA: Elig Index, Elig Confirm - Add reCAPTCHA text, hide flag #2586

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix: reCAPTCHA + HTML5 form validation #1071

Fix: reCAPTCHA + HTML5 form validation #1071

thekaveman commented Oct 14, 2022

machikoyasuda commented Oct 24, 2022

machikoyasuda left a comment

thekaveman commented Oct 24, 2022

angela-tran left a comment

machikoyasuda commented Oct 24, 2022

Fix: reCAPTCHA + HTML5 form validation #1071

Fix: reCAPTCHA + HTML5 form validation #1071

Conversation

thekaveman commented Oct 14, 2022

What this PR does

Uses programmatic reCAPTCHA validation

Refactors how templates get reCAPTCHA data

Makes the eligibility:unverified path a redirect

Testing this PR

Setup

Eligibility index

Eligibility confirm

Other pages

machikoyasuda commented Oct 24, 2022

machikoyasuda left a comment

Choose a reason for hiding this comment

thekaveman commented Oct 24, 2022

angela-tran left a comment

Choose a reason for hiding this comment

machikoyasuda commented Oct 24, 2022

Makes the `eligibility:unverified` path a redirect