From 787d70192ef8eb615895af11e7b5e6c46cf310ec Mon Sep 17 00:00:00 2001 From: Pierre Ambroise <74421318+Fan2Shrek@users.noreply.github.com> Date: Thu, 21 Mar 2024 10:18:37 +0100 Subject: [PATCH] Email users on account security changes (#1426) Co-authored-by: Jordi Boggiano --- src/Controller/ChangePasswordController.php | 5 ++- src/Controller/GitHubLoginController.php | 9 ++-- src/Controller/ProfileController.php | 20 +++++++-- src/Security/UserNotifier.php | 48 +++++++++++++++++++++ templates/email/alert_change.txt.twig | 10 +++++ 5 files changed, 85 insertions(+), 7 deletions(-) create mode 100644 src/Security/UserNotifier.php create mode 100644 templates/email/alert_change.txt.twig diff --git a/src/Controller/ChangePasswordController.php b/src/Controller/ChangePasswordController.php index d7b980f1a..f5227b008 100644 --- a/src/Controller/ChangePasswordController.php +++ b/src/Controller/ChangePasswordController.php @@ -14,6 +14,7 @@ use App\Entity\User; use App\Form\ChangePasswordFormType; +use App\Security\UserNotifier; use Symfony\Component\Security\Http\Attribute\IsGranted; use Symfony\Component\HttpFoundation\Request; use Symfony\Component\HttpFoundation\Response; @@ -25,7 +26,7 @@ class ChangePasswordController extends Controller { #[IsGranted('ROLE_USER')] #[Route(path: '/profile/change-password', name: 'change_password')] - public function changePasswordAction(Request $request, UserPasswordHasherInterface $passwordHasher, #[CurrentUser] User $user): Response + public function changePasswordAction(Request $request, UserPasswordHasherInterface $passwordHasher, UserNotifier $userNotifier, #[CurrentUser] User $user): Response { $form = $this->createForm(ChangePasswordFormType::class, $user); $form->handleRequest($request); @@ -42,6 +43,8 @@ public function changePasswordAction(Request $request, UserPasswordHasherInterfa $this->getEM()->persist($user); $this->getEM()->flush(); + $userNotifier->notifyChange($user->getEmail(), 'Your password has been changed'); + return $this->redirectToRoute('my_profile'); } diff --git a/src/Controller/GitHubLoginController.php b/src/Controller/GitHubLoginController.php index 342713055..6027cc4e0 100644 --- a/src/Controller/GitHubLoginController.php +++ b/src/Controller/GitHubLoginController.php @@ -13,6 +13,7 @@ namespace App\Controller; use App\Entity\User; +use App\Security\UserNotifier; use App\Service\Scheduler; use KnpU\OAuth2ClientBundle\Client\ClientRegistry; use KnpU\OAuth2ClientBundle\Exception\InvalidStateException; @@ -72,7 +73,7 @@ public function login(ClientRegistry $clientRegistry): RedirectResponse * in config/packages/knpu_oauth2_client.yaml */ #[Route(path: '/connect/github/check', name: 'connect_github_check')] - public function connectCheck(Request $request, ClientRegistry $clientRegistry, Scheduler $scheduler, #[CurrentUser] User $user): RedirectResponse + public function connectCheck(Request $request, ClientRegistry $clientRegistry, Scheduler $scheduler, UserNotifier $userNotifier, #[CurrentUser] User $user): RedirectResponse { /** @var \KnpU\OAuth2ClientBundle\Client\Provider\GithubClient $client */ $client = $clientRegistry->getClient('github'); @@ -113,8 +114,9 @@ public function connectCheck(Request $request, ClientRegistry $clientRegistry, S $this->getEM()->flush(); $scheduler->scheduleUserScopeMigration($user->getId(), $oldScope, $user->getGithubScope() ?? ''); + $userNotifier->notifyChange($user->getEmail(), 'A GitHub account ('.$ghUser->getNickname().') has been connected to your Packagist.org account.'); - $this->addFlash('success', 'You have connected your GitHub account '.$ghUser->getNickname().' to your Packagist.org account.'); + $this->addFlash('success', 'You have connected your GitHub account ('.$ghUser->getNickname().') to your Packagist.org account.'); } catch (IdentityProviderException | InvalidStateException $e) { $this->addFlash('error', 'Failed OAuth Login: '.$e->getMessage()); } @@ -133,7 +135,7 @@ public function loginCheck(Request $request, ClientRegistry $clientRegistry): vo } #[Route(path: '/oauth/github/disconnect', name: 'user_github_disconnect')] - public function disconnect(Request $req, CsrfTokenManagerInterface $csrfTokenManager, #[CurrentUser] User $user): RedirectResponse + public function disconnect(Request $req, CsrfTokenManagerInterface $csrfTokenManager, UserNotifier $userNotifier, #[CurrentUser] User $user): RedirectResponse { if (!$this->isCsrfTokenValid('unlink_github', $req->query->get('token', ''))) { throw new AccessDeniedException('Invalid CSRF token'); @@ -143,6 +145,7 @@ public function disconnect(Request $req, CsrfTokenManagerInterface $csrfTokenMan $this->disconnectUser($user); $this->getEM()->persist($user); $this->getEM()->flush(); + $userNotifier->notifyChange($user->getEmail(), 'Your GitHub account has been disconnected from your Packagist.org account.'); } return $this->redirectToRoute('edit_profile'); diff --git a/src/Controller/ProfileController.php b/src/Controller/ProfileController.php index 50d2c6dfe..c973a97db 100644 --- a/src/Controller/ProfileController.php +++ b/src/Controller/ProfileController.php @@ -19,6 +19,7 @@ use App\Form\Type\ProfileFormType; use App\Model\DownloadManager; use App\Model\FavoriteManager; +use App\Security\UserNotifier; use Pagerfanta\Doctrine\ORM\QueryAdapter; use Pagerfanta\Pagerfanta; use Symfony\Component\HttpFoundation\Request; @@ -112,7 +113,7 @@ public function packagesAction(Request $req, #[VarName('name')] User $user, Favo } #[Route(path: '/profile/edit', name: 'edit_profile')] - public function editAction(Request $request): Response + public function editAction(Request $request, UserNotifier $userNotifier): Response { $user = $this->getUser(); if (!$user instanceof User) { @@ -120,13 +121,26 @@ public function editAction(Request $request): Response } $oldEmail = $user->getEmail(); + $oldUsername = $user->getUsername(); $form = $this->createForm(ProfileFormType::class, $user); $form->handleRequest($request); if ($form->isSubmitted() && $form->isValid()) { - if ($oldEmail !== $user->getEmail()) { - $user->resetPasswordRequest(); + $diffs = array_filter([ + $oldEmail !== $user->getEmail() ? 'email ('.$oldEmail.' => '.$user->getEmail().')' : null, + $oldUsername !== $user->getUsername() ? 'username ('.$oldUsername.' => '.$user->getUsername().')' : null, + ]); + + if (!empty($diffs)) { + $reason = sprintf('Your %s has been changed', implode(' and ', $diffs)); + + if ($oldEmail !== $user->getEmail()) { + $userNotifier->notifyChange($oldEmail, $reason); + $user->resetPasswordRequest(); + } + + $userNotifier->notifyChange($user->getEmail(), $reason); } $this->getEM()->persist($user); diff --git a/src/Security/UserNotifier.php b/src/Security/UserNotifier.php new file mode 100644 index 000000000..d5e992739 --- /dev/null +++ b/src/Security/UserNotifier.php @@ -0,0 +1,48 @@ + + * Nils Adermann + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + +namespace App\Security; + +use Symfony\Bridge\Twig\Mime\TemplatedEmail; +use Symfony\Component\Mailer\MailerInterface; +use Symfony\Component\Mime\Address; + +/** + * @author Pierre Ambroise + */ +class UserNotifier +{ + private MailerInterface $mailer; + private string $mailFromEmail; + private string $mailFromName; + + public function __construct(string $mailFromEmail, string $mailFromName, MailerInterface $mailer) + { + $this->mailer = $mailer; + $this->mailFromEmail = $mailFromEmail; + $this->mailFromName = $mailFromName; + } + + public function notifyChange(string $email, string $reason): void + { + $email = (new TemplatedEmail()) + ->from(new Address($this->mailFromEmail, $this->mailFromName)) + ->to($email) + ->subject('A change has been made to your account') + ->textTemplate('email/alert_change.txt.twig') + ->context([ + 'reason' => $reason, + ]); + + $this->mailer->send($email); + } +} diff --git a/templates/email/alert_change.txt.twig b/templates/email/alert_change.txt.twig new file mode 100644 index 000000000..21447e871 --- /dev/null +++ b/templates/email/alert_change.txt.twig @@ -0,0 +1,10 @@ +{% autoescape false -%} + +A critical account security change has been made to your account: + +------------------------------- +Change: {{ reason }} +Time: {{ 'now'|date('c') }} +------------------------------- + +{%- endautoescape %}