Skip to content

Include FormResponse error details for reCAPTCHA at sign-in#11456

Merged
aduth merged 3 commits intomainfrom
aduth-sign-in-recaptcha-form-errors
Nov 6, 2024
Merged

Include FormResponse error details for reCAPTCHA at sign-in#11456
aduth merged 3 commits intomainfrom
aduth-sign-in-recaptcha-form-errors

Conversation

@aduth
Copy link
Copy Markdown
Contributor

@aduth aduth commented Nov 5, 2024

🛠 Summary of changes

Updates sign-in analytics properties to include properties from SignInRecaptchaForm#to_h, notably error_details.

Why?

  • So that we can attribute failure reasons when a reCAPTCHA response isn't received due to form-specific validations like token presence
  • As an incremental step toward refactoring 'Email and Password Authentication' to include conventional FormResponse success and error details
    • For phone setup, we merge the reCAPTCHA result into the phone setup errors to ensure its errors are included in the corresponding "Multi-Factor Authentication Setup" event

📜 Testing Plan

rspec spec/controllers/users/sessions_controller_spec.rb

Verify that if reCAPTCHA form validation fails for a reason other than a failing score (e.g. absent token), you are able to observe this in analytics logging.

You can simulate this by preventing the reCAPTCHA script from loading, similar to what's being addressed in #11451. Currently, reCAPTCHA will submit immediately if the user attempts to submit the form before reCAPTCHA has loaded, which will not include the expected reCAPTCHA token.

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 %>

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. In a separate terminal process, run make watch_events
  2. In a private browsing tab, go to http://localhost:3000
  3. Enter email and password
  4. Click "Sign in"
  5. Observe that "Email and Password Authentication" event includes event_properties.error_details.recaptcha_token.blank = true

changelog: Internal, Analytics, Include FormResponse error details for reCAPTCHA at sign-in
@aduth aduth requested a review from a team November 5, 2024 19:39
Copy link
Copy Markdown
Contributor

@kevinsmaster5 kevinsmaster5 left a comment

Choose a reason for hiding this comment

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

Looks good, local test behaves as expected in PR description.

@aduth aduth merged commit 0c37e09 into main Nov 6, 2024
@aduth aduth deleted the aduth-sign-in-recaptcha-form-errors branch November 6, 2024 12:24
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.

2 participants