devise: v4.6.0 breaks activeadmin forms that have `password` field

Environment

Ruby 2.6.1
Rails 5.2.2
Devise 4.6.1

Current behavior

It appears that PR #4261 introduced a change where encrypted_password could be set to nil even when no attempt was made to change password. The relevant comment is here https://github.com/plataformatec/devise/pull/4261#discussion_r260024837

This results in user model forms that have a password field in activeadmin to break with a validation failure when the password field remains empty on submit:

ActiveRecord::NotNullViolation (PG::NotNullViolation: ERROR:  null value in column "encrypted_password" violates not-null constraint

Expected behavior

No attempt should be made to set encrypted_password to nil if the password field doesn’t have a value. This was the case in devise versions prior to v4.6.0.

About this issue

Original URL
State: closed
Created 5 years ago
Reactions: 6
Comments: 24 (5 by maintainers)

Most upvoted comments

Hi everyone, sorry for the delay.

The original change was meant to keep the attributes in sync (password and encrypted_password) - which seemed to make sense - and also because they being different could cause a security breach. But since we didn’t know for now of a real exploit and this update is breaking so many existing applications, we decided to revert the original change. If someday in the future we find an exploit based on this, we can then think in another way to fix it without causing that much trouble or even include it in a major version.

We’ll be releasing a patch version later today.

Thanks again and sorry for all the trouble.

tegon on Mar 25, 2019

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

tegon on Mar 26, 2019

In case it helps others, I am using this workaround in the activeadmin pages that have an editable devise password field:

  before_save do |user|
    if user.will_save_change_to_encrypted_password?
      user.restore_encrypted_password! unless user.encrypted_password.present?
    end
  end

gsar on Feb 26, 2019

I know this issue is closed but I figured this might help some others that might encounter a problem we had. I’m not sure how our problem relates, but upgrading to the 4.6.2 release from 4.6.1 fixed it.

We have a name and email field for a user. The email field is required and the name field is only required under certain circumstances. Upgrading to 4.6.1 caused this name field to be autopopulated with the value of email if left to be nil when using factory bot. Upgrading to 4.6.2 then fixed the issue without any changes in our codebase.

Thanks for all your good work!

hawaiigal on Apr 5, 2019

In case it helps others, I am using this workaround in the activeadmin pages that have an editable devise password field:
  before_save do |user|
    if user.will_save_change_to_encrypted_password?
      user.restore_encrypted_password! unless user.encrypted_password.present?
    end
  end

before_save do |user|
  if user.will_save_change_to_encrypted_password?
    user.restore_encrypted_password! if user.encrypted_password.blank?
  end
end

Rubocop recommends not nesting a sole nested conditional:

before_save do |user|
    user.restore_encrypted_password! if user.will_save_change_to_encrypted_password? && user.encrypted_password.blank?
  end

chiperific on Nov 5, 2020

@nosretep your solution breaks logins! If I submit an account form to update the account and leave password and password_confirmation empty, then the account should be updated without set encrypted_password to blank! If it get set to blank, then the account can no more login with his password!

phlegx on Mar 14, 2019

Broke for me too. Just add encrypted_password: "" and that should satisfy the not null, and is also the default value of “”.

t.string :encrypted_password, null: false, default: "" from the migration

nosretep on Mar 14, 2019