[10.x] Support rehashing user passwords with BC support

[valorin]

Following from the comments on #48665, this is a backwards compatible version of the password rehashing fix. Unlike the Laravel 11 targeted PR, this doesn't require any changes to the User contract or model.

It will check for the existence of the getAuthPasswordName() method on the User model - allowing for custom password attributes, and if that doesn't exist, it checks for the existence of the password attribute. If the attribute exists, and the hashes match, it assumes this is the valid password hash field and checks if it needs to rehash it.

I'll mark the other PR as a draft for now - it will need changes to finish the upgrade and remove the BC checks when this has been merged.

Here is the summary and impacts from the other PR, to save a click:

Summary

As was discovered after updating bcrypt rounds from 10 to 12 in laravel/laravel#6245 and #48494, user passwords are not currently being rehashed during login when the hashing configuration has changed. This should be considered a security risk as rehashing passwords is an important part of the authentication process. Rehashing passwords during login ensures updates to the hashing configuration, such as increasing rounds/cost, is applied to existing hashes when the plaintext password is available during login.

This PR implements rehashing directly within the Eloquent and Database User Providers, making it a part of the core framework authentication system. The rehashing will happen automatically when user credentials are validated by *UserProvider::validateCredentials(), which is called by the Auth::attempt() helper. This helper is used by both Breeze and Fortify/Jetstream to authenticate users, and is the recommended method in the Laravel docs. This ensures that with Laravel 11, any apps using the authentication system will be properly rehashing passwords.

I modelled the new DatabaseUserProvider::rehashUserPassword() method on the existing rehashUserPassword() method, to ensure it is compatible for apps that don't use Eloquent.

Upgrade Impacts

1. Rehashing passwords incurs a performance hit, and after this is deployed, every user who logs in will trigger a password rehash. This is unlike to have an impact as logins are generally spread out and servers usually have spare resources. However apps with large numbers of concurrent logins and limited resources may notice increased resource usage.
2. It's not technically a breaking change, but when a password is rehashed it will force all other active sessions, remember-me tokens, and anything else that relies on the current password hash to be reset automatically. This may cause an increase in login attempts as users reauthenticate different devices.

[valorin]

FQN's added. 🙂

[SanderMuller]

Now the hash gets checked twice (once in validateCredentials and here).

I guess this is done to be able to make this a public method so it won't rehash passwords that don't match the user's password. But the downside is double hash checking for every login attempt.

Would it make more sense to split this in two methods, one private without the hash check and one public that uses the private method after validating the hash?

[valorin]

hash_equals() is a basic string comparison function, I could have used ===, but it's best practice to use that for hashes.

You're thinking of password_verify().

[RTippin]

Would this not be a BC for those of us who use an Attribute mutator to set and hash the password?

protected function password(): Attribute
{
    return new Attribute(
        set: fn (string $value): string => Hash::make($value),
    );
}

[valorin]

Yeah, it probably would.

The built-in hash cast checks if it's hashed before hashing:

return $value !== null && ! Hash::isHashed($value) ? Hash::make($value) : $value;

But a custom cast like your example wouldn't do this. 🤔

[valorin]

I can see three options to avoid the BC issue @RTippin identified:

1. Update the raw value, bypassing any casts.
   Works in theory, as it checks we have a matching hash, but the custom cast could have different hashing rules, so is most likely a bad idea.
2. Don't rehash when there is a custom cast.
   Anyone implementing a custom cast would need to handle rehashing themselves, or switch to the Eloquent hashed cast that handles it.
3. Push this back to Laravel 11.
   This would allow for a BC break and upgrade instructions that can advise updating the custom cast to support the rehashing.

Any other options I haven't considered?

I'm leaning towards pushing back to L11 out of caution, since we're already in the murky waters of checking for compatibility, and this is going to make it even more murky.

It's also been recommended to add a toggle into the config to disable rehashing, which I'll need to do.

[RTippin]

@valorin Definitely a fan of this PR, so I am sorry to be the bearer of bad news. I'll be updating to the hashed cast myself as I completely forgot that existed.

Given the options, I'd say this would be a BC for anyone like me, who would otherwise not know to change their ways without reading the upgrade guide, so a L11 target seems best.

[crynobone]

This potentially may trigger User model observer and maybe not be desirable in some scenarios.

[crynobone]

I believe this may also create breaking change for any 3rd party authentication package such as https://github.com/DirectoryTree/LdapRecord-Laravel/blob/master/src/Auth/DatabaseUserProvider.php

I do believe if we want to introduce this in a minor release it should be opt-in to avoid breaking existing projects.

[valorin]

Thank you everyone for your feedback! It's pretty clear there are a lot of potential issues with trying to fit this in 10.x, so I'm going to close this PR and shift my attention back to 11.x in #48665.

I will be moving a few things around in there, and adding a toggle, so go subscribe to that PR to continue the discussion as this evolves.

Folks can manually patch this locally if it's urgent.