Skip to content

[5.8] Break out password reset credentials into a method#28370

Merged
taylorotwell merged 1 commit intolaravel:5.8from
dwightwatson:reset-password
Apr 30, 2019
Merged

[5.8] Break out password reset credentials into a method#28370
taylorotwell merged 1 commit intolaravel:5.8from
dwightwatson:reset-password

Conversation

@dwightwatson
Copy link
Contributor

This breaks out the fetching of credentials from a password reset request into a separate method in a similar fashion to how it occurs in the AuthenticatedUsers trait.

My use-case for this is that I'm using Postgres which is case-sensitive by default, so when a user attempts to reset their password but uses capitals then it tells them their account doesn't exist and leaves them confused.

I'm already overriding credentials for authentication in my own app for this. It might be nice to see Laravel Str::lower email addresses out of the box, but as it appears Postgres-specific it might be overkill.

This solution brings SendsPasswordResetEmails into line with AuthenticatesUsers and allows developers, where necessary, to support email normalization.

@dwightwatson dwightwatson changed the title Break out password reset credentials into a method [5.8] Break out password reset credentials into a method Apr 30, 2019
@devcircus
Copy link
Contributor

I understand the purpose here, but it doesn't seem like "credentials" is the correct method name for "retrieve email address from current request."

@taylorotwell taylorotwell merged commit 6e8be71 into laravel:5.8 Apr 30, 2019
@taylorotwell
Copy link
Member

@devcircus that is what PasswordBroker calls that array of data.

@devcircus
Copy link
Contributor

Cool. That makes sense.

@bobbybouwmann
Copy link
Contributor

bobbybouwmann commented Apr 30, 2019

@driesvints @taylorotwell This is a breaking change if you're using Spark. In spark we have the following code:

class PasswordController extends Controller
{
    use SendsPasswordResetEmails, ResetsPasswords {
        SendsPasswordResetEmails::broker insteadof ResetsPasswords;
    }

    // ...
}

Because both traits now have a credentials method we get the following error

Whoops\Exception\ErrorException : Trait method credentials has not been applied, because there are collisions with other trait methods on Laravel\Spark\Http\Controllers\Auth\PasswordController

Can we revert this?

@devcircus
Copy link
Contributor

devcircus commented Apr 30, 2019

There should be a way to make this work without reverting.

Edit: maybe not?

@driesvints
Copy link
Member

@devcircus another override in Spark needs to be applied like the one already in place.

@devcircus
Copy link
Contributor

10-4. Ive ran into similar situations in various projects so I thought it could be fixed without reverting this.

driesvints added a commit to laravel/spark-aurelius that referenced this pull request May 1, 2019
This addresses an issue that was introduced by laravel/framework#28370 which added a conflicting method name.
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.

6 participants