Skip to content

Avoid setting reCAPTCHA token on failed execute#11503

Merged
aduth merged 3 commits intomainfrom
aduth-recaptcha-execute-token-blank
Nov 13, 2024
Merged

Avoid setting reCAPTCHA token on failed execute#11503
aduth merged 3 commits intomainfrom
aduth-recaptcha-execute-token-blank

Conversation

@aduth
Copy link
Contributor

@aduth aduth commented Nov 13, 2024

🛠 Summary of changes

Updates the implementation of the reCAPTCHA submit button to avoid setting a token value if reCAPTCHA fails to execute.

Previously, if reCAPTCHA failed to execute (throws an error), the value would be undefined, which would be stringified to the form input as the literal string "undefined". By not setting the token value, we can get more accurate error details on the backend for a "blank" form error reason (source).

📜 Testing Plan

yarn app/javascript/packages/captcha-submit-button/captcha-submit-button-element.spec.ts

Previously, this would fail with:

      AssertionError: expected { recaptcha_token: 'undefined' } to deeply equal { recaptcha_token: '' }
      + expected - actual

       {
      -  "recaptcha_token": "undefined"
      +  "recaptcha_token": ""
       }

Verify token is sent empty for failed reCAPTCHA execute:

  1. Repeat testing plan from Add error handling for failed reCAPTCHA execute #11449, with browser developer tools "Network" tab open, and "Preserve log" checked
  2. After clicking "Sign in", observe click on the POST request to the sign-in route
  3. Click "Payload" tab
  4. Observe that recaptcha_token is empty, rather than "undefined"

changelog: Internal, Anti-Fraud, Avoid setting reCAPTCHA token on failed execute
If this were set previously, we'd get TypeScript warnings on attempting to assign potentially-undefined value to input field
@aduth aduth requested a review from a team November 13, 2024 21:08
const { recaptchaSiteKey: siteKey, recaptchaAction: action } = this;

let token;
let token: string | undefined;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

If this TypeScript type were assigned previously, it'd also have been flagged as an issue:

app/javascript/packages/captcha-submit-button/captcha-submit-button-element.ts:65:7 - error TS2322: Type 'string | undefined' is not assignable to type 'string'.
  Type 'undefined' is not assignable to type 'string'.

65       this.tokenInput.value = token;
         ~~~~~~~~~~~~~~~~~~~~~

@aduth aduth merged commit e3e6f32 into main Nov 13, 2024
@aduth aduth deleted the aduth-recaptcha-execute-token-blank branch November 13, 2024 21:54
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