Skip to content

Commit

Permalink
Honeypot fields and logic (#48)
Browse files Browse the repository at this point in the history
Co-authored-by: Rocket <[email protected]>
  • Loading branch information
SammySteiner and rocketnova authored Jun 28, 2024
1 parent 4220508 commit c7e83fa
Show file tree
Hide file tree
Showing 19 changed files with 110 additions and 9 deletions.
5 changes: 3 additions & 2 deletions app-rails/app/controllers/users/passwords_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,8 @@ def forgot

def send_reset_password_instructions
email = params[:users_forgot_password_form][:email]
@form = Users::ForgotPasswordForm.new(email: email)
spam_trap = params[:users_forgot_password_form][:spam_trap]
@form = Users::ForgotPasswordForm.new(email: email, spam_trap: spam_trap)

if @form.invalid?
flash.now[:errors] = @form.errors.full_messages
Expand Down Expand Up @@ -59,6 +60,6 @@ def auth_service
end

def reset_password_params
params.require(:users_reset_password_form).permit(:email, :code, :password)
params.require(:users_reset_password_form).permit(:email, :code, :password, :spam_trap)
end
end
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,7 @@ def auth_service
end

def registration_params
params.require(:users_registration_form).permit(:email, :password, :password_confirmation, :role)
params.require(:users_registration_form).permit(:email, :password, :password_confirmation, :role, :spam_trap)
end

def verify_account_params
Expand Down
2 changes: 1 addition & 1 deletion app-rails/app/controllers/users/sessions_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,7 @@ def auth_service
def new_session_params
# If :users_new_session_form is renamed, make sure to also update it in
# cognito_authenticatable.rb otherwise login will not work.
params.require(:users_new_session_form).permit(:email, :password)
params.require(:users_new_session_form).permit(:email, :password, :spam_trap)
end

# This is similar to the default Devise SessionController implementation
Expand Down
4 changes: 3 additions & 1 deletion app-rails/app/forms/users/forgot_password_form.rb
Original file line number Diff line number Diff line change
@@ -1,7 +1,9 @@
class Users::ForgotPasswordForm
include ActiveModel::Model

attr_accessor :email
attr_accessor :email, :spam_trap

validates :email, format: { with: URI::MailTo::EMAIL_REGEXP }

validates :spam_trap, absence: true
end
4 changes: 3 additions & 1 deletion app-rails/app/forms/users/new_session_form.rb
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,10 @@
class Users::NewSessionForm
include ActiveModel::Model

attr_accessor :email, :password
attr_accessor :email, :password, :spam_trap

validates :email, :password, presence: true
validates :email, format: { with: URI::MailTo::EMAIL_REGEXP }, if: -> { email.present? }

validates :spam_trap, absence: true
end
4 changes: 3 additions & 1 deletion app-rails/app/forms/users/registration_form.rb
Original file line number Diff line number Diff line change
Expand Up @@ -3,10 +3,12 @@
class Users::RegistrationForm
include ActiveModel::Model

attr_accessor :email, :password, :password_confirmation, :role
attr_accessor :email, :password, :password_confirmation, :role, :spam_trap

validates :email, :password, :role, presence: true
validates :email, format: { with: URI::MailTo::EMAIL_REGEXP }, if: -> { email.present? }

validates :password, confirmation: true, if: -> { password.present? }

validates :spam_trap, absence: true
end
4 changes: 3 additions & 1 deletion app-rails/app/forms/users/reset_password_form.rb
Original file line number Diff line number Diff line change
Expand Up @@ -3,9 +3,11 @@
class Users::ResetPasswordForm
include ActiveModel::Model

attr_accessor :email, :password, :code
attr_accessor :email, :password, :code, :spam_trap

validates :email, :password, :code, presence: true
validates :email, format: { with: URI::MailTo::EMAIL_REGEXP }, if: -> { email.present? }
validates :code, length: { is: 6 }, if: -> { code.present? }

validates :spam_trap, absence: true
end
10 changes: 10 additions & 0 deletions app-rails/app/helpers/uswds_form_builder.rb
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,16 @@ def submit(value = nil, options = {})
super(value, options)
end

def honeypot_field
spam_trap_classes = "opacity-0 position-absolute z-bottom top-0 left-0 height-0 width-0"
label_text = "Do not fill in this field. It is an anti-spam measure."

@template.content_tag(:div, class: "usa-form-group #{spam_trap_classes}") do
label(:spam_trap, label_text, { tabindex: -1, class: "usa-label #{spam_trap_classes}" }) +
@template.text_field(@object_name, :spam_trap, { autocomplete: "false", tabindex: -1, class: "usa-input #{spam_trap_classes}" })
end
end

########################################
# Custom helpers
########################################
Expand Down
1 change: 1 addition & 0 deletions app-rails/app/views/users/passwords/forgot.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@

<%= us_form_with model: @form, url: users_forgot_password_path, local: true do |f| %>
<%= f.email_field :email, { autocomplete: "username" } %>
<%= f.honeypot_field %>

<%= f.submit t(".submit") %>
<% end %>
1 change: 1 addition & 0 deletions app-rails/app/views/users/passwords/reset.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
<h1><%= t(".title") %></h1>

<%= us_form_with model: @form, url: users_reset_password_path, local: true do |f| %>
<%= f.honeypot_field %>
<%= f.text_field :code, { autocomplete: "off", label: t('.code'), width: "md" } %>

<%= f.email_field :email, { autocomplete: "username" } %>
Expand Down
1 change: 1 addition & 0 deletions app-rails/app/views/users/registrations/new.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@

<%= us_form_with model: @form, url: users_registrations_path, local: true do |f| %>
<%= f.hidden_field :role %>
<%= f.honeypot_field %>
<%= f.email_field :email %>

<%= f.password_field :password, autocomplete: "new-password", id: "new-password", hint: t("users.password_hint") %>
Expand Down
1 change: 1 addition & 0 deletions app-rails/app/views/users/sessions/new.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
</h1>

<%= us_form_with model: @form, url: new_user_session_path, local: true do |f| %>
<%= f.honeypot_field %>
<%= f.email_field :email, { autocomplete: "username" } %>

<%= f.password_field :password, {id: "password", autocomplete: "current-password"} %>
Expand Down
4 changes: 4 additions & 0 deletions app-rails/config/locales/defaults/en.yml
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,10 @@ en:
default: "%m/%d/%Y"
short: "%b %d"
long: "%B %d, %Y"
errors:
attributes:
spam_trap:
present: "This field should be left empty. It is intended to prevent bots from submitting spam using this form."
helpers:
submit:
create: "Submit"
Expand Down
23 changes: 23 additions & 0 deletions app-rails/spec/controllers/users/passwords_controller_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,15 @@

expect(response.status).to eq(422)
end

it "handles submission by bots" do
post :send_reset_password_instructions, params: {
users_forgot_password_form: { email: "[email protected]", spam_trap: "I am a bot" },
locale: "en"
}

expect(response.status).to eq(422)
end
end

describe "GET reset" do
Expand Down Expand Up @@ -95,5 +104,19 @@

expect(response.status).to eq(422)
end

it "handles submission by bots" do
post :confirm_reset, params: {
users_reset_password_form: {
email: "[email protected]",
code: "123456",
password: "password",
spam_trap: "I am a bot"
},
locale: "en"
}

expect(response.status).to eq(422)
end
end
end
16 changes: 16 additions & 0 deletions app-rails/spec/controllers/users/registrations_controller_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,22 @@

expect(response.status).to eq(422)
end

it "handles submission by bots" do
email = "[email protected]"

post :create, params: {
users_registration_form: {
email: email,
password: "password",
role: "employer",
spam_trap: "I am a bot"
},
locale: "en"
}

expect(response.status).to eq(422)
end
end

describe "GET new_account_verification" do
Expand Down
15 changes: 15 additions & 0 deletions app-rails/spec/controllers/users/sessions_controller_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -91,6 +91,21 @@
expect(session[:challenge_email]).to eq("[email protected]")
expect(response).to redirect_to(session_challenge_path)
end

it "handles submission by bots" do
create(:user, uid: uid)

post :create, params: {
users_new_session_form: {
email: "[email protected]",
password: "password",
spam_trap: "I am a bot"
},
locale: "en"
}

expect(response.status).to eq(422)
end
end

describe "GET challenge" do
Expand Down
11 changes: 11 additions & 0 deletions app-rails/spec/forms/users/new_session_form_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -29,4 +29,15 @@
expect(form).not_to be_valid
expect(form.errors.of_kind?(:email, :invalid)).to be_truthy
end

it "requires the honeypot field to be empty" do
form = Users::NewSessionForm.new(
email: "[email protected]",
password: "password",
spam_trap: "I am a bot"
)

expect(form).not_to be_valid
expect(form.errors.of_kind?(:spam_trap, :present)).to be_truthy
end
end
9 changes: 9 additions & 0 deletions app-rails/spec/forms/users/registration_form_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -36,4 +36,13 @@
expect(form).not_to be_valid
expect(form.errors.of_kind?(:email, :invalid)).to be_truthy
end

it "requires the honeypot field is empty" do
form.email = "[email protected]"
form.password = valid_password
form.spam_trap = "I am a bot"

expect(form).not_to be_valid
expect(form.errors.of_kind?(:spam_trap, :present)).to be_truthy
end
end
2 changes: 1 addition & 1 deletion docs/app-rails/application-security.md
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ There is currently no file upload or download functionality at this time, so ple
- [x] Use a secondary verification when users change their password
- Note: Change password requires 6 digit code from email sent to user's email address.
- [ ] Require user's password when changing email.
- [ ] Include honeypot fields and logic on Non logged in forms to catch bots that spam all fields (good resource: https://nedbatchelder.com/text/stopbots.html).
- [x] Include honeypot fields and logic on Non logged in forms to catch bots that spam all fields (good resource: https://nedbatchelder.com/text/stopbots.html).
- [ ] Consider using Captcha on account creation, login, change password, and change email forms.
- Note: Captchas are often not accessible to screen readers and their use should be part of a UX discussion.
- [x] Filter log entries so they do not include passwords or secrets
Expand Down

0 comments on commit c7e83fa

Please sign in to comment.