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

Honeypot fields and logic #48

Merged
merged 12 commits into from
Jun 28, 2024
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)
hp_field = params[:users_forgot_password_form][:hp_field]
@form = Users::ForgotPasswordForm.new(email: email, hp_field: hp_field)

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, :hp_field)
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, :hp_field)
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, :hp_field)
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, :hp_field

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

validates :hp_field, 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, :hp_field

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

validates :hp_field, 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, :hp_field

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 :hp_field, 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, :hp_field

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 :hp_field, 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
hp_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 #{hp_classes}") do
label(:hp_field, label_text, { tabindex: -1, class: "usa-label #{hp_classes}" }) +
@template.text_field(@object_name, :hp_field, { autocomplete: "false", tabindex: -1, class: "usa-input #{hp_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:
hp_field:
present: "You entered text in a field that is meant to catch spambots."
SammySteiner marked this conversation as resolved.
Show resolved Hide resolved
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]", hp_field: "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",
hp_field: "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",
hp_field: "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",
hp_field: "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 is empty" do
SammySteiner marked this conversation as resolved.
Show resolved Hide resolved
form = Users::NewSessionForm.new(
email: "[email protected]",
password: "password",
hp_field: "I am a bot"
)

expect(form).not_to be_valid
expect(form.errors.of_kind?(:hp_field, :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.hp_field = "I am a bot"

expect(form).not_to be_valid
expect(form.errors.of_kind?(:hp_field, :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
Loading