Remove Valid Password Constraints from DatabaseAuthenticatable Concern

[tagCincy]

Overview

PR #4261 and subsequently #4970 were added to address issue #4245. In doing so, a hard requirement for having a non-nil, non-blank password was injected into DatabaseAuthenticatable. As many of us are upgrading from version < 4.6 to 4.6.1 to address #4981 we are unable to do so because we have a pre-existing implementation that allows the creation of a Devise model with no password. A common use case of this is pre-creating an account when a user submits an email for marketing purposes.

However, in my opinion, a valid use case is irrelevant to what this change did. It broke the nice separation of concerns that existed in Devise prior to version 4.6.0. DatabaseAuthenticatable should not be deciding what constitutes validity for a password, that is what the Validatable concern is for. Given that Validatable#password_required? exists, which allows implementations to override default behavior and explicitly not require a password, is further evidence why this change was a mistake.

Furthermore, the original issue that sparked this change, #4245, was either inaccurate to begin with, or had been properly fixed in 2+ years since being reported and the change being merged. I have run the same "test" on an implementation using this branch:

UPDATE: A colleague of mine pointed out that my change was different than the pre-4.6 behavior and that is why the test case from #4245 was passing. I updated my change to set encrypted_password to a blank string if the passed password argument is blank as well. This should mimic the pre-4.6 behavior as well as keeping the "default" encrypted_password value the same as defined in the schema.

Changelog

Changes

• removed returning nil from DatabaseAuthenticatable#password_digest if the passed password argument is nil or blank
• returns '' if a blank password argument is passed
• adds a simple unit test to match the test case from the original issue (:database_authenticatable issue with clean_passwords #4245)

Removed

• unit tests referencing the prior behavior

[djfpaagman]

+1 here, running into the exact same issue.

[JulienItard]

Any idea when this will be merged ? I got same issue ! Thanks a lot 😄

[tegon]

Hi @tagCincy, thanks for submitting this pull request but we decided to just revert original commit in a new pull request (#5051) in order to keep the code the same as it was before.

Thanks again!