backdrop-contrib / honeypot

Backdrop port of Drupal module. Uses both the honeypot and timestamp methods of deterring spam bots from completing forms on your site.
GNU General Public License v2.0
3 stars 1 forks source link

Caching on pages with protected forms immediately broken when honeypot is added to forms #20

Closed jenlampton closed 7 years ago

jenlampton commented 7 years ago

This is one of my biggest frustrations with the Honeypot drupal module as well. When you enable the module, caching is instantly broken for your site because the default time limit is set to 5. (I refer to this problem as this "being honeypotted" as I see it often.)

Instead, we should set the default time limit to 0, so that enabling honeypot does not break caching. Additionally, we should do a better job of warning users that their caching will be broken as soon as they set a limit.

Additionally, we should set some more serious messaging when caching has been disabled on these pages. I've been hired to diagnose performance issues on several sites that have been honeypotted because the site architects either missed the current warning, or didn't fully consider the ramifications.


PR: https://github.com/backdrop-contrib/honeypot/pull/21

geerlingguy commented 7 years ago

Just as an aside—Honeypot only disables caching on pages where there is a form with Honeypot protection enabled. It's not intended for use with search forms or similar forms which don't necessarily submit information to be stored in Drupal or distributed via email (e.g. something like user signup form, webform, etc.).

This is why I'm still comfortable with the 5 second time limit being enabled by default—since Honeypot doesn't automatically enable itself on any system forms (the admin/site builder has to manually enable it for select forms, or all forms).

jenlampton commented 7 years ago

Fair enough :) I updated the title to reflect that caching is only disabled on pages where there is a protected form. However, if you are using the comment module, that still ends up being almost every page on a site. (My most-common use-case for needing honeypot is on sites using the comment module.)

jenlampton commented 7 years ago

I've pushed a PR that sets the default value to 0, and adds better warning messaging about when caching may be broken. @geerlingguy I'll file this as a patch agains the Drupal module too (but just the messaging bit) and you can see if you want to use it there too. Thanks for weighing in here :)

jlfranklin commented 7 years ago

I disable it because I can touch type and easily beat the 5 second timer.

herbdool commented 7 years ago

I'm inclined to merge this even though the time limit is pretty useful. In Backdrop the post content type by default shows the comment form on the same page as the node so that would hurt caching quite a bit. Too bad we can't have the time limit enabled per form or is that too messy? Even that is just until there's a workaround to enable caching and the time limit.

jenlampton commented 7 years ago

Too bad we can't have the time limit enabled per form or is that too messy?

It's possible to do this already, but you need to do it in code.

The most valuable part of the module is the honeypot. The time-limit is an added bonus, but it does have risks. I like the idea of providing it, but not exposing everyone to those risks unless they are prepared to take them on.

A per-form setting would be nice, but it is tricky to build a sensible UI for how to specify per-form settings. Take a look at captcha as an example, it's attempted there. There's a weird form_id table and a bunch admin-only tools to help people get the correct form IDs.