Rename profile password fields so password managers understand

Previously, password managers and browsers were confused about
which field on the profile password change form was the current
password. This renames the fields so the current password field
is called 'password', while new password fields are appropriately
named.

Changelog: security

app/controllers/profiles/passwords_controller.rb

@@ -15,17 +15,11 @@ class Profiles::PasswordsController < Profiles::ApplicationController
   end
 
   def create
-    unless @user.password_automatically_set || @user.valid_password?(user_params[:current_password])
+    unless @user.password_automatically_set || @user.valid_password?(user_params[:password])
       redirect_to new_profile_password_path, alert: _('You must provide a valid current password')
       return
     end
 
-    password_attributes = {
-      password: user_params[:password],
-      password_confirmation: user_params[:password_confirmation],
-      password_automatically_set: false
-    }
-
     result = Users::UpdateService.new(current_user, password_attributes.merge(user: @user)).execute
 
     if result[:status] == :success
@@ -41,12 +35,7 @@ class Profiles::PasswordsController < Profiles::ApplicationController
   end
 
   def update
-    password_attributes = user_params.select do |key, value|
-      %w(password password_confirmation).include?(key.to_s)
-    end
-    password_attributes[:password_automatically_set] = false
-
-    unless @user.password_automatically_set || @user.valid_password?(user_params[:current_password])
+    unless @user.password_automatically_set || @user.valid_password?(user_params[:password])
       handle_invalid_current_password_attempt!
 
       redirect_to edit_profile_password_path, alert: _('You must provide a valid current password')
@@ -94,6 +83,14 @@ class Profiles::PasswordsController < Profiles::ApplicationController
   end
 
   def user_params
-    params.require(:user).permit(:current_password, :password, :password_confirmation)
+    params.require(:user).permit(:password, :new_password, :password_confirmation)
+  end
+
+  def password_attributes
+    {
+      password: user_params[:new_password],
+      password_confirmation: user_params[:password_confirmation],
+      password_automatically_set: false
+    }
   end
 end

app/views/profiles/passwords/edit.html.haml

@@ -19,16 +19,16 @@
 
       - unless @user.password_automatically_set?
         .form-group
-          = f.label :current_password, _('Current password'), class: 'label-bold'
-          = f.password_field :current_password, required: true, class: 'form-control gl-form-input', data: { qa_selector: 'current_password_field' }
+          = f.label :password, _('Current password'), class: 'label-bold'
+          = f.password_field :password, required: true, autocomplete: 'current-password', class: 'form-control gl-form-input', data: { qa_selector: 'current_password_field' }
           %p.form-text.text-muted
             = _('You must provide your current password in order to change it.')
       .form-group
-        = f.label :password, _('New password'), class: 'label-bold'
-        = f.password_field :password, required: true, class: 'form-control gl-form-input', data: { qa_selector: 'new_password_field' }
+        = f.label :new_password, _('New password'), class: 'label-bold'
+        = f.password_field :new_password, required: true, autocomplete: 'new-password', class: 'form-control gl-form-input', data: { qa_selector: 'new_password_field' }
       .form-group
         = f.label :password_confirmation, _('Password confirmation'), class: 'label-bold'
-        = f.password_field :password_confirmation, required: true, class: 'form-control gl-form-input', data: { qa_selector: 'confirm_password_field' }
+        = f.password_field :password_confirmation, required: true, autocomplete: 'new-password', class: 'form-control gl-form-input', data: { qa_selector: 'confirm_password_field' }
       .gl-mt-3.gl-mb-3
         = f.submit _('Save password'), class: "gl-button btn btn-confirm gl-mr-3", data: { qa_selector: 'save_password_button' }
         - unless @user.password_automatically_set?

app/views/profiles/passwords/new.html.haml

@@ -14,18 +14,18 @@
   - unless @user.password_automatically_set?
     .form-group.row
       .col-sm-2.col-form-label
-        = f.label :current_password, _('Current password')
+        = f.label :password, _('Current password')
       .col-sm-10
-        = f.password_field :current_password, required: true, class: 'form-control gl-form-input', data: { qa_selector: 'current_password_field' }
+        = f.password_field :password, required: true, autocomplete: 'current-password', class: 'form-control gl-form-input', data: { qa_selector: 'current_password_field' }
   .form-group.row
     .col-sm-2.col-form-label
-      = f.label :password, _('New password')
+      = f.label :new_password, _('New password')
     .col-sm-10
-      = f.password_field :password, required: true, class: 'form-control gl-form-input', data: { qa_selector: 'new_password_field' }
+      = f.password_field :new_password, required: true, autocomplete: 'new-password', class: 'form-control gl-form-input', data: { qa_selector: 'new_password_field' }
   .form-group.row
     .col-sm-2.col-form-label
       = f.label :password_confirmation, _('Password confirmation')
     .col-sm-10
-      = f.password_field :password_confirmation, required: true, class: 'form-control gl-form-input', data: { qa_selector: 'confirm_password_field' }
+      = f.password_field :password_confirmation, required: true, autocomplete: 'new-password', class: 'form-control gl-form-input', data: { qa_selector: 'confirm_password_field' }
   .form-actions
     = f.submit _('Set new password'), class: 'gl-button btn btn-confirm', data: { qa_selector: 'set_new_password_button' }

spec/features/profiles/password_spec.rb

@@ -89,7 +89,7 @@ RSpec.describe 'Profile > Password' do
     shared_examples 'user enters an incorrect current password' do
       subject do
         page.within '.update-password' do
-          fill_in 'user_current_password', with: user_current_password
+          fill_in 'user_password', with: user_current_password
           fill_passwords(new_password, new_password)
         end
       end
@@ -131,7 +131,7 @@ RSpec.describe 'Profile > Password' do
     end
 
     context 'when current password is incorrect' do
-      let(:user_current_password) {'invalid' }
+      let(:user_current_password) { 'invalid' }
 
       it_behaves_like 'user enters an incorrect current password'
     end
@@ -139,7 +139,7 @@ RSpec.describe 'Profile > Password' do
     context 'when the password reset is successful' do
       subject do
         page.within '.update-password' do
-          fill_in "user_current_password", with: user.password
+          fill_in "user_password", with: user.password
           fill_passwords(new_password, new_password)
         end
       end
@@ -169,8 +169,8 @@ RSpec.describe 'Profile > Password' do
 
       expect(current_path).to eq new_profile_password_path
 
-      fill_in :user_current_password,      with: user.password
-      fill_in :user_password,              with: '12345678'
+      fill_in :user_password,      with: user.password
+      fill_in :user_new_password,  with: '12345678'
       fill_in :user_password_confirmation, with: '12345678'
       click_button 'Set new password'
 

spec/features/users/login_spec.rb

@@ -866,8 +866,8 @@ RSpec.describe 'Login', :clean_gitlab_redis_shared_state do
 
         expect(current_path).to eq(new_profile_password_path)
 
-        fill_in 'user_current_password', with: '12345678'
-        fill_in 'user_password', with: 'new password'
+        fill_in 'user_password', with: '12345678'
+        fill_in 'user_new_password', with: 'new password'
         fill_in 'user_password_confirmation', with: 'new password'
         click_button 'Set new password'