Skip to content

THREESCALE-14842: Add recaptcha to reset password screen - #4359

Merged
jlledom merged 7 commits into
masterfrom
THREESCALE-14842-recaptcha-reset-password
Aug 6, 2026
Merged

THREESCALE-14842: Add recaptcha to reset password screen#4359
jlledom merged 7 commits into
masterfrom
THREESCALE-14842-recaptcha-reset-password

Conversation

@jlledom

@jlledom jlledom commented Jul 29, 2026

Copy link
Copy Markdown
Contributor

What this PR does / why we need it:

This adds the recaptcha challenge to the "Forgot password" screen and creates the corresponding tests.

Additionally, some recaptcha tests has been added for the LoginForm component. Not directly related to the PR but the component was lacking such tests and it was straightforward to add them.

Which issue(s) this PR fixes

https://redhat.atlassian.net/browse/THREESCALE-14842

Verification steps

  1. Configure recaptcha keys via env vars:
RECAPTCHA_PROJECT_ID='project_id'
RECAPTCHA_PUBLIC_KEY='public_key'
RECAPTCHA_PRIVATE_KEY='private_key'
RECAPTCHA_MIN_BOT_SCORE=0.5
  1. Enable recaptcha on Account Settings -> Integrate -> Security

  2. Go to the reset password screen

  3. Recaptcha notice "This site is protected by reCAPTCHA..." should appear

  4. Intriduce an email and submit the form

    1. With RECAPTCHA_MIN_BOT_SCORE=0.99 recpatcha challenge should fail
    2. With RECAPTCHA_MIN_BOT_SCORE=0.5 recpatcha challenge should pass

jlledom added 4 commits July 29, 2026 12:17
The admin portal Forgot Password page lacked bot protection while the
Sign In page already had reCAPTCHA v3. This creates an inconsistency
and exposes the password reset endpoint to automated attacks.

Add reCAPTCHA v3 protection using the existing admin_bot_protection_level
setting (same as the login page). The controller includes
BotProtection::Controller, overrides bot_protection_level to read the
admin setting, and checks the token in the destroy action. The view
passes recaptcha config to the React component, which conditionally
renders ReCaptchaV3.

Assisted-by: Claude Code
Verify that the reCAPTCHA check in the destroy action correctly blocks
requests when verification fails (renders the reset page with an error
message) and allows them through when it passes or when bot protection
is disabled.

Assisted-by: Claude Code
Add end-to-end scenarios covering the reCAPTCHA protection on the
admin Forgot Password page: captcha presence when enabled/disabled,
bot rejection, and successful submission.

Also update the bot protection step definition to stub at the Recaptcha
module level (skip_env?, invalid_response?, verify_via_api_call) instead
of stubbing ApplicationController#verify_recaptcha. This lets the full
controller flow run and produces the gem's real flash message.

Assisted-by: Claude Code
Add tests to RequestPasswordForm verifying that the ReCaptchaV3 input is
present when enabled and absent when disabled.

Also added similar lacking tests to LoginForm.

Update defaultProps in RequestPasswordForm and RequestPasswordPage specs
to include the new required recaptcha prop, and regenerate the
RequestPasswordForm snapshot to reflect it.

Assisted-by: Claude Code
@qltysh

qltysh Bot commented Jul 29, 2026

Copy link
Copy Markdown

All good ✅

Comment thread app/javascript/src/Login/components/RequestPasswordForm.tsx
Comment thread app/javascript/src/Login/components/RequestPasswordForm.tsx
Comment thread app/javascript/src/Login/components/RequestPasswordPage.tsx
Comment thread app/javascript/src/Login/components/RequestPasswordPage.tsx
Comment thread app/javascript/src/Login/components/RequestPasswordPage.tsx
@jlledom jlledom changed the title THREESCALE-14842 recaptcha reset password THREESCALE-14842: Add recaptcha to reset password screen Jul 29, 2026
jlledom added 2 commits July 29, 2026 13:23
ESLint react/prefer-read-only-props requires props that are never
mutated to be declared as readonly. The recaptcha prop in
RequestPasswordForm and RequestPasswordPage fits this rule.

Assisted-by: Claude Code
ApplicationController.any_instance.stubs(:verify_recaptcha).returns(!bot)
Recaptcha.stubs(:skip_env?).returns(false)
Recaptcha.stubs(:invalid_response?).returns(false)
Recaptcha.stubs(:verify_via_api_call).returns([!bot, {}])

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.

The step definition stubs changed from stubbing the public API (ApplicationController.any_instance.stubs(:verify_recaptcha)) to stubbing gem internals (Recaptcha.stubs(:skip_env?), Recaptcha.stubs(:invalid_response?), Recaptcha.stubs(:verify_via_api_call)). This couples the tests to the recaptcha gem's internal implementation — a gem version bump could silently break these stubs without any app code change.

Was there a specific reason the previous approach didn't work here? If so, it might be worth adding a comment explaining why, so future maintainers don't revert it.

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.

akostadinov
akostadinov previously approved these changes Jul 31, 2026

@akostadinov akostadinov left a comment

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.

looks good, if you want to take into concern akostadinov-bot's comment or not, that's fine with me

Gem internals are stubbed instead of our :verify_recaptcha method so
that the flash message is set by the gem itself, matching production
behavior. Add a comment explaining this non-obvious choice.

@madnialihussain madnialihussain left a comment

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.

lgtm

@jlledom
jlledom merged commit ad61869 into master Aug 6, 2026
22 of 26 checks passed
@jlledom
jlledom deleted the THREESCALE-14842-recaptcha-reset-password branch August 6, 2026 08:17
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.

3 participants