From a6ba8647f919cca5f37f663502186d8b6b7642ec Mon Sep 17 00:00:00 2001
From: Robert Speicher <rspeicher@gmail.com>
Date: Tue, 19 Apr 2016 16:00:45 -0400
Subject: [PATCH] Improve uniqueness of field names on the signup form
Closes https://gitlab.com/gitlab-org/gitlab-ce/issues/15075
---
app/controllers/registrations_controller.rb | 7 ++++++
app/views/devise/shared/_signup_box.html.haml | 4 ++--
spec/features/signup_spec.rb | 24 +++++++++----------
spec/features/users_spec.rb | 16 ++++++-------
4 files changed, 29 insertions(+), 22 deletions(-)
diff --git a/app/controllers/registrations_controller.rb b/app/controllers/registrations_controller.rb
index c48175a4c5a..b441b34d0be 100644
--- a/app/controllers/registrations_controller.rb
+++ b/app/controllers/registrations_controller.rb
@@ -8,6 +8,13 @@ class RegistrationsController < Devise::RegistrationsController
def create
if !Gitlab::Recaptcha.load_configurations! || verify_recaptcha
+ # To avoid duplicate form fields on the login page, the registration form
+ # names fields using `new_user`, but Devise still wants the params in
+ # `user`.
+ if params["new_#{resource_name}"].present? && params[resource_name].blank?
+ params[resource_name] = params.delete(:"new_#{resource_name}")
+ end
+
super
else
flash[:alert] = "There was an error with the reCAPTCHA code below. Please re-enter the code."
diff --git a/app/views/devise/shared/_signup_box.html.haml b/app/views/devise/shared/_signup_box.html.haml
index e5607dacd0d..510215bb8cd 100644
--- a/app/views/devise/shared/_signup_box.html.haml
+++ b/app/views/devise/shared/_signup_box.html.haml
@@ -6,7 +6,7 @@
.login-heading
%h3 Create an account
.login-body
- = form_for(resource, as: resource_name, url: registration_path(resource_name)) do |f|
+ = form_for(resource, as: "new_#{resource_name}", url: registration_path(resource_name)) do |f|
.devise-errors
= devise_error_messages!
%div
@@ -16,7 +16,7 @@
%div
= f.email_field :email, class: "form-control middle", placeholder: "Email", required: true
.form-group.append-bottom-20#password-strength
- = f.password_field :password, class: "form-control bottom", id: "user_password_sign_up", placeholder: "Password", required: true
+ = f.password_field :password, class: "form-control bottom", placeholder: "Password", required: true
%div
- if current_application_settings.recaptcha_enabled
= recaptcha_tags
diff --git a/spec/features/signup_spec.rb b/spec/features/signup_spec.rb
index 01472743b2a..a9b81733897 100644
--- a/spec/features/signup_spec.rb
+++ b/spec/features/signup_spec.rb
@@ -7,10 +7,10 @@ feature 'Signup', feature: true do
visit root_path
- fill_in 'user_name', with: user.name
- fill_in 'user_username', with: user.username
- fill_in 'user_email', with: user.email
- fill_in 'user_password_sign_up', 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_password', with: user.password
click_button "Sign up"
expect(current_path).to eq user_session_path
@@ -25,10 +25,10 @@ feature 'Signup', feature: true do
visit root_path
- fill_in 'user_name', with: user.name
- fill_in 'user_username', with: user.username
- fill_in 'user_email', with: existing_user.email
- fill_in 'user_password_sign_up', 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: existing_user.email
+ fill_in 'new_user_password', with: user.password
click_button "Sign up"
expect(current_path).to eq user_registration_path
@@ -42,10 +42,10 @@ feature 'Signup', feature: true do
visit root_path
- fill_in 'user_name', with: user.name
- fill_in 'user_username', with: user.username
- fill_in 'user_email', with: existing_user.email
- fill_in 'user_password_sign_up', 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: existing_user.email
+ fill_in 'new_user_password', with: user.password
click_button "Sign up"
expect(current_path).to eq user_registration_path
diff --git a/spec/features/users_spec.rb b/spec/features/users_spec.rb
index c1248162031..cf116040394 100644
--- a/spec/features/users_spec.rb
+++ b/spec/features/users_spec.rb
@@ -5,10 +5,10 @@ feature 'Users', feature: true do
scenario 'GET /users/sign_in creates a new user account' do
visit new_user_session_path
- fill_in 'user_name', with: 'Name Surname'
- fill_in 'user_username', with: 'Great'
- fill_in 'user_email', with: 'name@mail.com'
- fill_in 'user_password_sign_up', 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_password', with: 'password1234'
expect { click_button 'Sign up' }.to change { User.count }.by(1)
end
@@ -31,10 +31,10 @@ feature 'Users', feature: true do
scenario 'Should show one error if email is already taken' do
visit new_user_session_path
- fill_in 'user_name', with: 'Another user name'
- fill_in 'user_username', with: 'anotheruser'
- fill_in 'user_email', with: user.email
- fill_in 'user_password_sign_up', 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_password', with: '12341234'
expect { click_button 'Sign up' }.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}'
--
GitLab