Merge branch '14552-signup-password-leak' into 'master'

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.

[Closes #14552]

See merge request !3691
This commit is contained in:
Rémy Coutable 2016-04-18 08:27:19 +00:00
commit 6d899f46b5
3 changed files with 60 additions and 5 deletions

View file

@ -56,6 +56,7 @@ v 8.7.0 (unreleased)
- Decouple membership and notifications - Decouple membership and notifications
- Fix creation of merge requests for orphaned branches (Stan Hu) - Fix creation of merge requests for orphaned branches (Stan Hu)
- API: Ability to retrieve a single tag (Robert Schilling) - 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) - 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) - Remove "Congratulations!" tweet button on newly-created project. (Connor Shea)
- Fix admin/projects when using visibility levels on search (PotHix) - Fix admin/projects when using visibility levels on search (PotHix)

View file

@ -6,18 +6,17 @@
.login-heading .login-heading
%h3 Create an account %h3 Create an account
.login-body .login-body
- user = params[:user].present? ? params[:user] : {}
= form_for(resource, as: resource_name, url: registration_path(resource_name)) do |f| = form_for(resource, as: resource_name, url: registration_path(resource_name)) do |f|
.devise-errors .devise-errors
= devise_error_messages! = devise_error_messages!
%div %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 %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 %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 .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 %div
- if current_application_settings.recaptcha_enabled - if current_application_settings.recaptcha_enabled
= recaptcha_tags = recaptcha_tags

View file

@ -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