reCAPTCHA #1647

Dantemss · 2025-05-27T17:48:13Z

Initial attempt at adding reCAPTCHA to Accounts
Fully removed legacy signup controller, views, routes

Dantemss · 2025-05-27T18:35:42Z

Well I haven't tested this yet at all, but maybe you can take a look.

github-actions · 2025-05-29T16:00:48Z

❌ Migration check failed. Check the workflow run for details.

…ky feature specs

Dantemss · 2025-06-30T19:16:48Z

@mwvolo this should be ready for review, though there are still some flaky feature specs

Copilot

Pull Request Overview

Adds Google reCAPTCHA to the new Sign-up flows and removes legacy signup controllers, views, and routes.

Introduces recaptcha gem configuration, view helpers, and verification in signup controllers
Injects reCAPTCHA widgets into student and educator signup/change-email forms
Cleans up legacy signup code and updates feature specs to use Capybara’s have_current_path matcher

Reviewed Changes

Copilot reviewed 32 out of 34 changed files in this pull request and generated 1 comment.

Show a summary per file

File	Description
config/secrets.yml	Adds `recaptcha.site_key` and `secret_key` entries
config/initializers/recaptcha.rb	Configures Recaptcha gem, stub behavior for non-prod
app/controllers/newflow/signup_controller.rb	Includes RecaptchaController helper
app/controllers/newflow/student_signup_controller.rb	Adds `verify_recaptcha_with_fallback` around signup
app/controllers/newflow/educator_signup_controller.rb	Adds `verify_recaptcha_with_fallback` around signup
app/views/newflow/*/_signup_form.html.erb	Injects `<%= recaptcha_with_disclaimer_and_fallback %>`
app/views/newflow/signup/_change_signup_email_form_template…	Injects recaptcha into email-change form
config/routes.rb	Removes legacy signup routes
app/views/legacy/signup/ and app/controllers/legacy//*	Entire legacy signup flow removed
spec/*/.rb	Replaces `expect(page.current_path)` with `have_current_path` and related matcher updates

Comments suppressed due to low confidence (2)

config/initializers/recaptcha.rb:47

No existing feature specs cover the fallback case when reCAPTCHA verification fails (i.e. force_recaptcha_failure is true). Consider adding an integration test that injects force_recaptcha_failure: 'true' to verify that the form re-renders with the failure message.

    force_recaptcha_failure = params[:force_recaptcha_failure] == 'true'

README.md:167

[nitpick] It would be helpful to document how to configure the new reCAPTCHA keys (e.g. RECAPTCHA_SITE_KEY and RECAPTCHA_SECRET_KEY environment variables) in the README under a new "reCAPTCHA Configuration" section.

The most straightforward way to sign up is by visiting the `newflow_signup_path`  without any parameters in the URL and without any state in the session.

Copilot · 2025-06-30T22:17:11Z

app/controllers/newflow/student_signup_controller.rb

-          render :student_signup_form
-        }
-      )
+      if verify_recaptcha_with_fallback


[nitpick] The if verify_recaptcha_with_fallback ... else blocks in both StudentSignupController and EducatorSignupController duplicate the rendering logic. Consider extracting the recaptcha check into a shared before_action or method to DRY up these controllers.

Suggested change

def student_signup

handle_with(

StudentSignup::SignupForm,

contracts_required: !contracts_not_required,

client_app: get_client_app,

success: lambda {

user = @handler_result.outputs.user

save_unverified_user(user.id)

security_log(:student_signed_up, { user: user })

log_posthog(user, "student_started_signup")

redirect_to student_email_verification_form_path

},

failure: lambda {

email = @handler_result.outputs.email

error_codes = @handler_result.errors.map(&:code)

security_log(:student_sign_up_failed, { reason: error_codes, email: email })

render :student_signup_form

}

)

if verify_recaptcha_with_fallback

private

def handle_recaptcha_failure(failure_view)

if verify_recaptcha_with_fallback

yield

else

instance_variable_set(:@email, unverified_user.email_addresses.first.value) if failure_view == :student_change_signup_email_form

render failure_view

end

end

def student_signup

handle_recaptcha_failure(:student_signup_form) do

mwvolo · 2025-07-01T17:16:03Z

app/controllers/legacy/sessions_controller.rb

    def maybe_skip_to_sign_up
      if %w{signup student_signup}.include?(params[:go])
-        redirect_to signup_path(set_param_to_permit_legacy_flow)
+        redirect_to newflow_signup_path


do we still use this? I think this was a Tutor thing

we can maybe remove this then

I saw you message asking about this, but I'm going to go ahead and merge this to unblock Roy's PRs.
We can open another one to clean up remaining legacy stuff.

mwvolo · 2025-07-01T17:17:21Z

app/controllers/legacy/signup_controller.rb

-    def password; end
-    def social; end
-
    def profile


is stuff staying here for specs / making migration easier? I don't think (hope) this profile method isn't called.

It's called somewhere but all it does is redirect

Nvm I got that wrong, it doesn't just redirect. It's called from here:

accounts/config/initializers/controllers.rb

Line 33 in c58a912

redirect_to '/signup/profile' if current_user.is_needs_profile?

also this newflow test expects to reach it: https://github.com/openstax/accounts/blob/main/spec/features/newflow/student_login_flow_spec.rb#L54

mwvolo

looks good to me, asked a few questions, but not blocking merge

Dantemss added 2 commits May 27, 2025 10:27

Remove legacy signup controller, views, routes

c3de697

Initial attempt at adding recaptcha

fd469db

Dantemss self-assigned this May 27, 2025

Fix Rails secrets typo

08f43e2

Dantemss requested a review from mwvolo May 27, 2025 18:35

Dantemss marked this pull request as ready for review May 27, 2025 18:35

Dantemss added 3 commits May 28, 2025 08:53

Fix how recaptcha secrets are handled

ead50b3

Set recaptcha config in an initializer instead

d528a8c

Default recaptcha badge to bottomleft

9bab4fe

Dantemss added 8 commits May 29, 2025 11:13

Don't blow up if recaptcha secrets are not defined

55f96e1

Remove redirects to now-defunct legacy signup

25564aa

Hide the recaptcha badge and add a text disclaimer instead

98f3cdb

Attempt to force recaptcha failure for testing

1151788

Rename modules to prevent collision with gem namespace

a27974f

Moved lib/recaptcha.rb to config/initializers/recaptcha.rb

550c90a

Remove continue-on-error from tests

231a15b

Replaced recaptcha fallback with error message

dedefd2

Dantemss force-pushed the captcha branch from 5362be1 to dedefd2 Compare June 2, 2025 17:16

Dantemss added 2 commits June 2, 2025 12:19

Set color for recaptcha failure message

00e7668

Add link to support in captcha error message

99635b8

openstax deleted a comment from github-actions bot Jun 4, 2025

mwvolo added 2 commits June 4, 2025 09:38

Merge branch 'main' into captcha

05cb389

Merge branch 'main' into captcha

8b7e953

openstax deleted a comment from github-actions bot Jun 12, 2025

Dantemss added 3 commits June 25, 2025 18:55

Prevent recaptcha in tests, re-add some legacy signup routes, fix fla…

c7ae63a

…ky feature specs

Fixed path in spec that had an OR before

c9ca36c

Added back a before_action that I shouldn't have removed

7f3ef15

mwvolo requested a review from Copilot June 30, 2025 22:16

Copilot AI reviewed Jun 30, 2025

View reviewed changes

mwvolo reviewed Jul 1, 2025

View reviewed changes

mwvolo approved these changes Jul 1, 2025

View reviewed changes

Merge branch 'main' into captcha

eb5c49d

openstax deleted a comment from github-actions bot Jul 9, 2025

bracket fix from conflict merge

e8253a3

openstax deleted a comment from github-actions bot Jul 9, 2025

mwvolo merged commit 6305d61 into main Jul 9, 2025
6 of 7 checks passed

mwvolo deleted the captcha branch July 9, 2025 16:08

-    def student_signup
-      handle_with(
-        StudentSignup::SignupForm,
-        contracts_required: !contracts_not_required,
-        client_app: get_client_app,
-        success: lambda {
-          user = @handler_result.outputs.user
-          save_unverified_user(user.id)
-          security_log(:student_signed_up, { user: user })
-          log_posthog(user, "student_started_signup")
-          redirect_to student_email_verification_form_path
-        },
-        failure: lambda {
-          email = @handler_result.outputs.email
-          error_codes = @handler_result.errors.map(&:code)
-          security_log(:student_sign_up_failed, { reason: error_codes, email: email })
-          render :student_signup_form
-        }
-      )
-      if verify_recaptcha_with_fallback
+    private
+    def handle_recaptcha_failure(failure_view)
+      if verify_recaptcha_with_fallback
+        yield
+      else
+        instance_variable_set(:@email, unverified_user.email_addresses.first.value) if failure_view == :student_change_signup_email_form
+        render failure_view
+      end
+    end
+    def student_signup
+      handle_recaptcha_failure(:student_signup_form) do

reCAPTCHA #1647

reCAPTCHA #1647

Conversation

Dantemss commented May 27, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Dantemss commented May 27, 2025

Uh oh!

github-actions bot commented May 29, 2025

Uh oh!

Dantemss commented Jun 30, 2025

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Uh oh!

Copilot AI Jun 30, 2025

Choose a reason for hiding this comment

Uh oh!

mwvolo Jul 1, 2025

Choose a reason for hiding this comment

Uh oh!

Dantemss Jul 7, 2025

Choose a reason for hiding this comment

Uh oh!

mwvolo Jul 9, 2025

Choose a reason for hiding this comment

Uh oh!

mwvolo Jul 1, 2025

Choose a reason for hiding this comment

Uh oh!

Dantemss Jul 7, 2025

Choose a reason for hiding this comment

Uh oh!

Dantemss Jul 7, 2025

Choose a reason for hiding this comment

Uh oh!

Dantemss Jul 7, 2025

Choose a reason for hiding this comment

Uh oh!

mwvolo left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Dantemss commented May 27, 2025 •

edited

Loading