#valid_password? returns false when updating resource with empty password fields

[codeminator]

Environment

• Ruby 2.4.4
• Rails 5.2.2
• Devise 4.6.1

Description

In prior versions of devise (I tested with v.4.4.2 since, it's the earliest version compatible with Rails ~> 5.2), it was possible to call update statement on a resource with some fields, while submitting password and password_confirmation fields as empty strings, and still old password is valid:

> pw = 'thisIsPassword'
> user.update(password: pw, password_confirmation: pw)
User Update (7.5ms)  UPDATE `users` SET `updated_at` = '2019-03-07 10:18:45', `encrypted_password` = '$2a$10$mh8fvYDEujHZrDIAPao8k.rj.EweEwT/uf4i/Ax71d0oSMYYtvUHe' WHERE `users`.`id` = 5
   (1.4ms)  COMMIT
=> true
> user.valid_password?(pw)
=> true
user.update(email: "another_email@mail.com", password: "", password_confirmation: "")
   (0.4ms)  BEGIN
  User Update (0.7ms)  UPDATE `users` SET `email` = 'another_email@mail.com', `updated_at` = '2019-03-07 10:19:40' WHERE `users`.`id` = 5
   (3.9ms)  COMMIT
=> true
> user.valid_password?(pw)
=> true

While in v.4.6.1 (the latest), #valid_password? returns false when doing the same steps (Basically, it sets encrypted-password column with NULL):

> pw = 'thisIsPasswordTest'
=> "thisIsPasswordTest"
> user.update(password: pw, password_confirmation: pw)
   (0.2ms)  BEGIN
  User Update (0.5ms)  UPDATE `users` SET `updated_at` = '2019-03-07 10:21:57', `encrypted_password` = '$2a$10$uRBpZWVLF2epl3J.apIiZ.KdQ9hFM0Qmn53hMpmNNgvUccCyjmPnq' WHERE `users`.`id` = 5
   (2.0ms)  COMMIT
=> true
> user.valid_password?(pw)
=> true
> user.update(email: "another_email_updated@test.com", password: "", password_confirmation: "")
   (0.2ms)  BEGIN
  User Update (0.4ms)  UPDATE `users` SET `email` = 'another_email_updated@test.com', `updated_at` = '2019-03-07 10:23:13', `encrypted_password` = NULL WHERE `users`.`id` = 5
   (0.5ms)  COMMIT
=> true
[6] pry(main)> user.valid_password?(pw)
=> false

Current behavior

valid_password? with old password returns false when password/password_confirmation fields have been submitted as empty strings within #update call.

Expected behavior

valid_password? with old password returns true password/password_confirmation fields have been submitted as empty strings within #update call (older versions are working that way).

I know that we have already methods such as update_without_password and update_with_password. The question is what if you have a large app, that is updating user information from a form or something that contains password fields, then you have to either edit the code to check the presence of password/password_confirmation params, or change the user experience.

If it's intended behaviour, kindly clarify.

[tegon]

Hello @codeminator, thanks for your report.

We changed this in #4245 because we thought it made sense to keep the attributes in sync (password and encrypted_password).

I did not understand, however, what you said about your controller code that is not working anymore due to this. Are you calling #update in a controller action for many forms - e.g. one has the password field and the other hasn't?
If you can share some of the code here that would be great.

[codeminator]

@tegon Thanks for your feedback.

Actually I encountered the case when I was upgrading a Rails app to latest version of Rails 5, and I had to upgrade devise on the way.

In this app, there is an already-succeeded unit test about updating user field(s) should work fine when submitting empty password fields. This test failed after upgrading devise to the latest version.

The console code I pasted in my previous comment is the exact same thing that the test case is doing.

Please let me know if you need further clarification.

[timkrins]

It seems like your issue isn't really related to valid_password? returning false.

As you pointed out in your example, the encrypted_password value is being set to NULL in your database when you call update.
Therefore, valid_password? should return false.

Your real issue is what you pointed out - your encrypted_password attribute is set to NULL when you update with a blank password and password_confirmation.

I would check you haven't got anything strange happening with your models (make sure that you actually have the devise database_authenticatable methods included on your model, make sure no monkey-patch interference etc).

According to https://github.com/plataformatec/devise/blob/master/lib/devise/models/database_authenticatable.rb#L95
a blank password param (and password_confirmation) is deleted.

[codeminator]

@timkrins Thanks, no monkey patch interference and it's not about including database_authenticatable methods, let me explain why:

• In older devise version, when you try to update user attribute(s), while having password/password confirmation fields submitted as "", then it doesn't set encrypted_password with NULL, and update the other attribute(s).
• In latest devise version, it sets the encrypted_password field with NULL along with updating other attributes.

The thing is that the user form scenario was just an example of a real-life use case, what I actually encountered in the project that I am working with, was a test spec:

assert user.valid_password? # returns true
assert user.update(email: 'another_email@mail.com', password: '', password_confirmation: '') # returns true (user email updated)
assert user.valid_password? # still returns true, since password didn't "actually" change.

Based on the line you sent, now devise is removing password params from update params whenever they are blank but when you use #update_with_password not just #update, so this case should never happen, This eventually will end up with same desired result: User attribute(s) has been updated without affecting the password.

But if you use #update i think it will still set encrypted_password with nil (please correct me if I am wrong).

[DaniG2k]

+1, same issue here.

We want certain super-users to be able to update another user's information if necessary. With previous versions of Devise, we could allow them to do this without modifying the user's password.

With the latest version of Devise, this behavior has changed. It is now mandatory for the password field to be filled.

This appears to be a bug. @codeminator mentioned, the encrypted_password field gets set to NULL so the validation error we are seeing is Encrypted password can't be blank.

[timkrins]

@codeminator yeah, looks like it could be a bug

Same underlying issue as #5033
and looks related to PR #4261 (the result of #4245 tegor mentioned)

[timkrins]

PR (which i've closed) should explain the why - but now, yeah, I am curious about the change in the first place (as there is a test which states test_should_set_encrypted_password_to_nil_if_password_is_nil)

#5044

[tagCincy]

+1 to this issue for us

We are trying to update to 4.6.1 to address #4981 and are blocked because of the change from #4261.

Our use case allows users to be created with just an email. IMO, this framework should not be making the decision whether users must have a non-null password.

@timkrins I think the original issue from #4245 was that is the user entered password did not pass validation, it was cleaning password and password_confirmation, but not encrypted_password. Though that was true, I would have liked to see more of an actual problematic use case. Also, did #4261 really address that original issue?

[mrcasals]

I'm hitting the same problem in decidim/decidim#4986 (seems to be the same underlying issue as in #5033), there are failings CI jobs in my PR in case they help debugging this problem!

[tagCincy]

I submitted #5048 to address this issue. The only difference between that and how devise behaved pre-4.6 is the encrypted_password column will be populated with digest hash of nil instead of a blank string.

[tegon]

I'm closing this in favor of #5033.

TL;DR we're going to revert the change since like @tagCincy mentioned, there were no real use cases for this. We thought this would be a simple change but since it's causing so much trouble for existing applications, we're going to revert it.

Thanks.

[tegon]

Released in version 4.6.2. Please let us know if something is missing.