navapbc / template-application-rails

Ruby on Rails with USWDS template, including CI/CD, for teams building web applications
Apache License 2.0
1 stars 1 forks source link

Honeypot fields and logic #48

Closed SammySteiner closed 3 months ago

SammySteiner commented 3 months ago

Ticket

Changes

Context for reviewers

Added honeypot fields to publicly accessible forms to combat spam bots, following guidance from the rails security best practices doc .

Testing

SammySteiner commented 3 months ago

Thank you for working on this!

For app/controllers and app/forms/users, could we use the variable honeypot instead of hp_field, so that the field naming convention is consistent and the purpose of the field is spelled out? Without knowing in advance, I think it is hard to guess what hp stands for in this context.

The guidance is not to have the name or id of the fields include the word honeypot as some bots are programmed to ignore fields with that word in the name or id, which is why I went with hp_field. Here are some alternatives:

  1. pick some other descriptive name like,trap_for_spambots or something else
  2. something like h_o_n_e_y_p_o_t should provide some obfuscation
  3. alias the attribute using alias_attribute in the app/forms/users which while a bit messy will contain or obfuscated name and the word honeypot in both forms and controllers.

i.e. in the app/users/form:

attr_accessor :email, :hp_field 
alias_attribute :honeypot_field, :hp_field
validates :honeypot_field, absence: true

in the controller:

honeypot_field = params[:users_forgot_password_form][:hp_field]
@form = Users::ForgotPasswordForm.new(email: email, honeypot_field: honeypot_field)

params.require(:users_reset_password_form).permit(:email, :code, :password, :hp_field)

Thoughts?

rocketnova commented 3 months ago

The guidance is not to have the name or id of the fields include the word honeypot as some bots are programmed to ignore fields with that word in the name or id, which is why I went with hp_field.

Thank you for explaining. What about spamtrap or spam_trap? I also think hp would be acceptable if we added comments everywhere it appeared to explain what it is.

SammySteiner commented 3 months ago

The guidance is not to have the name or id of the fields include the word honeypot as some bots are programmed to ignore fields with that word in the name or id, which is why I went with hp_field.

Thank you for explaining. What about spamtrap or spam_trap? I also think hp would be acceptable if we added comments everywhere it appeared to explain what it is.

Sounds good, I went with spam_trap

Let me know if you think we should also rename the honeypot helper in the form builder to spam_trap? But I like that the honeypot method creates a spam trap 🤷

rocketnova commented 3 months ago

Sounds good, I went with spam_trap

👍

Let me know if you think we should also rename the honeypot helper in the form builder to spam_trap? But I like that the honeypot method creates a spam trap 🤷

🤔 I don't feel strongly about it. I think that if it's named the same thing consistently, there's less mental translation that someone has to do as they navigate the code, but I also think we can always rename it in the future if we hear feedback that it is cumbersome.