From 888b4ef238ff6c51357182712283286608e22b2e Mon Sep 17 00:00:00 2001 From: Jo Sprague Date: Thu, 10 Jul 2025 13:38:38 +0700 Subject: [PATCH 1/4] Add honeypot field to newsletter form to prevent spam submissions Fixes https://github.com/mozilla/bedrock/issues/16231 - Added `office_fax` as a honeypot field in the form to trap bots. - Implemented validation to check for any input in the honeypot field, logging a warning if filled. - Updated the newsletter subscription view to handle errors related to the honeypot field. - Added corresponding tests to ensure the honeypot functionality works as intended. - Styled the honeypot field to be hidden from users while remaining accessible to bots. --- bedrock/mozorg/forms.py | 5 ++- bedrock/newsletter/forms.py | 15 +++++++- .../templates/newsletter/includes/form.html | 3 ++ bedrock/newsletter/tests/test_forms.py | 38 +++++++++++++++++++ bedrock/newsletter/views.py | 3 ++ .../protocol/components/newsletter-form.scss | 17 ++++++++- 6 files changed, 77 insertions(+), 4 deletions(-) diff --git a/bedrock/mozorg/forms.py b/bedrock/mozorg/forms.py index ce8c0f63ff4..4ab4e19c862 100644 --- a/bedrock/mozorg/forms.py +++ b/bedrock/mozorg/forms.py @@ -45,11 +45,12 @@ def render(self, name, value, attrs=None, renderer=None): # semi-randomized in case we have more than one per page. # this is maybe/probably overthought honeypot_id = "office-fax-" + str(randrange(1001)) + "-" + str(datetime.now().strftime("%Y%m%d%H%M%S%f")) + tabindex_attr = 'tabindex="-1"' return mark_safe( '
' '' - '' - "
" % (honeypot_id, honeypot_txt, honeypot_id) + '' + "" % (honeypot_id, honeypot_txt, honeypot_id, tabindex_attr) ) diff --git a/bedrock/newsletter/forms.py b/bedrock/newsletter/forms.py index 9c1d486d959..60209916847 100644 --- a/bedrock/newsletter/forms.py +++ b/bedrock/newsletter/forms.py @@ -10,7 +10,7 @@ from product_details import product_details -from bedrock.mozorg.forms import EmailInput, PrivacyWidget, strip_parenthetical +from bedrock.mozorg.forms import EmailInput, HoneyPotWidget, PrivacyWidget, strip_parenthetical from bedrock.newsletter import utils from lib.l10n_utils.fluent import ftl, ftl_lazy @@ -186,6 +186,8 @@ class NewsletterFooterForm(forms.Form): privacy = forms.BooleanField(widget=PrivacyWidget(attrs={"data-testid": "newsletter-privacy-checkbox"})) source_url = forms.CharField(required=False) newsletters = forms.MultipleChoiceField(widget=forms.CheckboxSelectMultiple()) + # office_fax is a honeypot field + office_fax = forms.CharField(widget=HoneyPotWidget(attrs={"tabindex": "-1"}), required=False, empty_value="") # has to take a newsletters argument so it can figure # out which languages to list in the form. @@ -251,6 +253,17 @@ def clean_source_url(self): return su + def clean_office_fax(self): + # Check raw data to catch any value, including whitespace-only + office_fax = self.data.get("office_fax", "") + if office_fax: + import logging + + logger = logging.getLogger("b.newsletter") + logger.warning(f'Honeypot field filled with value: "{office_fax}"') + raise forms.ValidationError("Invalid submission") + return "" + class EmailForm(forms.Form): """ diff --git a/bedrock/newsletter/templates/newsletter/includes/form.html b/bedrock/newsletter/templates/newsletter/includes/form.html index 15dbbbb931f..9ca3a70452a 100644 --- a/bedrock/newsletter/templates/newsletter/includes/form.html +++ b/bedrock/newsletter/templates/newsletter/includes/form.html @@ -15,6 +15,9 @@ {% endif %} + {# Honeypot field #} + {{ form.office_fax|safe }} + {% if include_title and is_multi_newsletter_form %}
diff --git a/bedrock/newsletter/tests/test_forms.py b/bedrock/newsletter/tests/test_forms.py index 3fe7cc2309f..ba434e801a1 100644 --- a/bedrock/newsletter/tests/test_forms.py +++ b/bedrock/newsletter/tests/test_forms.py @@ -233,3 +233,41 @@ def test_multiple_newsletters(self): form = NewsletterFooterForm(spacey_newsletters, "en-US", data=data.copy()) self.assertTrue(form.is_valid()) self.assertEqual(form.cleaned_data["newsletters"], newsletters) + + def test_honeypot_empty_valid(self): + """Honeypot field should be valid when empty""" + data = { + "email": "foo@example.com", + "lang": "fr", + "privacy": True, + "newsletters": [self.newsletter_name], + "office_fax": "", # honeypot field + } + form = NewsletterFooterForm(self.newsletter_name, locale="en-US", data=data.copy()) + self.assertTrue(form.is_valid(), form.errors) + + def test_honeypot_filled_invalid(self): + """Honeypot field should be invalid when filled""" + data = { + "email": "foo@example.com", + "lang": "fr", + "privacy": True, + "newsletters": [self.newsletter_name], + "office_fax": "some value", # honeypot field + } + form = NewsletterFooterForm(self.newsletter_name, locale="en-US", data=data.copy()) + self.assertFalse(form.is_valid()) + self.assertIn("office_fax", form.errors) + + def test_honeypot_whitespace_invalid(self): + """Honeypot field should be invalid when filled with whitespace only""" + data = { + "email": "foo@example.com", + "lang": "fr", + "privacy": True, + "newsletters": [self.newsletter_name], + "office_fax": " ", # honeypot field + } + form = NewsletterFooterForm(self.newsletter_name, locale="en-US", data=data.copy()) + self.assertFalse(form.is_valid()) + self.assertIn("office_fax", form.errors) diff --git a/bedrock/newsletter/views.py b/bedrock/newsletter/views.py index ecee8ff0253..8fb8a87cfc8 100644 --- a/bedrock/newsletter/views.py +++ b/bedrock/newsletter/views.py @@ -262,6 +262,9 @@ def newsletter_subscribe(request): errors.append(ftl("newsletter-form-please-enter-a-valid")) if "privacy" in form.errors: errors.append(ftl("newsletter-form-you-must-agree-to")) + if "office_fax" in form.errors: + # Honeypot field was filled + errors.append(ftl("newsletter-form-we-are-sorry-but-there")) for fieldname in ("newsletters", "lang", "country"): if fieldname in form.errors: errors.extend(form.errors[fieldname]) diff --git a/media/css/protocol/components/newsletter-form.scss b/media/css/protocol/components/newsletter-form.scss index 17d1729ec0b..b619e311056 100644 --- a/media/css/protocol/components/newsletter-form.scss +++ b/media/css/protocol/components/newsletter-form.scss @@ -42,4 +42,19 @@ $image-path: '/media/protocol/img'; } } -/* stylelint-enable declaration-no-important */ +// Honeypot field styling - hide from users but keep accessible to bots +.super-priority-field { + position: absolute !important; + left: -9999px !important; + top: -9999px !important; + width: 1px !important; + height: 1px !important; + overflow: hidden !important; + opacity: 0 !important; + pointer-events: none !important; + + // Hide from screen readers + label { + display: none !important; + } +} From caf0fbcf29ef78dab18200c616d7eb6f0de78ca6 Mon Sep 17 00:00:00 2001 From: Jo Sprague Date: Mon, 14 Jul 2025 13:51:30 +0700 Subject: [PATCH 2/4] Simplify clean_office_fax Follow existing clean_office_fax pattern from this codebase for consistency and because we don't need to be logging this --- bedrock/newsletter/forms.py | 12 +++--------- 1 file changed, 3 insertions(+), 9 deletions(-) diff --git a/bedrock/newsletter/forms.py b/bedrock/newsletter/forms.py index 60209916847..44c2903008a 100644 --- a/bedrock/newsletter/forms.py +++ b/bedrock/newsletter/forms.py @@ -254,15 +254,9 @@ def clean_source_url(self): return su def clean_office_fax(self): - # Check raw data to catch any value, including whitespace-only - office_fax = self.data.get("office_fax", "") - if office_fax: - import logging - - logger = logging.getLogger("b.newsletter") - logger.warning(f'Honeypot field filled with value: "{office_fax}"') - raise forms.ValidationError("Invalid submission") - return "" + honeypot = self.cleaned_data.pop("office_fax", None) + if honeypot: + raise forms.ValidationError("Your submission could not be processed") class EmailForm(forms.Form): From b607795bb0ae88029199f76a16e698acfe8efb12 Mon Sep 17 00:00:00 2001 From: Jo Sprague Date: Mon, 14 Jul 2025 14:09:26 +0700 Subject: [PATCH 3/4] Remove unnecessary test The case of the field containing a whitespace-only value is not likely to happen and removing this test allows us to simplify the clean_office_fax function implementation --- bedrock/newsletter/tests/test_forms.py | 13 ------------- 1 file changed, 13 deletions(-) diff --git a/bedrock/newsletter/tests/test_forms.py b/bedrock/newsletter/tests/test_forms.py index ba434e801a1..f2ecd90d016 100644 --- a/bedrock/newsletter/tests/test_forms.py +++ b/bedrock/newsletter/tests/test_forms.py @@ -258,16 +258,3 @@ def test_honeypot_filled_invalid(self): form = NewsletterFooterForm(self.newsletter_name, locale="en-US", data=data.copy()) self.assertFalse(form.is_valid()) self.assertIn("office_fax", form.errors) - - def test_honeypot_whitespace_invalid(self): - """Honeypot field should be invalid when filled with whitespace only""" - data = { - "email": "foo@example.com", - "lang": "fr", - "privacy": True, - "newsletters": [self.newsletter_name], - "office_fax": " ", # honeypot field - } - form = NewsletterFooterForm(self.newsletter_name, locale="en-US", data=data.copy()) - self.assertFalse(form.is_valid()) - self.assertIn("office_fax", form.errors) From 2e228ce719f50d823a5947aa7efa0b52fcc2fe77 Mon Sep 17 00:00:00 2001 From: Jo Sprague Date: Mon, 14 Jul 2025 16:12:39 +0700 Subject: [PATCH 4/4] Use Protocol mixin for honeypot field styling in newsletter form Replaced explicit styles for the honeypot field with a Protocol mixin for visually hidden elements --- media/css/protocol/components/newsletter-form.scss | 9 +-------- 1 file changed, 1 insertion(+), 8 deletions(-) diff --git a/media/css/protocol/components/newsletter-form.scss b/media/css/protocol/components/newsletter-form.scss index b619e311056..43d6ee520bd 100644 --- a/media/css/protocol/components/newsletter-form.scss +++ b/media/css/protocol/components/newsletter-form.scss @@ -44,14 +44,7 @@ $image-path: '/media/protocol/img'; // Honeypot field styling - hide from users but keep accessible to bots .super-priority-field { - position: absolute !important; - left: -9999px !important; - top: -9999px !important; - width: 1px !important; - height: 1px !important; - overflow: hidden !important; - opacity: 0 !important; - pointer-events: none !important; + @include visually-hidden; // Hide from screen readers label {