Skip to content

Commit

Permalink
Email users on account security changes (#1426)
Browse files Browse the repository at this point in the history
Co-authored-by: Jordi Boggiano <[email protected]>
  • Loading branch information
Fan2Shrek and Seldaek authored Mar 21, 2024
1 parent 7f04d3d commit 787d701
Show file tree
Hide file tree
Showing 5 changed files with 85 additions and 7 deletions.
5 changes: 4 additions & 1 deletion src/Controller/ChangePasswordController.php
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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);
Expand All @@ -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');
}

Expand Down
9 changes: 6 additions & 3 deletions src/Controller/GitHubLoginController.php
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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');
Expand Down Expand Up @@ -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());
}
Expand All @@ -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');
Expand All @@ -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');
Expand Down
20 changes: 17 additions & 3 deletions src/Controller/ProfileController.php
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -112,21 +113,34 @@ 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) {
throw $this->createAccessDeniedException('This user does not have access to this section.');
}

$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);
Expand Down
48 changes: 48 additions & 0 deletions src/Security/UserNotifier.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@
<?php declare(strict_types=1);

/*
* This file is part of Packagist.
*
* (c) Jordi Boggiano <[email protected]>
* Nils Adermann <[email protected]>
*
* 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 <[email protected]>
*/
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);
}
}
10 changes: 10 additions & 0 deletions templates/email/alert_change.txt.twig
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
{% autoescape false -%}

A critical account security change has been made to your account:

-------------------------------
Change: {{ reason }}
Time: {{ 'now'|date('c') }}
-------------------------------

{%- endautoescape %}

0 comments on commit 787d701

Please sign in to comment.