Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve race condition handling for slow reCAPTCHA load #11451

Open
wants to merge 5 commits into
base: main
Choose a base branch
from

Conversation

aduth
Copy link
Member

@aduth aduth commented Nov 5, 2024

🛠 Summary of changes

Updates CaptchaSubmitButton to better handle a scenario where reCAPTCHA is slow to load.

Currently, if reCAPTCHA is not loaded by the time the user submits the form, the form will submit immediately, which may produce a failing result from the backend if reCAPTCHA is being enforced.

This is rather unlikely to happen, since the initial reCAPTCHA script is small (roughly 2kb in size), but still technically possible, particularly if someone is using browser extensions or system features to autofill the sign-in form.

The proposed changes here take advantage of documented reCAPTCHA features to "queue" callbacks prior to reCAPTCHA being loaded†, with an additional fallback to submit the form if reCAPTCHA fails to load within a reasonable timeframe (currently 5 seconds).

†: While this feature is not explicitly documented in the reCAPTCHA Enterprise documentation, I've manually confirmed the presence of this queue behavior in the enterprise.js script.

📜 Testing Plan

Verify that reCAPTCHA validation is not impacted by these changes:

Configure reCAPTCHA for local development. You can use any credentials, they don't need to be valid.

# config/application.yml
development:
  sign_in_recaptcha_score_threshold: 0.3
  sign_in_recaptcha_percent_tested: 100
  recaptcha_site_key: 'test'
  recaptcha_enterprise_api_key: 'test'
  recaptcha_enterprise_project_id: 'test'
  recaptcha_mock_validator: false
  1. Go to http://localhost:3000
  2. Enter email and password
  3. Click "Sign in"
  4. Confirm that you are redirected to either MFA or directly to your account dashboard

Verify that the reCAPTCHA script failing to load doesn't prevent form submission:

You can simulate this by removing the reCAPTCHA script from CaptchaSubmitButtonComponent's markup:

diff --git a/app/components/captcha_submit_button_component.html.erb b/app/components/captcha_submit_button_component.html.erb
index 88e7033c4a..b6a7699b10 100644
--- a/app/components/captcha_submit_button_component.html.erb
+++ b/app/components/captcha_submit_button_component.html.erb
@@ -36,5 +36,2 @@
       ).with_content(content) %>
-  <% if recaptcha_script_src.present? %>
-    <%= content_tag(:script, '', src: recaptcha_script_src, async: true) %>
-  <% end %>
 <% end %>
  1. Go to http://localhost:3000
  2. Enter email and password
  3. Click "Sign in"
  4. After 5 seconds of the button showing as a spinner button, confirm that you are redirected to either MFA or directly to your account dashboard

@aduth aduth marked this pull request as ready for review November 5, 2024 14:53
@aduth aduth requested a review from a team November 5, 2024 14:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant