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

Add invisible_captcha to user registration #333

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

Conversation

julianrubisch
Copy link
Contributor

In my latest BT installation I noticed some bot signups.

I thought that installing invisible_captcha would be a sensible default for all apps.

@jagthedrummer
Copy link
Contributor

I love the idea of including this, but I'm on the fence about having it turned on by default, and with no obvious way to disable it. Could we make a config option that would allow devs to toggle it on or off via an initializer or something?

@andrewculver @gazayas @kaspth, what do y'all think?

@gazayas
Copy link
Contributor

gazayas commented Aug 4, 2023

Yeah, I think something like captcha_enabled? which checks an environment variable in bullet_train.rb would be perfect 👌

@julianrubisch
Copy link
Contributor Author

Will see to it when I'm back from 🏝️

@kaspth
Copy link
Contributor

kaspth commented Aug 7, 2023

@julianrubisch I can also take this over, if you'd rather enjoy your vacation without worrying about it 😄

@julianrubisch
Copy link
Contributor Author

@julianrubisch I can also take this over, if you'd rather enjoy your vacation without worrying about it 😄

Suit yourself, but please don't feel obliged!

@kaspth
Copy link
Contributor

kaspth commented Sep 1, 2023

@julianrubisch I didn't get to this if you're back from vacation and want to follow it up. I can still take a look too!

@jagthedrummer
Copy link
Contributor

jagthedrummer commented Sep 1, 2023

I pulled this down and have run through signup a few times trying to trigger this and I seem to be getting inconsistent results.

I'm going to the signup page and then inspecting the form and disabling the CSS rules that hide the honeypot field. Then I'm putting something in the honeypot, filling out the rest of the form, and hitting the "Sign Up" button.

CleanShot 2023-09-01 at 11 54 38

Sometimes the registration works like normal and the invisible_captcha seems to be missing the fact that I've populated the honeypot.

Other times it seems that invisible_captcha does catch that I've populated the honeypot and it blocks sign up, but it doesn't return an error or anything. It appears that the form just doesn't submit.

CleanShot 2023-09-01 at 12 03 50

(^ That's after clicking "Sign Up" once.)

When this happens I can see an error in the rails console that says:

[Invisible Captcha] Potential spam detected for IP 127.0.0.1. Honeypot param 'registration.oqafjyulnzdbevkhr-' was present.
Filter chain halted as #<Proc:0x000000010e1bf030 /Users/jgreen/.asdf/installs/ruby/3.2.2/lib/ruby/gems/3.2.0/gems/invisible_captcha-2.1.0/lib/invisible_captcha/controller_ext.rb:12> rendered or redirected

Weirdly the console also says that the request was successful:

Completed 200 OK in 1ms (ActiveRecord: 0.0ms | Allocations: 642)

I can see in the browser dev tools that it does return a 200 OK:

CleanShot 2023-09-01 at 12 04 44

And there's an error in the browser console that says:

Error: Form responses must redirect to another location

CleanShot 2023-09-01 at 12 07 19

If I then click "Sign Up" again the form gets blanked out (it loses info I filled in) and I get an error saying:

Sorry, that was too quick! Please resubmit.

CleanShot 2023-09-01 at 12 13 50

So I think something isn't quite right about the integration. I have no idea why it's blocking sign up sometimes, but not all the times when I fill out the honeypot.

I was also reading through the docs for invisible_captcha and I think there are at least a couple of config options that we need to figure out how to handle so that people have a reasonably good experience when deploying the app. (Or maybe these are arguments for leaving it disabled by default, and then having some docs about what steps need to be taken to enable it.)

From the docs:

secret: customize the secret key to encode some internal values. By default, it reads the environment variable ENV['INVISIBLE_CAPTCHA_SECRET'] and fallbacks to random value. Be careful, if you are running multiple Rails instances behind a load balancer, use always the same value via the environment variable.

I think that means that if someone were to deploy to Heroku (for instance) and have their app configured to run on multiple dynos that the captcha would be flaky and might even trigger false positives (that is, block legitimate users from signing up). Some discussion of that possibility in this issue.

There's also a whole section in the docs about running multiple Rails instances. I think we'd want to have that bit of configuration in the repo and setup to be automatically enabled if the captcha_enabled? flag (proposed above) is turned on.

I wouldn't be surprised if the flakiness I'm seeing is related to one of these issues.

It also looks like adding this breaks a few tests, so we'll need to figure out how to either disable this in testing, or we'll need to modify tests to accommodate.

Still love the idea of having this included and easy to turn on for people who want it.

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.

4 participants