valid_password? checkes for match with the currently changed state of the password
Reported by LacKac | February 12th, 2009 @ 10:11 AM
If valid_password? is called on an unsaved object
which has an updated password field, then it validates the
attempted password with the new unsaved password which in my
opinion is the wrong behaviour. It should still validate with the
original password.
E.g. this could be used to check the old password when the user
wants to change her password. Three fields would be submitted:
old_password, password and
password_confirmation, which would all be set on the
user object by the update_attributes method. Then
somewhere in the validation process a method would check if
old_password is valid. By that time
password and password_confirmation are
set and crypted_password and
password_salt are also set accordingly. This causes a
problem when valid_password? is called since it will
encrypt with the new password_salt and check against
the new crypted_password value.
A suggested fix for this is commited to my fork at github: http://github.com/lackac/authlog... (a test is provided as well)
Comments and changes to this ticket
-
Ben Johnson February 12th, 2009 @ 01:25 PM
- State changed from “new” to “open”
I agree with you, but the problem is that not everyone has dirty attributes. They might have an older version of AR that doesn't have it. I guess in that instance I could fall back to using the current password.
-
Ben Johnson February 13th, 2009 @ 04:13 PM
Alright, so I took a stab at implementing this and it causes problems with new records. That being said, what is wrong with checking the password before your change it? I would imagine that would make the most sense anyways, instead of the other way aropund. What do you think?
-
LacKac February 14th, 2009 @ 12:21 PM
What problems are we talking about? I'd argue with checking first making the most sense. Changing the password is an update on the account object really, which needs an extra validation. I would put this into the model this way not needing anything extra in the controller.
I guess the compatibility problem could be solved by saving the
crypted_passwordandpassword_saltfields to an instance variable on first modification of these values and using these if they exist when validating the password. For new records none of these is necessary, since we surely don't have an old password to validate.I'll try to do this modifications later tonight and submit a patch if it works out.
-
Ben Johnson February 17th, 2009 @ 03:18 AM
Sounds good, it should be an easy change, but I was having issues with my tests. I'll see if I can work on it more as well, but help would be appreciated. Thanks!
-
LacKac March 16th, 2009 @ 06:08 AM
I tried to follow up and make it compatible with Rails 2.0 and below (pre Dirty feature), when I noticed that you use the dirty featureset too in validations to check if the email of login field was changed.
So I guess there's nothing wrong using it for old password validation as well.
-
Ben Johnson March 23rd, 2009 @ 03:19 AM
Sorry for the delay. I'm kind of on the fence with this, but I am leaning towards your side. How about this, if you can update your fork, make the changes, and have the tests pass then I'll include. I also agree that the "dirty" feature can be assumed installed, since I am using it throughout Authlogic.
-
Ben Johnson April 3rd, 2009 @ 01:21 AM
- State changed from “open” to “resolved”
Please let me know if this is still an issue.
-
Aaron April 20th, 2009 @ 07:22 PM
I'd like to see this change implemented as well. I just ran into the same error in my application:
def validate_on_update unless self.old_password.nil?
self.errors.add(:old_password, "Wrong password") unless valid_password?(old_password)end end
This checks against the new password when user.password and user.password_confirmation are sent instead of checking against the old one
-
Ben Johnson April 22nd, 2009 @ 06:18 PM
I decided to add better support for this. See this commit: http://github.com/binarylogic/au...
-
-
Please Sign in or create a free account to add a new ticket.
With your very own profile, you can contribute to projects, track your activity, watch tickets, receive and update tickets through your email and much more.
Create new ticket
Create your profile
Help contribute to this project by taking a few moments to create your personal profile. Create your profile ยป
Object based authentication solution that handles all of the non sense for you. It's as easy as ActiveRecord is with a database.
Aaron
Ben Johnson