Rails 4 - Allow password change only if current password is correct

Question

In my app, users can edit their profile information. On the edit profile form, the user can make changes to all fields (name, title, and more). On this same form are three fields: current_password, password, and password_confirmation. I am using bcrypt's has_secure_password feature for password authentication. I am NOT using Devise at all.

I want users to only be able to change their password if they have supplied a correct current password. I've got this working before with the following code in the update method of my Users controller:

# Check if the user tried changing his/her password and CANNOT be authenticated with the entered current password
if !the_params[:password].blank? && !@user.authenticate(the_params[:current_password])
  # Add an error that states the user's current password is incorrect
  @user.errors.add(:base, "Current password is incorrect.")
else    
  # Try to update the user
  if @user.update_attributes(the_params)
    # Notify the user that his/her profile was updated
    flash.now[:success] = "Your changes have been saved"
  end
end

However, the problem with this approach is that it discards all changes to the user model if just the current password is incorrect. I want to save all changes to the user model but NOT the password change if the current password is incorrect. I've tried splitting up the IF statements like so:

# Check if the user tried changing his/her password and CANNOT be authenticated with the entered current password
if !the_params[:password].blank? && !@user.authenticate(the_params[:current_password])
  # Add an error that states the user's current password is incorrect
  @user.errors.add(:base, "Current password is incorrect.")
end

# Try to update the user
if @user.update_attributes(the_params)
  # Notify the user that his/her profile was updated
  flash.now[:success] = "Your changes have been saved"
end

This doesn't work because the user is able to change his/her password even if the current password is incorrect. When stepping through the code, although the "Current password is incorrect." error is added to @user, after running through the update_attributes method, it seems to ignore this error message.

By the way, the current_password field is a virtual attribute in my User model:

attr_accessor :current_password

I've been stuck trying to figure this out for a couple of hours now, so I can really use some help.

Thanks!

Solution

Thanks to papirtiger, I got this working. I changed the code around a little bit from his answer. Below is my code. Note that either code snippet will work just fine.

In the User model (user.rb)

class User < ActiveRecord::Base
  has_secure_password

  attr_accessor :current_password

  # Validate current password when the user is updated
  validate :current_password_is_correct, on: :update

  # Check if the inputted current password is correct when the user tries to update his/her password
  def current_password_is_correct
    # Check if the user tried changing his/her password
    if !password.blank?
      # Get a reference to the user since the "authenticate" method always returns false when calling on itself (for some reason)
      user = User.find_by_id(id)

      # Check if the user CANNOT be authenticated with the entered current password
      if (user.authenticate(current_password) == false)
        # Add an error stating that the current password is incorrect
        errors.add(:current_password, "is incorrect.")
      end
    end
  end
end

And the code in my Users controller is now simply:

# Try to update the user
if @user.update_attributes(the_params)
  # Notify the user that his/her profile was updated
  flash.now[:success] = "Your changes have been saved"
end

max · Accepted Answer · 2015-05-18T03:58:50.793

7

You could add a custom validation on the model level which checks if the password has changed:

class User < ActiveRecord::Base
  has_secure_password

  validate :current_password_is_correct,
           if: :validate_password?, on: :update

  def current_password_is_correct
    # For some stupid reason authenticate always returns false when called on self
    if User.find(id).authenticate(current_password) == false
      errors.add(:current_password, "is incorrect.")
    end
  end

  def validate_password?
    !password.blank?
  end

  attr_accessor :current_password
end

edited May 18 '15 at 03:58

answered May 18 '15 at 00:39

max

96,212
14
104
165

