Skip to content

Commit

Permalink
♻️Remove PlainPasswordTrait from User
Browse files Browse the repository at this point in the history
  • Loading branch information
0x46616c6b committed Aug 31, 2024
1 parent 3d36b20 commit 85d7dd2
Show file tree
Hide file tree
Showing 10 changed files with 14 additions and 87 deletions.
6 changes: 1 addition & 5 deletions config/packages/dev/mailer.yaml
Original file line number Diff line number Diff line change
@@ -1,7 +1,3 @@
# See https://symfony.com/doc/current/mailer.html#always-send-to-the-same-address
framework:
mailer:
dsn: '%env(MAILER_DSN)%?verify_peer=0'
envelope:
# send all emails to a specific address
recipients: ['%env(MAILER_DELIVERY_ADDRESS)%']
dsn: 'null://null'
2 changes: 0 additions & 2 deletions src/Controller/RecoveryController.php
Original file line number Diff line number Diff line change
Expand Up @@ -205,8 +205,6 @@ public function recoveryToken(Request $request): Response
$form->handleRequest($request);

if ($form->isSubmitted() && $form->isValid()) {
$user->setPlainPassword($recoveryTokenModel->password);

// Check if user has a MailCrypt key
if ($user->hasMailCryptSecretBox()) {
// Decrypt the MailCrypt key
Expand Down
6 changes: 4 additions & 2 deletions src/Entity/User.php
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@
use App\Traits\PasswordTrait;
use App\Traits\PasswordVersionTrait;
use App\Traits\PlainMailCryptPrivateKeyTrait;
use App\Traits\PlainPasswordTrait;
use App\Traits\PlainRecoveryTokenTrait;
use App\Traits\QuotaTrait;
use App\Traits\RecoverySecretBoxTrait;
Expand Down Expand Up @@ -53,7 +52,6 @@ class User implements UserInterface, PasswordAuthenticatedUserInterface, Passwor
use SaltTrait;
use DeleteTrait;
use InvitationVoucherTrait;
use PlainPasswordTrait;
use DomainAwareTrait;
use LastLoginTimeTrait;
use PasswordVersionTrait;
Expand Down Expand Up @@ -142,4 +140,8 @@ public function getPasswordHasherName(): ?string
// use default encoder
return null;
}

public function eraseCredentials()
{
}
}
29 changes: 6 additions & 23 deletions src/EventListener/LoginListener.php
Original file line number Diff line number Diff line change
Expand Up @@ -4,28 +4,24 @@

use App\Entity\User;
use App\Event\LoginEvent;
use App\Helper\PasswordUpdater;
use Doctrine\ORM\EntityManagerInterface;
use Symfony\Component\EventDispatcher\EventSubscriberInterface;
use Symfony\Component\Security\Http\Event\InteractiveLoginEvent;
use Symfony\Component\Security\Http\SecurityEvents;

class LoginListener implements EventSubscriberInterface
readonly class LoginListener implements EventSubscriberInterface
{
/**
* LoginListener constructor.
*/
public function __construct(private readonly EntityManagerInterface $manager, private readonly PasswordUpdater $passwordUpdater)
public function __construct(private EntityManagerInterface $manager)
{
}

public function onSecurityInteractiveLogin(InteractiveLoginEvent $event): void
{
$request = $event->getRequest();
/** @var User $user */
$user = $event->getAuthenticationToken()->getUser();
$user->setPlainPassword($request->get('_password'));
$this->handleLogin($user);

if ($user instanceof User) {
$this->handleLogin($user);
}
}

public function onLogin(LoginEvent $event): void
Expand All @@ -34,25 +30,12 @@ public function onLogin(LoginEvent $event): void
}

private function handleLogin(User $user): void
{
// update password hash if necessary
if (($user->getPasswordVersion() < User::CURRENT_PASSWORD_VERSION) && null !== $plainPassword = $user->getPlainPassword()) {
$user->setPasswordVersion(User::CURRENT_PASSWORD_VERSION);
$this->passwordUpdater->updatePassword($user, $plainPassword);
}
$this->updateLastLogin($user);
}

private function updateLastLogin(User $user): void
{
$user->updateLastLoginTime();
$this->manager->persist($user);
$this->manager->flush();
}

