From 446810bc5e38a9319a72f21464c52f5502d5db56 Mon Sep 17 00:00:00 2001 From: Guy Sartorelli Date: Thu, 14 Dec 2023 12:47:05 +1300 Subject: [PATCH 1/2] FIX Allow new password to save even if there's an error sending email --- src/Security/Member.php | 33 +++++++++++++++++++++------------ 1 file changed, 21 insertions(+), 12 deletions(-) diff --git a/src/Security/Member.php b/src/Security/Member.php index dc309b0da7a..6b9496cfc0b 100644 --- a/src/Security/Member.php +++ b/src/Security/Member.php @@ -4,6 +4,7 @@ use IntlDateFormatter; use InvalidArgumentException; +use Psr\Log\LoggerInterface; use SilverStripe\Admin\LeftAndMain; use SilverStripe\CMS\Controllers\CMSMain; use SilverStripe\Control\Director; @@ -34,7 +35,9 @@ use SilverStripe\ORM\UnsavedRelationList; use SilverStripe\ORM\ValidationException; use SilverStripe\ORM\ValidationResult; +use Symfony\Component\Mailer\Exception\TransportExceptionInterface; use Symfony\Component\Mailer\MailerInterface; +use Symfony\Component\Mime\Exception\RfcComplianceException; /** * The member class which represents the users of the system @@ -772,18 +775,24 @@ public function onBeforeWrite() && static::config()->get('notify_password_change') && $this->isInDB() ) { - $email = Email::create() - ->setHTMLTemplate('SilverStripe\\Control\\Email\\ChangePasswordEmail') - ->setData($this) - ->setTo($this->Email) - ->setSubject(_t( - __CLASS__ . '.SUBJECTPASSWORDCHANGED', - "Your password has been changed", - 'Email subject' - )); - - $this->extend('updateChangedPasswordEmail', $email); - $email->send(); + try { + $email = Email::create() + ->setHTMLTemplate('SilverStripe\\Control\\Email\\ChangePasswordEmail') + ->setData($this) + ->setTo($this->Email) + ->setSubject(_t( + __CLASS__ . '.SUBJECTPASSWORDCHANGED', + "Your password has been changed", + 'Email subject' + )); + + $this->extend('updateChangedPasswordEmail', $email); + $email->send(); + } catch (TransportExceptionInterface | RfcComplianceException $e) { + /** @var LoggerInterface $logger */ + $logger = Injector::inst()->get(LoggerInterface::class . '.errorhandler'); + $logger->error('Error sending email in ' . __FILE__ . ' line ' . __LINE__ . ": {$e->getMessage()}"); + } } // The test on $this->ID is used for when records are initially created. Note that this only works with From dd3a0dba242d3c826614c4d87a1808ea2bac5769 Mon Sep 17 00:00:00 2001 From: Guy Sartorelli Date: Thu, 14 Dec 2023 12:47:29 +1300 Subject: [PATCH 2/2] FIX Don't break the page if password recover email fails to send --- lang/en.yml | 1 + .../LostPasswordHandler.php | 51 ++++++++++++++----- 2 files changed, 38 insertions(+), 14 deletions(-) diff --git a/lang/en.yml b/lang/en.yml index 0fd9893a967..866c7496fbd 100644 --- a/lang/en.yml +++ b/lang/en.yml @@ -290,6 +290,7 @@ en: CURRENT_PASSWORD: 'Current Password' EDIT_PASSWORD: 'New Password' EMAIL: Email + EMAIL_FAILED: 'There was an error when trying to email you a password reset link.' EMPTYNEWPASSWORD: "The new password can't be empty, please try again" ENTEREMAIL: 'Please enter an email address to get a password reset link.' ERRORLOCKEDOUT2: 'Your account has been temporarily disabled because of too many failed attempts at logging in. Please try again in {count} minutes.' diff --git a/src/Security/MemberAuthenticator/LostPasswordHandler.php b/src/Security/MemberAuthenticator/LostPasswordHandler.php index a7a91a5b4e6..f5cb4538d96 100644 --- a/src/Security/MemberAuthenticator/LostPasswordHandler.php +++ b/src/Security/MemberAuthenticator/LostPasswordHandler.php @@ -2,15 +2,19 @@ namespace SilverStripe\Security\MemberAuthenticator; +use Psr\Log\LoggerInterface; use SilverStripe\Control\Controller; use SilverStripe\Control\Email\Email; use SilverStripe\Control\HTTPResponse; use SilverStripe\Control\RequestHandler; use SilverStripe\Core\Convert; +use SilverStripe\Core\Injector\Injector; use SilverStripe\Forms\Form; use SilverStripe\ORM\FieldType\DBField; use SilverStripe\Security\Member; use SilverStripe\Security\Security; +use Symfony\Component\Mailer\Exception\TransportExceptionInterface; +use Symfony\Component\Mime\Exception\RfcComplianceException; /** * Handle login requests from MemberLoginForm @@ -173,7 +177,18 @@ public function forgotPassword(array $data, Form $form): HTTPResponse if ($member) { $token = $member->generateAutologinTokenAndStoreHash(); - $this->sendEmail($member, $token); + $success = $this->sendEmail($member, $token); + if (!$success) { + $form->sessionMessage( + _t( + Member::class . '.EMAIL_FAILED', + 'There was an error when trying to email you a password reset link.' + ), + 'bad' + ); + + return $this->redirectToLostPassword(); + } } return $this->redirectToSuccess($data); @@ -225,20 +240,28 @@ protected function getMemberFromData(array $data) */ protected function sendEmail($member, $token) { - /** @var Email $email */ - $email = Email::create() - ->setHTMLTemplate('SilverStripe\\Control\\Email\\ForgotPasswordEmail') - ->setData($member) - ->setSubject(_t( - 'SilverStripe\\Security\\Member.SUBJECTPASSWORDRESET', - "Your password reset link", - 'Email subject' - )) - ->addData('PasswordResetLink', Security::getPasswordResetLink($member, $token)) - ->setTo($member->Email); + try { + /** @var Email $email */ + $email = Email::create() + ->setHTMLTemplate('SilverStripe\\Control\\Email\\ForgotPasswordEmail') + ->setData($member) + ->setSubject(_t( + 'SilverStripe\\Security\\Member.SUBJECTPASSWORDRESET', + "Your password reset link", + 'Email subject' + )) + ->addData('PasswordResetLink', Security::getPasswordResetLink($member, $token)) + ->setTo($member->Email); - $member->extend('updateForgotPasswordEmail', $email); - return $email->send(); + $member->extend('updateForgotPasswordEmail', $email); + $email->send(); + return true; + } catch (TransportExceptionInterface | RfcComplianceException $e) { + /** @var LoggerInterface $logger */ + $logger = Injector::inst()->get(LoggerInterface::class . '.errorhandler'); + $logger->error('Error sending email in ' . __FILE__ . ' line ' . __LINE__ . ": {$e->getMessage()}"); + return false; + } } /**