Problem/Motivation

Drupal allows for forms to be submitted programatically through \Drupal\Core\Form\FormBuilderInterface::submitForm. Currently, honeypot fields and validation applies to forms that are submitted in that way. Since these form submissions are triggered from within the site itself, they should be implicitly trusted, and there is no need to apply honeypot's logic.

First patch attached shows how Honeypot is currently applied to those types of form submissions. Second patch contains tests + fix.

Proposed resolution

Allow programmatically submitted forms to pass without Honeypot protection.

Remaining tasks

Review.

User interface changes

None.

API changes

None.

Data model changes

None.

CommentFileSizeAuthor
#14 honeypot_should_not_be-2677126-14.patch3.43 KBmr.baileys
#10 interdiff.txt475 bytesmr.baileys
#10 honeypot_should_not_be-2677126-10.patch4.38 KBmr.baileys
#7 honeypot_should_not_be-2677126-7.patch4.35 KBmr.baileys
#7 interdiff.txt1.06 KBmr.baileys
honeypot-programmatic-submission.patch5.29 KBmr.baileys
honeypot-programmatic-submission-test-only.patch3.68 KBmr.baileys
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

mr.baileys created an issue. See original summary.

The last submitted patch, honeypot-programmatic-submission-test-only.patch, failed testing.

geerlingguy’s picture

geerlingguy CreditAttribution: geerlingguy as a volunteer commented

Just out of curiosity... is there any equivalent to $form_state->isProgrammed() in Drupal 7? I haven't seen one, and this looks like a handy feature in D8. Will review patch in depth later, when I get some time to also finish up the installation bug #2676888: Installing Honeypot via UI results in unexpected error due to unavailable honeypot.config route.

mr.baileys’s picture

mr.baileys
Primary language Dutch
Location 🇧🇪 (Ghent)
CreditAttribution: mr.baileys at Calibrate commented
Issue tags: +Needs backport to D7

is there any equivalent to $form_state->isProgrammed() in Drupal 7?

In Drupal 7 the equivalent is drupal_form_submit(), prior to D7 it was called drupal_execute(). Probably makes sense to consider backporting to D7 if this lands.

geerlingguy’s picture

geerlingguy CreditAttribution: geerlingguy as a volunteer commented

But how can you detect if those functions were used? Maybe resort to PHP's backtrace functionality, or is there an API method/form array key?

mr.baileys’s picture

mr.baileys
Primary language Dutch
Location 🇧🇪 (Ghent)
CreditAttribution: mr.baileys at Calibrate commented

Ah, sorry, I misread your question. In D7, I think the $form_state has a 'programmed' property set to true for programmatic submissions:

From drupal_build_form:

programmed: If TRUE, the form was submitted programmatically, usually invoked via drupal_form_submit(). Defaults to FALSE.

mr.baileys’s picture

mr.baileys
Primary language Dutch
Location 🇧🇪 (Ghent)
CreditAttribution: mr.baileys at Calibrate commented
FileSize
interdiff.txt1.06 KB
honeypot_should_not_be-2677126-7.patch4.35 KB

On second thought, we only need to bypass the time protection on the submitted form, not the hidden field protection. Amended patch attached.

geerlingguy’s picture

geerlingguy CreditAttribution: geerlingguy as a volunteer commented
Status: Needs review » Reviewed & tested by the community

Looks good to me, and it looks like we can probably get things working well for D7 too, then!

geerlingguy’s picture

geerlingguy CreditAttribution: geerlingguy as a volunteer commented
Status: Reviewed & tested by the community » Needs work 
+++ b/tests/modules/honeypot_test/honeypot_test.info.yml
@@ -0,0 +1,6 @@
+description: Honeypot Test

Let's make it more explicit this is not a module to be enabled under normal circumstances—e.g. "Honeypot test module used for internal testing purposes. This module shouldn't normally be enabled."

mr.baileys’s picture

mr.baileys
Primary language Dutch
Location 🇧🇪 (Ghent)
CreditAttribution: mr.baileys at Calibrate commented
Status: Needs work » Needs review
FileSize
honeypot_should_not_be-2677126-10.patch4.38 KB
interdiff.txt475 bytes

I tweaked the description a bit to be more like the descriptions on Core's own test modules ("Support module for..."), and moved the module to package "Testing", which is where all the other test modules reside.

Note though that you cannot enable or see this module (not even through drush), unless you explicitly make all test modules visible through $settings['extension_discovery_scan_tests'] = TRUE;, which normally is the case in development environments ony, and only for people who know what they are doing.

geerlingguy’s picture

geerlingguy CreditAttribution: geerlingguy as a volunteer commented
Status: Needs review » Reviewed & tested by the community

Back to RTBC, will merge soon.

geerlingguy’s picture

geerlingguy CreditAttribution: geerlingguy as a volunteer commented
Version: 8.x-1.x-dev » 7.x-1.x-dev
Status: Reviewed & tested by the community » Patch (to be ported)

Ready for 7.x-1.x now. Hopefully things are as straightforward there. Seems like it would be about the same, architecturally.

mr.baileys’s picture

mr.baileys
Primary language Dutch
Location 🇧🇪 (Ghent)
CreditAttribution: mr.baileys at Calibrate commented
Status: Patch (to be ported) » Needs review
FileSize
honeypot_should_not_be-2677126-14.patch3.43 KB

Thanks!

D7 backport attached.

geerlingguy’s picture

geerlingguy CreditAttribution: geerlingguy as a volunteer commented
Status: Needs review » Reviewed & tested by the community
geerlingguy’s picture

geerlingguy CreditAttribution: geerlingguy as a volunteer commented
Status: Reviewed & tested by the community » Fixed

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.