Skip to content

Commit

Permalink
feature #36666 [Security] Renamed VerifyAuthenticatorCredentialsEvent…
Browse files Browse the repository at this point in the history
… to CheckPassportEvent (wouterj)

This PR was merged into the 5.1-dev branch.

Discussion
----------

[Security] Renamed VerifyAuthenticatorCredentialsEvent to CheckPassportEvent

| Q             | A
| ------------- | ---
| Branch?       | master
| Bug fix?      | no
| New feature?  | no
| Deprecations? | no
| Tickets       | Fix #36608
| License       | MIT
| Doc PR        | -

This event was named long before we introduced the concept of passports. Listeners on this event check the user, the credentials and any badges of the Security passport. I think `CheckPassportEvent` makes the most sense (more than `CheckCredentialsEvent`).

Also, I managed to break fabbot in the large PR. Just checked all new classes and added license headers in case they were missing (fabbot complained about most of them in this PR already).

Commits
-------

5ba4d1d Renamed VerifyAuthenticatorCredentialsEvent to CheckPassportEvent
  • Loading branch information
fabpot committed May 3, 2020
2 parents c30d6f9 + 5ba4d1d commit 1308dd5
Show file tree
Hide file tree
Showing 12 changed files with 79 additions and 45 deletions.
Expand Up @@ -12,10 +12,10 @@
namespace Symfony\Bundle\SecurityBundle\EventListener;

use Symfony\Component\EventDispatcher\EventSubscriberInterface;
use Symfony\Component\Security\Http\Event\CheckPassportEvent;
use Symfony\Component\Security\Http\Event\LoginFailureEvent;
use Symfony\Component\Security\Http\Event\LoginSuccessEvent;
use Symfony\Component\Security\Http\Event\LogoutEvent;
use Symfony\Component\Security\Http\Event\VerifyAuthenticatorCredentialsEvent;
use Symfony\Contracts\EventDispatcher\EventDispatcherInterface;

/**
Expand All @@ -39,7 +39,7 @@ public static function getSubscribedEvents(): array
LogoutEvent::class => 'bubbleEvent',
LoginFailureEvent::class => 'bubbleEvent',
LoginSuccessEvent::class => 'bubbleEvent',
VerifyAuthenticatorCredentialsEvent::class => 'bubbleEvent',
CheckPassportEvent::class => 'bubbleEvent',
];
}

Expand Down
Expand Up @@ -44,7 +44,7 @@

<!-- Listeners -->

<service id="security.listener.verify_authenticator_credentials" class="Symfony\Component\Security\Http\EventListener\VerifyAuthenticatorCredentialsListener">
<service id="security.listener.check_authenticator_credentials" class="Symfony\Component\Security\Http\EventListener\CheckCredentialsListener">
<tag name="kernel.event_subscriber" />
<argument type="service" id="security.encoder_factory" />
</service>
Expand Down
Expand Up @@ -26,10 +26,10 @@
use Symfony\Component\Security\Http\Authenticator\Passport\Badge\BadgeInterface;
use Symfony\Component\Security\Http\Authenticator\Passport\PassportInterface;
use Symfony\Component\Security\Http\Authenticator\Passport\SelfValidatingPassport;
use Symfony\Component\Security\Http\Event\CheckPassportEvent;
use Symfony\Component\Security\Http\Event\InteractiveLoginEvent;
use Symfony\Component\Security\Http\Event\LoginFailureEvent;
use Symfony\Component\Security\Http\Event\LoginSuccessEvent;
use Symfony\Component\Security\Http\Event\VerifyAuthenticatorCredentialsEvent;
use Symfony\Component\Security\Http\SecurityEvents;
use Symfony\Contracts\EventDispatcher\EventDispatcherInterface;

Expand Down Expand Up @@ -159,7 +159,7 @@ private function executeAuthenticator(AuthenticatorInterface $authenticator, Req
$passport = $authenticator->authenticate($request);

// check the passport (e.g. password checking)
$event = new VerifyAuthenticatorCredentialsEvent($authenticator, $passport);
$event = new CheckPassportEvent($authenticator, $passport);
$this->eventDispatcher->dispatch($event);

// check if all badges are resolved
Expand Down
@@ -1,5 +1,14 @@
<?php

/*
* This file is part of the Symfony package.
*
* (c) Fabien Potencier <fabien@symfony.com>
*
* For the full copyright and license information, please view the LICENSE
* file that was distributed with this source code.
*/

namespace Symfony\Component\Security\Http\Authenticator\Token;

