Verify current password on password change (#12174)

* Verify current password on password change

* Address PR comments

* Allow password reset for logged in users

* Make password change message more explicit

* Reformat message

* Fix specs

app/controllers/passwords_controller.rb

@@ -0,0 +1,15 @@
+class PasswordsController < Devise::PasswordsController
+  # allow already signed in users to reset their password
+  skip_before_action :require_no_authentication
+
+  def new
+    super
+    session[:referrer] = request.referer
+  end
+
+  protected
+
+  def after_sending_reset_password_instructions_path_for(_resource_name)
+    session[:referrer]
+  end
+end

app/controllers/users_controller.rb

@@ -1,7 +1,8 @@
 class UsersController < ApplicationController
   before_action :set_no_cache_header
-  before_action :raise_suspended, only: %i[update]
-  before_action :set_user, only: %i[update request_destroy full_delete remove_identity]
+  before_action :raise_suspended, only: %i[update update_password]
+  before_action :set_user,
+                only: %i[update update_password request_destroy full_delete remove_identity]
   # rubocop:disable Layout/LineLength
   after_action :verify_authorized, except: %i[index signout_confirm add_org_admin remove_org_admin remove_from_org confirm_destroy]
   # rubocop:enable Layout/LineLength
@@ -42,8 +43,7 @@ class UsersController < ApplicationController
 
     # preferred_languages is handled manually
     @user.language_settings["preferred_languages"] = Languages::LIST.keys & params[:user][:preferred_languages].to_a
-
-    @user.attributes = permitted_attributes(@user)
+    @user.assign_attributes(permitted_attributes(@user))
 
     if @user.save
       # NOTE: [@rhymes] this queues a job to fetch the feed each time the profile is updated, regardless if the user
@@ -269,6 +269,24 @@ class UsersController < ApplicationController
     end
   end
 
+  def update_password
+    set_current_tab("account")
+
+    if @user.update_with_password(password_params)
+      redirect_to user_settings_path(@tab)
+    else
+      Honeycomb.add_field("error", @user.errors.messages.reject { |_, v| v.empty? })
+      Honeycomb.add_field("errored", true)
+
+      if @tab
+        render :edit, status: :bad_request
+      else
+        flash[:error] = @user.errors_as_sentence
+        redirect_to user_settings_path
+      end
+    end
+  end
+
   private
 
   def sanitize_user_params
@@ -359,4 +377,8 @@ class UsersController < ApplicationController
   def profile_params
     params[:profile].permit(Profile.attributes)
   end
+
+  def password_params
+    params.permit(:current_password, :password, :password_confirmation)
+  end
 end

app/models/user.rb

@@ -90,7 +90,8 @@ class User < ApplicationRecord
   }x.freeze
 
   attr_accessor :scholar_email, :new_note, :note_for_current_role, :user_status, :pro, :merge_user_id,
-                :add_credits, :remove_credits, :add_org_credits, :remove_org_credits, :ip_address
+                :add_credits, :remove_credits, :add_org_credits, :remove_org_credits, :ip_address,
+                :current_password
 
   rolify after_add: :index_roles, after_remove: :index_roles
 

app/policies/user_policy.rb

@@ -7,6 +7,7 @@ class UserPolicy < ApplicationPolicy
     config_font
     config_theme
     config_navbar
+    current_password
     currently_hacking_on
     currently_learning
     display_announcements
@@ -116,6 +117,10 @@ class UserPolicy < ApplicationPolicy
     (user.has_role?(:trusted) || minimal_admin?) && !user.banned
   end
 
+  def update_password?
+    current_user?
+  end
+
   def permitted_attributes
     PERMITTED_ATTRIBUTES
   end

app/views/users/_account_set_password.html.erb

@@ -5,9 +5,15 @@
       </h2>
       <p class="color-base-70">
         As an alternative to signing in via linked social accounts, you may create a password.
+
+        If you signed up with a social media account, please <a href="<%= new_user_password_path %>">reset the password</a> for your primary email address (<%= current_user.email %>) first.
       </p>
     </header>
-    <%= form_for @user do |f| %>
+    <%= form_with url: user_update_password_path do |f| %>
+      <div class="crayons-field mb-4">
+        <%= f.label :current_password, class: "crayons-field__label" %>
+        <%= f.password_field :current_password, class: "crayons-textfield", autocomplete: "off" %>
+      </div>
       <div class="crayons-field mb-4">
         <%= f.label :password, class: "crayons-field__label" %>
         <%= f.password_field :password, class: "crayons-textfield", autocomplete: "off" %>

config/routes.rb

@@ -10,7 +10,8 @@ Rails.application.routes.draw do
   devise_for :users, controllers: {
     omniauth_callbacks: "omniauth_callbacks",
     registrations: "registrations",
-    invitations: "invitations"
+    invitations: "invitations",
+    passwords: "passwords"
   }
 
   devise_scope :user do
@@ -386,6 +387,7 @@ Rails.application.routes.draw do
     post "organizations/generate_new_secret" => "organizations#generate_new_secret"
     post "users/api_secrets" => "api_secrets#create", :as => :users_api_secrets
     delete "users/api_secrets/:id" => "api_secrets#destroy", :as => :users_api_secret
+    post "users/update_password", to: "users#update_password", as: :user_update_password
 
     # The priority is based upon order of creation: first created -> highest priority.
     # See how all your routes lay out with "rake routes".

spec/requests/user/user_changes_password_spec.rb

@@ -0,0 +1,59 @@
+require "rails_helper"
+
+RSpec.describe "User changes password", type: :request do
+  let(:password) { "password123" }
+  let(:user) { create(:user, password: password, password_confirmation: password) }
+
+  describe "POST /users/update_password" do
+    it "does not update the password if the current password is wrong" do
+      sign_in user
+      post user_update_password_path,
+           params: {
+             current_password: "wrongwrong",
+             password: "testtest",
+             password_confirmation: "testtest"
+           }
+
+      expect(response.body).to include("Current password is invalid")
+    end
+
+    it "does not update the password if the new password is too short" do
+      sign_in user
+      post user_update_password_path,
+           params: {
+             current_password: password,
+             password: "test",
+             password_confirmation: "test"
+           }
+
+      expect(response.body).to include("Password is too short (minimum is 8 characters)")
+    end
+
+    it "does not update the password if the new passwords don't match" do
+      sign_in user
+      post user_update_password_path,
+           params: {
+             current_password: password,
+             password: "testtest",
+             password_confirmation: "testtesy"
+           }
+
+      expect(response.body).to include("error")
+      expected_message = CGI.escapeHTML("Password doesn't match password confirmation")
+      expect(response.body).to include(expected_message)
+    end
+
+    it "updates the password if all params are valid" do
+      sign_in user
+      post user_update_password_path,
+           params: {
+             current_password: password,
+             password: "testtest",
+             password_confirmation: "testtest"
+           }
+
+      expect(response.body).not_to include("error")
+      expect(response).to redirect_to("/settings/account")
+    end
+  end
+end