Skip to content
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

feat: Implement a honeypot field on our main search #14297

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

petesfrench
Copy link
Contributor

@petesfrench petesfrench commented Sep 16, 2024

Done

This work aims to add a basic honeypot field to our search to catch automated attacks

  • Adds a honeypot input field to the navigation search form and search.html search form
  • Creates search.js, a script responsible for catching and redirecting submission to the honey pot field. This is made globally available through main.js

QA

The follow QA steps should be complete for the navigation search & the search found under /search

  • Make a regular search submission and see everything goes through as normal and the is only one query string present in the url q=
  • Using the inspection tool, find the honey pot field. It is an input with the name 'search'. Add something to the value of this field.
  • Add a value to the regular search field (this is required) and submit
  • You should be redirected to /search. With no query parameters present in the URL and no search result returned.

We should make sure this doesn't interfere with any other searches

Issue / Card

Fixes https://warthogs.atlassian.net/browse/WD-14707

@webteam-app
Copy link

@petesfrench
Copy link
Contributor Author

@samhotep I am not sure the failing playwright tests are related to this PR (even though they are on a 'search' field). As they are failing on my other ubuntu.com PRs also

@samhotep
Copy link
Member

samhotep commented Sep 18, 2024

@petesfrench LGTM!
The only worry I have is that disabling JS will also effectively disable the honeypot, but setting up the honeypot in the backend would probably require changes in the search module itself.

const honeyPotField = searchForm.querySelector("input[name='search']");
// If the honeypot field has a value, it might be a bot, so redirect
if (honeyPotField && honeyPotField.value !== "") {
console.log("Honeypot field has a value, redirecting to search page");
Copy link
Member

@samhotep samhotep Sep 18, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You might want to remove this console log, to give the bot's maintainer no hints :)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was wondering about this. Maybe all the code should be obscured in some way

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we minify the JS?

@petesfrench
Copy link
Contributor Author

@samhotep So I originally wanted to do this from within the search module, but as they works of request there was no way to strip the second search field from the url and it was looking very ugly. An alternative may be some 3rd party middleware. But we can look into that depending on how this goes.

@samhotep
Copy link
Member

@samhotep So I originally wanted to do this from within the search module, but as they works of request there was no way to strip the second search field from the url and it was looking very ugly. An alternative may be some 3rd party middleware. But we can look into that depending on how this goes.

Sounds good to me, its definitely out of scope for this PR

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants