-
Notifications
You must be signed in to change notification settings - Fork 946
Add honeypot field to newsletter form to prevent spam submissions #16406
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
Adds a honeypot field to the newsletter footer form to trap automated spam submissions.
- Introduces an
office_fax
honeypot field in the form, widget, validation, view logic, and template. - Hides the honeypot field via SCSS and ensures it doesn’t affect user interaction.
- Covers honeypot behavior with form-level tests for empty, filled, and whitespace inputs.
Reviewed Changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.
Show a summary per file
File | Description |
---|---|
media/css/protocol/components/newsletter-form.scss | Styles .super-priority-field to hide the honeypot input |
bedrock/newsletter/forms.py | Adds office_fax field with HoneyPotWidget and validation |
bedrock/newsletter/views.py | Handles honeypot validation errors and appends an FTL error |
bedrock/newsletter/templates/newsletter/includes/form.html | Renders the honeypot field in the subscription form |
bedrock/newsletter/tests/test_forms.py | Tests for valid/invalid honeypot inputs |
Comments suppressed due to low confidence (4)
bedrock/newsletter/forms.py:190
- The
NewsletterFooterForm
docstring doesn't mention the newoffice_fax
honeypot field. Consider updating the class docstring to describe its purpose so maintainers know why it's present.
office_fax = forms.CharField(widget=HoneyPotWidget(), required=False, empty_value='')
bedrock/newsletter/tests/test_forms.py:273
- There are good form‐level tests for the honeypot, but no integration test for the view’s response message. Consider adding a test that submits via the view and asserts the correct FTL error message is returned in the response.
self.assertIn("office_fax", form.errors)
media/css/protocol/components/newsletter-form.scss:60
- The CSS property
tab-index
is invalid. To remove the field from tab order, set thetabindex="-1"
attribute on the input element via the form widget or markup rather than in CSS.
tab-index: -1 !important;
media/css/protocol/components/newsletter-form.scss:48
- [nitpick] Consider using Protocol’s built-in
visually-hidden
or equivalent utility class instead of a custom.super-priority-field
to reduce duplicated hide/show styles and improve consistency.
.super-priority-field {
cbe8b85
to
5e04b3a
Compare
Fixes #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.
c145fdb
to
888b4ef
Compare
Please excuse the force-pushes. My local pre-commit hooks stopped working for some reason, and there were linter issues to fix, so I force-pushed to keep the commit history clean. |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #16406 +/- ##
=======================================
Coverage 79.82% 79.83%
=======================================
Files 158 158
Lines 8517 8525 +8
=======================================
+ Hits 6799 6806 +7
- Misses 1718 1719 +1 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Follow existing clean_office_fax pattern from this codebase for consistency and because we don't need to be logging this
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
Replaced explicit styles for the honeypot field with a Protocol mixin for visually hidden elements
Fixes #16231
office_fax
as a honeypot field in the form to trap bots.If this changeset needs to go into the FXC codebase, please add the
WMO and FXC
label.One-line summary
Add honeypot for email capture in footer on mozilla.org
Significant changes and points to review
Issue / Bugzilla link
Fixes #16231
Testing
http://localhost:8000/en-US/newsletter/