Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -1,17 +1,20 @@
import quibble from 'quibble';
import type { SinonStub } from 'sinon';
import userEvent from '@testing-library/user-event';
import baseUserEvent from '@testing-library/user-event';
import { screen, waitFor, fireEvent } from '@testing-library/dom';
import { useSandbox, useDefineProperty } from '@18f/identity-test-helpers';
import '@18f/identity-spinner-button/spinner-button-element';

describe('CaptchaSubmitButtonElement', () => {
const sandbox = useSandbox();
let FAILED_LOAD_DELAY_MS: number;
const sandbox = useSandbox({ useFakeTimers: true });
const { clock } = sandbox;
const userEvent = baseUserEvent.setup({ advanceTimers: clock.tick });
const trackError = sandbox.stub();

before(async () => {
quibble('@18f/identity-analytics', { trackError });
await import('./captcha-submit-button-element');
({ FAILED_LOAD_DELAY_MS } = await import('./captcha-submit-button-element'));
});

afterEach(() => {
Expand Down Expand Up @@ -117,7 +120,6 @@ describe('CaptchaSubmitButtonElement', () => {
await userEvent.click(button);
await waitFor(() => expect((form.submit as SinonStub).called).to.be.true());

expect(grecaptcha.ready).to.have.been.called();
expect(grecaptcha.execute).to.have.been.calledWith(RECAPTCHA_SITE_KEY, {
action: RECAPTCHA_ACTION_NAME,
});
Expand All @@ -126,6 +128,57 @@ describe('CaptchaSubmitButtonElement', () => {
});
});

context('with recaptcha not loaded by time of submission', () => {
beforeEach(() => {
delete (global as any).grecaptcha;
});

it('enqueues the challenge callback to be run once recaptcha loads', async () => {
const button = screen.getByRole('button', { name: 'Submit' });
const form = document.querySelector('form')!;
sandbox.stub(form, 'submit');

await userEvent.click(button);

expect(form.submit).not.to.have.been.called();
/* eslint-disable no-underscore-dangle */
expect((globalThis as any).___grecaptcha_cfg).to.have.keys('fns');
expect((globalThis as any).___grecaptcha_cfg.fns)
.to.be.an('array')
.with.lengthOf.greaterThan(0);
(globalThis as any).___grecaptcha_cfg.fns.forEach((callback) => callback());
/* eslint-enable no-underscore-dangle */

await expect(form.submit).to.eventually.be.called();
});
});

context('with only recaptcha loader script loaded by time of submission', () => {
// The loader script will define the `grecaptcha` global and `ready` function, but it will
// not define `execute`.
beforeEach(() => {
delete (global as any).grecaptcha.execute;
delete (global as any).grecaptcha.enterprise.execute;
});

it('enqueues the challenge callback to be run once recaptcha loads', async () => {
const button = screen.getByRole('button', { name: 'Submit' });
const form = document.querySelector('form')!;
sandbox.stub(form, 'submit');

await userEvent.click(button);

expect(grecaptcha.ready).to.have.been.called();

// Simulate reCAPTCHA full script loaded
(global as any).grecaptcha.execute = sandbox.stub().resolves(RECAPTCHA_TOKEN_VALUE);
const callback = (grecaptcha.ready as SinonStub).getCall(0).args[0];
callback();

await expect(form.submit).to.eventually.be.called();
});
});

context('with recaptcha enterprise', () => {
beforeEach(() => {
const element = document.querySelector('lg-captcha-submit-button')!;
Expand All @@ -141,7 +194,6 @@ describe('CaptchaSubmitButtonElement', () => {
await userEvent.click(button);
await waitFor(() => expect((form.submit as SinonStub).called).to.be.true());

expect(grecaptcha.enterprise.ready).to.have.been.called();
expect(grecaptcha.enterprise.execute).to.have.been.calledWith(RECAPTCHA_SITE_KEY, {
action: RECAPTCHA_ACTION_NAME,
});
Expand All @@ -156,19 +208,18 @@ describe('CaptchaSubmitButtonElement', () => {
delete (global as any).grecaptcha;
});

it('does not prevent default form submission', async () => {
it('submits the form if recaptcha is still not loaded after reasonable delay', async () => {
const button = screen.getByRole('button', { name: 'Submit' });
const form = document.querySelector('form')!;

let didSubmit = false;
form.addEventListener('submit', (event) => {
expect(event.defaultPrevented).to.equal(false);
event.preventDefault();
didSubmit = true;
});
sandbox.stub(form, 'submit');

await userEvent.click(button);
await waitFor(() => expect(didSubmit).to.be.true());

expect(form.submit).not.to.have.been.called();
clock.tick(FAILED_LOAD_DELAY_MS - 1);
expect(form.submit).not.to.have.been.called();
clock.tick(1);
expect(form.submit).to.have.been.called();
});
});

Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,11 @@
import { trackError } from '@18f/identity-analytics';

/**
* Maximum time (in milliseconds) to wait on reCAPTCHA to finish loading once a form is submitted
* before considering reCAPTCHA as having failed to load.
*/
export const FAILED_LOAD_DELAY_MS = 5_000;

class CaptchaSubmitButtonElement extends HTMLElement {
form: HTMLFormElement | null;

Expand Down Expand Up @@ -46,7 +52,7 @@ class CaptchaSubmitButtonElement extends HTMLElement {
}

invokeChallenge() {
this.recaptchaClient!.ready(async () => {
this.#onReady(async () => {
const { recaptchaSiteKey: siteKey, recaptchaAction: action } = this;

let token;
Expand All @@ -62,7 +68,7 @@ class CaptchaSubmitButtonElement extends HTMLElement {
}

shouldInvokeChallenge(): boolean {
return !!(this.recaptchaSiteKey && this.recaptchaClient);
return !!this.recaptchaSiteKey;
}

handleFormSubmit = (event: SubmitEvent) => {
Expand All @@ -71,6 +77,25 @@ class CaptchaSubmitButtonElement extends HTMLElement {
this.invokeChallenge();
}
};

#onReady(callback: Parameters<ReCaptchaV2.ReCaptcha['ready']>[0]) {
if (this.recaptchaClient) {
this.recaptchaClient.ready(callback);
} else {
// If reCAPTCHA hasn't finished loading by the time the form is submitted, we can enqueue the
// callback to be invoked once loaded by appending a callback to the ___grecaptcha_cfg global.
//
// See: https://developers.google.com/recaptcha/docs/loading

const failedLoadTimeoutId = setTimeout(() => this.submit(), FAILED_LOAD_DELAY_MS);
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does this mean we wait an additional 5 seconds? That seems like a very long time

Copy link
Copy Markdown
Contributor Author

@aduth aduth Nov 13, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The logic here is meant to be a race: Either reCAPTCHA finishes loading in the next 5 seconds (and we cancel this timeout), or the form submits (without the reCAPTCHA token). From a user's perspective, the form will submit as soon as reCAPTCHA is finished its assessment.

We could also just remove this logic, but then it risks reCAPTCHA failing to load altogether and the submission appearing to hang indefinitely. They'll be unsuccessful if they submit without a reCAPTCHA token, but at least it gives them a path forward.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it's important to keep things moving, but my guess is that if the page is responsive enough for the user to submit, but the reCAPTCHA hasn't loaded yet, it probably won't? I would have added like a 1 second delay

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've got a few other optimizations in the pipeline to drive the load time down further that we could be more aggressive, but I'm hoping this is exceedingly unlikely as it is to not worry too much about it.

const clearFailedLoadTimeout = () => clearTimeout(failedLoadTimeoutId);

/* eslint-disable no-underscore-dangle */
globalThis.___grecaptcha_cfg ??= { fns: [] };
globalThis.___grecaptcha_cfg.fns.push(clearFailedLoadTimeout, callback);
/* eslint-enable no-underscore-dangle */
}
}
}

declare global {
Expand Down