use Symfony\Component\Security\Core\Authentication\Token\AbstractToken;
Expand Down
@@ -1,9 +1,16 @@
<?php

/*
* This file is part of the Symfony package.
*
* (c) Fabien Potencier <fabien@symfony.com>
*
* For the full copyright and license information, please view the LICENSE
* file that was distributed with this source code.
*/

namespace Symfony\Component\Security\Http\Event;

use Symfony\Component\Security\Core\Authentication\Token\TokenInterface;
use Symfony\Component\Security\Core\User\UserInterface;
use Symfony\Component\Security\Http\Authenticator\AuthenticatorInterface;
use Symfony\Component\Security\Http\Authenticator\Passport\PassportInterface;
use Symfony\Contracts\EventDispatcher\Event;
Expand All @@ -17,7 +24,7 @@
*
* @author Wouter de Jong <wouter@wouterj.nl>
*/
class VerifyAuthenticatorCredentialsEvent extends Event
class CheckPassportEvent extends Event
{
private $authenticator;
private $passport;
Expand Down
@@ -1,5 +1,14 @@
<?php

/*
* This file is part of the Symfony package.
*
* (c) Fabien Potencier <fabien@symfony.com>
*
* For the full copyright and license information, please view the LICENSE
* file that was distributed with this source code.
*/

namespace Symfony\Component\Security\Http\EventListener;

use Symfony\Component\EventDispatcher\EventSubscriberInterface;
Expand All @@ -8,7 +17,7 @@
use Symfony\Component\Security\Http\Authenticator\Passport\Credentials\CustomCredentials;
use Symfony\Component\Security\Http\Authenticator\Passport\Credentials\PasswordCredentials;
use Symfony\Component\Security\Http\Authenticator\Passport\UserPassportInterface;
use Symfony\Component\Security\Http\Event\VerifyAuthenticatorCredentialsEvent;
use Symfony\Component\Security\Http\Event\CheckPassportEvent;

/**
* This listeners uses the interfaces of authenticators to
Expand All @@ -19,7 +28,7 @@
* @final
* @experimental in 5.1
*/
class VerifyAuthenticatorCredentialsListener implements EventSubscriberInterface
class CheckCredentialsListener implements EventSubscriberInterface
{
private $encoderFactory;

Expand All @@ -28,7 +37,7 @@ public function __construct(EncoderFactoryInterface $encoderFactory)
$this->encoderFactory = $encoderFactory;
}

public function onAuthenticating(VerifyAuthenticatorCredentialsEvent $event): void
public function checkPassport(CheckPassportEvent $event): void
{
$passport = $event->getPassport();
if ($passport instanceof UserPassportInterface && $passport->hasBadge(PasswordCredentials::class)) {
Expand Down Expand Up @@ -74,6 +83,6 @@ public function onAuthenticating(VerifyAuthenticatorCredentialsEvent $event): vo

public static function getSubscribedEvents(): array
{
return [VerifyAuthenticatorCredentialsEvent::class => ['onAuthenticating', 128]];
return [CheckPassportEvent::class => 'checkPassport'];
}
}
Expand Up @@ -16,7 +16,7 @@
use Symfony\Component\Security\Csrf\CsrfToken;
use Symfony\Component\Security\Csrf\CsrfTokenManagerInterface;
use Symfony\Component\Security\Http\Authenticator\Passport\Badge\CsrfTokenBadge;
use Symfony\Component\Security\Http\Event\VerifyAuthenticatorCredentialsEvent;
use Symfony\Component\Security\Http\Event\CheckPassportEvent;

/**
* @author Wouter de Jong <wouter@wouterj.nl>
Expand All @@ -33,7 +33,7 @@ public function __construct(CsrfTokenManagerInterface $csrfTokenManager)
$this->csrfTokenManager = $csrfTokenManager;
}

public function verifyCredentials(VerifyAuthenticatorCredentialsEvent $event): void
public function checkPassport(CheckPassportEvent $event): void
{
$passport = $event->getPassport();
if (!$passport->hasBadge(CsrfTokenBadge::class)) {
Expand All @@ -57,6 +57,6 @@ public function verifyCredentials(VerifyAuthenticatorCredentialsEvent $event): v

public static function getSubscribedEvents(): array
{
return [VerifyAuthenticatorCredentialsEvent::class => ['verifyCredentials', 256]];
return [CheckPassportEvent::class => ['checkPassport', 128]];
}
}
@@ -1,13 +1,22 @@
<?php

/*
* This file is part of the Symfony package.
*
* (c) Fabien Potencier <fabien@symfony.com>
*
* For the full copyright and license information, please view the LICENSE
* file that was distributed with this source code.
*/

namespace Symfony\Component\Security\Http\EventListener;

use Symfony\Component\EventDispatcher\EventSubscriberInterface;
use Symfony\Component\Security\Core\User\UserCheckerInterface;
use Symfony\Component\Security\Http\Authenticator\Passport\Badge\PreAuthenticatedUserBadge;
use Symfony\Component\Security\Http\Authenticator\Passport\UserPassportInterface;
use Symfony\Component\Security\Http\Event\CheckPassportEvent;
use Symfony\Component\Security\Http\Event\LoginSuccessEvent;
use Symfony\Component\Security\Http\Event\VerifyAuthenticatorCredentialsEvent;

/**
* @author Wouter de Jong <wouter@wouterj.nl>
Expand All @@ -24,7 +33,7 @@ public function __construct(UserCheckerInterface $userChecker)
$this->userChecker = $userChecker;
}

public function preCredentialsVerification(VerifyAuthenticatorCredentialsEvent $event): void
public function preCheckCredentials(CheckPassportEvent $event): void
{
$passport = $event->getPassport();
if (!$passport instanceof UserPassportInterface || $passport->hasBadge(PreAuthenticatedUserBadge::class)) {
Expand All @@ -34,7 +43,7 @@ public function preCredentialsVerification(VerifyAuthenticatorCredentialsEvent $
$this->userChecker->checkPreAuth($passport->getUser());
}

public function postCredentialsVerification(LoginSuccessEvent $event): void
public function postCheckCredentials(LoginSuccessEvent $event): void
{
$passport = $event->getPassport();
if (!$passport instanceof UserPassportInterface || null === $passport->getUser()) {
Expand All @@ -47,8 +56,8 @@ public function postCredentialsVerification(LoginSuccessEvent $event): void
public static function getSubscribedEvents(): array
{
return [
VerifyAuthenticatorCredentialsEvent::class => [['preCredentialsVerification', 256]],
LoginSuccessEvent::class => ['postCredentialsVerification', 256],
CheckPassportEvent::class => ['preCheckCredentials', 256],
LoginSuccessEvent::class => ['postCheckCredentials', 256],
];
}
}
Expand Up @@ -24,7 +24,7 @@
use Symfony\Component\Security\Http\Authenticator\Passport\Credentials\PasswordCredentials;
use Symfony\Component\Security\Http\Authenticator\Passport\Passport;
use Symfony\Component\Security\Http\Authenticator\Passport\SelfValidatingPassport;
use Symfony\Component\Security\Http\Event\VerifyAuthenticatorCredentialsEvent;
use Symfony\Component\Security\Http\Event\CheckPassportEvent;

class AuthenticatorManagerTest extends TestCase
{
Expand Down Expand Up @@ -95,7 +95,7 @@ public function testAuthenticateRequest($matchingAuthenticatorIndex)
$matchingAuthenticator->expects($this->any())->method('authenticate')->willReturn(new SelfValidatingPassport($this->user));

$listenerCalled = false;
$this->eventDispatcher->addListener(VerifyAuthenticatorCredentialsEvent::class, function (VerifyAuthenticatorCredentialsEvent $event) use (&$listenerCalled, $matchingAuthenticator) {
$this->eventDispatcher->addListener(CheckPassportEvent::class, function (CheckPassportEvent $event) use (&$listenerCalled, $matchingAuthenticator) {
if ($event->getAuthenticator() === $matchingAuthenticator && $event->getPassport()->getUser() === $this->user) {
$listenerCalled = true;
}
Expand All @@ -106,7 +106,7 @@ public function testAuthenticateRequest($matchingAuthenticatorIndex)

$manager = $this->createManager($authenticators);
$this->assertNull($manager->authenticateRequest($this->request));
$this->assertTrue($listenerCalled, 'The VerifyAuthenticatorCredentialsEvent listener is not called');
$this->assertTrue($listenerCalled, 'The CheckPassportEvent listener is not called');
}

public function provideMatchingAuthenticatorIndex()
Expand Down
Expand Up @@ -21,10 +21,10 @@
use Symfony\Component\Security\Http\Authenticator\Passport\Credentials\PasswordCredentials;
use Symfony\Component\Security\Http\Authenticator\Passport\Passport;
use Symfony\Component\Security\Http\Authenticator\Passport\SelfValidatingPassport;
use Symfony\Component\Security\Http\Event\VerifyAuthenticatorCredentialsEvent;
use Symfony\Component\Security\Http\EventListener\VerifyAuthenticatorCredentialsListener;
use Symfony\Component\Security\Http\Event\CheckPassportEvent;
use Symfony\Component\Security\Http\EventListener\CheckCredentialsListener;

class VerifyAuthenticatorCredentialsListenerTest extends TestCase
class CheckCredentialsListenerTest extends TestCase
{
private $encoderFactory;
private $listener;
Expand All @@ -33,7 +33,7 @@ class VerifyAuthenticatorCredentialsListenerTest extends TestCase
protected function setUp(): void
{
$this->encoderFactory = $this->createMock(EncoderFactoryInterface::class);
$this->listener = new VerifyAuthenticatorCredentialsListener($this->encoderFactory);
$this->listener = new CheckCredentialsListener($this->encoderFactory);
$this->user = new User('wouter', 'encoded-password');
}

Expand All @@ -53,7 +53,7 @@ public function testPasswordAuthenticated($password, $passwordValid, $result)
}

$credentials = new PasswordCredentials($password);
$this->listener->onAuthenticating($this->createEvent(new Passport($this->user, $credentials)));
$this->listener->checkPassport($this->createEvent(new Passport($this->user, $credentials)));

if (true === $result) {
$this->assertTrue($credentials->isResolved());
Expand All @@ -74,7 +74,7 @@ public function testEmptyPassword()
$this->encoderFactory->expects($this->never())->method('getEncoder');

$event = $this->createEvent(new Passport($this->user, new PasswordCredentials('')));
$this->listener->onAuthenticating($event);
$this->listener->checkPassport($event);
}

/**
Expand All @@ -91,7 +91,7 @@ public function testCustomAuthenticated($result)
$credentials = new CustomCredentials(function () use ($result) {
return $result;
}, ['password' => 'foo']);
$this->listener->onAuthenticating($this->createEvent(new Passport($this->user, $credentials)));
$this->listener->checkPassport($this->createEvent(new Passport($this->user, $credentials)));

if (true === $result) {
$this->assertTrue($credentials->isResolved());
Expand All @@ -109,11 +109,11 @@ public function testNoCredentialsBadgeProvided()
$this->encoderFactory->expects($this->never())->method('getEncoder');

$event = $this->createEvent(new SelfValidatingPassport($this->user));
$this->listener->onAuthenticating($event);
$this->listener->checkPassport($event);
}

private function createEvent($passport)
{
return new VerifyAuthenticatorCredentialsEvent($this->createMock(AuthenticatorInterface::class), $passport);
return new CheckPassportEvent($this->createMock(AuthenticatorInterface::class), $passport);
}
}
Expand Up @@ -19,7 +19,7 @@
use Symfony\Component\Security\Http\Authenticator\AuthenticatorInterface;
use Symfony\Component\Security\Http\Authenticator\Passport\Badge\CsrfTokenBadge;
use Symfony\Component\Security\Http\Authenticator\Passport\SelfValidatingPassport;
use Symfony\Component\Security\Http\Event\VerifyAuthenticatorCredentialsEvent;
use Symfony\Component\Security\Http\Event\CheckPassportEvent;
use Symfony\Component\Security\Http\EventListener\CsrfProtectionListener;

class CsrfProtectionListenerTest extends TestCase
Expand All @@ -38,7 +38,7 @@ public function testNoCsrfTokenBadge()
$this->csrfTokenManager->expects($this->never())->method('isTokenValid');

$event = $this->createEvent($this->createPassport(null));
$this->listener->verifyCredentials($event);
$this->listener->checkPassport($event);
}

public function testValidCsrfToken()
Expand All @@ -49,7 +49,7 @@ public function testValidCsrfToken()
->willReturn(true);

$event = $this->createEvent($this->createPassport(new CsrfTokenBadge('authenticator_token_id', 'abc123')));
$this->listener->verifyCredentials($event);
$this->listener->checkPassport($event);

$this->expectNotToPerformAssertions();
}
Expand All @@ -65,12 +65,12 @@ public function testInvalidCsrfToken()
->willReturn(false);

$event = $this->createEvent($this->createPassport(new CsrfTokenBadge('authenticator_token_id', 'abc123')));
$this->listener->verifyCredentials($event);
$this->listener->checkPassport($event);
}

private function createEvent($passport)
{
return new VerifyAuthenticatorCredentialsEvent($this->createMock(AuthenticatorInterface::class), $passport);
return new CheckPassportEvent($this->createMock(AuthenticatorInterface::class), $passport);
}

private function createPassport(?CsrfTokenBadge $badge)
Expand Down

0 comments on commit 1308dd5

Please sign in to comment.