Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Subscribe to correct LoginEvent name #507

Merged
merged 6 commits into from
Nov 7, 2023
Merged
Show file tree
Hide file tree
Changes from 5 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .env
Original file line number Diff line number Diff line change
Expand Up @@ -50,5 +50,5 @@ MAILER_DELIVERY_ADDRESS="[email protected]"
# DATABASE_URL="sqlite:///%kernel.project_dir%/var/data.db"
# DATABASE_URL="mysql://app:[email protected]:3306/app?serverVersion=8&charset=utf8mb4"
# DATABASE_URL="postgresql://app:[email protected]:5432/app?serverVersion=14&charset=utf8"
DATABASE_URL="mysql://mail:[email protected]:3306/mail?serverVersion=mariadb-10.3.23&charset=utf8mb4"
DATABASE_URL="mysql://mail:[email protected]:3306/mail?charset=utf8mb4"
###< doctrine/doctrine-bundle ###
4 changes: 3 additions & 1 deletion src/Command/UsersRegistrationMailCommand.php
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@
*
* @throws \Exception
*/
protected function execute(InputInterface $input, OutputInterface $output)
protected function execute(InputInterface $input, OutputInterface $output): int

Check warning on line 43 in src/Command/UsersRegistrationMailCommand.php

View check run for this annotation

Codecov / codecov/patch

src/Command/UsersRegistrationMailCommand.php#L43

Added line #L43 was not covered by tests
{
$email = $input->getOption('user');
$locale = $input->getOption('locale');
Expand All @@ -50,5 +50,7 @@
}

$this->welcomeMessageSender->send($user, $locale);

return 0;

Check warning on line 54 in src/Command/UsersRegistrationMailCommand.php

View check run for this annotation

Codecov / codecov/patch

src/Command/UsersRegistrationMailCommand.php#L54

Added line #L54 was not covered by tests
}
}
2 changes: 1 addition & 1 deletion src/Event/RecoveryProcessEvent.php
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
use Symfony\Contracts\EventDispatcher\Event;

/**
* Class LoginEvent.
* Class RecoveryProcessEvent.
*/
class RecoveryProcessEvent extends Event
{
Expand Down
2 changes: 1 addition & 1 deletion src/EventListener/LoginListener.php
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ public static function getSubscribedEvents(): array
{
return [
SecurityEvents::INTERACTIVE_LOGIN => 'onSecurityInteractiveLogin',
LoginEvent::class => 'onLogin',
LoginEvent::NAME => 'onLogin',
];
}
}
60 changes: 51 additions & 9 deletions tests/Command/UsersCheckPasswordCommandTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@
use App\Command\UsersCheckPasswordCommand;
use App\Entity\User;
use App\Enum\Roles;
use App\Event\LoginEvent;
use App\EventListener\LoginListener;
use App\Handler\MailCryptKeyHandler;
use App\Handler\UserAuthenticationHandler;
use App\Helper\FileDescriptorReader;
Expand All @@ -13,6 +15,10 @@
use PHPUnit\Framework\TestCase;
use Symfony\Component\Console\Exception\InvalidArgumentException;
use Symfony\Component\Console\Tester\CommandTester;
use Symfony\Component\EventDispatcher\EventDispatcher;
use Symfony\Component\EventDispatcher\EventDispatcherInterface;
use Symfony\Component\PasswordHasher\Hasher\PasswordHasherFactoryInterface;
use Symfony\Component\PasswordHasher\PasswordHasherInterface;

class UsersCheckPasswordCommandTest extends TestCase
{
Expand All @@ -21,16 +27,21 @@ class UsersCheckPasswordCommandTest extends TestCase
protected $quotaUser;
protected $mailCryptUser;
protected $spamUser;
protected $loginListener;

public function setUp(): void
{
$this->plainUser = new User();
$this->plainUser->setPassword('passwordhash');
$this->quotaUser = new User();
$this->quotaUser->setPassword('passwordhash');
$this->quotaUser->setQuota(1024);
$this->mailCryptUser = new User();
$this->mailCryptUser->setPassword('passwordhash');
$this->mailCryptUser->setMailCrypt(true);
$this->mailCryptUser->setMailCryptPublicKey('somePublicKey');
$this->spamUser = new User();
$this->spamUser->setPassword('passwordhash');
$this->spamUser->setRoles([Roles::SPAM]);
}

Expand Down Expand Up @@ -92,6 +103,35 @@ public function testExecuteFd3($inputStream, $returnCode): void
$this->assertEquals($returnCode, $commandTester->getStatusCode());
}

public function testExecuteCallsLoginListener(): void
{
$inputStream = "[email protected]\x00password";
$returnCode = 0;

$manager = $this->getManager();
$reader = $this->getReaderFd3($inputStream);
$handler = $this->getHandler();
$mailCryptKeyHandler = $this->getMailCryptKeyHandler();
$mailCrypt = 2;
$mailUID = 5000;
$mailGID = 5000;
$mailLocation = 'var/vmail';

$command = new UsersCheckPasswordCommand($manager,
$reader,
$handler,
$mailCryptKeyHandler,
$mailCrypt,
$mailUID,
$mailGID,
$mailLocation);
$commandTester = new CommandTester($command);

$this->loginListener->expects(self::once())->method('onLogin');
$commandTester->execute([]);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I do not see any assertation here. It would be good to assert that the command returns successfully and the mocked services were called as expected.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I changed the code logic now so that $this->loginListener->expects(self::once())->method('onLogin') is in the test function itself. Probably it's easier to follow the logic of the test that way.

I also added an assertion to make sure the execute() call succeeded, but that's not what we want to test here. We want to test whether LoginListener->onLogin() got triggered.

self::assertEquals($returnCode, $commandTester->getStatusCode());
}

/**
* @dataProvider validContentProvider
*/
Expand Down Expand Up @@ -239,19 +279,21 @@ public function getManager(): EntityManagerInterface

public function getHandler(): UserAuthenticationHandler
{
$handler = $this->getMockBuilder(UserAuthenticationHandler::class)
->disableOriginalConstructor()
->getMock();
$handler->method('authenticate')->willReturnMap(
$passwordHasher = $this->createMock(PasswordHasherInterface::class);
$passwordHasher->method('verify')->willReturnMap(
[
[$this->plainUser, 'password', $this->plainUser],
[$this->quotaUser, 'password', $this->quotaUser],
[$this->mailCryptUser, 'password', $this->mailCryptUser],
[$this->spamUser, 'password', $this->spamUser],
['passwordhash', 'password', true],
['passwordhash', 'wrongpassword', false],
['passwordhash', '', false],
]
);
$passwordHasherFactory = $this->createMock(PasswordHasherFactoryInterface::class);
$passwordHasherFactory->method('getPasswordHasher')->willReturn($passwordHasher);

return $handler;
$this->loginListener = $this->createMock(LoginListener::class);
$eventDispatcher = new EventDispatcher();
$eventDispatcher->addListener(LoginEvent::NAME, [$this->loginListener, 'onLogin']);
return new UserAuthenticationHandler($passwordHasherFactory, $eventDispatcher);
}

public function getMailCryptKeyHandler(): MailCryptKeyHandler
Expand Down
2 changes: 1 addition & 1 deletion tests/EventListener/LoginListenerTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,7 @@ public function testGetSubscribedEvents(): void
{
$this->assertEquals([
SecurityEvents::INTERACTIVE_LOGIN => 'onSecurityInteractiveLogin',
LoginEvent::class => 'onLogin',
LoginEvent::NAME => 'onLogin',
],
$this->listener::getSubscribedEvents());
}
Expand Down
Loading