[5.8] Pass notifiable email through reset link #28475
Conversation
dwightwatson
changed the title
|
Attempted here: #21633 As you can see, you should probably use the contract method to get the email. Secondly, several people commented it would break their applications - though I'm not sure how or why. |
|
This was breaking for me due to my own customizations. Not relevant any longer and many people have wanted this. Not sure why but the PRs for it will likely never stop so I say 👍🏻. |
|
Gotcha, that looks a lot better. I'll update this PR to match, but let me know if you'd prefer to target 5.9 instead. I feel like it's worth fixing this because the rest of the piping is already in place, it's just one missing piece of the puzzle, and it's a better experience for users. |
| @@ -47,7 +47,7 @@ public function via($notifiable) | |||
| /** | |||
| * Build the mail representation of the notification. | |||
| * | |||
| * @param mixed $notifiable | |||
| * @param \Illuminate\Contracts\Auth\CanResetPassword $notifiable | |||
There was a problem hiding this comment.
This doesn't makes sense because you expect a notifiable here and not a CanResetPassword instance. You can't be 100% sure that it's a User instance you're receiving here. The change below is ok for me but let's keep the mixed type-hint.
There was a problem hiding this comment.
That's fair, I was just lining up with the aforementioned PR. I'll fix this now.
There was a problem hiding this comment.
Why would a non-CanResetPassword implementation be trying to, well, reset their password?
There was a problem hiding this comment.
@martinbean There's, for example, the if (static::$toMailCallback) { at the beginning which could let you do whatever you want with whatever implementation you have.
|
@dwightwatson @taylorotwell @driesvints Sending email parameter on GET requests seems to have a security risk. We should prevent servers from logging sensitive information. I believe it's a bad approach "Retrieve from email, and then verify token". Just follow "Retrieve from token". |
When you click the link from the password reset email, the controller/view try to get your email address from the request to pre-fill the email field of the form. It's a nice improvement to the user experience as it doesn't force them to enter their email again, but it isn't actually working at the moment.
This change will pass the notifiable's email address through the query string so that this feature actually works.