mirror of
https://github.com/projectsend/projectsend.git
synced 2026-09-16 16:45:07 +00:00
ignacionelson
351da21e8d
Laravel's notification view writes the subcopy URL as [$url]($url). The HTML half parses that into an anchor; the text half parses nothing, so it arrives as literal brackets around a duplicated address. With the button line above it the URL appeared three times in one message. It reads as broken, and it reads broken in a specific direction: a long opaque token, the recipient's address in the query string, and a duplicated link in brackets is the shape of a phishing template. On a password reset, which is often the first mail an installation ever sends somebody, from a domain with no reputation yet. Fixed the way every other component in that message already handles the same split -- one name, two files, Laravel picks per half. Which meant publishing the framework's view for a one-line change, so there is a note in it saying to re-copy on upgrade. Seen in a real reset mail, not in a test.
66 lines
2.3 KiB
PHP
66 lines
2.3 KiB
PHP
<?php
|
|
|
|
declare(strict_types=1);
|
|
|
|
use App\Models\User;
|
|
use App\Modules\Identity\Notifications\ResetPasswordNotification;
|
|
use Illuminate\Mail\Markdown;
|
|
|
|
/**
|
|
* Every notification carrying an action button repeats its URL in the
|
|
* subcopy, for somebody whose mail client will not let them click it.
|
|
*
|
|
* Laravel's own view writes that as `[$url]($url)`, which is right for
|
|
* the HTML half and wrong for the text one: nothing parses markdown in a
|
|
* text/plain body, so it arrives as literal brackets around a duplicated
|
|
* address — the shape a badly-built phishing mail has, on what is often
|
|
* the first message an installation ever sends anybody. Seen in the wild
|
|
* on a real password reset before it was fixed.
|
|
*/
|
|
beforeEach(function () {
|
|
User::factory()->create();
|
|
});
|
|
|
|
/** The two halves Laravel builds for one markdown notification. */
|
|
function renderResetMail(): array
|
|
{
|
|
$user = User::factory()->create();
|
|
$mail = (new ResetPasswordNotification(str_repeat('a', 64)))->toMail($user);
|
|
$mail->viewData['actionText'] = $mail->actionText;
|
|
|
|
$markdown = app(Markdown::class);
|
|
$view = $mail->markdown ?: 'notifications::email';
|
|
$data = array_merge($mail->toArray(), $mail->viewData);
|
|
|
|
return [
|
|
'text' => (string) $markdown->renderText($view, $data),
|
|
'html' => (string) $markdown->render($view, $data),
|
|
'url' => $mail->actionUrl,
|
|
];
|
|
}
|
|
|
|
it('spells the action URL out plainly in the text half', function () {
|
|
['text' => $text, 'url' => $url] = renderResetMail();
|
|
|
|
expect($text)->toContain($url)
|
|
->and($text)->not->toContain('](')
|
|
->and($text)->not->toContain('['.$url);
|
|
});
|
|
|
|
it('still links the action URL in the html half', function () {
|
|
['html' => $html, 'url' => $url] = renderResetMail();
|
|
|
|
// Twice: the button itself, and the subcopy that repeats it.
|
|
expect(substr_count($html, 'href="'.e($url).'"'))->toBe(2)
|
|
// The markdown must have been parsed, not passed through.
|
|
->and($html)->not->toContain('['.e($url).']');
|
|
});
|
|
|
|
it('does not repeat the URL more than the two places that need it', function () {
|
|
['text' => $text, 'url' => $url] = renderResetMail();
|
|
|
|
// Once after the button label, once in the subcopy. A third meant the
|
|
// markdown link had been left in place.
|
|
expect(substr_count($text, $url))->toBe(2);
|
|
});
|