Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[12.x] Check if internal Hasher::verifyConfiguration() method exists on driver before forwarding call #54833

Merged
merged 1 commit into from
Feb 27, 2025

Conversation

rodrigopedra
Copy link
Contributor

While migrating an older application to Laravel 12, I updated its User model to start using the hashed cast for the password attribute, as it is used on the updated skeleton.

This application extends the Illuminate\Hashing\HashManager to use a custom class which implements the Illuminate\Contracts\Hashing\Hasher interface. Please read the "motivation" section if you are interested in the reasoning to using a custom Hasher implementation.

The Illuminate\Database\Eloquent\Concerns\HasAttributes@castAttributeAsHashedString() method calls a
Hash::verifyConfiguration() method on the Hash façade, which blindly delegates this call to the underlying driver.

/** @phpstan-ignore staticMethod.notFound */
if (! Hash::verifyConfiguration($value)) {
throw new RuntimeException("Could not verify the hashed value's configuration.");
}

But the verifyConfiguration() method wasn't added to the Illuminate\Contracts\Hashing\Hasher, and therefore wasn't required to be implemented at first glance.

This PR:

  • Updates the HashManager@verifyConfiguration() method to verify if the verifyConfiguration() method exists before delegating to the current driver.

If this PR is accepted, I can send another PR later to add the verifyConfiguration() to the Illuminate\Contracts\Hashing\Hasher interface, in case it is wanted.

Maybe this change should be backported to Laravel 11 - or even Laravel 10, when that check was introduced - as any application using a custom Hasher implementation, and moving to use the hashed cast, would raise this error.

As a workaround, I added the verifyConfiguration() to the custom Hasher implementation.

Motivation

The app is from a 30-year-old project (yes, from 1995). I was contracted in 2019 to convert the codebase to Laravel, and earlier this month I was tasked with modernizing it again to the latest Laravel version.

For being such a long-lived application, there were many developers along the way, and many decisions about password hashing were made.

By 2019, each user would have their password hashed using a different hashing mechanism, bound to when them created their account.

What we ended up doing was creating a Illuminate\Contracts\Hashing\Hasher implementation for each hashing strategy, and a composite class which aggregates them, including Laravel's BcryptHasher, which:

  • Hasher@make() always uses Laravel's BcryptHasher
  • Hasher@check() iterates over each implementation
  • Hasher@needsRehash() always returns true if we verify it is a legacy hash, otherwise delegates to Laravel's BcryptHasher

That way, slowly on logging in, the users have their password converted over bcrypt.

@taylorotwell taylorotwell merged commit 84565cc into laravel:12.x Feb 27, 2025
41 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants