From e57a9551df0a6b6de218bf881322fcba5a77485b Mon Sep 17 00:00:00 2001 From: Timothy Andrew Date: Wed, 13 Apr 2016 12:02:20 +0530 Subject: [PATCH 1/5] Don't populate the password field on signup validation errors. - Previously, we were pulling `params[:user][:password] as the default value for the password field. This is incorrect; we should be pulling it from `@user.password` or the like. --- app/views/devise/shared/_signup_box.html.haml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/views/devise/shared/_signup_box.html.haml b/app/views/devise/shared/_signup_box.html.haml index cb93ff2465..7f23dbfed9 100644 --- a/app/views/devise/shared/_signup_box.html.haml +++ b/app/views/devise/shared/_signup_box.html.haml @@ -17,7 +17,7 @@ %div = f.email_field :email, class: "form-control middle", value: user[:email], placeholder: "Email", required: true .form-group.append-bottom-20#password-strength - = f.password_field :password, class: "form-control bottom", value: user[:password], id: "user_password_sign_up", placeholder: "Password", required: true + = f.password_field :password, class: "form-control bottom", id: "user_password_sign_up", placeholder: "Password", required: true %div - if current_application_settings.recaptcha_enabled = recaptcha_tags From d5f44d8a4b23b20f8292147db9c7f7730de70a3b Mon Sep 17 00:00:00 2001 From: Timothy Andrew Date: Wed, 13 Apr 2016 12:57:40 +0530 Subject: [PATCH 2/5] Don't retrieve default values from `params` - In the signup page. --- app/views/devise/shared/_signup_box.html.haml | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/app/views/devise/shared/_signup_box.html.haml b/app/views/devise/shared/_signup_box.html.haml index 7f23dbfed9..e5607dacd0 100644 --- a/app/views/devise/shared/_signup_box.html.haml +++ b/app/views/devise/shared/_signup_box.html.haml @@ -6,16 +6,15 @@ .login-heading %h3 Create an account .login-body - - user = params[:user].present? ? params[:user] : {} = form_for(resource, as: resource_name, url: registration_path(resource_name)) do |f| .devise-errors = devise_error_messages! %div - = f.text_field :name, class: "form-control top", value: user[:name], placeholder: "Name", required: true + = f.text_field :name, class: "form-control top", placeholder: "Name", required: true %div - = f.text_field :username, class: "form-control middle", value: user[:username], placeholder: "Username", required: true + = f.text_field :username, class: "form-control middle", placeholder: "Username", required: true %div - = f.email_field :email, class: "form-control middle", value: user[:email], placeholder: "Email", required: true + = 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 %div From cfcd95b0c3c0c3699de8724e60cc2bce687a9132 Mon Sep 17 00:00:00 2001 From: Timothy Andrew Date: Fri, 15 Apr 2016 10:03:21 +0530 Subject: [PATCH 3/5] Update CHANGELOG --- CHANGELOG | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG b/CHANGELOG index 071e35167f..963d2ad681 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -43,6 +43,7 @@ v 8.7.0 (unreleased) - Update number of Todos in the sidebar when it's marked as "Done". !3600 - API: Expose 'updated_at' for issue, snippet, and merge request notes (Robert Schilling) - API: User can leave a project through the API when not master or owner. !3613 + - While signing up, don't persist the user password across form redisplays v 8.6.6 - Fix error on language detection when repository has no HEAD (e.g., master branch). !3654 (Jeroen Bobbeldijk) From 40bc8e7677cb27f87fef7d4362fbd17bee8980ef Mon Sep 17 00:00:00 2001 From: Timothy Andrew Date: Mon, 18 Apr 2016 12:19:11 +0530 Subject: [PATCH 4/5] Add acceptance test to check if the user password persists after form redisplays. - While signing up. --- spec/features/signup_spec.rb | 55 ++++++++++++++++++++++++++++++++++++ 1 file changed, 55 insertions(+) create mode 100644 spec/features/signup_spec.rb diff --git a/spec/features/signup_spec.rb b/spec/features/signup_spec.rb new file mode 100644 index 0000000000..01472743b2 --- /dev/null +++ b/spec/features/signup_spec.rb @@ -0,0 +1,55 @@ +require 'spec_helper' + +feature 'Signup', feature: true do + describe 'signup with no errors' do + it 'creates the user account and sends a confirmation email' do + user = build(:user) + + 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 + click_button "Sign up" + + expect(current_path).to eq user_session_path + expect(page).to have_content("A message with a confirmation link has been sent to your email address.") + end + end + + describe 'signup with errors' do + it "displays the errors" do + existing_user = create(:user) + user = build(:user) + + 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 + click_button "Sign up" + + 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("Email has already been taken") + end + + it 'does not redisplay the password' do + existing_user = create(:user) + user = build(:user) + + 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 + click_button "Sign up" + + expect(current_path).to eq user_registration_path + expect(page.body).not_to match(/#{user.password}/) + end + end +end From 38557ec400d8c28ea73df4bc5142e156c7ab8855 Mon Sep 17 00:00:00 2001 From: Timothy Andrew Date: Mon, 18 Apr 2016 12:30:04 +0530 Subject: [PATCH 5/5] Move CHANGELOG entry to a random place. --- CHANGELOG | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG b/CHANGELOG index 225751cbb0..ede0c00e90 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -56,6 +56,7 @@ v 8.7.0 (unreleased) - Decouple membership and notifications - Fix creation of merge requests for orphaned branches (Stan Hu) - API: Ability to retrieve a single tag (Robert Schilling) + - While signing up, don't persist the user password across form redisplays - Fall back to `In-Reply-To` and `References` headers when sub-addressing is not available (David Padilla) - Remove "Congratulations!" tweet button on newly-created project. (Connor Shea) - Fix admin/projects when using visibility levels on search (PotHix) @@ -66,7 +67,6 @@ v 8.7.0 (unreleased) - Update number of Todos in the sidebar when it's marked as "Done". !3600 - API: Expose 'updated_at' for issue, snippet, and merge request notes (Robert Schilling) - API: User can leave a project through the API when not master or owner. !3613 - - While signing up, don't persist the user password across form redisplays - Fix repository cache invalidation issue when project is recreated with an empty repo (Stan Hu) - Fix: Allow empty recipients list for builds emails service when pushed is added (Frank Groeneveld) - Improved markdown forms