Edit: You can just check if password is updated. I think the digest is generated on save which would give a false negative. – max May 18 '15 at 00:48
Thanks for the answer. Yes, the digest did give a false negative. Even though the error message was being displayed, the model was still updating. With this new code, I am getting an error: `undefined method password_changed?` – Alexander May 18 '15 at 00:52
Hmm. The magic [attr]_changed? methods come from [ActiveRecord::Dirty](http://www.rubydoc.info/docs/rails/4.1.7/ActiveModel/Dirty#attribute_changed%3F-instance_method). But I guess since password is a virtual attribute than it is not tracked. I haven't used has_secure_password very much but I think that you could just check if password is nil. – max May 18 '15 at 01:02
Edited, check for blank? Instead of nil? – max May 18 '15 at 01:09
I've removed the first IF statement from my second code snippet. Should I have done that? When I try to change my password now, the check for the current password is ignored, just like before. :/ As you suggested, I changed my code to check for `blank?` – Alexander May 18 '15 at 01:30
I will settle with two separate forms by tomorrow afternoon (EST) if I can't figure this out by then. Part of me thinks I might have to redesign the experience. Either way, I think I've been worrying about this aspect of my site too much, especially since my app is in alpha :D – Alexander May 18 '15 at 02:14
Edited the answer. I tested this in a rails sample app, and it covers your requirement but for some strange reason `authenticate(current_password)` always returns false on my development server. When I do it it from the console it returns the user... – max May 18 '15 at 02:35
Edited again - there is some strange bug in `ActiveModel::SecurePassword` which makes authenticate always return false when calling on self. I was able to confirm that this works as expected. – max May 18 '15 at 03:14
Thank you so much for your help; I really appreciate it!! This works beautifully! I changed your code a little bit to fit my preferred style, and I will put it in an edit to my question. – Alexander May 18 '15 at 22:29

Austio · Answer 2 · 2015-05-18T00:35:29.897

1

So thinking from a user perspective, if someone enters the wrong password wouldn't you want the other stuff to not change as well? Normally people will have a password update where it is just email and password. If the current password is incorrect then don't update anything.

If you have to do it this way then just move the logic and have two sets of params or delete the password from the params. Here would be psuedocode for it.

if not_authenticated_correctly
  params = params_minus_password_stuff (or use slice, delete, etc)
end

#Normal update user logic

edited May 18 '15 at 00:35

answered May 18 '15 at 00:24

Austio

5,939
20
34

I kind of disagree with you here. I find it pretty annoying with services that require my password to change my profile. And there is nothing really wrong with having the password change on the same form. – max May 18 '15 at 00:45
Thanks for the answer. If needed, I will use two separate forms like I did before. However, I'm trying to avoid that since my Edit Profile page is a tabbed interface. One of the tabs is the password change. I feel that users will change some settings in one tab and go to another tab before clicking Save. They will lose all of their changes if each tab is a different form. Maybe I need to rethink my design. – Alexander May 18 '15 at 00:55
@papirtiger - it is matter of opinion for sure. If i saw this in a pr my comment would be that it does add some nested complexity to this form though that wouldn't be there if it were a separate thing. So not necessarily wrong, just something that needs to be discussed. When i have the choice i generally do this in an account action that only contains the email/password. – Austio May 18 '15 at 01:01
@Alexander that is understandable, hope this helped you get on the right track. – Austio May 18 '15 at 01:03

John Saltarelli · Answer 3 · 2018-11-02T16:48:28.977

Another approach is to use a custom validator instead of embedding this validation within the model. You can store these custom validators in app/validators and they will be automatically loaded by Rails. I called this one password_match_validator.rb.

In addition to being reusable, this strategy also removes the need to re-query for User when authenticating because the User instance is passed to the validator automatically by rails as the "record" argument.

class PasswordMatchValidator < ActiveModel::EachValidator

   # Password Match Validator
   #
   # We need to validate the users current password
   # matches what we have on-file before we change it
   #
   def validate_each(record, attribute, value)
     unless value.present? && password_matches?(record, value)
       record.errors.add attribute, "does not match"
     end
   end

   private

   # Password Matches?
   #
   # Need to validate if the current password matches
   # based on what the password_digest was. has_secure_password
   # changes the password_digest whenever password is changed.
   #
   # @return Boolean
   #
   def password_matches?(record, value)
     BCrypt::Password.new(record.password_digest_was).is_password?(value)
   end
 end

Once you add the validator to your project you can use it in any model as shown below.

class User < ApplicationRecord

  has_secure_password

  # Add an accessor so you can have a field to validate
  # that is seperate from password, password_confirmation or 
  # password_digest...
  attr_accessor :current_password

  # Validation should only happen if the user is updating 
  # their password after the account has been created.
  validates :current_password, presence: true, password_match: true, on: :update, if: :password_digest_changed?

end

If you don't want to add the attr_accessor to every model you could combine this with a concern but that is probably overkill. Works well if you have separate models for an administrator vs a user. Note that the name of the file, the class name AND the key used on the validator all have to match.

Updated answer, discovered a bug where if you use authenticate() it will look at the "latest" password_digest instead of the one persisted to the DB. Therefore, we need to write our own method within the validator that uses "password_digest_was" — John Saltarelli, Nov 02 '18 at 16:50

score 1 · Answer 4 · answered Dec 16 '21 at 15:29

Just posting it, works for ror 6.x

form.erb file:

  <div class="field">
    <%= form.label :current_password, 'Current password:' %>
    <%= form.password_field :current_password, size: 40 %>
  </div>

  <div class="field">
    <%= form.label :password, 'Password:'%>
    <%= form.password_field :password, size:40 %>
  </div>

  <div class="field">
    <%= form.label :password_confirmation, 'Confirm:' %>
    <%= form.password_field :password_confirmation, id: :user_password_confirmation, size:40 %>
  </div>

  <div class="actions">
    <%= form.submit %>
  </div>

user.rb:

  has_secure_password

  # virtual attribute
  attr_accessor :current_password

  # Validate current password when the user is updated
  validate :current_password_is_correct, on: :update

  # Check if the inputted current password is correct when the user tries to update his/her password
  def current_password_is_correct
    # Check if the user tried changing his/her password
    return if password.blank?

    # Get a reference to the user since the "authenticate" method always returns false when calling on itself (for some reason)
    user = User.find(id)

    # Check if the user CANNOT be authenticated with the entered current password
    if user.authenticate(current_password) == false
      # Add an error stating that the current password is incorrect
      errors.add(:current_password, "is incorrect.")
    end
  end

users_controller.rb:

only need to add ":current_password" to def user_params or pass change wont work and in server log will be written:

Unpermitted parameter: :current_password

Rails 4 - Allow password change only if current password is correct

Solution

4 Answers4

Linked