summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorDrew Blessing <drew@gitlab.com>2016-11-11 18:47:32 -0600
committerDrew Blessing <drew@gitlab.com>2017-01-03 00:16:37 -0600
commit33b41bc8a821974d07d01ffdee98db35d1df4840 (patch)
tree45bbf00cc05c87f0659160cb343953ce727e5602
parent46920f7e371debe6af526ab9476aef6ca452185b (diff)
downloadgitlab-ce-33b41bc8a821974d07d01ffdee98db35d1df4840.tar.gz
Add email and password confirmation fields to registration form
It's too easy to mistype an email or password when signing up. The support team is receiving an increasing number of requests because users mistype their email. We can eliminate this problem by requiring users to confirm the email before registering. The same issue can occur for the password field so we should add this, too. We should note that password confirmation is part of the default Devise forms. I don't know why/when GitLab removed it.
-rw-r--r--app/controllers/registrations_controller.rb2
-rw-r--r--app/models/user.rb1
-rw-r--r--app/views/devise/shared/_signup_box.html.haml3
-rw-r--r--changelogs/unreleased/add_email_password_confirmation.yml4
-rw-r--r--spec/features/signup_spec.rb21
-rw-r--r--spec/features/users_spec.rb18
6 files changed, 31 insertions, 18 deletions
diff --git a/app/controllers/registrations_controller.rb b/app/controllers/registrations_controller.rb
index c45196cc3e9..5e652ebe27d 100644
--- a/app/controllers/registrations_controller.rb
+++ b/app/controllers/registrations_controller.rb
@@ -57,7 +57,7 @@ class RegistrationsController < Devise::RegistrationsController
end
def sign_up_params
- params.require(:user).permit(:username, :email, :name, :password, :password_confirmation)
+ params.require(:user).permit(:username, :email, :email_confirmation, :name, :password)
end
def resource_name
diff --git a/app/models/user.rb b/app/models/user.rb
index 899a89a4eaa..94f262d97b3 100644
--- a/app/models/user.rb
+++ b/app/models/user.rb
@@ -99,6 +99,7 @@ class User < ActiveRecord::Base
#
# Note: devise :validatable above adds validations for :email and :password
validates :name, presence: true
+ validates_confirmation_of :email
validates :notification_email, presence: true
validates :notification_email, email: true, if: ->(user) { user.notification_email != user.email }
validates :public_email, presence: true, uniqueness: true, email: true, allow_blank: true
diff --git a/app/views/devise/shared/_signup_box.html.haml b/app/views/devise/shared/_signup_box.html.haml
index 3133f6de2e8..14c72f2080e 100644
--- a/app/views/devise/shared/_signup_box.html.haml
+++ b/app/views/devise/shared/_signup_box.html.haml
@@ -15,6 +15,9 @@
%div.form-group
= f.label :email
= f.email_field :email, class: "form-control middle", required: true, title: "Please provide a valid email address."
+ %div.form-group
+ = f.label :email_confirmation
+ = f.email_field :email_confirmation, class: "form-control middle", required: true, title: "Please retype the email address."
.form-group.append-bottom-20#password-strength
= f.label :password
= f.password_field :password, class: "form-control bottom", required: true, pattern: ".{#{@minimum_password_length},}", title: "Minimum length is #{@minimum_password_length} characters."
diff --git a/changelogs/unreleased/add_email_password_confirmation.yml b/changelogs/unreleased/add_email_password_confirmation.yml
new file mode 100644
index 00000000000..92f9b9b7a6d
--- /dev/null
+++ b/changelogs/unreleased/add_email_password_confirmation.yml
@@ -0,0 +1,4 @@
+---
+title: Add email confirmation field to registration form
+merge_request: 7432
+author:
diff --git a/spec/features/signup_spec.rb b/spec/features/signup_spec.rb
index 65544f79eba..9fde8d6e5cf 100644
--- a/spec/features/signup_spec.rb
+++ b/spec/features/signup_spec.rb
@@ -10,10 +10,11 @@ feature 'Signup', feature: true do
visit root_path
- fill_in 'new_user_name', with: user.name
- fill_in 'new_user_username', with: user.username
- fill_in 'new_user_email', with: user.email
- fill_in 'new_user_password', with: user.password
+ fill_in 'new_user_name', with: user.name
+ fill_in 'new_user_username', with: user.username
+ fill_in 'new_user_email', with: user.email
+ fill_in 'new_user_email_confirmation', with: user.email
+ fill_in 'new_user_password', with: user.password
click_button "Register"
expect(current_path).to eq users_almost_there_path
@@ -29,10 +30,11 @@ feature 'Signup', feature: true do
visit root_path
- fill_in 'new_user_name', with: user.name
- fill_in 'new_user_username', with: user.username
- fill_in 'new_user_email', with: user.email
- fill_in 'new_user_password', with: user.password
+ fill_in 'new_user_name', with: user.name
+ fill_in 'new_user_username', with: user.username
+ fill_in 'new_user_email', with: user.email
+ fill_in 'new_user_email_confirmation', with: user.email
+ fill_in 'new_user_password', with: user.password
click_button "Register"
expect(current_path).to eq dashboard_projects_path
@@ -55,8 +57,9 @@ feature 'Signup', feature: true do
click_button "Register"
expect(current_path).to eq user_registration_path
- expect(page).to have_content("error prohibited this user from being saved")
+ expect(page).to have_content("errors prohibited this user from being saved")
expect(page).to have_content("Email has already been taken")
+ expect(page).to have_content("Email confirmation doesn't match")
end
it 'does not redisplay the password' do
diff --git a/spec/features/users_spec.rb b/spec/features/users_spec.rb
index afa98f3f715..2de0fbe7ab2 100644
--- a/spec/features/users_spec.rb
+++ b/spec/features/users_spec.rb
@@ -6,10 +6,11 @@ feature 'Users', feature: true, js: true do
scenario 'GET /users/sign_in creates a new user account' do
visit new_user_session_path
click_link 'Register'
- fill_in 'new_user_name', with: 'Name Surname'
- fill_in 'new_user_username', with: 'Great'
- fill_in 'new_user_email', with: 'name@mail.com'
- fill_in 'new_user_password', with: 'password1234'
+ fill_in 'new_user_name', with: 'Name Surname'
+ fill_in 'new_user_username', with: 'Great'
+ fill_in 'new_user_email', with: 'name@mail.com'
+ fill_in 'new_user_email_confirmation', with: 'name@mail.com'
+ fill_in 'new_user_password', with: 'password1234'
expect { click_button 'Register' }.to change { User.count }.by(1)
end
@@ -33,10 +34,11 @@ feature 'Users', feature: true, js: true do
scenario 'Should show one error if email is already taken' do
visit new_user_session_path
click_link 'Register'
- fill_in 'new_user_name', with: 'Another user name'
- fill_in 'new_user_username', with: 'anotheruser'
- fill_in 'new_user_email', with: user.email
- fill_in 'new_user_password', with: '12341234'
+ fill_in 'new_user_name', with: 'Another user name'
+ fill_in 'new_user_username', with: 'anotheruser'
+ fill_in 'new_user_email', with: user.email
+ fill_in 'new_user_email_confirmation', with: user.email
+ fill_in 'new_user_password', with: '12341234'
expect { click_button 'Register' }.to change { User.count }.by(0)
expect(page).to have_text('Email has already been taken')
expect(number_of_errors_on_page(page)).to be(1), 'errors on page:\n #{errors_on_page page}'