Timestamp detection - Githubissues

amnesia7 commented 9 years ago

Hi,

I like the idea of the invisible captcha. I was looking how to apply something like this when I found your gem. I also found a site mentioning about comparing the timestamp of the user and wondered if you were planning on implementing something along those lines in your gem as well (http://blog.gingerlime.com/2012/simple-detection-of-comment-spam-in-rails/) or whether I should apply this to my site separately?

markets commented 9 years ago

Hi @amnesia7,

Thanks for trying it! Regarding that "timestamp" addition, yes, would be really nice to introduce it to InvisibleCaptcha. Would you like to start to work on it and send PR? I would be very happy to merge it :smiley: :+1:

amnesia7 commented 8 years ago

@markets each of the method calls are named invisible_captcha including the one in the particular controller to check for spam so how would you expect it to be done in this case because it would require a before action to be run in the application controller as well as the topics controller? Would you give the application controller call a different name or would you do the same code but check which controller it was being called from, i.e. application or not to decide which method to run (set timestamp or check for spam)? Yoav who wrote the article doesn't have the time available to help with the code but says it is fine for us to use it.

markets commented 8 years ago

Hi @amnesia7,

To check the "timestamp" I'd like to re-use the same invisible_captcha method (this method should also accept some options, ie: threshold: 10). But to save the timestamp, I'm not sure, there are some different approaches:

1) add a new before_filter to save it into the session object 2) render the timestamp along with the view/form helper, as a hidden input and with

Option 2) seems easy to implement and we don't need to add a before_filter to the whole app, but, could some smart bots detect that input? Is it enough "secure"?

Once we ship this new feature, we can ping Yoav and request a link back to this repo :+1:

amnesia7 commented 8 years ago

I've wondered about a timestamp field before and it does mean that you would either need to hard-code the field name or have a random name that you would need to validate upon submission. It also gives spammers something to latch onto and is visible in the html as something you're using. The field value could also be overridden if possible either with a past or future timestamp depending on how the validation worked.

After some thought I don't think another before_action/before_filter is actually required on the application controller. The form submission needs to work no matter which controller the user hit the site on...they could actually hit the form page as the first page they go to so it needs to be able to work immediately.

For this reason, I think adding the code to set the session variable in the existing invisible_captcha method is the one to go for. However, the readme currently advises to call the method only on :create, :update which wouldn't work with this suggestion because it would need to set the session variable on :new, :edit in this case.

The method would therefore need to be called on :new, :create, :edit, :update. If it is a GET request then it would set the session variable if it hasn't been already. If it is anything else (i.e. POST/PUT/PATCH) then the spam checking function that Yoav advises would be applied (as well as the existing spam checking functionality already provided by this gem).

I assume as well as the threshold option there would also need to be a config option for setting the error message if the form was submitted too quickly by a human?!

What do you think @markets?

markets commented 8 years ago

@amnesia7 I thought about your proposal but I'm not sure if it's enough clear or can cause some weird behaviour. For example, using this approach, all GET requests to that controller will set a timestamp to the session, even if the user hits a page without an invisible_captcha form, no?

What about: set the timestamp while rendering the captcha hidden field through the view helper? Do you see any problem with this approach?

About the threshold options, yes!

Thanks for your help.

P.S. Definitely, we should add this feature, could be really useful, but we need to see the best way to implement it, especially in terms of final api: good defaults, easy to customize, no surprises.

amnesia7 commented 8 years ago

I can't think of any reason why we couldn't set the session variable while rendering the captcha field. On the one hand doing it this way means the variable is only set if that particular view helper is called but on the other the view helper would have to be called anyway so I can't see an issue.

This would mean the controller method can remain only being called for the submissions.

set the session variable using the view helper (if it doesn't already exist)
set a default value for the threshold
on submit, compare the timestamps (if the session variable exists - just in case the user has decided to handcraft their own hidden field) against the threshold value
return the default "too quick" error message using errors :base (or the user's own message if they have set one in config.

Does that cover it?

markets commented 8 years ago

That seems a good plan. I'd add:

threshold and error message should be configurable generally (initializer), and allow to override it via the view helper
allow to configure a callback for "too quick" response, same as :on_spam option? using a different method :on_timestamp_spam ?

amnesia7 commented 8 years ago

@markets what version of rails have you been using for developing this gem because bundle has installed rails 4.2 but the console shows warnings about config.serve_static_assets, config.eager_load and secret_key_base. It does appear to run rspec though and has the initial 12 examples, 0 failures (before I add any extra ones for this new feature. Do you want to update the intregrated dummy app to rails 4.2 before I start adding this feature?

markets commented 8 years ago

Hi @amnesia7, I started the gem with Rails 3, so the dummy app was generated with this version. Anyway, the plugin works with Rails 3 (at least 1 app in pro) and Rails 4 (at least 3 apps in pro). Could you please update the app in the same PR?

I'm planning to integrate Appraisals to ensure compatibility along different versions, but I'll do it after this new feature.

amnesia7 commented 8 years ago

Dummy app updated to rails 4.2 so far and existing tests passing ok. I'll move onto the feature itself as soon as I get chance.

amnesia7 commented 8 years ago

@markets I'm not sure how to allow override of threshold and error message via the view helper but the rest seems to be ok so far.

markets commented 8 years ago

That sounds pretty nice @amnesia7, no prob, send the PR and I'll take a look to those overrides :+1:

amnesia7 commented 8 years ago

@markets a few failing tests still but can you take a look at https://github.com/amnesia7/invisible_captcha/tree/check-timestamp and see if I'm on the right track and see if you can see where I'm going wrong for the remaining failing tests; which includes the action in response to the threshold error because I'm not quite sure how you're doing that. Thanks

markets / invisible_captcha

Timestamp detection #6