Logic bug regarding spinner-based protection

[se2342]

Expected Behaviour

When setting spinner_enabled = true requests with an empty params[:spinner] should fail regardless of whether the client has requested a view that invokes invisible_captcha form helper.

Actual Behaviour

When spinner_enabled = true is set but no previous Rails view has been called, session[:invisible_captcha_spinner] is empty thus not making valid params[:spinner] a prerequisite for the request to be executed i.e. malicious requests are not detected as spam, effectively bypassing the spinner-based protection.

See https://github.com/markets/invisible_captcha/blob/fc5dd708837f4afbea0ad3a3636d11889949ecc3/lib/invisible_captcha/controller_ext.rb#L78

Proposal

Improve conditional logic as follows:

if InvisibleCaptcha.spinner_enabled && (params[:spinner].blank? || params[:spinner] != session[:invisible_captcha_spinner])

[markets]

Hello @se2342,

I think that makes sense, I can't remember now a valid reason to not have this extra check in place.

[markets]

Hello again @se2342,

I just pushed this change in #134. I'll cut a new release to RubyGems soon: https://github.com/markets/invisible_captcha/compare/v2.2.0...master

[markets]

UPDATE I finally released a new version now 🚀 https://rubygems.org/gems/invisible_captcha/versions/2.3.0

[se2342]

Awesome. Thank you for your work as a maintainer. 👏