/**
* {@inheritdoc}
*/
public static function getSubscribedEvents(): array
{
return [
Expand Down
1 change: 0 additions & 1 deletion src/Handler/RegistrationHandler.php
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,6 @@ private function buildUser(Registration $registration): User
{
$user = new User();
$user->setEmail(strtolower((string)$registration->getEmail()));
$user->setPlainPassword($registration->getPlainPassword());
$user->setRoles([Roles::USER]);

if (null !== $domain = $this->domainGuesser->guess($registration->getEmail())) {
Expand Down
2 changes: 0 additions & 2 deletions src/Handler/UserAuthenticationHandler.php
Original file line number Diff line number Diff line change
Expand Up @@ -25,8 +25,6 @@ public function authenticate(User $user, string $password): ?User
if (!$hasher->verify($user->getPassword(), $password)) {
return null;
}
$user->setPlainPassword($password);

$this->eventDispatcher->dispatch(new LoginEvent($user), LoginEvent::NAME);

return $user;
Expand Down
10 changes: 0 additions & 10 deletions tests/Entity/UserTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -55,16 +55,6 @@ public function testGetPasswordHasherName(): void
self::assertEquals('legacy', $user->getPasswordHasherName());
}

public function testPlainPassword(): void
{
$user = new User();
self::assertEquals(null, $user->getPlainPassword());
$user->setPlainPassword('test');
self::assertEquals('test', $user->getPlainPassword());
$user->eraseCredentials();
self::assertEquals(null, $user->getPlainPassword());
}

public function testHasRecoverySecretBox(): void
{
$user = new User();
Expand Down
39 changes: 3 additions & 36 deletions tests/EventListener/LoginListenerTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@
use App\EventListener\LoginListener;
use App\Helper\PasswordUpdater;
use Doctrine\ORM\EntityManagerInterface;
use PHPUnit\Framework\MockObject\MockObject;
use PHPUnit\Framework\TestCase;
use Symfony\Component\HttpFoundation\Request;
use Symfony\Component\Security\Core\Authentication\Token\TokenInterface;
Expand All @@ -17,33 +16,20 @@
class LoginListenerTest extends TestCase
{
private EntityManagerInterface $manager;
private PasswordUpdater $passwordUpdater;
private LoginListener $listener;

public function setUp(): void
{
$this->manager = $this->getMockBuilder(EntityManagerInterface::class)
->disableOriginalConstructor()
->getMock();
$this->passwordUpdater = $this->getMockBuilder(PasswordUpdater::class)
->disableOriginalConstructor()
->getMock();
$this->listener = new LoginListener($this->manager, $this->passwordUpdater);
$this->listener = new LoginListener($this->manager);
}

/**
* @dataProvider provider
*/
public function testOnSecurityInteractiveLogin(User $user, bool $update): void
public function testOnSecurityInteractiveLogin(): void
{
$user = new User();
$this->manager->expects($this->once())->method('flush');

if ($update) {
$this->passwordUpdater->expects($this->once())->method('updatePassword');
} else {
$this->passwordUpdater->expects($this->never())->method('updatePassword');
}

$event = $this->getEvent($user);

$this->listener->onSecurityInteractiveLogin($event);
Expand Down Expand Up @@ -74,25 +60,6 @@ private function getEvent(User $user): InteractiveLoginEvent
return $event;
}

public function provider(): array
{
return [
[$this->getUser(null), true],
[$this->getUser(0), true],
[$this->getUser(1), true],
[$this->getUser(2), false],
[$this->getUser(3), false],
];
}

public function getUser(?int $passwordVersion): User
{
$user = new User();
$user->setPasswordVersion($passwordVersion);

return $user;
}

public function testGetSubscribedEvents(): void
{
$this->assertEquals([
Expand Down
5 changes: 0 additions & 5 deletions tests/Handler/RecoveryTokenHandlerTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,6 @@ public function testCreateExceptionPlainMailCryptPrivateKeyNull(): void
$handler = $this->createHandler();
$user = new User();

$user->setPlainPassword('password');
$handler->create($user);
}

Expand All @@ -35,7 +34,6 @@ public function testCreate(): void
$handler = $this->createHandler();
$user = new User();

$user->setPlainPassword('password');
$user->setPlainMailCryptPrivateKey('dummyKey');
$handler->create($user);

Expand All @@ -49,7 +47,6 @@ public function testVerify(): void

self::assertFalse($handler->verify($user, 'recoveryToken'));

$user->setPlainPassword('password');
$user->setPlainMailCryptPrivateKey('dummyKey');
$handler->create($user);

Expand Down Expand Up @@ -78,7 +75,6 @@ public function testDecryptExceptionDecryptionFailed(): void
$this->expectExceptionMessage('decryption of recoverySecretBox failed');
$handler = $this->createHandler();
$user = new User();
$user->setPlainPassword('password');
$user->setPlainMailCryptPrivateKey('privateKey');
$handler->create($user);

Expand All @@ -89,7 +85,6 @@ public function testDecrypt(): void
{
$handler = $this->createHandler();
$user = new User();
$user->setPlainPassword('password');
$user->setPlainMailCryptPrivateKey('privateKey');
$handler->create($user);
$recoveryToken = $user->getPlainRecoveryToken();
Expand Down
1 change: 0 additions & 1 deletion tests/Helper/AdminPasswordUpdaterTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,6 @@ public function setUp(): void
public function testUpdateAdminPassword(): void
{
$admin = new User();
$admin->setPlainPassword('password');
$admin->setPassword('impossible_login');

$adminPasswordUpdater = new AdminPasswordUpdater(
Expand Down

0 comments on commit 85d7dd2

Please sign in to comment.