ambethia / recaptcha

ReCaptcha helpers for ruby apps
http://github.com/ambethia/recaptcha
MIT License
1.97k stars 440 forks source link

Captcha parameter is required for verification, even when test credentials are used #271

Open andreyprostakov opened 6 years ago

andreyprostakov commented 6 years ago

Greetings.

I use credentials specified here https://developers.google.com/recaptcha/docs/faq#id-like-to-run-automated-tests-with-recaptcha-v2-what-should-i-do to run automated tests in test environment. I expected verification to be skipped, but that's not how it works with the gem.

In my case false is returned here without sending a verification request. However, if I set params['g-recaptcha-response'] = "FOOBAR", the request is being sent, and as a result verification is skipped.

Expected behavior: verify_recaptcha returns true when test credentials are used.

Gem version: 4.6.6.

grosser commented 6 years ago

if credentials are set then it does a real request ... and the request returns an empty response ... which means it failed ? are you saying the library should know when test credentials were used and then ignore failed requests ?

andreyprostakov commented 6 years ago

@grosser not quite. Sorry, I'll try to formalize better

Given that we have a simple Rails app with recaptcha gem, configured with fake credentials And the app has a form with ReCAPTCHA on it. When a user submits the form without passing ReCAPTCHA test, Expected result: Then ReCAPTCHA backend verification should pass successfully. Actual result: Then ReCAPTCHA backend verification fails.

When we call verify_recaptcha from the controller, it returns false right away, without sending a request for verification. That's because params['g-recaptcha-response'] is blank. I tried a few things:

Hence, the only thing that prevents verification is that if recaptcha_response.empty? check.

I assume it's there for speed optimization. Perhaps it would make sense to check credentials in that place and, if they're fake, allow the verification request to be sent :man_shrugging: That seems a bit ugly though, I hope there is a better way

grosser commented 6 years ago

I kinda like that it returns false even when using fake credentials so users can test the 'not submitted' path. I don't like sending requests on blank params since that makes ddos easier. If it's easy to check for the fake credentials then returning true early would be alright, and link to that page as reason for why it is that way.

artur-intech commented 6 years ago

Given this gem is an API for Google ReCaptcha, embedding test keys, specified in https://developers.google.com/recaptcha/docs/faq#id-like-to-run-automated-tests-with-recaptcha-v2-what-should-i-do would simplify integration testing, when I need to test successful path (captcha is solved).

Currently I don't see a good way to test it.

Here is an example how I tested it before I updated the gem to the latest version: https://github.com/internetee/rest-whois/blob/edc8fe4589a1cb8e1642b6802ef94848d2c070ab/test/integration/whois_records/html_test.rb#L57

grosser commented 6 years ago

adding something like test_keys: true could make things simpler test_credentials: true or so ... but improving the docs would already help ...

artur-intech commented 6 years ago

Perhaps test_mode: true?

grosser commented 6 years ago

test_mode is kinda ambiguous (what does it mean ?) ... but either works

On Wed, May 23, 2018 at 10:12 AM Artur Beljajev notifications@github.com wrote:

Perhaps test_mode: true?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/ambethia/recaptcha/issues/271#issuecomment-391427472, or mute the thread https://github.com/notifications/unsubscribe-auth/AAAsZ0lKXPmZ7OwPnwFAf1-N0IlD9YBFks5t1ZiRgaJpZM4Tjjk7 .