Framework: Reset Password email does not include port number

[nklein]

• Laravel Version: 5.7.*
• PHP Version: 7.2.11
• Database Driver & Version: irrelevant

Description:

The email sent when you are verifying your email address includes the port number of the web server. The email sent when you have forgotten your password does not include the port number of the web server.

Steps To Reproduce:

• Configure Laravel to be able to send email
• Ensure you have one account with an email address you have access to
• Run Laravel on a non-standard port (e.g. 8000 or 8080)
• Navigate to /password/reset, enter your email address, and submit

Expected: clicking on the link in the email will take you to the non-standard port
Actual: clicking on the link in the email tries to go to the default port (80).

Why this happens and how to fix it

The file laravel/framework/src/Illuminate/Auth/Notifications/ResetPassword.php does the following in the toMail() method:

->action(Lang::getFromJson('Reset Password'), url(config('app.url').route('password.reset', $this->token, false)))

Instead, it should do this like the laravel/framework/src/Illuminate/Auth/Notifications/VerifyEmail.php does:

->action(Lang::getFromJson('Reset Password'), route('password.reset', $this->token, true))

ResetPasswordEmailUrl.patch.txt

[ankurk91]

See
https://github.com/laravel/framework/commit/cef10551820530632a86fa6f1306fee95c5cac43

[seriquynh]

Have you configured the APP_URL variable inside the .env file? This is used to generate full url via both route and url methods.

[nklein]

I have not configured the APP_URL, and I don’t like that idea that much because depending on what else is going on, I switch ports. And, I would have to special case the value for dev vs. prod.

Anyhow, I haven’t looked too closely at the other thread yet. I don’t understand fully (yet) the use case that made the password reset email use one method of finding the route but the verify email use a different approach.

-- Patrick [e-mail]

On Oct 25, 2018, at 8:08 PM, Nguyen Xuan Quynh [e-mail] wrote:

Have you configure the APP_URL variable inside the .env file? This is used to generate full url via both route and url methods.

—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub, or mute the thread.

[seriquynh]

I don't know why the code has done below. But they have the same sense, the third parameter that is passed into the "route" method detects that it will generate full URL or not. They will provide the same results.

route('password.reset', $this->token, true) ~ config('app.url').route('password.reset', $this->token, false) 
url(config('app.url').route('password.reset', $this->token, false)) ~ route('password.reset', $this->token, true)

"And, I would have to special case the value for dev vs. prod."

I don't think so. It's necessary to change the configuration when switching among many computer environment. And the production configuration such as domain, database is very stable, it will not be changed unless there are some real specified conditions.

I think no problem here. Anyway, feel free to submit a pull request to solve what you want.

[driesvints]

Think a PR would be appropriate? It's probably best to also include a test.

[phroggyy]

I think this shouldn't be considered a bug – as @xuanquynh says, you should set the APP_URL env variable correctly instead. For example, there's no guarantee that the server you're sending the email from is the same as the one you serve the frontend from. For example you could have a worker instance that sends queued emails, _if_ this server was at all web accessible, it might be on a different IP and different port.

Further, it's likely that PHP itself is running under FPM on port 9000 – it's not up to Laravel to decide what the port is as you can have a complex setup of load balancers, reverse proxies, and worker instances. The only proper solution is to set APP_URL correctly.

[srmklive]

I completely agree with @phroggyy . This can simply be done by setting the APP_URL in the .env file by including the appropriate port like this:

APP_URL=https://yoursite.com:880/

[driesvints]

Gonna close this in favor of the suggestions made by others above.

[adromil]

I know this might be too either late or outdated. But I'm also in the same sentiments with @nklein.
But maybe we could try to add _temporarily signed_ URL by updating the laravel/framework/src/Illuminate/Auth/Notifications/ResetPassword.php file and replace this line :

->action(Lang::get('Reset Password'), url(config('app.url').route('password.reset', ['token' => $this->token, 'email' => $notifiable->getEmailForPasswordReset()], false)))

with something like:

->action(Lang::get('Reset Password'), URL::temporarySignedRoute('password.reset', Carbon::now()->addMinutes(Config::get('auth.passwords.'.config('auth.defaults.passwords').'.expire')), ['token' => $this->token, 'email' => $notifiable->getEmailForPasswordReset()], true))

It would generate a rather long URL compare to the original but will certainly have the _host_ together with the _port_ you are using.